Skip to content

Commit

Permalink
fix(cli): fix tracking annotation diff for non-namespaced resources (a…
Browse files Browse the repository at this point in the history
…rgoproj#13924)

Signed-off-by: Maxime Brunet <[email protected]>
  • Loading branch information
maxbrunet committed Jul 12, 2023
1 parent 22281c5 commit 103a419
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 10 deletions.
10 changes: 5 additions & 5 deletions cmd/argocd/commands/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,7 @@ func findandPrintDiff(ctx context.Context, app *argoappv1.Application, proj *arg
items := make([]objKeyLiveTarget, 0)
if diffOptions.local != "" {
localObjs := groupObjsByKey(getLocalObjects(ctx, app, proj, diffOptions.local, diffOptions.localRepoRoot, argoSettings.AppLabelKey, diffOptions.cluster.Info.ServerVersion, diffOptions.cluster.Info.APIVersions, argoSettings.KustomizeOptions, argoSettings.TrackingMethod), liveObjs, app.Spec.Destination.Namespace)
items = groupObjsForDiff(resources, localObjs, items, argoSettings, app.InstanceName(argoSettings.ControllerNamespace))
items = groupObjsForDiff(resources, localObjs, items, argoSettings, app.InstanceName(argoSettings.ControllerNamespace), app.Spec.Destination.Namespace)
} else if diffOptions.revision != "" {
var unstructureds []*unstructured.Unstructured
for _, mfst := range diffOptions.res.Manifests {
Expand All @@ -1036,7 +1036,7 @@ func findandPrintDiff(ctx context.Context, app *argoappv1.Application, proj *arg
unstructureds = append(unstructureds, obj)
}
groupedObjs := groupObjsByKey(unstructureds, liveObjs, app.Spec.Destination.Namespace)
items = groupObjsForDiff(resources, groupedObjs, items, argoSettings, app.InstanceName(argoSettings.ControllerNamespace))
items = groupObjsForDiff(resources, groupedObjs, items, argoSettings, app.InstanceName(argoSettings.ControllerNamespace), app.Spec.Destination.Namespace)
} else if diffOptions.serversideRes != nil {
var unstructureds []*unstructured.Unstructured
for _, mfst := range diffOptions.serversideRes.Manifests {
Expand All @@ -1045,7 +1045,7 @@ func findandPrintDiff(ctx context.Context, app *argoappv1.Application, proj *arg
unstructureds = append(unstructureds, obj)
}
groupedObjs := groupObjsByKey(unstructureds, liveObjs, app.Spec.Destination.Namespace)
items = groupObjsForDiff(resources, groupedObjs, items, argoSettings, app.InstanceName(argoSettings.ControllerNamespace))
items = groupObjsForDiff(resources, groupedObjs, items, argoSettings, app.InstanceName(argoSettings.ControllerNamespace), app.Spec.Destination.Namespace)
} else {
for i := range resources.Items {
res := resources.Items[i]
Expand Down Expand Up @@ -1105,7 +1105,7 @@ func findandPrintDiff(ctx context.Context, app *argoappv1.Application, proj *arg
return foundDiffs
}

func groupObjsForDiff(resources *application.ManagedResourcesResponse, objs map[kube.ResourceKey]*unstructured.Unstructured, items []objKeyLiveTarget, argoSettings *settings.Settings, appName string) []objKeyLiveTarget {
func groupObjsForDiff(resources *application.ManagedResourcesResponse, objs map[kube.ResourceKey]*unstructured.Unstructured, items []objKeyLiveTarget, argoSettings *settings.Settings, appName, namespace string) []objKeyLiveTarget {
resourceTracking := argo.NewResourceTracking()
for _, res := range resources.Items {
var live = &unstructured.Unstructured{}
Expand All @@ -1120,7 +1120,7 @@ func groupObjsForDiff(resources *application.ManagedResourcesResponse, objs map[
}
if local, ok := objs[key]; ok || live != nil {
if local != nil && !kube.IsCRD(local) {
err = resourceTracking.SetAppInstance(local, argoSettings.AppLabelKey, appName, "", argoappv1.TrackingMethod(argoSettings.GetTrackingMethod()))
err = resourceTracking.SetAppInstance(local, argoSettings.AppLabelKey, appName, namespace, argoappv1.TrackingMethod(argoSettings.GetTrackingMethod()))
errors.CheckError(err)
}

Expand Down
34 changes: 33 additions & 1 deletion test/e2e/cluster_objects_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import (

"github.com/argoproj/gitops-engine/pkg/health"
. "github.com/argoproj/gitops-engine/pkg/sync/common"
"github.com/stretchr/testify/assert"

. "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
. "github.com/argoproj/argo-cd/v2/test/e2e/fixture"
. "github.com/argoproj/argo-cd/v2/test/e2e/fixture/app"
"github.com/argoproj/argo-cd/v2/util/argo"
)

// ensure that cluster scoped objects, like a cluster role, as a hok, can be successfully deployed
func TestClusterRoleBinding(t *testing.T) {
Given(t).
Path("cluster-role").
Expand All @@ -20,5 +22,35 @@ func TestClusterRoleBinding(t *testing.T) {
Then().
Expect(OperationPhaseIs(OperationSucceeded)).
Expect(HealthIs(health.HealthStatusHealthy)).
Expect(SyncStatusIs(SyncStatusCodeSynced)).
And(func(app *Application) {
diffOutput, err := RunCli("app", "diff", app.Name, "--revision=HEAD")
assert.NoError(t, err)
assert.Empty(t, diffOutput)
}).
When().
SetTrackingMethod(string(argo.TrackingMethodAnnotation)).
Sync().
Then().
Expect(OperationPhaseIs(OperationSucceeded)).
Expect(SyncStatusIs(SyncStatusCodeSynced)).
Expect(HealthIs(health.HealthStatusHealthy)).
And(func(app *Application) {
diffOutput, err := RunCli("app", "diff", app.Name, "--revision=HEAD")
assert.NoError(t, err)
assert.Empty(t, diffOutput)
})
}

// ensure that cluster scoped objects, like a cluster role, as a hook, can be successfully deployed
func TestClusterRoleBindingHook(t *testing.T) {
Given(t).
Path("cluster-role-hook").
When().
CreateApp().
Sync().
Then().
Expect(OperationPhaseIs(OperationSucceeded)).
Expect(HealthIs(health.HealthStatusHealthy)).
Expect(SyncStatusIs(SyncStatusCodeSynced))
}
15 changes: 15 additions & 0 deletions test/e2e/testdata/cluster-role-hook/cluster-role.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
namespace: cert-manager
name: my-cluster-role-binding
annotations:
argocd.argoproj.io/hook: PreSync
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: cluster-admin
subjects:
- kind: ServiceAccount
name: default
namespace: default
File renamed without changes.
5 changes: 1 addition & 4 deletions test/e2e/testdata/cluster-role/cluster-role.yaml
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
namespace: cert-manager
name: my-cluster-role-binding
annotations:
argocd.argoproj.io/hook: PreSync
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: cluster-admin
subjects:
- kind: ServiceAccount
name: default
namespace: default
namespace: default

0 comments on commit 103a419

Please sign in to comment.