From e410c487806fd8c3366cf238f2dce04b797956e1 Mon Sep 17 00:00:00 2001 From: pashavictorovich Date: Wed, 6 Oct 2021 15:50:22 +0300 Subject: [PATCH 1/2] resource id normalizer Signed-off-by: pashavictorovich --- cmd/argocd/commands/admin/app.go | 6 +- cmd/argocd/commands/admin/settings.go | 2 +- cmd/argocd/commands/app.go | 2 +- controller/appcontroller.go | 6 +- controller/cache/cache.go | 8 +- controller/state.go | 12 +-- reposerver/cache/cache.go | 4 +- reposerver/repository/repository.go | 8 +- reposerver/repository/repository_test.go | 4 +- reposerver/server.go | 4 +- server/application/application.go | 6 +- test/e2e/deployment_test.go | 8 +- util/argo/argo.go | 6 +- util/argo/normalizer.go | 13 ++-- util/argo/normalizers/diff_normalizer.go | 2 +- .../argo/normalizers/knowntypes_normalizer.go | 2 +- .../normalizers/resource_id_normalizer.go | 75 +++++++++++++++++++ .../resource_tracking.go | 6 +- .../resource_tracking_test.go | 24 +++--- 19 files changed, 145 insertions(+), 53 deletions(-) create mode 100644 util/argo/normalizers/resource_id_normalizer.go rename util/{argo => resource_tracking}/resource_tracking.go (96%) rename util/{argo => resource_tracking}/resource_tracking_test.go (68%) diff --git a/cmd/argocd/commands/admin/app.go b/cmd/argocd/commands/admin/app.go index a44557a171e65..2749064d649d0 100644 --- a/cmd/argocd/commands/admin/app.go +++ b/cmd/argocd/commands/admin/app.go @@ -10,7 +10,7 @@ import ( "sort" "time" - "github.com/argoproj/argo-cd/v2/util/argo" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" "github.com/ghodss/yaml" "github.com/spf13/cobra" @@ -368,7 +368,7 @@ func reconcileApplications( ) appStateManager := controller.NewAppStateManager( - argoDB, appClientset, repoServerClient, namespace, kubeutil.NewKubectl(), settingsMgr, stateCache, projInformer, server, cache, time.Second, argo.NewResourceTracking()) + argoDB, appClientset, repoServerClient, namespace, kubeutil.NewKubectl(), settingsMgr, stateCache, projInformer, server, cache, time.Second, resource_tracking.NewResourceTracking()) appsList, err := appClientset.ArgoprojV1alpha1().Applications(namespace).List(context.Background(), v1.ListOptions{LabelSelector: selector}) if err != nil { @@ -410,5 +410,5 @@ func reconcileApplications( } func newLiveStateCache(argoDB db.ArgoDB, appInformer kubecache.SharedIndexInformer, settingsMgr *settings.SettingsManager, server *metrics.MetricsServer) cache.LiveStateCache { - return cache.NewLiveStateCache(argoDB, appInformer, settingsMgr, kubeutil.NewKubectl(), server, func(managedByApp map[string]bool, ref apiv1.ObjectReference) {}, nil, argo.NewResourceTracking()) + return cache.NewLiveStateCache(argoDB, appInformer, settingsMgr, kubeutil.NewKubectl(), server, func(managedByApp map[string]bool, ref apiv1.ObjectReference) {}, nil, resource_tracking.NewResourceTracking()) } diff --git a/cmd/argocd/commands/admin/settings.go b/cmd/argocd/commands/admin/settings.go index 78bf8da0839d0..b61b27b716aa0 100644 --- a/cmd/argocd/commands/admin/settings.go +++ b/cmd/argocd/commands/admin/settings.go @@ -411,7 +411,7 @@ argocd admin settings resource-overrides ignore-differences ./deploy.yaml --argo normalizedRes := res.DeepCopy() logs := collectLogs(func() { - errors.CheckError(normalizer.Normalize(normalizedRes)) + errors.CheckError(normalizer.Normalize(normalizedRes, nil, nil)) }) if logs != "" { _, _ = fmt.Println(logs) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index da601b7741e8b..8eb05de3064cb 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -885,7 +885,7 @@ func NewApplicationDiffCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co val := argoSettings.ResourceOverrides[k] overrides[k] = *val } - normalizer, err := argo.NewDiffNormalizer(app.Spec.IgnoreDifferences, overrides) + normalizer, err := argo.NewDiffNormalizer(app.Spec.IgnoreDifferences, overrides, argoSettings.GetTrackingMethod()) errors.CheckError(err) diffRes, err := diff.Diff(item.target, item.live, diff.WithNormalizer(normalizer)) diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 7d1bd9e04c4f3..5ce7576254ff8 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -14,6 +14,8 @@ import ( "sync" "time" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" + clustercache "github.com/argoproj/gitops-engine/pkg/cache" "github.com/argoproj/gitops-engine/pkg/diff" "github.com/argoproj/gitops-engine/pkg/health" @@ -195,8 +197,8 @@ func NewApplicationController( return nil, err } } - stateCache := statecache.NewLiveStateCache(db, appInformer, ctrl.settingsMgr, kubectl, ctrl.metricsServer, ctrl.handleObjectUpdated, clusterFilter, argo.NewResourceTracking()) - appStateManager := NewAppStateManager(db, applicationClientset, repoClientset, namespace, kubectl, ctrl.settingsMgr, stateCache, projInformer, ctrl.metricsServer, argoCache, ctrl.statusRefreshTimeout, argo.NewResourceTracking()) + stateCache := statecache.NewLiveStateCache(db, appInformer, ctrl.settingsMgr, kubectl, ctrl.metricsServer, ctrl.handleObjectUpdated, clusterFilter, resource_tracking.NewResourceTracking()) + appStateManager := NewAppStateManager(db, applicationClientset, repoClientset, namespace, kubectl, ctrl.settingsMgr, stateCache, projInformer, ctrl.metricsServer, argoCache, ctrl.statusRefreshTimeout, resource_tracking.NewResourceTracking()) ctrl.appInformer = appInformer ctrl.appLister = appLister ctrl.projInformer = projInformer diff --git a/controller/cache/cache.go b/controller/cache/cache.go index c17c1199acb7f..70854f2d2059f 100644 --- a/controller/cache/cache.go +++ b/controller/cache/cache.go @@ -8,6 +8,8 @@ import ( "sync" "time" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" + clustercache "github.com/argoproj/gitops-engine/pkg/cache" "github.com/argoproj/gitops-engine/pkg/health" "github.com/argoproj/gitops-engine/pkg/utils/kube" @@ -106,7 +108,7 @@ func NewLiveStateCache( metricsServer *metrics.MetricsServer, onObjectUpdated ObjectUpdatedHandler, clusterFilter func(cluster *appv1.Cluster) bool, - resourceTracking argo.ResourceTracking) LiveStateCache { + resourceTracking resource_tracking.ResourceTracking) LiveStateCache { return &liveStateCache{ appInformer: appInformer, @@ -136,7 +138,7 @@ type liveStateCache struct { settingsMgr *settings.SettingsManager metricsServer *metrics.MetricsServer clusterFilter func(cluster *appv1.Cluster) bool - resourceTracking argo.ResourceTracking + resourceTracking resource_tracking.ResourceTracking // listSemaphore is used to limit the number of concurrent memory consuming operations on the // k8s list queries results across all clusters to avoid memory spikes during cache initialization. @@ -288,7 +290,7 @@ func (c *liveStateCache) getCluster(server string) (clustercache.ClusterCache, e return nil, fmt.Errorf("controller is configured to ignore cluster %s", cluster.Server) } - trackingMethod := argo.GetTrackingMethod(c.settingsMgr) + trackingMethod := resource_tracking.GetTrackingMethod(c.settingsMgr) clusterCache = clustercache.NewClusterCache(cluster.RESTConfig(), clustercache.SetListSemaphore(c.listSemaphore), clustercache.SetResyncTimeout(K8SClusterResyncDuration), diff --git a/controller/state.go b/controller/state.go index a5c121d484897..4cbd343f75782 100644 --- a/controller/state.go +++ b/controller/state.go @@ -7,6 +7,8 @@ import ( "reflect" "time" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" + "github.com/argoproj/gitops-engine/pkg/diff" "github.com/argoproj/gitops-engine/pkg/health" "github.com/argoproj/gitops-engine/pkg/sync" @@ -98,7 +100,7 @@ type appStateManager struct { cache *appstatecache.Cache namespace string statusRefreshTimeout time.Duration - resourceTracking argo.ResourceTracking + resourceTracking resource_tracking.ResourceTracking } func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, source v1alpha1.ApplicationSource, appLabelKey, revision string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, *apiclient.ManifestResponse, error) { @@ -176,7 +178,7 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, source v1alpha1 ApiVersions: argo.APIGroupsToVersions(apiGroups), VerifySignature: verifySignature, HelmRepoCreds: permittedHelmCredentials, - TrackingMethod: string(argo.GetTrackingMethod(m.settingsMgr)), + TrackingMethod: string(resource_tracking.GetTrackingMethod(m.settingsMgr)), }) if err != nil { return nil, nil, err @@ -259,7 +261,7 @@ func (m *appStateManager) getComparisonSettings(app *appv1.Application) (string, if err != nil { return "", nil, nil, nil, err } - diffNormalizer, err := argo.NewDiffNormalizer(app.Spec.IgnoreDifferences, resourceOverrides) + diffNormalizer, err := argo.NewDiffNormalizer(app.Spec.IgnoreDifferences, resourceOverrides, string(resource_tracking.GetTrackingMethod(m.settingsMgr))) if err != nil { return "", nil, nil, nil, err } @@ -463,7 +465,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *ap } } - trackingMethod := argo.GetTrackingMethod(m.settingsMgr) + trackingMethod := resource_tracking.GetTrackingMethod(m.settingsMgr) for _, liveObj := range liveObjByKey { if liveObj != nil { @@ -686,7 +688,7 @@ func NewAppStateManager( metricsServer *metrics.MetricsServer, cache *appstatecache.Cache, statusRefreshTimeout time.Duration, - resourceTracking argo.ResourceTracking, + resourceTracking resource_tracking.ResourceTracking, ) AppStateManager { return &appStateManager{ liveStateCache: liveStateCache, diff --git a/reposerver/cache/cache.go b/reposerver/cache/cache.go index 265d9a0aabab6..1cb472637754f 100644 --- a/reposerver/cache/cache.go +++ b/reposerver/cache/cache.go @@ -10,7 +10,7 @@ import ( "strings" "time" - "github.com/argoproj/argo-cd/v2/util/argo" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" "github.com/go-git/go-git/v5/plumbing" "github.com/go-redis/redis/v8" @@ -194,7 +194,7 @@ func (c *Cache) DeleteManifests(revision string, appSrc *appv1.ApplicationSource func appDetailsCacheKey(revision string, appSrc *appv1.ApplicationSource, trackingMethod appv1.TrackingMethod) string { if trackingMethod == "" { - trackingMethod = argo.TrackingMethodLabel + trackingMethod = resource_tracking.TrackingMethodLabel } return fmt.Sprintf("appdetails|%s|%d|%s", revision, appSourceKey(appSrc), trackingMethod) } diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index a6b40f06c5a6f..15b7c60a94bf4 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -16,7 +16,7 @@ import ( "strings" "time" - "github.com/argoproj/argo-cd/v2/util/argo" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" "github.com/Masterminds/semver" "github.com/TomOnTime/utfutil" @@ -69,7 +69,7 @@ type Service struct { cache *reposervercache.Cache parallelismLimitSemaphore *semaphore.Weighted metricsServer *metrics.MetricsServer - resourceTracking argo.ResourceTracking + resourceTracking resource_tracking.ResourceTracking newGitClient func(rawRepoURL string, creds git.Creds, insecure bool, enableLfs bool, proxy string, opts ...git.ClientOpts) (git.Client, error) newHelmClient func(repoURL string, creds helm.Creds, enableOci bool, proxy string, opts ...helm.ClientOpts) helm.Client initConstants RepoServerInitConstants @@ -85,7 +85,7 @@ type RepoServerInitConstants struct { } // NewService returns a new instance of the Manifest service -func NewService(metricsServer *metrics.MetricsServer, cache *reposervercache.Cache, initConstants RepoServerInitConstants, resourceTracking argo.ResourceTracking) *Service { +func NewService(metricsServer *metrics.MetricsServer, cache *reposervercache.Cache, initConstants RepoServerInitConstants, resourceTracking resource_tracking.ResourceTracking) *Service { var parallelismLimitSemaphore *semaphore.Weighted if initConstants.ParallelismLimit > 0 { parallelismLimitSemaphore = semaphore.NewWeighted(initConstants.ParallelismLimit) @@ -715,7 +715,7 @@ func GenerateManifests(appPath, repoRoot, revision string, q *apiclient.Manifest var targetObjs []*unstructured.Unstructured var dest *v1alpha1.ApplicationDestination - resourceTracking := argo.NewResourceTracking() + resourceTracking := resource_tracking.NewResourceTracking() appSourceType, err := GetAppSourceType(q.ApplicationSource, appPath, q.AppName) if err != nil { return nil, err diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index 6b70b9aa0eb23..9cee16a39ac62 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -14,7 +14,7 @@ import ( "testing" "time" - "github.com/argoproj/argo-cd/v2/util/argo" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" "github.com/ghodss/yaml" "github.com/stretchr/testify/assert" @@ -72,7 +72,7 @@ func newServiceWithOpt(cf clientFunc) (*Service, *gitmocks.Client) { cacheutil.NewCache(cacheutil.NewInMemoryCache(1*time.Minute)), 1*time.Minute, 1*time.Minute, - ), RepoServerInitConstants{ParallelismLimit: 1}, argo.NewResourceTracking()) + ), RepoServerInitConstants{ParallelismLimit: 1}, resource_tracking.NewResourceTracking()) chart := "my-chart" version := "1.1.0" diff --git a/reposerver/server.go b/reposerver/server.go index ecc86ca95de65..7207c061ea1f6 100644 --- a/reposerver/server.go +++ b/reposerver/server.go @@ -5,7 +5,7 @@ import ( "fmt" "os" - "github.com/argoproj/argo-cd/v2/util/argo" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" @@ -94,7 +94,7 @@ func (a *ArgoCDRepoServer) CreateGRPC() *grpc.Server { versionpkg.RegisterVersionServiceServer(server, version.NewServer(nil, func() (bool, error) { return true, nil })) - manifestService := repository.NewService(a.metricsServer, a.cache, a.initConstants, argo.NewResourceTracking()) + manifestService := repository.NewService(a.metricsServer, a.cache, a.initConstants, resource_tracking.NewResourceTracking()) apiclient.RegisterRepoServerServiceServer(server, manifestService) healthService := health.NewServer() diff --git a/server/application/application.go b/server/application/application.go index d621649059477..7fd23cd3e6276 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -11,6 +11,8 @@ import ( "strings" "time" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" + "github.com/Masterminds/semver" "github.com/argoproj/gitops-engine/pkg/diff" "github.com/argoproj/gitops-engine/pkg/sync/common" @@ -331,7 +333,7 @@ func (s *Server) GetManifests(ctx context.Context, q *application.ApplicationMan KubeVersion: serverVersion, ApiVersions: argo.APIGroupsToVersions(apiGroups), HelmRepoCreds: helmCreds, - TrackingMethod: string(argoutil.GetTrackingMethod(s.settingsMgr)), + TrackingMethod: string(resource_tracking.GetTrackingMethod(s.settingsMgr)), }) return err }) @@ -415,7 +417,7 @@ func (s *Server) Get(ctx context.Context, q *application.ApplicationQuery) (*app KustomizeOptions: kustomizeOptions, Repos: helmRepos, NoCache: true, - TrackingMethod: string(argoutil.GetTrackingMethod(s.settingsMgr)), + TrackingMethod: string(resource_tracking.GetTrackingMethod(s.settingsMgr)), }) return err }); err != nil { diff --git a/test/e2e/deployment_test.go b/test/e2e/deployment_test.go index acc913c5fe7fc..f21e0c6e12810 100644 --- a/test/e2e/deployment_test.go +++ b/test/e2e/deployment_test.go @@ -4,9 +4,9 @@ import ( "fmt" "testing" - "github.com/stretchr/testify/assert" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" - "github.com/argoproj/argo-cd/v2/util/argo" + "github.com/stretchr/testify/assert" "github.com/argoproj/gitops-engine/pkg/health" . "github.com/argoproj/gitops-engine/pkg/sync/common" @@ -41,7 +41,7 @@ func TestDeployment(t *testing.T) { func TestDeploymentWithAnnotationTrackingMode(t *testing.T) { ctx := Given(t) - SetTrackingMethod(string(argo.TrackingMethodAnnotation)) + SetTrackingMethod(string(resource_tracking.TrackingMethodAnnotation)) ctx. Path("deployment"). When(). @@ -64,7 +64,7 @@ func TestDeploymentWithAnnotationTrackingMode(t *testing.T) { func TestDeploymentWithLabelTrackingMode(t *testing.T) { ctx := Given(t) - SetTrackingMethod(string(argo.TrackingMethodLabel)) + SetTrackingMethod(string(resource_tracking.TrackingMethodLabel)) ctx. Path("deployment"). When(). diff --git a/util/argo/argo.go b/util/argo/argo.go index e567636cead4a..966f34dd256da 100644 --- a/util/argo/argo.go +++ b/util/argo/argo.go @@ -7,6 +7,8 @@ import ( "strings" "time" + "github.com/argoproj/argo-cd/v2/util/resource_tracking" + "github.com/r3labs/diff" "github.com/argoproj/gitops-engine/pkg/utils/kube" @@ -245,7 +247,7 @@ func ValidateRepo( KustomizeOptions: kustomizeOptions, // don't use case during application change to make sure to fetch latest git/helm revisions NoRevisionCache: true, - TrackingMethod: string(GetTrackingMethod(settingsMgr)), + TrackingMethod: string(resource_tracking.GetTrackingMethod(settingsMgr)), }) if err != nil { conditions = append(conditions, argoappv1.ApplicationCondition{ @@ -499,7 +501,7 @@ func verifyGenerateManifests( KubeVersion: kubeVersion, ApiVersions: apiVersions, HelmRepoCreds: repositoryCredentials, - TrackingMethod: string(GetTrackingMethod(settingsMgr)), + TrackingMethod: string(resource_tracking.GetTrackingMethod(settingsMgr)), } req.Repo.CopyCredentialsFromRepo(repoRes) req.Repo.CopySettingsFrom(repoRes) diff --git a/util/argo/normalizer.go b/util/argo/normalizer.go index 7b1e0d2c36477..91c227dde8690 100644 --- a/util/argo/normalizer.go +++ b/util/argo/normalizer.go @@ -9,7 +9,7 @@ import ( ) // NewDiffNormalizer creates normalizer that uses Argo CD and application settings to normalize the resource prior to diffing. -func NewDiffNormalizer(ignore []v1alpha1.ResourceIgnoreDifferences, overrides map[string]v1alpha1.ResourceOverride) (diff.Normalizer, error) { +func NewDiffNormalizer(ignore []v1alpha1.ResourceIgnoreDifferences, overrides map[string]v1alpha1.ResourceOverride, trackingMethod string) (diff.Normalizer, error) { ignoreNormalizer, err := normalizers.NewIgnoreNormalizer(ignore, overrides) if err != nil { return nil, err @@ -18,8 +18,11 @@ func NewDiffNormalizer(ignore []v1alpha1.ResourceIgnoreDifferences, overrides ma if err != nil { return nil, err } - - return &composableNormalizer{normalizers: []diff.Normalizer{ignoreNormalizer, knownTypesNorm}}, nil + resourceIdNormalizer, err := normalizers.NewResourceIdNormalizer(trackingMethod) + if err != nil { + return nil, err + } + return &composableNormalizer{normalizers: []diff.Normalizer{ignoreNormalizer, knownTypesNorm, resourceIdNormalizer}}, nil } type composableNormalizer struct { @@ -27,9 +30,9 @@ type composableNormalizer struct { } // Normalize performs resource normalization. -func (n *composableNormalizer) Normalize(un *unstructured.Unstructured) error { +func (n *composableNormalizer) Normalize(un, config, live *unstructured.Unstructured) error { for i := range n.normalizers { - if err := n.normalizers[i].Normalize(un); err != nil { + if err := n.normalizers[i].Normalize(un, config, live); err != nil { return err } } diff --git a/util/argo/normalizers/diff_normalizer.go b/util/argo/normalizers/diff_normalizer.go index 41061ea6c15ae..9819b251d90c1 100644 --- a/util/argo/normalizers/diff_normalizer.go +++ b/util/argo/normalizers/diff_normalizer.go @@ -151,7 +151,7 @@ func NewIgnoreNormalizer(ignore []v1alpha1.ResourceIgnoreDifferences, overrides } // Normalize removes fields from supplied resource using json paths from matching items of specified resources ignored differences list -func (n *ignoreNormalizer) Normalize(un *unstructured.Unstructured) error { +func (n *ignoreNormalizer) Normalize(un, config, live *unstructured.Unstructured) error { matched := make([]normalizerPatch, 0) for _, patch := range n.patches { groupKind := un.GroupVersionKind().GroupKind() diff --git a/util/argo/normalizers/knowntypes_normalizer.go b/util/argo/normalizers/knowntypes_normalizer.go index 403065c58a6df..ca105299621a0 100644 --- a/util/argo/normalizers/knowntypes_normalizer.go +++ b/util/argo/normalizers/knowntypes_normalizer.go @@ -145,7 +145,7 @@ func remarshal(fieldVal interface{}, field knownTypeField) (interface{}, error) // Normalize re-format custom resource fields using built-in Kubernetes types JSON marshaler. // This technique allows avoiding false drift detections in CRDs that import data structures from Kubernetes codebase. -func (n *knownTypesNormalizer) Normalize(un *unstructured.Unstructured) error { +func (n *knownTypesNormalizer) Normalize(un, config, live *unstructured.Unstructured) error { if fields, ok := n.typeFields[un.GroupVersionKind().GroupKind()]; ok { for _, field := range fields { err := normalize(un.Object, field, field.fieldPath) diff --git a/util/argo/normalizers/resource_id_normalizer.go b/util/argo/normalizers/resource_id_normalizer.go new file mode 100644 index 0000000000000..a990eb539e93b --- /dev/null +++ b/util/argo/normalizers/resource_id_normalizer.go @@ -0,0 +1,75 @@ +package normalizers + +import ( + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/argoproj/argo-cd/v2/util/resource_tracking" + + "github.com/argoproj/argo-cd/v2/common" + "github.com/argoproj/argo-cd/v2/util/kube" +) + +type resourceIdNormalizer struct { + trackingMethod string +} + +func init() { + +} + +// NewResourceIdNormalizer +func NewResourceIdNormalizer(trackingMethod string) (*resourceIdNormalizer, error) { + normalizer := resourceIdNormalizer{trackingMethod: trackingMethod} + return &normalizer, nil +} + +// Normalize re-format custom resource fields using built-in Kubernetes types JSON marshaler. +// This technique allows avoiding false drift detections in CRDs that import data structures from Kubernetes codebase. +func (n *resourceIdNormalizer) Normalize(origin, config, live *unstructured.Unstructured) error { + if resource_tracking.IsOldTrackingMethod(n.trackingMethod) { + return nil + } + + if live == nil { + return nil + } + + if n.trackingMethod == string(resource_tracking.TrackingMethodAnnotation) { + annotation := kube.GetAppInstanceAnnotation(origin, common.AnnotationKeyAppInstance) + _ = kube.SetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance, annotation) + if config != nil { + _ = kube.SetAppInstanceAnnotation(config, common.AnnotationKeyAppInstance, annotation) + _ = kube.SetAppInstanceLabel(config, common.LabelKeyAppInstance, kube.GetAppInstanceLabel(live, common.LabelKeyAppInstance)) + } + } + + if n.trackingMethod == string(resource_tracking.TrackingMethodAnnotationAndLabel) { + annotation := kube.GetAppInstanceAnnotation(origin, common.AnnotationKeyAppInstance) + _ = kube.SetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance, annotation) + if config != nil { + _ = kube.SetAppInstanceAnnotation(config, common.AnnotationKeyAppInstance, annotation) + } + } + + // + //label := kube.GetAppInstanceLabel(origin, common.LabelKeyAppInstance) + // + //lannotation := kube.GetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance) + //llabel := kube.GetAppInstanceLabel(live, common.LabelKeyAppInstance) + // + //if annotation != "" { + //} + // + //if label != "" { + // _ = kube.SetAppInstanceLabel(live, common.LabelKeyAppInstance, label) + //} + // + //if lannotation != "" { + // _ = kube.SetAppInstanceAnnotation(origin, common.AnnotationKeyAppInstance, lannotation) + //} + // + //if llabel != "" { + // _ = kube.SetAppInstanceLabel(origin, common.LabelKeyAppInstance, llabel) + //} + return nil +} diff --git a/util/argo/resource_tracking.go b/util/resource_tracking/resource_tracking.go similarity index 96% rename from util/argo/resource_tracking.go rename to util/resource_tracking/resource_tracking.go index d55d088add2d1..5dc0c480a49b5 100644 --- a/util/argo/resource_tracking.go +++ b/util/resource_tracking/resource_tracking.go @@ -1,4 +1,4 @@ -package argo +package resource_tracking import ( "fmt" @@ -57,6 +57,10 @@ func GetTrackingMethod(settingsMgr *settings.SettingsManager) v1alpha1.TrackingM return v1alpha1.TrackingMethod(tm) } +func IsOldTrackingMethod(trackingMethod string) bool { + return trackingMethod == "" || trackingMethod == string(TrackingMethodLabel) +} + // GetAppName retrieve application name base on tracking method func (rt *resourceTracking) GetAppName(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod) string { retrieveAppInstanceValue := func() string { diff --git a/util/argo/resource_tracking_test.go b/util/resource_tracking/resource_tracking_test.go similarity index 68% rename from util/argo/resource_tracking_test.go rename to util/resource_tracking/resource_tracking_test.go index 2d12a4459c005..46950c3ae8fb7 100644 --- a/util/argo/resource_tracking_test.go +++ b/util/resource_tracking/resource_tracking_test.go @@ -1,4 +1,4 @@ -package argo +package resource_tracking import ( "io/ioutil" @@ -21,9 +21,9 @@ func TestSetAppInstanceLabel(t *testing.T) { resourceTracking := NewResourceTracking() - err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodLabel) + err = SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodLabel) assert.Nil(t, err) - app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodLabel) + app := GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodLabel) assert.Equal(t, "my-app", app) } @@ -37,10 +37,10 @@ func TestSetAppInstanceAnnotation(t *testing.T) { resourceTracking := NewResourceTracking() - err = resourceTracking.SetAppInstance(&obj, common.AnnotationKeyAppInstance, "my-app", "", TrackingMethodAnnotation) + err = SetAppInstance(&obj, common.AnnotationKeyAppInstance, "my-app", "", TrackingMethodAnnotation) assert.Nil(t, err) - app := resourceTracking.GetAppName(&obj, common.AnnotationKeyAppInstance, TrackingMethodAnnotation) + app := GetAppName(&obj, common.AnnotationKeyAppInstance, TrackingMethodAnnotation) assert.Equal(t, "my-app", app) } @@ -53,10 +53,10 @@ func TestSetAppInstanceAnnotationAndLabel(t *testing.T) { resourceTracking := NewResourceTracking() - err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodAnnotationAndLabel) + err = SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodAnnotationAndLabel) assert.Nil(t, err) - app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel) + app := GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel) assert.Equal(t, "my-app", app) } @@ -70,13 +70,13 @@ func TestSetAppInstanceAnnotationNotFound(t *testing.T) { resourceTracking := NewResourceTracking() - app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotation) + app := GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotation) assert.Equal(t, "", app) } func TestParseAppInstanceValue(t *testing.T) { resourceTracking := NewResourceTracking() - appInstanceValue, err := resourceTracking.ParseAppInstanceValue("app:/:/") + appInstanceValue, err := ParseAppInstanceValue("app:/:/") if !assert.NoError(t, err) { t.Fatal() } @@ -89,18 +89,18 @@ func TestParseAppInstanceValue(t *testing.T) { func TestParseAppInstanceValueWrongFormat1(t *testing.T) { resourceTracking := NewResourceTracking() - _, err := resourceTracking.ParseAppInstanceValue("app") + _, err := ParseAppInstanceValue("app") assert.Error(t, err, WrongResourceTrackingFormat) } func TestParseAppInstanceValueWrongFormat2(t *testing.T) { resourceTracking := NewResourceTracking() - _, err := resourceTracking.ParseAppInstanceValue("app;group/kind/ns") + _, err := ParseAppInstanceValue("app;group/kind/ns") assert.Error(t, err, WrongResourceTrackingFormat) } func TestParseAppInstanceValueCorrectFormat(t *testing.T) { resourceTracking := NewResourceTracking() - _, err := resourceTracking.ParseAppInstanceValue("app:group/kind:test/ns") + _, err := ParseAppInstanceValue("app:group/kind:test/ns") assert.NoError(t, err) } From f7aca36305924ca1eb3157849c232280b3703178 Mon Sep 17 00:00:00 2001 From: pashavictorovich Date: Wed, 6 Oct 2021 17:47:20 +0300 Subject: [PATCH 2/2] add tests Signed-off-by: pashavictorovich --- util/argo/normalizers/diff_normalizer_test.go | 16 +++--- .../normalizers/knowntypes_normalizer_test.go | 10 ++-- .../normalizers/resource_id_normalizer.go | 49 ++++++------------- .../resource_id_normalizer_test.go | 44 +++++++++++++++++ util/argo/normalizers/testdata/svc.yaml | 11 +++++ .../resource_tracking_test.go | 22 ++++----- util/resource_tracking/testdata/svc.yaml | 11 +++++ 7 files changed, 104 insertions(+), 59 deletions(-) create mode 100644 util/argo/normalizers/resource_id_normalizer_test.go create mode 100644 util/argo/normalizers/testdata/svc.yaml create mode 100644 util/resource_tracking/testdata/svc.yaml diff --git a/util/argo/normalizers/diff_normalizer_test.go b/util/argo/normalizers/diff_normalizer_test.go index 74ce5cb33136a..02be8e4d83a15 100644 --- a/util/argo/normalizers/diff_normalizer_test.go +++ b/util/argo/normalizers/diff_normalizer_test.go @@ -26,7 +26,7 @@ func TestNormalizeObjectWithMatchedGroupKind(t *testing.T) { assert.Nil(t, err) assert.True(t, has) - err = normalizer.Normalize(deployment) + err = normalizer.Normalize(deployment, nil, nil) assert.Nil(t, err) _, has, err = unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "containers") assert.Nil(t, err) @@ -44,7 +44,7 @@ func TestNormalizeNoMatchedGroupKinds(t *testing.T) { deployment := test.NewDeployment() - err = normalizer.Normalize(deployment) + err = normalizer.Normalize(deployment, nil, nil) assert.Nil(t, err) _, hasSpec, err := unstructured.NestedMap(deployment.Object, "spec") @@ -67,7 +67,7 @@ func TestNormalizeMatchedResourceOverrides(t *testing.T) { assert.Nil(t, err) assert.True(t, has) - err = normalizer.Normalize(deployment) + err = normalizer.Normalize(deployment, nil, nil) assert.Nil(t, err) _, has, err = unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "containers") assert.Nil(t, err) @@ -105,14 +105,14 @@ func TestNormalizeMissingJsonPointer(t *testing.T) { deployment := test.NewDeployment() - err = normalizer.Normalize(deployment) + err = normalizer.Normalize(deployment, nil, nil) assert.NoError(t, err) crd := unstructured.Unstructured{} err = yaml.Unmarshal([]byte(testCRDYAML), &crd) assert.NoError(t, err) - err = normalizer.Normalize(&crd) + err = normalizer.Normalize(&crd, nil, nil) assert.NoError(t, err) } @@ -131,7 +131,7 @@ func TestNormalizeGlobMatch(t *testing.T) { assert.Nil(t, err) assert.True(t, has) - err = normalizer.Normalize(deployment) + err = normalizer.Normalize(deployment, nil, nil) assert.Nil(t, err) _, has, err = unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "containers") assert.Nil(t, err) @@ -160,7 +160,7 @@ func TestNormalizeJQPathExpression(t *testing.T) { assert.True(t, has) assert.Len(t, actualInitContainers, 2) - err = normalizer.Normalize(deployment) + err = normalizer.Normalize(deployment, nil, nil) assert.Nil(t, err) actualInitContainers, has, err = unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "initContainers") assert.Nil(t, err) @@ -197,7 +197,7 @@ func TestNormalizeJQPathExpressionWithError(t *testing.T) { originalDeployment, err := deployment.MarshalJSON() assert.Nil(t, err) - err = normalizer.Normalize(deployment) + err = normalizer.Normalize(deployment, nil, nil) assert.Nil(t, err) normalizedDeployment, err := deployment.MarshalJSON() diff --git a/util/argo/normalizers/knowntypes_normalizer_test.go b/util/argo/normalizers/knowntypes_normalizer_test.go index ce6acfeff4c32..c43924d2310b6 100644 --- a/util/argo/normalizers/knowntypes_normalizer_test.go +++ b/util/argo/normalizers/knowntypes_normalizer_test.go @@ -82,7 +82,7 @@ func TestNormalize_MapField(t *testing.T) { rollout := mustUnmarshalYAML(someCRDYaml) - err = normalizer.Normalize(rollout) + err = normalizer.Normalize(rollout, nil, nil) if !assert.NoError(t, err) { return } @@ -123,7 +123,7 @@ func TestNormalize_FieldInNestedSlice(t *testing.T) { return } - err = normalizer.Normalize(rollout) + err = normalizer.Normalize(rollout, nil, nil) if !assert.NoError(t, err) { return } @@ -175,7 +175,7 @@ spec: return } - err = normalizer.Normalize(rollout) + err = normalizer.Normalize(rollout, nil, nil) if !assert.NoError(t, err) { return } @@ -216,7 +216,7 @@ spec: return } - err = normalizer.Normalize(rollout) + err = normalizer.Normalize(rollout, nil, nil) if !assert.NoError(t, err) { return } @@ -242,7 +242,7 @@ func TestFieldDoesNotExist(t *testing.T) { return } - err = normalizer.Normalize(rollout) + err = normalizer.Normalize(rollout, nil, nil) if !assert.NoError(t, err) { return } diff --git a/util/argo/normalizers/resource_id_normalizer.go b/util/argo/normalizers/resource_id_normalizer.go index a990eb539e93b..567b899c52066 100644 --- a/util/argo/normalizers/resource_id_normalizer.go +++ b/util/argo/normalizers/resource_id_normalizer.go @@ -3,10 +3,10 @@ package normalizers import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "github.com/argoproj/argo-cd/v2/util/resource_tracking" - "github.com/argoproj/argo-cd/v2/common" "github.com/argoproj/argo-cd/v2/util/kube" + + "github.com/argoproj/argo-cd/v2/util/resource_tracking" ) type resourceIdNormalizer struct { @@ -34,42 +34,21 @@ func (n *resourceIdNormalizer) Normalize(origin, config, live *unstructured.Unst return nil } - if n.trackingMethod == string(resource_tracking.TrackingMethodAnnotation) { - annotation := kube.GetAppInstanceAnnotation(origin, common.AnnotationKeyAppInstance) - _ = kube.SetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance, annotation) - if config != nil { - _ = kube.SetAppInstanceAnnotation(config, common.AnnotationKeyAppInstance, annotation) - _ = kube.SetAppInstanceLabel(config, common.LabelKeyAppInstance, kube.GetAppInstanceLabel(live, common.LabelKeyAppInstance)) - } + annotation := kube.GetAppInstanceAnnotation(origin, common.AnnotationKeyAppInstance) + err := kube.SetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance, annotation) + if err != nil { + return err } - - if n.trackingMethod == string(resource_tracking.TrackingMethodAnnotationAndLabel) { - annotation := kube.GetAppInstanceAnnotation(origin, common.AnnotationKeyAppInstance) - _ = kube.SetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance, annotation) - if config != nil { - _ = kube.SetAppInstanceAnnotation(config, common.AnnotationKeyAppInstance, annotation) + if config != nil { + err = kube.SetAppInstanceAnnotation(config, common.AnnotationKeyAppInstance, annotation) + if err != nil { + return err + } + err = kube.SetAppInstanceLabel(config, common.LabelKeyAppInstance, kube.GetAppInstanceLabel(live, common.LabelKeyAppInstance)) + if err != nil { + return err } } - // - //label := kube.GetAppInstanceLabel(origin, common.LabelKeyAppInstance) - // - //lannotation := kube.GetAppInstanceAnnotation(live, common.AnnotationKeyAppInstance) - //llabel := kube.GetAppInstanceLabel(live, common.LabelKeyAppInstance) - // - //if annotation != "" { - //} - // - //if label != "" { - // _ = kube.SetAppInstanceLabel(live, common.LabelKeyAppInstance, label) - //} - // - //if lannotation != "" { - // _ = kube.SetAppInstanceAnnotation(origin, common.AnnotationKeyAppInstance, lannotation) - //} - // - //if llabel != "" { - // _ = kube.SetAppInstanceLabel(origin, common.LabelKeyAppInstance, llabel) - //} return nil } diff --git a/util/argo/normalizers/resource_id_normalizer_test.go b/util/argo/normalizers/resource_id_normalizer_test.go new file mode 100644 index 0000000000000..d9f708fc3a74b --- /dev/null +++ b/util/argo/normalizers/resource_id_normalizer_test.go @@ -0,0 +1,44 @@ +package normalizers + +import ( + "io/ioutil" + "testing" + + "github.com/argoproj/argo-cd/v2/util/kube" + + "github.com/argoproj/argo-cd/v2/util/resource_tracking" + + "github.com/ghodss/yaml" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/argoproj/argo-cd/v2/common" +) + +func TestResourceIdNormalizer_Normalize(t *testing.T) { + yamlBytes, err := ioutil.ReadFile("testdata/svc.yaml") + assert.Nil(t, err) + var obj *unstructured.Unstructured + err = yaml.Unmarshal(yamlBytes, &obj) + assert.Nil(t, err) + + resourceTracking := resource_tracking.NewResourceTracking() + + err = resourceTracking.SetAppInstance(obj, common.LabelKeyAppInstance, "my-app", "", resource_tracking.TrackingMethodLabel) + + yamlBytes, err = ioutil.ReadFile("testdata/svc.yaml") + assert.Nil(t, err) + var obj2 *unstructured.Unstructured + err = yaml.Unmarshal(yamlBytes, &obj2) + assert.Nil(t, err) + + err = resourceTracking.SetAppInstance(obj2, common.AnnotationKeyAppInstance, "my-app2", "", resource_tracking.TrackingMethodAnnotation) + + normalizer, _ := NewResourceIdNormalizer("annotation") + + _ = normalizer.Normalize(obj2, nil, obj) + + annotation := kube.GetAppInstanceAnnotation(obj2, common.AnnotationKeyAppInstance) + + assert.Equal(t, obj.GetAnnotations()[common.AnnotationKeyAppInstance], annotation) +} diff --git a/util/argo/normalizers/testdata/svc.yaml b/util/argo/normalizers/testdata/svc.yaml new file mode 100644 index 0000000000000..11aeb957db7c3 --- /dev/null +++ b/util/argo/normalizers/testdata/svc.yaml @@ -0,0 +1,11 @@ +kind: Service +apiVersion: v1 +metadata: + name: my-service +spec: + selector: + app: MyApp + ports: + - protocol: TCP + port: 80 + targetPort: 9376 diff --git a/util/resource_tracking/resource_tracking_test.go b/util/resource_tracking/resource_tracking_test.go index 46950c3ae8fb7..b15c341d7aa28 100644 --- a/util/resource_tracking/resource_tracking_test.go +++ b/util/resource_tracking/resource_tracking_test.go @@ -21,9 +21,9 @@ func TestSetAppInstanceLabel(t *testing.T) { resourceTracking := NewResourceTracking() - err = SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodLabel) + err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodLabel) assert.Nil(t, err) - app := GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodLabel) + app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodLabel) assert.Equal(t, "my-app", app) } @@ -37,10 +37,10 @@ func TestSetAppInstanceAnnotation(t *testing.T) { resourceTracking := NewResourceTracking() - err = SetAppInstance(&obj, common.AnnotationKeyAppInstance, "my-app", "", TrackingMethodAnnotation) + err = resourceTracking.SetAppInstance(&obj, common.AnnotationKeyAppInstance, "my-app", "", TrackingMethodAnnotation) assert.Nil(t, err) - app := GetAppName(&obj, common.AnnotationKeyAppInstance, TrackingMethodAnnotation) + app := resourceTracking.GetAppName(&obj, common.AnnotationKeyAppInstance, TrackingMethodAnnotation) assert.Equal(t, "my-app", app) } @@ -53,10 +53,10 @@ func TestSetAppInstanceAnnotationAndLabel(t *testing.T) { resourceTracking := NewResourceTracking() - err = SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodAnnotationAndLabel) + err = resourceTracking.SetAppInstance(&obj, common.LabelKeyAppInstance, "my-app", "", TrackingMethodAnnotationAndLabel) assert.Nil(t, err) - app := GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel) + app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotationAndLabel) assert.Equal(t, "my-app", app) } @@ -70,13 +70,13 @@ func TestSetAppInstanceAnnotationNotFound(t *testing.T) { resourceTracking := NewResourceTracking() - app := GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotation) + app := resourceTracking.GetAppName(&obj, common.LabelKeyAppInstance, TrackingMethodAnnotation) assert.Equal(t, "", app) } func TestParseAppInstanceValue(t *testing.T) { resourceTracking := NewResourceTracking() - appInstanceValue, err := ParseAppInstanceValue("app:/:/") + appInstanceValue, err := resourceTracking.ParseAppInstanceValue("app:/:/") if !assert.NoError(t, err) { t.Fatal() } @@ -89,18 +89,18 @@ func TestParseAppInstanceValue(t *testing.T) { func TestParseAppInstanceValueWrongFormat1(t *testing.T) { resourceTracking := NewResourceTracking() - _, err := ParseAppInstanceValue("app") + _, err := resourceTracking.ParseAppInstanceValue("app") assert.Error(t, err, WrongResourceTrackingFormat) } func TestParseAppInstanceValueWrongFormat2(t *testing.T) { resourceTracking := NewResourceTracking() - _, err := ParseAppInstanceValue("app;group/kind/ns") + _, err := resourceTracking.ParseAppInstanceValue("app;group/kind/ns") assert.Error(t, err, WrongResourceTrackingFormat) } func TestParseAppInstanceValueCorrectFormat(t *testing.T) { resourceTracking := NewResourceTracking() - _, err := ParseAppInstanceValue("app:group/kind:test/ns") + _, err := resourceTracking.ParseAppInstanceValue("app:group/kind:test/ns") assert.NoError(t, err) } diff --git a/util/resource_tracking/testdata/svc.yaml b/util/resource_tracking/testdata/svc.yaml new file mode 100644 index 0000000000000..11aeb957db7c3 --- /dev/null +++ b/util/resource_tracking/testdata/svc.yaml @@ -0,0 +1,11 @@ +kind: Service +apiVersion: v1 +metadata: + name: my-service +spec: + selector: + app: MyApp + ports: + - protocol: TCP + port: 80 + targetPort: 9376