Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for CDEvents notifications #18853

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

afrittoli
Copy link

@afrittoli afrittoli commented Jun 26, 2023

Comprehensive Summary of your change

Introduce a new notifier format "CDEvents", as described in the CDEvents proposal.

The CDEvents formatter, for now, supports only PUSH_ARTIFACT events.
If other events are selected in the UI, they raise an error in the logs and no event is sent.

The PR is made of two commits:

  • add the new notification format to Harbor code
  • add the CDEvents option in the format dropdown in the UI

Issue being fixed

Fixes #community/225

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@afrittoli afrittoli requested a review from a team as a code owner June 26, 2023 11:17
@afrittoli
Copy link
Author

/label release-note/new-feature

Signed-off-by: Andrea Frittoli <[email protected]>
Signed-off-by: Andrea Frittoli <[email protected]>
@chlins
Copy link
Member

chlins commented Jul 3, 2023

@afrittoli Hi, thanks for your work! Please update to resolve the conflict.

@chlins chlins added the release-note/new-feature New Harbor Feature label Jul 3, 2023
@codecov
Copy link

codecov bot commented Jul 3, 2023

Codecov Report

Merging #18853 (5ffc9bd) into main (b822952) will not change coverage.
Report is 117 commits behind head on main.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #18853   +/-   ##
=======================================
  Coverage   44.71%   44.71%           
=======================================
  Files         235      235           
  Lines       13083    13083           
  Branches     2671     2671           
=======================================
  Hits         5850     5850           
- Misses       6938     6939    +1     
+ Partials      295      294    -1     
Flag Coverage Δ
unittests 44.71% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...al/src/app/base/project/webhook/webhook.service.ts 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

// Only TopicPullArtifact is supported for now.
// Other event types are not striclty a failure (they are missing by design),
// but we still report them upstream as unknown types
if he.EventType != event.TopicPushArtifact {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I agreed that other event types are not a failure case, user can subscribe other event types from UI but can only receive PUSH events, IMO the better solution is that extend the capability to support limit the event types.

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

@github-actions github-actions bot added the Stale label Sep 1, 2023
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

This PR was closed because it has been stalled for 30 days with no activity. If this PR is still relevant, please re-open a new PR against main.

@github-actions github-actions bot closed this Oct 2, 2023
@afrittoli
Copy link
Author

I would like to continue working on this - is it an option to re-open? If not I will open a new PR.

@OrlinVasilev
Copy link
Member

@afrittoli reopen :) please excuse the automation :D

@github-actions github-actions bot removed the Stale label Oct 3, 2023
@Vad1mo Vad1mo added the never-stale Do not stale label Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
never-stale Do not stale release-note/new-feature New Harbor Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants