From d8dcc97f95865a8e8b3823d04b986e7928c8bb6b Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Fri, 21 Jul 2023 14:30:55 -0400 Subject: [PATCH] fix: webhook handler fails to refresh when alternate application namespaces are configured (#13976) (#14652) * 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 * 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. * fix: Use existing NamespacedName type Use the existing type instead of a custom type --------- Signed-off-by: Nikolas Skoufis Co-authored-by: Nik Skoufis --- util/webhook/webhook.go | 3 ++- util/webhook/webhook_test.go | 13 +++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/util/webhook/webhook.go b/util/webhook/webhook.go index c47323f2cdbbb..ca4742e31a1f1 100644 --- a/util/webhook/webhook.go +++ b/util/webhook/webhook.go @@ -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 diff --git a/util/webhook/webhook_test.go b/util/webhook/webhook_test.go index 899c3ecb73203..cf11162febc6c 100644 --- a/util/webhook/webhook_test.go +++ b/util/webhook/webhook_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io" + "k8s.io/apimachinery/pkg/types" "net/http" "net/http/httptest" "os" @@ -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 } @@ -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()