diff --git a/controller/cache/cache.go b/controller/cache/cache.go index 42e1242099ffb..99f85d50bbe9e 100644 --- a/controller/cache/cache.go +++ b/controller/cache/cache.go @@ -242,6 +242,10 @@ func (c *liveStateCache) loadCacheSettings() (*cacheSettings, error) { if err != nil { return nil, err } + trackingMethod, err := c.settingsMgr.GetTrackingMethod() + if err != nil { + return nil, err + } installationID, err := c.settingsMgr.GetInstallationID() if err != nil { return nil, err @@ -267,7 +271,7 @@ func (c *liveStateCache) loadCacheSettings() (*cacheSettings, error) { ResourcesFilter: resourcesFilter, } - return &cacheSettings{clusterSettings, appInstanceLabelKey, argo.GetTrackingMethod(c.settingsMgr), installationID, resourceUpdatesOverrides, ignoreResourceUpdatesEnabled}, nil + return &cacheSettings{clusterSettings, appInstanceLabelKey, appv1.TrackingMethod(trackingMethod), installationID, resourceUpdatesOverrides, ignoreResourceUpdatesEnabled}, nil } func asResourceNode(r *clustercache.Resource) appv1.ResourceNode { diff --git a/controller/state.go b/controller/state.go index 2c73113986adc..1a2345f470a0a 100644 --- a/controller/state.go +++ b/controller/state.go @@ -163,6 +163,11 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp return nil, nil, false, fmt.Errorf("failed to get Helm settings: %w", err) } + trackingMethod, err := m.settingsMgr.GetTrackingMethod() + if err != nil { + return nil, nil, false, fmt.Errorf("failed to get trackingMethod: %w", err) + } + installationID, err := m.settingsMgr.GetInstallationID() if err != nil { return nil, nil, false, fmt.Errorf("failed to get installation ID: %w", err) @@ -249,7 +254,7 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp ApplicationSource: &source, KubeVersion: serverVersion, ApiVersions: apiVersions, - TrackingMethod: string(argo.GetTrackingMethod(m.settingsMgr)), + TrackingMethod: trackingMethod, RefSources: refSources, HasMultipleSources: app.Spec.HasMultipleSources(), InstallationID: installationID, @@ -286,7 +291,7 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp ApiVersions: apiVersions, VerifySignature: verifySignature, HelmRepoCreds: permittedHelmCredentials, - TrackingMethod: string(argo.GetTrackingMethod(m.settingsMgr)), + TrackingMethod: trackingMethod, EnabledSourceTypes: enabledSourceTypes, HelmOptions: helmOptions, HasMultipleSources: app.Spec.HasMultipleSources(), @@ -435,24 +440,28 @@ func normalizeClusterScopeTracking(targetObjs []*unstructured.Unstructured, info // getComparisonSettings will return the system level settings related to the // diff/normalization process. -func (m *appStateManager) getComparisonSettings() (string, map[string]v1alpha1.ResourceOverride, *settings.ResourcesFilter, string, error) { +func (m *appStateManager) getComparisonSettings() (string, map[string]v1alpha1.ResourceOverride, *settings.ResourcesFilter, string, string, error) { resourceOverrides, err := m.settingsMgr.GetResourceOverrides() if err != nil { - return "", nil, nil, "", err + return "", nil, nil, "", "", err } appLabelKey, err := m.settingsMgr.GetAppInstanceLabelKey() if err != nil { - return "", nil, nil, "", err + return "", nil, nil, "", "", err } resFilter, err := m.settingsMgr.GetResourcesFilter() if err != nil { - return "", nil, nil, "", err + return "", nil, nil, "", "", err } installationID, err := m.settingsMgr.GetInstallationID() if err != nil { - return "", nil, nil, "", err + return "", nil, nil, "", "", err } - return appLabelKey, resourceOverrides, resFilter, installationID, nil + trackingMethod, err := m.settingsMgr.GetTrackingMethod() + if err != nil { + return "", nil, nil, "", "", err + } + return appLabelKey, resourceOverrides, resFilter, installationID, trackingMethod, nil } // verifyGnuPGSignature verifies the result of a GnuPG operation for a given git @@ -503,7 +512,7 @@ func isManagedNamespace(ns *unstructured.Unstructured, app *v1alpha1.Application // revision and overrides in the app spec. func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localManifests []string, hasMultipleSources bool, rollback bool) (*comparisonResult, error) { ts := stats.NewTimingStats() - appLabelKey, resourceOverrides, resFilter, installationID, err := m.getComparisonSettings() + appLabelKey, resourceOverrides, resFilter, installationID, trackingMethod, err := m.getComparisonSettings() ts.AddCheckpoint("settings_ms") @@ -612,10 +621,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 infoProvider = &resourceInfoProviderStub{} } - trackingMethod := argo.GetTrackingMethod(m.settingsMgr) - err = normalizeClusterScopeTracking(targetObjs, infoProvider, func(u *unstructured.Unstructured) error { - return m.resourceTracking.SetAppInstance(u, appLabelKey, app.InstanceName(m.namespace), app.Spec.Destination.Namespace, trackingMethod, installationID) + return m.resourceTracking.SetAppInstance(u, appLabelKey, app.InstanceName(m.namespace), app.Spec.Destination.Namespace, v1alpha1.TrackingMethod(trackingMethod), installationID) }) if err != nil { msg := "Failed to normalize cluster-scoped resource tracking: " + err.Error() @@ -682,7 +689,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 for _, liveObj := range liveObjByKey { if liveObj != nil { - appInstanceName := m.resourceTracking.GetAppName(liveObj, appLabelKey, trackingMethod, installationID) + appInstanceName := m.resourceTracking.GetAppName(liveObj, appLabelKey, v1alpha1.TrackingMethod(trackingMethod), installationID) if appInstanceName != "" && appInstanceName != app.InstanceName(m.namespace) { fqInstanceName := strings.ReplaceAll(appInstanceName, "_", "/") conditions = append(conditions, v1alpha1.ApplicationCondition{ @@ -821,7 +828,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 } gvk := obj.GroupVersionKind() - isSelfReferencedObj := m.isSelfReferencedObj(liveObj, targetObj, app.GetName(), trackingMethod, installationID) + isSelfReferencedObj := m.isSelfReferencedObj(liveObj, targetObj, app.GetName(), v1alpha1.TrackingMethod(trackingMethod), installationID) resState := v1alpha1.ResourceStatus{ Namespace: obj.GetNamespace(), @@ -1145,7 +1152,7 @@ func (m *appStateManager) isSelfReferencedObj(live, config *unstructured.Unstruc // If tracking method doesn't contain required metadata for this check, // we are not able to determine and just assume the object to be managed. - if trackingMethod == argo.TrackingMethodLabel { + if trackingMethod == v1alpha1.TrackingMethodLabel { return true } diff --git a/controller/state_test.go b/controller/state_test.go index 4ee6389dc421a..e033a1b7cfef6 100644 --- a/controller/state_test.go +++ b/controller/state_test.go @@ -31,7 +31,6 @@ import ( "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v3/reposerver/apiclient" "github.com/argoproj/argo-cd/v3/test" - "github.com/argoproj/argo-cd/v3/util/argo" ) // TestCompareAppStateEmpty tests comparison when both git and live have no objects @@ -1423,8 +1422,8 @@ func TestIsLiveResourceManaged(t *testing.T) { configObj := managedObj.DeepCopy() // then - assert.True(t, manager.isSelfReferencedObj(managedObj, configObj, appName, argo.TrackingMethodLabel, "")) - assert.True(t, manager.isSelfReferencedObj(managedObj, configObj, appName, argo.TrackingMethodAnnotation, "")) + assert.True(t, manager.isSelfReferencedObj(managedObj, configObj, appName, v1alpha1.TrackingMethodLabel, "")) + assert.True(t, manager.isSelfReferencedObj(managedObj, configObj, appName, v1alpha1.TrackingMethodAnnotation, "")) }) t.Run("will return true if tracked with label", func(t *testing.T) { // given @@ -1432,43 +1431,43 @@ func TestIsLiveResourceManaged(t *testing.T) { configObj := managedObjWithLabel.DeepCopy() // then - assert.True(t, manager.isSelfReferencedObj(managedObjWithLabel, configObj, appName, argo.TrackingMethodLabel, "")) + assert.True(t, manager.isSelfReferencedObj(managedObjWithLabel, configObj, appName, v1alpha1.TrackingMethodLabel, "")) }) t.Run("will handle if trackingId has wrong resource name and config is nil", func(t *testing.T) { // given t.Parallel() // then - assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongName, nil, appName, argo.TrackingMethodLabel, "")) - assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongName, nil, appName, argo.TrackingMethodAnnotation, "")) + assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongName, nil, appName, v1alpha1.TrackingMethodLabel, "")) + assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongName, nil, appName, v1alpha1.TrackingMethodAnnotation, "")) }) t.Run("will handle if trackingId has wrong resource group and config is nil", func(t *testing.T) { // given t.Parallel() // then - assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongGroup, nil, appName, argo.TrackingMethodLabel, "")) - assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongGroup, nil, appName, argo.TrackingMethodAnnotation, "")) + assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongGroup, nil, appName, v1alpha1.TrackingMethodLabel, "")) + assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongGroup, nil, appName, v1alpha1.TrackingMethodAnnotation, "")) }) t.Run("will handle if trackingId has wrong kind and config is nil", func(t *testing.T) { // given t.Parallel() // then - assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongKind, nil, appName, argo.TrackingMethodLabel, "")) - assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongKind, nil, appName, argo.TrackingMethodAnnotation, "")) + assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongKind, nil, appName, v1alpha1.TrackingMethodLabel, "")) + assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongKind, nil, appName, v1alpha1.TrackingMethodAnnotation, "")) }) t.Run("will handle if trackingId has wrong namespace and config is nil", func(t *testing.T) { // given t.Parallel() // then - assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongNamespace, nil, appName, argo.TrackingMethodLabel, "")) - assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongNamespace, nil, appName, argo.TrackingMethodAnnotationAndLabel, "")) + assert.True(t, manager.isSelfReferencedObj(unmanagedObjWrongNamespace, nil, appName, v1alpha1.TrackingMethodLabel, "")) + assert.False(t, manager.isSelfReferencedObj(unmanagedObjWrongNamespace, nil, appName, v1alpha1.TrackingMethodAnnotationAndLabel, "")) }) t.Run("will return true if live is nil", func(t *testing.T) { t.Parallel() - assert.True(t, manager.isSelfReferencedObj(nil, nil, appName, argo.TrackingMethodAnnotation, "")) + assert.True(t, manager.isSelfReferencedObj(nil, nil, appName, v1alpha1.TrackingMethodAnnotation, "")) }) t.Run("will handle upgrade in desired state APIGroup", func(t *testing.T) { @@ -1478,7 +1477,7 @@ func TestIsLiveResourceManaged(t *testing.T) { delete(config.GetAnnotations(), common.AnnotationKeyAppInstance) // then - assert.True(t, manager.isSelfReferencedObj(managedWrongAPIGroup, config, appName, argo.TrackingMethodAnnotation, "")) + assert.True(t, manager.isSelfReferencedObj(managedWrongAPIGroup, config, appName, v1alpha1.TrackingMethodAnnotation, "")) }) } diff --git a/controller/sync.go b/controller/sync.go index 8e6e1982f2e36..7855ac144df7e 100644 --- a/controller/sync.go +++ b/controller/sync.go @@ -309,7 +309,11 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha log.Errorf("Could not get installation ID: %v", err) return } - trackingMethod := argo.GetTrackingMethod(m.settingsMgr) + trackingMethod, err := m.settingsMgr.GetTrackingMethod() + if err != nil { + log.Errorf("Could not get trackingMethod: %v", err) + return + } impersonationEnabled, err := m.settingsMgr.IsImpersonationEnabled() if err != nil { @@ -360,7 +364,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha return (len(syncOp.Resources) == 0 || isPostDeleteHook(target) || argo.ContainsSyncResource(key.Name, key.Namespace, schema.GroupVersionKind{Kind: key.Kind, Group: key.Group}, syncOp.Resources)) && - m.isSelfReferencedObj(live, target, app.GetName(), trackingMethod, installationID) + m.isSelfReferencedObj(live, target, app.GetName(), v1alpha1.TrackingMethod(trackingMethod), installationID) }), sync.WithManifestValidation(!syncOp.SyncOptions.HasOption(common.SyncOptionsDisableValidation)), sync.WithSyncWaveHook(delayBetweenSyncWaves), diff --git a/pkg/apis/application/v1alpha1/types.go b/pkg/apis/application/v1alpha1/types.go index 8577f181e808f..c6b31cd6829a4 100644 --- a/pkg/apis/application/v1alpha1/types.go +++ b/pkg/apis/application/v1alpha1/types.go @@ -102,6 +102,12 @@ func (id IgnoreDifferences) Equals(other IgnoreDifferences) bool { type TrackingMethod string +const ( + TrackingMethodAnnotation TrackingMethod = "annotation" + TrackingMethodLabel TrackingMethod = "label" + TrackingMethodAnnotationAndLabel TrackingMethod = "annotation+label" +) + // ResourceIgnoreDifferences contains resource filter and list of json paths which should be ignored during comparison with live state. type ResourceIgnoreDifferences struct { Group string `json:"group,omitempty" protobuf:"bytes,1,opt,name=group"` diff --git a/reposerver/cache/cache.go b/reposerver/cache/cache.go index 51c2f35657e6c..212d040d815ea 100644 --- a/reposerver/cache/cache.go +++ b/reposerver/cache/cache.go @@ -18,7 +18,6 @@ import ( appv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v3/reposerver/apiclient" - "github.com/argoproj/argo-cd/v3/util/argo" cacheutil "github.com/argoproj/argo-cd/v3/util/cache" "github.com/argoproj/argo-cd/v3/util/env" "github.com/argoproj/argo-cd/v3/util/hash" @@ -305,7 +304,7 @@ func manifestCacheKey(revision string, appSrc *appv1.ApplicationSource, srcRefs func trackingKey(appLabelKey string, trackingMethod string) string { trackingKey := appLabelKey - if text.FirstNonEmpty(trackingMethod, string(argo.TrackingMethodLabel)) != string(argo.TrackingMethodLabel) { + if text.FirstNonEmpty(trackingMethod, string(appv1.TrackingMethodLabel)) != string(appv1.TrackingMethodLabel) { trackingKey = trackingMethod + ":" + trackingKey } return trackingKey @@ -399,7 +398,7 @@ func (c *Cache) DeleteManifests(revision string, appSrc *appv1.ApplicationSource func appDetailsCacheKey(revision string, appSrc *appv1.ApplicationSource, srcRefs appv1.RefTargetRevisionMapping, trackingMethod appv1.TrackingMethod, refSourceCommitSHAs ResolvedRevisions) string { if trackingMethod == "" { - trackingMethod = argo.TrackingMethodLabel + trackingMethod = appv1.TrackingMethodLabel } return fmt.Sprintf("appdetails|%s|%d|%s", revision, appSourceKey(appSrc, srcRefs, refSourceCommitSHAs), trackingMethod) } diff --git a/server/application/application.go b/server/application/application.go index ce5e53466509d..77513d4fe18d1 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -528,6 +528,10 @@ func (s *Server) GetManifests(ctx context.Context, q *application.ApplicationMan if err != nil { return fmt.Errorf("error getting installation ID: %w", err) } + trackingMethod, err := s.settingsMgr.GetTrackingMethod() + if err != nil { + return fmt.Errorf("error getting trackingMethod from settings: %w", err) + } manifestInfo, err := client.GenerateManifest(ctx, &apiclient.ManifestRequest{ Repo: repo, @@ -542,7 +546,7 @@ func (s *Server) GetManifests(ctx context.Context, q *application.ApplicationMan ApiVersions: argo.APIResourcesToStrings(apiResources, true), HelmRepoCreds: helmCreds, HelmOptions: helmOptions, - TrackingMethod: string(argo.GetTrackingMethod(s.settingsMgr)), + TrackingMethod: trackingMethod, EnabledSourceTypes: enableGenerateManifests, ProjectName: proj.Name, ProjectSourceRepos: proj.Spec.SourceRepos, @@ -613,6 +617,11 @@ func (s *Server) GetManifestsWithFiles(stream application.ApplicationService_Get return fmt.Errorf("error getting app instance label key from settings: %w", err) } + trackingMethod, err := s.settingsMgr.GetTrackingMethod() + if err != nil { + return fmt.Errorf("error getting trackingMethod from settings: %w", err) + } + config, err := s.getApplicationClusterConfig(ctx, a) if err != nil { return fmt.Errorf("error getting application cluster config: %w", err) @@ -662,7 +671,7 @@ func (s *Server) GetManifestsWithFiles(stream application.ApplicationService_Get ApiVersions: argo.APIResourcesToStrings(apiResources, true), HelmRepoCreds: helmCreds, HelmOptions: helmOptions, - TrackingMethod: string(argo.GetTrackingMethod(s.settingsMgr)), + TrackingMethod: trackingMethod, EnabledSourceTypes: enableGenerateManifests, ProjectName: proj.Name, ProjectSourceRepos: proj.Spec.SourceRepos, @@ -777,6 +786,10 @@ func (s *Server) Get(ctx context.Context, q *application.ApplicationQuery) (*v1a if err != nil { return fmt.Errorf("error getting kustomize settings: %w", err) } + trackingMethod, err := s.settingsMgr.GetTrackingMethod() + if err != nil { + return fmt.Errorf("error getting trackingMethod from settings: %w", err) + } kustomizeOptions, err := kustomizeSettings.GetOptions(a.Spec.GetSource()) if err != nil { return fmt.Errorf("error getting kustomize settings options: %w", err) @@ -788,7 +801,7 @@ func (s *Server) Get(ctx context.Context, q *application.ApplicationQuery) (*v1a KustomizeOptions: kustomizeOptions, Repos: helmRepos, NoCache: true, - TrackingMethod: string(argo.GetTrackingMethod(s.settingsMgr)), + TrackingMethod: trackingMethod, EnabledSourceTypes: enabledSourceTypes, HelmOptions: helmOptions, }) diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go index a3d574a627f7a..33b43fa203794 100644 --- a/test/e2e/app_management_test.go +++ b/test/e2e/app_management_test.go @@ -1009,7 +1009,7 @@ func TestDuplicatedClusterResourcesAnnotationTracking(t *testing.T) { // (i.e. resources where metadata.namespace is set). Before the bugfix, this test would fail with a diff in the // tracking annotation. Given(t). - SetTrackingMethod(string(argo.TrackingMethodAnnotation)). + SetTrackingMethod(string(TrackingMethodAnnotation)). Path("duplicated-resources"). When(). CreateApp(). @@ -2600,7 +2600,7 @@ func TestSwitchTrackingMethod(t *testing.T) { ctx := Given(t) ctx. - SetTrackingMethod(string(argo.TrackingMethodAnnotation)). + SetTrackingMethod(string(TrackingMethodAnnotation)). Path("deployment"). When(). CreateApp(). @@ -2637,7 +2637,7 @@ func TestSwitchTrackingMethod(t *testing.T) { Expect(SyncStatusIs(SyncStatusCodeSynced)). Expect(HealthIs(health.HealthStatusHealthy)). When(). - SetTrackingMethod(string(argo.TrackingMethodLabel)). + SetTrackingMethod(string(TrackingMethodLabel)). Sync(). Then(). Expect(OperationPhaseIs(OperationSucceeded)). @@ -2692,7 +2692,7 @@ func TestSwitchTrackingMethod(t *testing.T) { func TestSwitchTrackingLabel(t *testing.T) { ctx := Given(t) - require.NoError(t, fixture.SetTrackingMethod(string(argo.TrackingMethodLabel))) + require.NoError(t, fixture.SetTrackingMethod(string(TrackingMethodLabel))) ctx. Path("deployment"). When(). @@ -2786,7 +2786,7 @@ func TestSwitchTrackingLabel(t *testing.T) { func TestAnnotationTrackingExtraResources(t *testing.T) { ctx := Given(t) - require.NoError(t, fixture.SetTrackingMethod(string(argo.TrackingMethodAnnotation))) + require.NoError(t, fixture.SetTrackingMethod(string(TrackingMethodAnnotation))) ctx. Path("deployment"). When(). @@ -2952,7 +2952,7 @@ data: func TestInstallationID(t *testing.T) { ctx := Given(t) ctx. - SetTrackingMethod(string(argo.TrackingMethodAnnotation)). + SetTrackingMethod(string(TrackingMethodAnnotation)). And(func() { _, err := fixture.KubeClientset.CoreV1().ConfigMaps(fixture.DeploymentNamespace()).Create( t.Context(), &corev1.ConfigMap{ diff --git a/test/e2e/cluster_objects_test.go b/test/e2e/cluster_objects_test.go index b06acce735490..790783609894f 100644 --- a/test/e2e/cluster_objects_test.go +++ b/test/e2e/cluster_objects_test.go @@ -11,7 +11,6 @@ import ( . "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" . "github.com/argoproj/argo-cd/v3/test/e2e/fixture" . "github.com/argoproj/argo-cd/v3/test/e2e/fixture/app" - "github.com/argoproj/argo-cd/v3/util/argo" ) func TestClusterRoleBinding(t *testing.T) { @@ -30,7 +29,7 @@ func TestClusterRoleBinding(t *testing.T) { assert.Empty(t, diffOutput) }). When(). - SetTrackingMethod(string(argo.TrackingMethodAnnotation)). + SetTrackingMethod(string(TrackingMethodAnnotation)). Sync(). Then(). Expect(OperationPhaseIs(OperationSucceeded)). diff --git a/test/e2e/deployment_test.go b/test/e2e/deployment_test.go index a827d45d34878..2bfbf228fbc4a 100644 --- a/test/e2e/deployment_test.go +++ b/test/e2e/deployment_test.go @@ -18,7 +18,6 @@ import ( "k8s.io/client-go/tools/clientcmd" "github.com/argoproj/argo-cd/v3/common" - "github.com/argoproj/argo-cd/v3/util/argo" "github.com/argoproj/argo-cd/v3/util/clusterauth" "github.com/argoproj/gitops-engine/pkg/health" @@ -54,7 +53,7 @@ func TestDeployment(t *testing.T) { func TestDeploymentWithAnnotationTrackingMode(t *testing.T) { ctx := Given(t) - require.NoError(t, SetTrackingMethod(string(argo.TrackingMethodAnnotation))) + require.NoError(t, SetTrackingMethod(string(TrackingMethodAnnotation))) ctx. Path("deployment"). When(). @@ -77,7 +76,7 @@ func TestDeploymentWithAnnotationTrackingMode(t *testing.T) { func TestDeploymentWithLabelTrackingMode(t *testing.T) { ctx := Given(t) - require.NoError(t, SetTrackingMethod(string(argo.TrackingMethodLabel))) + require.NoError(t, SetTrackingMethod(string(TrackingMethodLabel))) ctx. Path("deployment"). When(). diff --git a/test/e2e/fixture/app/context.go b/test/e2e/fixture/app/context.go index 435f8041723dc..f3343e7d1780c 100644 --- a/test/e2e/fixture/app/context.go +++ b/test/e2e/fixture/app/context.go @@ -11,7 +11,6 @@ import ( "github.com/argoproj/argo-cd/v3/test/e2e/fixture/certs" "github.com/argoproj/argo-cd/v3/test/e2e/fixture/gpgkeys" "github.com/argoproj/argo-cd/v3/test/e2e/fixture/repos" - "github.com/argoproj/argo-cd/v3/util/argo" "github.com/argoproj/argo-cd/v3/util/env" "github.com/argoproj/argo-cd/v3/util/settings" ) @@ -88,7 +87,7 @@ func GivenWithSameState(t *testing.T) *Context { timeout: timeout, project: "default", prune: true, - trackingMethod: argo.TrackingMethodLabel, + trackingMethod: v1alpha1.TrackingMethodLabel, } } diff --git a/util/argo/argo.go b/util/argo/argo.go index 428e951989734..bbe758a2826c6 100644 --- a/util/argo/argo.go +++ b/util/argo/argo.go @@ -773,6 +773,15 @@ func verifyGenerateManifests( continue } + trackingMethod, err := settingsMgr.GetTrackingMethod() + if err != nil { + conditions = append(conditions, argoappv1.ApplicationCondition{ + Type: argoappv1.ApplicationConditionInvalidSpecError, + Message: fmt.Sprintf("Error getting trackingMethod: %v", err), + }) + continue + } + verifySignature := false if len(proj.Spec.SignatureKeys) > 0 && gpg.IsGPGEnabled() { verifySignature = true @@ -798,7 +807,7 @@ func verifyGenerateManifests( ApiVersions: apiVersions, HelmOptions: helmOptions, HelmRepoCreds: repositoryCredentials, - TrackingMethod: string(GetTrackingMethod(settingsMgr)), + TrackingMethod: trackingMethod, EnabledSourceTypes: enableGenerateManifests, NoRevisionCache: true, HasMultipleSources: app.Spec.HasMultipleSources(), diff --git a/util/argo/resource_tracking.go b/util/argo/resource_tracking.go index ac6c5dab9607c..ac1535d52def8 100644 --- a/util/argo/resource_tracking.go +++ b/util/argo/resource_tracking.go @@ -12,13 +12,6 @@ import ( "github.com/argoproj/argo-cd/v3/common" "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v3/util/kube" - "github.com/argoproj/argo-cd/v3/util/settings" -) - -const ( - TrackingMethodAnnotation v1alpha1.TrackingMethod = "annotation" - TrackingMethodLabel v1alpha1.TrackingMethod = "label" - TrackingMethodAnnotationAndLabel v1alpha1.TrackingMethod = "annotation+label" ) var ( @@ -52,17 +45,8 @@ func NewResourceTracking() ResourceTracking { return &resourceTracking{} } -// GetTrackingMethod retrieve tracking method from settings -func GetTrackingMethod(settingsMgr *settings.SettingsManager) v1alpha1.TrackingMethod { - tm, err := settingsMgr.GetTrackingMethod() - if err != nil || tm == "" { - return TrackingMethodAnnotation - } - return v1alpha1.TrackingMethod(tm) -} - func IsOldTrackingMethod(trackingMethod string) bool { - return trackingMethod == "" || trackingMethod == string(TrackingMethodLabel) + return trackingMethod == "" || trackingMethod == string(v1alpha1.TrackingMethodLabel) } func (rt *resourceTracking) getAppInstanceValue(un *unstructured.Unstructured, installationID string) *AppInstanceValue { @@ -90,15 +74,15 @@ func (rt *resourceTracking) GetAppName(un *unstructured.Unstructured, key string return "" } switch trackingMethod { - case TrackingMethodLabel: + case v1alpha1.TrackingMethodLabel: label, err := kube.GetAppInstanceLabel(un, key) if err != nil { return "" } return label - case TrackingMethodAnnotationAndLabel: + case v1alpha1.TrackingMethodAnnotationAndLabel: return retrieveAppInstanceValue() - case TrackingMethodAnnotation: + case v1alpha1.TrackingMethodAnnotation: return retrieveAppInstanceValue() default: return retrieveAppInstanceValue() @@ -110,7 +94,7 @@ func (rt *resourceTracking) GetAppName(un *unstructured.Unstructured, key string // not be parsed, it returns nil. func (rt *resourceTracking) GetAppInstance(un *unstructured.Unstructured, trackingMethod v1alpha1.TrackingMethod, instanceID string) *AppInstanceValue { switch trackingMethod { - case TrackingMethodAnnotation, TrackingMethodAnnotationAndLabel: + case v1alpha1.TrackingMethodAnnotation, v1alpha1.TrackingMethodAnnotationAndLabel: return rt.getAppInstanceValue(un, instanceID) default: return nil @@ -152,15 +136,15 @@ func (rt *resourceTracking) SetAppInstance(un *unstructured.Unstructured, key, v return kube.SetAppInstanceAnnotation(un, common.AnnotationKeyAppInstance, rt.BuildAppInstanceValue(appInstanceValue)) } switch trackingMethod { - case TrackingMethodLabel: + case v1alpha1.TrackingMethodLabel: err := kube.SetAppInstanceLabel(un, key, val) if err != nil { return fmt.Errorf("failed to set app instance label: %w", err) } return nil - case TrackingMethodAnnotation: + case v1alpha1.TrackingMethodAnnotation: return setAppInstanceAnnotation() - case TrackingMethodAnnotationAndLabel: + case v1alpha1.TrackingMethodAnnotationAndLabel: err := setAppInstanceAnnotation() if err != nil { return err diff --git a/util/argo/resource_tracking_test.go b/util/argo/resource_tracking_test.go index 5c1f813bee889..582d610e2b3bb 100644 --- a/util/argo/resource_tracking_test.go +++ b/util/argo/resource_tracking_test.go @@ -12,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/argoproj/argo-cd/v3/common" + "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" ) func TestSetAppInstanceLabel(t *testing.T) { @@ -24,9 +25,9 @@ func TestSetAppInstanceLabel(t *testing.T) { resourceTracking := NewResourceTracking() - err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodLabel, "") + err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", v1alpha1.TrackingMethodLabel, "") require.NoError(t, err) - app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodLabel, "") + app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, v1alpha1.TrackingMethodLabel, "") assert.Equal(t, "my-app", app) } @@ -40,10 +41,10 @@ func TestSetAppInstanceAnnotation(t *testing.T) { resourceTracking := NewResourceTracking() - err = resourceTracking.SetAppInstance(&obj, common.AnnotationKeyAppInstance, "my-app", "", TrackingMethodAnnotation, "") + err = resourceTracking.SetAppInstance(&obj, common.AnnotationKeyAppInstance, "my-app", "", v1alpha1.TrackingMethodAnnotation, "") require.NoError(t, err) - app := resourceTracking.GetAppName(&obj, common.AnnotationKeyAppInstance, TrackingMethodAnnotation, "") + app := resourceTracking.GetAppName(&obj, common.AnnotationKeyAppInstance, v1alpha1.TrackingMethodAnnotation, "") assert.Equal(t, "my-app", app) } @@ -56,10 +57,10 @@ func TestSetAppInstanceAnnotationAndLabel(t *testing.T) { resourceTracking := NewResourceTracking() - err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodAnnotationAndLabel, "") + err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", v1alpha1.TrackingMethodAnnotationAndLabel, "") require.NoError(t, err) - app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel, "") + app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, v1alpha1.TrackingMethodAnnotationAndLabel, "") assert.Equal(t, "my-app", app) } @@ -72,11 +73,11 @@ func TestSetAppInstanceAnnotationAndLabelLongName(t *testing.T) { resourceTracking := NewResourceTracking() - err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app-with-an-extremely-long-name-that-is-over-sixty-three-characters", "", TrackingMethodAnnotationAndLabel, "") + err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app-with-an-extremely-long-name-that-is-over-sixty-three-characters", "", v1alpha1.TrackingMethodAnnotationAndLabel, "") require.NoError(t, err) // the annotation should still work, so the name from GetAppName should not be truncated - app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel, "") + app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, v1alpha1.TrackingMethodAnnotationAndLabel, "") assert.Equal(t, "my-app-with-an-extremely-long-name-that-is-over-sixty-three-characters", app) // the label should be truncated to 63 characters @@ -92,11 +93,11 @@ func TestSetAppInstanceAnnotationAndLabelLongNameBadEnding(t *testing.T) { resourceTracking := NewResourceTracking() - err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "the-very-suspicious-name-with-precisely-sixty-three-characters-with-hyphen", "", TrackingMethodAnnotationAndLabel, "") + err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "the-very-suspicious-name-with-precisely-sixty-three-characters-with-hyphen", "", v1alpha1.TrackingMethodAnnotationAndLabel, "") require.NoError(t, err) // the annotation should still work, so the name from GetAppName should not be truncated - app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel, "") + app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, v1alpha1.TrackingMethodAnnotationAndLabel, "") assert.Equal(t, "the-very-suspicious-name-with-precisely-sixty-three-characters-with-hyphen", app) // the label should be truncated to 63 characters, AND the hyphen should be removed @@ -112,7 +113,7 @@ func TestSetAppInstanceAnnotationAndLabelOutOfBounds(t *testing.T) { resourceTracking := NewResourceTracking() - err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "----------------------------------------------------------------", "", TrackingMethodAnnotationAndLabel, "") + err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "----------------------------------------------------------------", "", v1alpha1.TrackingMethodAnnotationAndLabel, "") // this should error because it can't truncate to a valid value assert.EqualError(t, err, "failed to set app instance label: unable to truncate label to not end with a special character") } @@ -127,7 +128,7 @@ func TestSetAppInstanceAnnotationNotFound(t *testing.T) { resourceTracking := NewResourceTracking() - app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotation, "") + app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, v1alpha1.TrackingMethodAnnotation, "") assert.Empty(t, app) } @@ -186,15 +187,15 @@ func TestResourceIdNormalizer_Normalize(t *testing.T) { // live object is a resource that has old style tracking label liveObj := sampleResource(t) - err := rt.SetAppInstance(liveObj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodLabel, "") + err := rt.SetAppInstance(liveObj, common.LabelKeyAppInstance, "my-app", "", v1alpha1.TrackingMethodLabel, "") require.NoError(t, err) // config object is a resource that has new style tracking annotation configObj := sampleResource(t) - err = rt.SetAppInstance(configObj, common.AnnotationKeyAppInstance, "my-app2", "", TrackingMethodAnnotation, "") + err = rt.SetAppInstance(configObj, common.AnnotationKeyAppInstance, "my-app2", "", v1alpha1.TrackingMethodAnnotation, "") require.NoError(t, err) - _ = rt.Normalize(configObj, liveObj, common.LabelKeyAppInstance, string(TrackingMethodAnnotation)) + _ = rt.Normalize(configObj, liveObj, common.LabelKeyAppInstance, string(v1alpha1.TrackingMethodAnnotation)) // the normalization should affect add the new style annotation and drop old tracking label from live object annotation, err := kube.GetAppInstanceAnnotation(configObj, common.AnnotationKeyAppInstance) @@ -243,7 +244,7 @@ func TestResourceIdNormalizer_NormalizeCRD(t *testing.T) { }, } - require.NoError(t, rt.Normalize(configObj, liveObj, common.LabelKeyAppInstance, string(TrackingMethodAnnotation))) + require.NoError(t, rt.Normalize(configObj, liveObj, common.LabelKeyAppInstance, string(v1alpha1.TrackingMethodAnnotation))) // the normalization should not apply any changes to the live object require.NotContains(t, liveObj.GetAnnotations(), common.AnnotationKeyAppInstance) } @@ -253,17 +254,17 @@ func TestResourceIdNormalizer_Normalize_ConfigHasOldLabel(t *testing.T) { // live object is a resource that has old style tracking label liveObj := sampleResource(t) - err := rt.SetAppInstance(liveObj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodLabel, "") + err := rt.SetAppInstance(liveObj, common.LabelKeyAppInstance, "my-app", "", v1alpha1.TrackingMethodLabel, "") require.NoError(t, err) // config object is a resource that has new style tracking annotation configObj := sampleResource(t) - err = rt.SetAppInstance(configObj, common.AnnotationKeyAppInstance, "my-app2", "", TrackingMethodAnnotation, "") + err = rt.SetAppInstance(configObj, common.AnnotationKeyAppInstance, "my-app2", "", v1alpha1.TrackingMethodAnnotation, "") require.NoError(t, err) - err = rt.SetAppInstance(configObj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodLabel, "") + err = rt.SetAppInstance(configObj, common.LabelKeyAppInstance, "my-app", "", v1alpha1.TrackingMethodLabel, "") require.NoError(t, err) - _ = rt.Normalize(configObj, liveObj, common.LabelKeyAppInstance, string(TrackingMethodAnnotation)) + _ = rt.Normalize(configObj, liveObj, common.LabelKeyAppInstance, string(v1alpha1.TrackingMethodAnnotation)) // the normalization should affect add the new style annotation and drop old tracking label from live object annotation, err := kube.GetAppInstanceAnnotation(configObj, common.AnnotationKeyAppInstance) @@ -274,5 +275,5 @@ func TestResourceIdNormalizer_Normalize_ConfigHasOldLabel(t *testing.T) { } func TestIsOldTrackingMethod(t *testing.T) { - assert.True(t, IsOldTrackingMethod(string(TrackingMethodLabel))) + assert.True(t, IsOldTrackingMethod(string(v1alpha1.TrackingMethodLabel))) } diff --git a/util/settings/settings.go b/util/settings/settings.go index e08ba50d9314f..2d398827c9135 100644 --- a/util/settings/settings.go +++ b/util/settings/settings.go @@ -827,7 +827,11 @@ func (mgr *SettingsManager) GetTrackingMethod() (string, error) { if err != nil { return "", err } - return argoCDCM.Data[settingsResourceTrackingMethodKey], nil + tm := argoCDCM.Data[settingsResourceTrackingMethodKey] + if tm == "" { + return string(v1alpha1.TrackingMethodAnnotation), nil + } + return tm, nil } func (mgr *SettingsManager) GetInstallationID() (string, error) { diff --git a/util/settings/settings_test.go b/util/settings/settings_test.go index 72e63db7e9b7b..4bef9c7cb06ae 100644 --- a/util/settings/settings_test.go +++ b/util/settings/settings_test.go @@ -210,12 +210,39 @@ func TestInClusterServerAddressEnabledByDefault(t *testing.T) { } func TestGetAppInstanceLabelKey(t *testing.T) { - _, settingsManager := fixtures(map[string]string{ - "application.instanceLabelKey": "testLabel", + t.Run("should get custom instanceLabelKey", func(t *testing.T) { + _, settingsManager := fixtures(map[string]string{ + "application.instanceLabelKey": "testLabel", + }) + label, err := settingsManager.GetAppInstanceLabelKey() + require.NoError(t, err) + assert.Equal(t, "testLabel", label) + }) + + t.Run("should get default instanceLabelKey if custom not defined", func(t *testing.T) { + _, settingsManager := fixtures(map[string]string{}) + label, err := settingsManager.GetAppInstanceLabelKey() + require.NoError(t, err) + assert.Equal(t, common.LabelKeyAppInstance, label) + }) +} + +func TestGetTrackingMethod(t *testing.T) { + t.Run("should get custom trackingMethod", func(t *testing.T) { + _, settingsManager := fixtures(map[string]string{ + "application.resourceTrackingMethod": string(v1alpha1.TrackingMethodLabel), + }) + label, err := settingsManager.GetTrackingMethod() + require.NoError(t, err) + assert.Equal(t, string(v1alpha1.TrackingMethodLabel), label) + }) + + t.Run("should get default trackingMethod if custom not defined", func(t *testing.T) { + _, settingsManager := fixtures(map[string]string{}) + label, err := settingsManager.GetTrackingMethod() + require.NoError(t, err) + assert.Equal(t, string(v1alpha1.TrackingMethodAnnotation), label) }) - label, err := settingsManager.GetAppInstanceLabelKey() - require.NoError(t, err) - assert.Equal(t, "testLabel", label) } func TestApplicationFineGrainedRBACInheritanceDisabledDefault(t *testing.T) {