Skip to content

Commit

Permalink
fix: webhook handler fails to refresh when alternate application name…
Browse files Browse the repository at this point in the history
…spaces are configured (#13976)

* fix: Add failing test for webhooks in all namespaces

This adds a failing test that properly exercises this functionality over
all namespaces. The issue with the code that is under test is that it
does not pass the namespace correctly to the patch of the application,
resulting in the patch not taking place in the correct namespace

Signed-off-by: Nikolas Skoufis <nskoufis@seek.com.au>

* fix: queue webhook refresh for apps in all namespaces

This passes the test in the previous commit, to ensure that webhooks
correctly refresh applications across all namespaces.

Signed-off-by: Nikolas Skoufis <nskoufis@seek.com.au>

* fix: Use existing NamespacedName type

Use the existing type instead of a custom type

Signed-off-by: Nikolas Skoufis <nskoufis@seek.com.au>

---------

Signed-off-by: Nikolas Skoufis <nskoufis@seek.com.au>
  • Loading branch information
Niksko authored Jul 21, 2023
1 parent db52c26 commit 5120026
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 7 deletions.
3 changes: 2 additions & 1 deletion util/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,8 @@ func (a *ArgoCDWebhookHandler) HandleEvent(payload interface{}) {
for _, source := range app.Spec.GetSources() {
if sourceRevisionHasChanged(source, revision, touchedHead) && sourceUsesURL(source, webURL, repoRegexp) {
if appFilesHaveChanged(&app, changedFiles) {
_, err = argo.RefreshApp(appIf, app.ObjectMeta.Name, v1alpha1.RefreshTypeNormal)
namespacedAppInterface := a.appClientset.ArgoprojV1alpha1().Applications(app.ObjectMeta.Namespace)
_, err = argo.RefreshApp(namespacedAppInterface, app.ObjectMeta.Name, v1alpha1.RefreshTypeNormal)
if err != nil {
log.Warnf("Failed to refresh app '%s' for controller reprocessing: %v", app.ObjectMeta.Name, err)
continue
Expand Down
13 changes: 7 additions & 6 deletions util/webhook/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"io"
"k8s.io/apimachinery/pkg/types"
"net/http"
"net/http/httptest"
"os"
Expand Down Expand Up @@ -149,10 +150,10 @@ func TestGitHubCommitEvent_MultiSource_Refresh(t *testing.T) {
func TestGitHubCommitEvent_AppsInOtherNamespaces(t *testing.T) {
hook := test.NewGlobal()

patchedApps := make([]string, 0, 3)
patchedApps := make([]types.NamespacedName, 0, 3)
reaction := func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
patchAction := action.(kubetesting.PatchAction)
patchedApps = append(patchedApps, patchAction.GetName())
patchedApps = append(patchedApps, types.NamespacedName{Name: patchAction.GetName(), Namespace: patchAction.GetNamespace()})
return true, nil, nil
}

Expand Down Expand Up @@ -231,10 +232,10 @@ func TestGitHubCommitEvent_AppsInOtherNamespaces(t *testing.T) {
assert.Contains(t, logMessages, "Requested app 'app-to-refresh-in-globbed-namespace' refresh")
assert.NotContains(t, logMessages, "Requested app 'app-to-ignore' refresh")

assert.Contains(t, patchedApps, "app-to-refresh-in-default-namespace")
assert.Contains(t, patchedApps, "app-to-refresh-in-exact-match-namespace")
assert.Contains(t, patchedApps, "app-to-refresh-in-globbed-namespace")
assert.NotContains(t, patchedApps, "app-to-ignore")
assert.Contains(t, patchedApps, types.NamespacedName{Name: "app-to-refresh-in-default-namespace", Namespace: "argocd"})
assert.Contains(t, patchedApps, types.NamespacedName{Name: "app-to-refresh-in-exact-match-namespace", Namespace: "end-to-end-tests"})
assert.Contains(t, patchedApps, types.NamespacedName{Name: "app-to-refresh-in-globbed-namespace", Namespace: "app-team-two"})
assert.NotContains(t, patchedApps, types.NamespacedName{Name: "app-to-ignore", Namespace: "kube-system"})
assert.Len(t, patchedApps, 3)

hook.Reset()
Expand Down

0 comments on commit 5120026

Please sign in to comment.