Skip to content

Commit

Permalink
feat(controller)!: deprecate glob for authorized App annotation
Browse files Browse the repository at this point in the history 
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
  • Loading branch information
@hiddeco
hiddeco committed Oct 1, 2024
1 parent bb4eb2d commit de0d229
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 143 deletions.
37 changes: 12 additions & 25 deletions internal/controller/promotion/argocd.go
View file
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
View file
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: "argocd: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
38 changes: 13 additions & 25 deletions internal/directives/argocd_updater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -23,8 +23,6 @@ import (
)

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

applicationOperationInitiator = "kargo-controller"
freightCollectionInfoKey = "kargo.akuity.io/freight-collection"
)
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit de0d229

Please sign in to comment.