feat: implement webhook receiver for Docker, GHCR and Harbor to receive triggers for image update #1159
Conversation
binhnguyenduc
requested review from
chengfang,
ishitasequeira,
jannfis and
pasha-codefresh
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR adds webhook server functionality to the image updater, enabling it to respond to container registry events (Docker Hub, GHCR, Harbor) and trigger ArgoCD application updates. Key changes include the implementation of a dedicated WebhookServer and multiple registry-specific webhook handlers with associated tests, as well as CLI configuration enhancements.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/webhook/webhook*.go | Core webhook handler definitions and registry type detection |
| pkg/webhook/server.go | New HTTP server implementation for webhook requests |
| pkg/webhook/harbor.go | Added Harbor registry webhook handler and payload parsing |
| pkg/webhook/ghcr.go | Added GHCR webhook handler with validation and payload parsing |
| pkg/webhook/docker*.go | Docker Hub webhook handler and tests |
| pkg/argocd/updater_config.go | New updater configuration for image update operations |
| cmd/webhook.go | New CLI command for starting the webhook server |
| cmd/run.go & cmd/main.go | Integration of the webhook server into the main application flow |
pkg/webhook/harbor.go
Outdated
| if err != nil { | ||
| return nil, fmt.Errorf("failed to read request body: %w", err) | ||
| } | ||
| fmt.Printf("Received Harbor webhook payload: %s", string(body)) |
There was a problem hiding this comment.
Consider replacing fmt.Printf with the project's logging utility (e.g. log.Infof) for consistency in logging throughout the codebase.
| fmt.Printf("Received Harbor webhook payload: %s", string(body)) | |
| log.Infof("Received Harbor webhook payload: %s", string(body)) |
| body, err := io.ReadAll(r.Body) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to read request body: %w", err) | ||
| } | ||
|
|
||
| // Reset body for later reading | ||
| r.Body = io.NopCloser(strings.NewReader(string(body))) | ||
|
|
There was a problem hiding this comment.
[nitpick] The pattern for reading and then resetting the request body is repeated across several webhook validation methods. Consider extracting this logic into a shared helper function to reduce duplication and maintain consistency.
| body, err := io.ReadAll(r.Body) | |
| if err != nil { | |
| return fmt.Errorf("failed to read request body: %w", err) | |
| } | |
| // Reset body for later reading | |
| r.Body = io.NopCloser(strings.NewReader(string(body))) | |
| body, err := readAndResetBody(r) | |
| if err != nil { | |
| return fmt.Errorf("failed to read request body: %w", err) | |
| } |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1159 +/- ##
==========================================
- Coverage 63.27% 60.80% -2.48%
==========================================
Files 15 22 +7
Lines 2358 2990 +632
==========================================
+ Hits 1492 1818 +326
- Misses 771 1068 +297
- Partials 95 104 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
UpdaterConfig in pkg/argocd/updater_config.go: is it possible to use other existing struct, instead of creating a new one? webhook secrets: can we keep them in a secret file, similar to https://argo-cd.readthedocs.io/en/latest/operator-manual/webhook/#2-configure-argo-cd-with-the-webhook-secret-optional ? |
Yes, the service, ingress and related manifests would be great to have. |
|
How do you currently use this feature, using the standalone |
I'll update as soon as I can |
Currently, we enable webhook with the run command (via env var) and reduce the interval to 30m or 60m. That way, we have both the webhook trigger and registry scanner running side by side. One way is to potentially run the webhook command, with the registry scanner off altogether, but we haven't actually done that in production. |
I'll see what I can do. |
…ve update of image pushes and trigger application updates Signed-off-by: Binh Nguyen <[email protected]>
Signed-off-by: Cheng Fang <[email protected]>
Signed-off-by: Cheng Fang <[email protected]>
|
Will merge this as an experimental feature and will continue to improve it. |
|
@binhnguyenduc + @chengfang - This PR is incredibly well-timed for what I think is an issue we're encountering at my job, with one caveat: we're using Artifactory. If we were to contribute a PR that followed the patterns of this PR and provided an Artifactory handler, would you want to include it with these current handlers as well? |
…ve triggers for image update (argoproj-labs#1159) Signed-off-by: Binh Nguyen <[email protected]> Signed-off-by: Cheng Fang <[email protected]> Co-authored-by: Binh Nguyen <[email protected]> Co-authored-by: Cheng Fang <[email protected]>
|
@phil-monroe PRs are welcome to enhance the webhook feature! |
|
I am trying to setup the If I don't set the webhook.harbor-secret or set it to empty string then I get this: Please help how should I set the signature. I was trying to use a random passphrase. |
What I did to make it work is that I deleted with kustomize patch the argocd-image-updater-secret so it works now without authentication |
5 participants
This PR is to address #1 and is essentially a continuation of #284
This pull request introduces a webhook server to handle container registry events, enabling automated image updates in ArgoCD applications. Key changes include adding webhook server functionality, integrating registry-specific webhook handlers, and providing configuration options for the webhook server.
Webhook Server Implementation:
WebhookServerclass to handle registry events, including initialization, event handling, and graceful shutdown. (cmd/main.go,cmd/run.go,pkg/webhook/docker.go,pkg/webhook/docker_test.go) [1] [2] [3] [4] [5] [6]WebhookOptionsand CLI command: Added a newwebhookCLI command with configuration options such as port, registry secrets, and ArgoCD integration settings. (cmd/webhook.go)Registry-Specific Webhook Handlers:
pkg/webhook/docker.go,pkg/webhook/docker_test.go) [1] [2]Configuration Enhancements:
UpdaterConfigto encapsulate settings for image updates, such as Git commit details and concurrency limits. (pkg/argocd/updater_config.go)Integration with ArgoCD:
kubernetesorargocd) for webhook server operations. (cmd/run.go,cmd/webhook.go) [1] [2]These changes collectively enhance the automation capabilities of the ArgoCD Image Updater by enabling it to respond to registry events and update applications accordingly.
I've also made sure to address concerns from the original PR:
enable-webhookand is off by defaultwebhook-portI have not update the manifests to add new Service and Ingress. If needed, let me know.
Disclaimer: the integration with Harbor is well tested and currently running on our own cluster. Unfortunately, I don't have access to Dockerhub and GHCR to test thoroughly, so any help is welcomed.