feat(controller)!: queue Stages for Application changes using annotat…

…ion (#2617)

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
(cherry picked from commit 0e79db7)

api/rbac/v1alpha1/annotations.go

@@ -1,20 +1,22 @@
 package v1alpha1
 
-import strings "strings"
+import "strings"
 
 const (
 	// AnnotationKeyManaged is an annotation key that can be set on a
 	// ServiceAccount, Role, or RoleBinding to indicate that it is managed by
 	// Kargo.
 	AnnotationKeyManaged = "rbac.kargo.akuity.io/managed"
 
-	// AnnotationKeyOIDCPrefix is the prefix of an annotation key that can be set on a
-	// ServiceAccount to associate it with any user authenticated via OIDC and having
-	// the claim indicated by the full annotation key with any of the values indicated by
-	// the annotation. The value of the annotation may be either a scalar string value or a
-	// comma-separated list.
+	// AnnotationKeyOIDCClaimNamePrefix is the prefix of an annotation key that
+	// can be set on a ServiceAccount to associate it with any user authenticated
+	// via OIDC and having the claim indicated by the full annotation key with
+	// any of the values indicated by the annotation. The value of the annotation
+	// may be either a scalar string value or a comma-separated list.
 	AnnotationKeyOIDCClaimNamePrefix = "rbac.kargo.akuity.io/claim."
 
+	// AnnotationValueTrue is a value that can be set on an annotation to indicate
+	// that it applies.
 	AnnotationValueTrue = "true"
 )
 

api/v1alpha1/annotations.go

@@ -32,6 +32,13 @@ const (
 	// resource.
 	AnnotationKeyDescription = "kargo.akuity.io/description"
 
+	// AnnotationKeyAuthorizedStage is an annotation key that can be set on a
+	// resource to indicate that a Stage is authorized to manage it. The value
+	// of the annotation should be in the format of "<project>:<stage>".
+	AnnotationKeyAuthorizedStage = "kargo.akuity.io/authorized-stage"
+
+	// AnnotationValueTrue is a value that can be set on an annotation to
+	// indicate that it applies.
 	AnnotationValueTrue = "true"
 )
 

go.mod

@@ -20,7 +20,6 @@ require (
 	github.com/evanphx/json-patch/v5 v5.9.0
 	github.com/fatih/structtag v1.2.0
 	github.com/fluxcd/pkg/kustomize v1.13.0
-	github.com/gobwas/glob v0.2.3
 	github.com/gogo/protobuf v1.3.2
 	github.com/golang-jwt/jwt/v5 v5.2.1
 	github.com/google/go-containerregistry v0.20.2
@@ -90,6 +89,7 @@ require (
 	github.com/fatih/color v1.16.0 // indirect
 	github.com/fxamacker/cbor/v2 v2.7.0 // indirect
 	github.com/go-gorp/gorp/v3 v3.1.0 // indirect
+	github.com/gobwas/glob v0.2.3 // indirect
 	github.com/gofrs/uuid v4.0.0+incompatible // indirect
 	github.com/google/go-github/v64 v64.0.0 // indirect
 	github.com/google/s2a-go v0.1.8 // indirect

internal/controller/promotion/argocd.go

@@ -7,7 +7,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/gobwas/glob"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -23,8 +22,6 @@ import (
 )
 
 const (
-	authorizedStageAnnotationKey = "kargo.akuity.io/authorized-stage"
-
 	applicationOperationInitiator = "kargo-controller"
 	freightCollectionInfoKey      = "kargo.akuity.io/freight-collection"
 )
@@ -561,44 +558,34 @@ func authorizeArgoCDAppUpdate(
 		stageMeta.Name,
 		stageMeta.Namespace,
 	)
-	if appMeta.Annotations == nil {
-		return permErr
-	}
-	allowedStage, ok := appMeta.Annotations[authorizedStageAnnotationKey]
+
+	allowedStage, ok := appMeta.Annotations[kargoapi.AnnotationKeyAuthorizedStage]
 	if !ok {
 		return permErr
 	}
+
 	tokens := strings.SplitN(allowedStage, ":", 2)
 	if len(tokens) != 2 {
 		return fmt.Errorf(
-			"unable to parse value of annotation %q (%q) on Argo CD Application "+
-				"%q in namespace %q",
-			authorizedStageAnnotationKey,
+			"unable to parse value of annotation %q (%q) on Argo CD Application %q in namespace %q",
+			kargoapi.AnnotationKeyAuthorizedStage,
 			allowedStage,
 			appMeta.Name,
 			appMeta.Namespace,
 		)
 	}
-	allowedNamespaceGlob, err := glob.Compile(tokens[0])
-	if err != nil {
-		return fmt.Errorf(
-			"Argo CD Application %q in namespace %q has invalid glob expression: %q",
-			appMeta.Name,
-			appMeta.Namespace,
-			tokens[0],
-		)
-	}
-	allowedNameGlob, err := glob.Compile(tokens[1])
-	if err != nil {
+
+	projectName, stageName := tokens[0], tokens[1]
+	if strings.Contains(projectName, "*") || strings.Contains(stageName, "*") {
 		return fmt.Errorf(
-			"Argo CD Application %q in namespace %q has invalid glob expression: %q",
+			"Argo CD Application %q in namespace %q has deprecated glob expression in annotation %q (%q)",
 			appMeta.Name,
 			appMeta.Namespace,
-			tokens[1],
+			kargoapi.AnnotationKeyAuthorizedStage,
+			allowedStage,
 		)
 	}
-	if !allowedNamespaceGlob.Match(stageMeta.Namespace) ||
-		!allowedNameGlob.Match(stageMeta.Name) {
+	if projectName != stageMeta.Namespace || stageName != stageMeta.Name {
 		return permErr
 	}
 	return nil

internal/controller/promotion/argocd_test.go

@@ -1198,7 +1198,7 @@ func TestArgoCDSyncApplication(t *testing.T) {
 					Name:      "fake-name",
 					Namespace: "fake-namespace",
 					Annotations: map[string]string{
-						authorizedStageAnnotationKey: "fake-namespace:fake-name",
+						kargoapi.AnnotationKeyAuthorizedStage: "fake-namespace:fake-name",
 					},
 				},
 			},
@@ -1224,7 +1224,7 @@ func TestArgoCDSyncApplication(t *testing.T) {
 					Name:      "fake-name",
 					Namespace: "fake-namespace",
 					Annotations: map[string]string{
-						authorizedStageAnnotationKey: "fake-namespace:fake-name",
+						kargoapi.AnnotationKeyAuthorizedStage: "fake-namespace:fake-name",
 					},
 				},
 			},
@@ -1402,7 +1402,7 @@ func TestArgoCDGetAuthorizedApplication(t *testing.T) {
 					Name:      "fake-name",
 					Namespace: "fake-namespace",
 					Annotations: map[string]string{
-						authorizedStageAnnotationKey: "fake-namespace:fake-stage",
+						kargoapi.AnnotationKeyAuthorizedStage: "fake-namespace:fake-stage",
 					},
 				},
 			},
@@ -1423,7 +1423,7 @@ func TestArgoCDGetAuthorizedApplication(t *testing.T) {
 					Name:      "fake-name",
 					Namespace: libargocd.Namespace(),
 					Annotations: map[string]string{
-						authorizedStageAnnotationKey: "*:fake-stage",
+						kargoapi.AnnotationKeyAuthorizedStage: "fake-namespace:fake-stage",
 					},
 				},
 			},
