fix: webhook handler fails to refresh when alternate application name…

…spaces are configured (argoproj#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 <[email protected]> * 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 <[email protected]> * fix: Use existing NamespacedName type Use the existing type instead of a custom type Signed-off-by: Nikolas Skoufis <[email protected]> --------- Signed-off-by: Nikolas Skoufis <[email protected]>
ornew · Jul 21, 2023 · 5120026 · 5120026
1 parent db52c26
commit 5120026
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 7 deletions.
diff --git 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
@@ -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()