Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(controller)!: queue Stages for Application changes using annotation #2617

Merged
merged 5 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions api/rbac/v1alpha1/annotations.go
Original file line number Diff line number Diff line change
@@ -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."
Comment on lines -11 to 16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this.


// AnnotationValueTrue is a value that can be set on an annotation to indicate
// that it applies.
AnnotationValueTrue = "true"
)

Expand Down
7 changes: 7 additions & 0 deletions api/v1alpha1/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
37 changes: 12 additions & 25 deletions internal/controller/promotion/argocd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -23,8 +22,6 @@ import (
)

const (
authorizedStageAnnotationKey = "kargo.akuity.io/authorized-stage"

applicationOperationInitiator = "kargo-controller"
freightCollectionInfoKey = "kargo.akuity.io/freight-collection"
)
Expand Down Expand Up @@ -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
Expand Down
64 changes: 17 additions & 47 deletions internal/controller/promotion/argocd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
},
Expand All @@ -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",
},
},
},
Expand Down Expand Up @@ -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",
},
},
},
Expand All @@ -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",
},
},
},
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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 {
Expand Down
14 changes: 3 additions & 11 deletions internal/controller/stages/stages.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,20 +247,17 @@
}

// 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 {

Check warning on line 250 in internal/controller/stages/stages.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/stages.go#L250

Added line #L250 was not covered by tests
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 {

Check warning on line 255 in internal/controller/stages/stages.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/stages.go#L255

Added line #L255 was not covered by tests
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 {

Check warning on line 260 in internal/controller/stages/stages.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/stages.go#L260

Added line #L260 was not covered by tests
return fmt.Errorf("index Stages by upstream Stages: %w", err)
}

Expand All @@ -269,11 +266,6 @@
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)
Expand Down
45 changes: 30 additions & 15 deletions internal/controller/stages/watches.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import (
"context"
"fmt"
"strings"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/fields"
Expand Down Expand Up @@ -329,26 +330,40 @@
) {
if appHealthOrSyncStatusChanged(ctx, e) {
newApp := any(e.ObjectNew).(*argocd.Application) // nolint: forcetypeassert

stageRef, ok := newApp.Annotations[kargoapi.AnnotationKeyAuthorizedStage]
if !ok {
return

Check warning on line 336 in internal/controller/stages/watches.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/watches.go#L334-L336

Added lines #L334 - L336 were not covered by tests
}
parts := strings.SplitN(stageRef, ":", 2)
if len(parts) != 2 {
return

Check warning on line 340 in internal/controller/stages/watches.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/watches.go#L338-L340

Added lines #L338 - L340 were not covered by tests
}
projectName, stageName := parts[0], parts[1]

Check warning on line 342 in internal/controller/stages/watches.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/watches.go#L342

Added line #L342 was not covered by tests

logger := logging.LoggerFromContext(ctx)
stages := &kargoapi.StageList{}
if err := u.kargoClient.List(
stage := &kargoapi.Stage{}
if err := u.kargoClient.Get(

Check warning on line 346 in internal/controller/stages/watches.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/watches.go#L345-L346

Added lines #L345 - L346 were not covered by tests
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,

Check warning on line 350 in internal/controller/stages/watches.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/watches.go#L348-L350

Added lines #L348 - L350 were not covered by tests
},
stage,

Check warning on line 352 in internal/controller/stages/watches.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/watches.go#L352

Added line #L352 was not covered by tests
); 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,
)

Check warning on line 361 in internal/controller/stages/watches.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/watches.go#L354-L361

Added lines #L354 - L361 were not covered by tests
}
return

Check warning on line 363 in internal/controller/stages/watches.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/watches.go#L363

Added line #L363 was not covered by tests
}
for _, stage := range stages.Items {

if u.shardSelector.Empty() || u.shardSelector.Matches(labels.Set(stage.Labels)) {

Check warning on line 366 in internal/controller/stages/watches.go

View check run for this annotation

Codecov / codecov/patch

internal/controller/stages/watches.go#L366

Added line #L366 was not covered by tests
wq.Add(
reconcile.Request{
NamespacedName: types.NamespacedName{
Expand Down
Loading