@@ -1460,9 +1460,12 @@ func TestArgoCDGetAuthorizedApplication(t *testing.T) {
 }
 
 func TestAuthorizeArgoCDAppUpdate(t *testing.T) {
-	permErr := "does not permit mutation"
-	parseErr := "unable to parse"
-	invalidGlobErr := "invalid glob expression"
+	const (
+		permErr           = "does not permit mutation"
+		parseErr          = "unable to parse"
+		deprecatedGlobErr = "deprecated glob expression"
+	)
+
 	testCases := []struct {
 		name    string
 		appMeta metav1.ObjectMeta
@@ -1484,7 +1487,7 @@ func TestAuthorizeArgoCDAppUpdate(t *testing.T) {
 			name: "annotation cannot be parsed",
 			appMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{
-					authorizedStageAnnotationKey: "bogus",
+					kargoapi.AnnotationKeyAuthorizedStage: "bogus",
 				},
 			},
 			errMsg: parseErr,
@@ -1493,7 +1496,7 @@ func TestAuthorizeArgoCDAppUpdate(t *testing.T) {
 			name: "mutation is not allowed",
 			appMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{
-					authorizedStageAnnotationKey: "ns-nope:name-nope",
+					kargoapi.AnnotationKeyAuthorizedStage: "ns-nope:name-nope",
 				},
 			},
 			errMsg: permErr,
@@ -1502,60 +1505,27 @@ func TestAuthorizeArgoCDAppUpdate(t *testing.T) {
 			name: "mutation is allowed",
 			appMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{
-					authorizedStageAnnotationKey: "ns-yep:name-yep",
+					kargoapi.AnnotationKeyAuthorizedStage: "ns-yep:name-yep",
 				},
 			},
 		},
 		{
 			name: "wildcard namespace with full name",
 			appMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{
-					authorizedStageAnnotationKey: "*:name-yep",
+					kargoapi.AnnotationKeyAuthorizedStage: "*:name-yep",
 				},
 			},
+			errMsg: deprecatedGlobErr,
 		},
 		{
 			name: "full namespace with wildcard name",
 			appMeta: metav1.ObjectMeta{
 				Annotations: map[string]string{
-					authorizedStageAnnotationKey: "ns-yep:*",
-				},
-			},
-		},
-		{
-			name: "partial wildcards in namespace and name",
-			appMeta: metav1.ObjectMeta{
-				Annotations: map[string]string{
-					authorizedStageAnnotationKey: "*-ye*:*-y*",
-				},
-			},
-		},
-		{
-			name: "wildcards do not match",
-			appMeta: metav1.ObjectMeta{
-				Annotations: map[string]string{
-					authorizedStageAnnotationKey: "*-nope:*-nope",
-				},
-			},
-			errMsg: permErr,
-		},
-		{
-			name: "invalid namespace glob",
-			appMeta: metav1.ObjectMeta{
-				Annotations: map[string]string{
-					authorizedStageAnnotationKey: "*[:*",
-				},
-			},
-			errMsg: invalidGlobErr,
-		},
-		{
-			name: "invalid name glob",
-			appMeta: metav1.ObjectMeta{
-				Annotations: map[string]string{
-					authorizedStageAnnotationKey: "*:*[",
+					kargoapi.AnnotationKeyAuthorizedStage: "ns-yep:*",
 				},
 			},
-			errMsg: invalidGlobErr,
+			errMsg: deprecatedGlobErr,
 		},
 	}
 	for _, testCase := range testCases {

internal/controller/stages/stages.go

@@ -247,20 +247,17 @@ func SetupReconcilerWithManager(
 	}
 
 	// Index Freight by Stages in which it has been verified
-	if err :=
-		kubeclient.IndexFreightByVerifiedStages(ctx, kargoMgr); err != nil {
+	if err := kubeclient.IndexFreightByVerifiedStages(ctx, kargoMgr); err != nil {
 		return fmt.Errorf("index Freight by Stages in which it has been verified: %w", err)
 	}
 
 	// Index Freight by Stages for which it has been approved
-	if err :=
-		kubeclient.IndexFreightByApprovedStages(ctx, kargoMgr); err != nil {
+	if err := kubeclient.IndexFreightByApprovedStages(ctx, kargoMgr); err != nil {
 		return fmt.Errorf("index Freight by Stages for which it has been approved: %w", err)
 	}
 
 	// Index Stages by upstream Stages
-	if err :=
-		kubeclient.IndexStagesByUpstreamStages(ctx, kargoMgr); err != nil {
+	if err := kubeclient.IndexStagesByUpstreamStages(ctx, kargoMgr); err != nil {
 		return fmt.Errorf("index Stages by upstream Stages: %w", err)
 	}
 
@@ -269,11 +266,6 @@ func SetupReconcilerWithManager(
 		return fmt.Errorf("index Stages by Warehouse: %w", err)
 	}
 
-	// Index Stages by Argo CD Applications
-	if err := kubeclient.IndexStagesByArgoCDApplications(ctx, kargoMgr, cfg.ShardName); err != nil {
-		return fmt.Errorf("index Stages by Argo CD Applications: %w", err)
-	}
-
 	// Index Stages by AnalysisRun
 	if err := kubeclient.IndexStagesByAnalysisRun(ctx, kargoMgr, cfg.ShardName); err != nil {
 		return fmt.Errorf("index Stages by Argo Rollouts AnalysisRun: %w", err)

internal/controller/stages/watches.go

@@ -3,6 +3,7 @@ package stages
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/fields"
@@ -329,26 +330,40 @@ func (u *updatedArgoCDAppHandler[T]) Update(
 ) {
 	if appHealthOrSyncStatusChanged(ctx, e) {
 		newApp := any(e.ObjectNew).(*argocd.Application) // nolint: forcetypeassert
+
+		stageRef, ok := newApp.Annotations[kargoapi.AnnotationKeyAuthorizedStage]
+		if !ok {
+			return
+		}
+		parts := strings.SplitN(stageRef, ":", 2)
+		if len(parts) != 2 {
+			return
+		}
+		projectName, stageName := parts[0], parts[1]
+
 		logger := logging.LoggerFromContext(ctx)
-		stages := &kargoapi.StageList{}
-		if err := u.kargoClient.List(
+		stage := &kargoapi.Stage{}
+		if err := u.kargoClient.Get(
 			ctx,
-			stages,
-			&client.ListOptions{
-				FieldSelector: fields.OneTermEqualSelector(
-					kubeclient.StagesByArgoCDApplicationsIndexField,
-					fmt.Sprintf("%s:%s", newApp.Namespace, newApp.Name),
-				),
-				LabelSelector: u.shardSelector,
+			types.NamespacedName{
+				Namespace: projectName,
+				Name:      stageName,
 			},
+			stage,
 		); err != nil {
-			logger.Error(
-				err, "error listing Stages for Application",
-				"app", newApp.Name,
-				"namespace", newApp.Namespace,
-			)
+			if client.IgnoreNotFound(err) != nil {
+				logger.Error(
+					err,
+					"error getting Stage for Application",
+					"namespace", projectName,
+					"stage", stageName,
+					"app", newApp.Name,
+				)
+			}
+			return
 		}
-		for _, stage := range stages.Items {
+
+		if u.shardSelector.Empty() || u.shardSelector.Matches(labels.Set(stage.Labels)) {
 			wq.Add(
 				reconcile.Request{
 					NamespacedName: types.NamespacedName{