From 57dc7dad1f979d3692ea912e1d6185e21322766f Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 1 Oct 2024 10:37:52 +0200 Subject: [PATCH 1/5] feat(api): add `kargo.akuity.io/authorized-stage` annotation Signed-off-by: Hidde Beydals --- api/rbac/v1alpha1/annotations.go | 14 ++++++++------ api/v1alpha1/annotations.go | 7 +++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/api/rbac/v1alpha1/annotations.go b/api/rbac/v1alpha1/annotations.go index ae05fc9f9..e648a2b3f 100644 --- a/api/rbac/v1alpha1/annotations.go +++ b/api/rbac/v1alpha1/annotations.go @@ -1,6 +1,6 @@ package v1alpha1 -import strings "strings" +import "strings" const ( // AnnotationKeyManaged is an annotation key that can be set on a @@ -8,13 +8,15 @@ const ( // 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" ) diff --git a/api/v1alpha1/annotations.go b/api/v1alpha1/annotations.go index 6df46b507..afe661a9d 100644 --- a/api/v1alpha1/annotations.go +++ b/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 ":". + AnnotationKeyAuthorizedStage = "kargo.akuity.io/authorized-stage" + + // AnnotationValueTrue is a value that can be set on an annotation to + // indicate that it applies. AnnotationValueTrue = "true" ) From c322a00751953a58cda36ad5b679b76b7789b3bf Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 1 Oct 2024 11:01:32 +0200 Subject: [PATCH 2/5] feat(controller)!: update ArgoCD app event handler to use annotation Signed-off-by: Hidde Beydals --- internal/controller/stages/watches.go | 45 ++++++++++++++++++--------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/internal/controller/stages/watches.go b/internal/controller/stages/watches.go index 1362d8414..bbd4aaeeb 100644 --- a/internal/controller/stages/watches.go +++ b/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{ From bb4eb2debad36116f112fdc6e3b94816685fea46 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 1 Oct 2024 11:20:06 +0200 Subject: [PATCH 3/5] chore(controller): remove Applications indexer Signed-off-by: Hidde Beydals --- internal/controller/stages/stages.go | 14 +--- internal/kubeclient/indexer.go | 58 ------------- internal/kubeclient/indexer_test.go | 117 --------------------------- 3 files changed, 3 insertions(+), 186 deletions(-) diff --git a/internal/controller/stages/stages.go b/internal/controller/stages/stages.go index c556ac651..2318f079a 100644 --- a/internal/controller/stages/stages.go +++ b/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) diff --git a/internal/kubeclient/indexer.go b/internal/kubeclient/indexer.go index 31c30bbbf..edc13576d 100644 --- a/internal/kubeclient/indexer.go +++ b/internal/kubeclient/indexer.go @@ -115,64 +115,6 @@ func indexStagesByAnalysisRun(shardName string) client.IndexerFunc { } } -// IndexStagesByArgoCDApplications sets up the indexing of Stages by the Argo CD -// Applications they are associated with. -// -// It configures the field indexer of the provided cluster to allow querying -// Stages by the Argo CD Applications they are associated with using the -// StagesByArgoCDApplicationsIndexField selector. -// -// When the provided shardName is non-empty, only Stages labeled with the -// provided shardName are indexed. When the provided shardName is empty, only -// Stages not labeled with a shardName are indexed. -func IndexStagesByArgoCDApplications(ctx context.Context, clstr cluster.Cluster, shardName string) error { - return clstr.GetFieldIndexer().IndexField( - ctx, - &kargoapi.Stage{}, - StagesByArgoCDApplicationsIndexField, - indexStagesByArgoCDApplications(shardName)) -} - -// indexStagesByArgoCDApplications returns a client.IndexerFunc that indexes -// Stages by the Argo CD Applications they are associated with. -// -// When the provided shardName is non-empty, only Stages labeled with the -// provided shardName are indexed. When the provided shardName is empty, only -// Stages not labeled with a shardName are indexed. -func indexStagesByArgoCDApplications(shardName string) client.IndexerFunc { - return func(obj client.Object) []string { - // Return early if: - // - // 1. This is the default controller, but the object is labeled for a - // specific shard. - // - // 2. This is a shard-specific controller, but the object is not labeled for - // this shard. - objShardName, labeled := obj.GetLabels()[kargoapi.ShardLabelKey] - if (shardName == "" && labeled) || - (shardName != "" && shardName != objShardName) { - return nil - } - - stage := obj.(*kargoapi.Stage) // nolint: forcetypeassert - // nolint: staticcheck - if stage.Spec.PromotionMechanisms == nil || len(stage.Spec.PromotionMechanisms.ArgoCDAppUpdates) == 0 { - return nil - } - // nolint: staticcheck - apps := make([]string, len(stage.Spec.PromotionMechanisms.ArgoCDAppUpdates)) - // nolint: staticcheck - for i, appCheck := range stage.Spec.PromotionMechanisms.ArgoCDAppUpdates { - namespace := appCheck.AppNamespace - if namespace == "" { - namespace = libargocd.Namespace() - } - apps[i] = fmt.Sprintf("%s:%s", namespace, appCheck.AppName) - } - return apps - } -} - // IndexPromotionsByStage sets up the indexing of Promotions by the Stage they // reference. // diff --git a/internal/kubeclient/indexer_test.go b/internal/kubeclient/indexer_test.go index df4540a35..3af58b4ac 100644 --- a/internal/kubeclient/indexer_test.go +++ b/internal/kubeclient/indexer_test.go @@ -237,123 +237,6 @@ func TestIndexStagesByAnalysisRun(t *testing.T) { } } -func TestIndexStagesByArgoCDApplications(t *testing.T) { - const testShardName = "test-shard" - t.Parallel() - testCases := []struct { - name string - controllerShardName string - stage *kargoapi.Stage - assertions func(*testing.T, []string) - }{ - { - name: "Stage belongs to another shard", - controllerShardName: testShardName, - stage: &kargoapi.Stage{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - kargoapi.ShardLabelKey: "another-shard", - }, - }, - Spec: kargoapi.StageSpec{ - PromotionMechanisms: &kargoapi.PromotionMechanisms{ - ArgoCDAppUpdates: []kargoapi.ArgoCDAppUpdate{ - { - AppNamespace: "fake-namespace", - AppName: "fake-app", - }, - }, - }, - }, - }, - assertions: func(t *testing.T, res []string) { - require.Nil(t, res) - }, - }, - { - name: "Stage belongs to this shard", - controllerShardName: testShardName, - stage: &kargoapi.Stage{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - kargoapi.ShardLabelKey: testShardName, - }, - }, - Spec: kargoapi.StageSpec{ - PromotionMechanisms: &kargoapi.PromotionMechanisms{ - ArgoCDAppUpdates: []kargoapi.ArgoCDAppUpdate{ - { - AppNamespace: "fake-namespace", - AppName: "fake-app", - }, - }, - }, - }, - }, - assertions: func(t *testing.T, res []string) { - require.Equal( - t, - []string{ - "fake-namespace:fake-app", - }, - res, - ) - }, - }, - { - name: "Stage is unlabeled and this is not the default controller", - controllerShardName: testShardName, - stage: &kargoapi.Stage{ - Spec: kargoapi.StageSpec{ - PromotionMechanisms: &kargoapi.PromotionMechanisms{ - ArgoCDAppUpdates: []kargoapi.ArgoCDAppUpdate{ - { - AppNamespace: "fake-namespace", - AppName: "fake-app", - }, - }, - }, - }, - }, - assertions: func(t *testing.T, res []string) { - require.Nil(t, res) - }, - }, - { - name: "Stage is unlabeled and this is the default controller", - controllerShardName: "", - stage: &kargoapi.Stage{ - Spec: kargoapi.StageSpec{ - PromotionMechanisms: &kargoapi.PromotionMechanisms{ - ArgoCDAppUpdates: []kargoapi.ArgoCDAppUpdate{ - { - AppNamespace: "fake-namespace", - AppName: "fake-app", - }, - }, - }, - }, - }, - assertions: func(t *testing.T, res []string) { - require.Equal( - t, - []string{ - "fake-namespace:fake-app", - }, - res, - ) - }, - }, - } - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - res := indexStagesByArgoCDApplications(tc.controllerShardName)(tc.stage) - tc.assertions(t, res) - }) - } -} - func TestIndexPromotionsByStage(t *testing.T) { testCases := map[string]struct { input *kargoapi.Promotion From 6cc228265f4c46320e8e5026de4a620a3d77bde3 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 1 Oct 2024 12:33:20 +0200 Subject: [PATCH 4/5] feat(controller)!: deprecate glob for authorized App annotation Signed-off-by: Hidde Beydals --- internal/controller/promotion/argocd.go | 37 ++++------- internal/controller/promotion/argocd_test.go | 64 ++++++-------------- internal/directives/argocd_updater.go | 38 ++++-------- internal/directives/argocd_updater_test.go | 62 +++++-------------- 4 files changed, 58 insertions(+), 143 deletions(-) diff --git a/internal/controller/promotion/argocd.go b/internal/controller/promotion/argocd.go index 636c8475b..c28049d2c 100644 --- a/internal/controller/promotion/argocd.go +++ b/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 diff --git a/internal/controller/promotion/argocd_test.go b/internal/controller/promotion/argocd_test.go index 421aa6439..87bdc8aad 100644 --- a/internal/controller/promotion/argocd_test.go +++ b/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,7 +1505,7 @@ 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", }, }, }, @@ -1510,52 +1513,19 @@ func TestAuthorizeArgoCDAppUpdate(t *testing.T) { 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 { diff --git a/internal/directives/argocd_updater.go b/internal/directives/argocd_updater.go index f070056d3..09dfc941c 100644 --- a/internal/directives/argocd_updater.go +++ b/internal/directives/argocd_updater.go @@ -7,13 +7,13 @@ import ( "strings" "time" - "github.com/gobwas/glob" "github.com/xeipuuv/gojsonschema" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client" + kargoapi "github.com/akuity/kargo/api/v1alpha1" libargocd "github.com/akuity/kargo/internal/argocd" argocd "github.com/akuity/kargo/internal/controller/argocd/api/v1alpha1" "github.com/akuity/kargo/internal/controller/freight" @@ -23,8 +23,6 @@ import ( ) const ( - authorizedStageAnnotationKey = "kargo.akuity.io/authorized-stage" - applicationOperationInitiator = "kargo-controller" freightCollectionInfoKey = "kargo.akuity.io/freight-collection" ) @@ -678,44 +676,34 @@ func (a *argocdUpdater) authorizeArgoCDAppUpdate( stepCtx.Stage, stepCtx.Project, ) - 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(stepCtx.Project) || - !allowedNameGlob.Match(stepCtx.Stage) { + if projectName != stepCtx.Project || stageName != stepCtx.Stage { return permErr } return nil diff --git a/internal/directives/argocd_updater_test.go b/internal/directives/argocd_updater_test.go index 192b4dc9b..02f4267ec 100644 --- a/internal/directives/argocd_updater_test.go +++ b/internal/directives/argocd_updater_test.go @@ -1289,7 +1289,7 @@ func Test_argoCDUpdater_syncApplication(t *testing.T) { Name: "fake-name", Namespace: "fake-namespace", Annotations: map[string]string{ - authorizedStageAnnotationKey: "fake-namespace:fake-name", + kargoapi.AnnotationKeyAuthorizedStage: "fake-namespace:fake-name", }, }, }, @@ -1324,7 +1324,7 @@ func Test_argoCDUpdater_syncApplication(t *testing.T) { Name: "fake-name", Namespace: "fake-namespace", Annotations: map[string]string{ - authorizedStageAnnotationKey: "fake-namespace:fake-name", + kargoapi.AnnotationKeyAuthorizedStage: "fake-namespace:fake-name", }, }, }, @@ -1504,7 +1504,7 @@ func Test_argoCDUpdater_getAuthorizedApplication(t *testing.T) { Name: "fake-app", Namespace: "fake-namespace", Annotations: map[string]string{ - authorizedStageAnnotationKey: "fake-namespace:fake-stage", + kargoapi.AnnotationKeyAuthorizedStage: "fake-namespace:fake-stage", }, }, }, @@ -1545,9 +1545,12 @@ func Test_argoCDUpdater_getAuthorizedApplication(t *testing.T) { } func Test_argoCDUpdater_authorizeArgoCDAppUpdate(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 @@ -1569,7 +1572,7 @@ func Test_argoCDUpdater_authorizeArgoCDAppUpdate(t *testing.T) { name: "annotation cannot be parsed", appMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - authorizedStageAnnotationKey: "bogus", + kargoapi.AnnotationKeyAuthorizedStage: "bogus", }, }, errMsg: parseErr, @@ -1578,7 +1581,7 @@ func Test_argoCDUpdater_authorizeArgoCDAppUpdate(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, @@ -1587,7 +1590,7 @@ func Test_argoCDUpdater_authorizeArgoCDAppUpdate(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", }, }, }, @@ -1595,52 +1598,19 @@ func Test_argoCDUpdater_authorizeArgoCDAppUpdate(t *testing.T) { 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, }, } From e117745e87126617e1a77c5a8b9dbb996087b36e Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 1 Oct 2024 14:13:46 +0200 Subject: [PATCH 5/5] chore: update Go Modules Signed-off-by: Hidde Beydals --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index e5d3a9b81..6812b3357 100644 --- a/go.mod +++ b/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