From f40cad8f317ccef28dd18ac8b0fc65e43983ccfc Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Sun, 15 Dec 2024 23:24:31 -0500 Subject: [PATCH] more fixes Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- .../controllers/applicationset_controller.go | 3 - cmd/argocd/commands/admin/app.go | 6 +- controller/appcontroller.go | 44 ++++++------ controller/appcontroller_test.go | 4 +- controller/cache/cache.go | 6 +- controller/cache/mocks/LiveStateCache.go | 2 +- controller/hook.go | 4 +- controller/state.go | 36 ++++------ controller/state_test.go | 72 ++++++++++--------- controller/sync.go | 10 +-- controller/sync_test.go | 20 +++--- util/argo/argo.go | 2 +- util/webhook/webhook.go | 4 +- 13 files changed, 106 insertions(+), 107 deletions(-) diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go index 4c9caeca48efa..70f97614a89ff 100644 --- a/applicationset/controllers/applicationset_controller.go +++ b/applicationset/controllers/applicationset_controller.go @@ -721,9 +721,6 @@ func (r *ApplicationSetReconciler) getCurrentApplications(ctx context.Context, a // deleteInCluster will delete Applications that are currently on the cluster, but not in appList. // The function must be called after all generators had been called and generated applications func (r *ApplicationSetReconciler) deleteInCluster(ctx context.Context, logCtx *log.Entry, applicationSet argov1alpha1.ApplicationSet, desiredApplications []argov1alpha1.Application) error { - // settingsMgr := settings.NewSettingsManager(context.TODO(), r.KubeClientset, applicationSet.Namespace) - // argoDB := db.NewDB(applicationSet.Namespace, settingsMgr, r.KubeClientset) - // clusterList, err := argoDB.ListClusters(ctx) clusterList, err := utils.ListClusters(ctx, r.KubeClientset, r.ArgoCDNamespace) if err != nil { return fmt.Errorf("error listing clusters: %w", err) diff --git a/cmd/argocd/commands/admin/app.go b/cmd/argocd/commands/admin/app.go index 0eb799b322570..cc48cbdf20377 100644 --- a/cmd/argocd/commands/admin/app.go +++ b/cmd/argocd/commands/admin/app.go @@ -437,7 +437,11 @@ func reconcileApplications( sources = append(sources, app.Spec.GetSource()) revisions = append(revisions, app.Spec.GetSource().TargetRevision) - res, err := appStateManager.CompareAppState(&app, proj, revisions, sources, false, false, nil, false, false) + destCluster, err := argo.GetDestinationCluster(ctx, app.Spec.Destination, argoDB) + if err != nil { + return nil, fmt.Errorf("error getting destination cluster: %w", err) + } + res, err := appStateManager.CompareAppState(destCluster, &app, proj, revisions, sources, false, false, nil, false, false) if err != nil { return nil, fmt.Errorf("error comparing app states: %w", err) } diff --git a/controller/appcontroller.go b/controller/appcontroller.go index cbf76872d9a8e..fb103dca58d7b 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -473,7 +473,7 @@ func (ctrl *ApplicationController) setAppManagedResources(destCluster *appv1.Clu logCtx = logCtx.WithField("time_ms", time.Since(ts.StartTime).Milliseconds()) logCtx.Debug("Finished setting app managed resources") }() - managedResources, err := ctrl.hideSecretData(a, comparisonResult) + managedResources, err := ctrl.hideSecretData(destCluster, a, comparisonResult) ts.AddCheckpoint("hide_secret_data_ms") if err != nil { return nil, fmt.Errorf("error getting managed resources: %w", err) @@ -576,7 +576,7 @@ func (ctrl *ApplicationController) getResourceTree(destCluster *appv1.Cluster, a managedResourcesKeys = append(managedResourcesKeys, kube.GetResourceKey(live)) } } - err = ctrl.stateCache.IterateHierarchyV2(a.Spec.Destination.Server, managedResourcesKeys, func(child appv1.ResourceNode, appName string) bool { + err = ctrl.stateCache.IterateHierarchyV2(destCluster.Server, managedResourcesKeys, func(child appv1.ResourceNode, appName string) bool { permitted, _ := proj.IsResourcePermitted(schema.GroupKind{Group: child.ResourceRef.Group, Kind: child.ResourceRef.Kind}, child.Namespace, destCluster, func(project string) ([]*appv1.Cluster, error) { clusters, err := ctrl.db.GetProjectClusters(context.TODO(), project) if err != nil { @@ -642,7 +642,7 @@ func (ctrl *ApplicationController) getResourceTree(destCluster *appv1.Cluster, a }) ts.AddCheckpoint("process_orphaned_resources_ms") - hosts, err := ctrl.getAppHosts(a, nodes) + hosts, err := ctrl.getAppHosts(destCluster, a, nodes) if err != nil { return nil, fmt.Errorf("failed to get app hosts: %w", err) } @@ -650,7 +650,7 @@ func (ctrl *ApplicationController) getResourceTree(destCluster *appv1.Cluster, a return &appv1.ApplicationTree{Nodes: nodes, OrphanedNodes: orphanedNodes, Hosts: hosts}, nil } -func (ctrl *ApplicationController) getAppHosts(a *appv1.Application, appNodes []appv1.ResourceNode) ([]appv1.HostInfo, error) { +func (ctrl *ApplicationController) getAppHosts(destCluster *appv1.Cluster, a *appv1.Application, appNodes []appv1.ResourceNode) ([]appv1.HostInfo, error) { ts := stats.NewTimingStats() defer func() { logCtx := getAppLog(a) @@ -675,7 +675,7 @@ func (ctrl *ApplicationController) getAppHosts(a *appv1.Application, appNodes [] allNodesInfo := map[string]statecache.NodeInfo{} allPodsByNode := map[string][]statecache.PodInfo{} appPodsByNode := map[string][]statecache.PodInfo{} - err := ctrl.stateCache.IterateResources(a.Spec.Destination.Server, func(res *clustercache.Resource, info *statecache.ResourceInfo) { + err := ctrl.stateCache.IterateResources(destCluster.Server, func(res *clustercache.Resource, info *statecache.ResourceInfo) { key := res.ResourceKey() switch { @@ -749,7 +749,7 @@ func (ctrl *ApplicationController) getAppHosts(a *appv1.Application, appNodes [] return hosts, nil } -func (ctrl *ApplicationController) hideSecretData(app *appv1.Application, comparisonResult *comparisonResult) ([]*appv1.ResourceDiff, error) { +func (ctrl *ApplicationController) hideSecretData(destCluster *appv1.Cluster, app *appv1.Application, comparisonResult *comparisonResult) ([]*appv1.ResourceDiff, error) { items := make([]*appv1.ResourceDiff, len(comparisonResult.managedResources)) for i := range comparisonResult.managedResources { res := comparisonResult.managedResources[i] @@ -788,7 +788,7 @@ func (ctrl *ApplicationController) hideSecretData(app *appv1.Application, compar return nil, fmt.Errorf("error getting tracking method: %w", err) } - clusterCache, err := ctrl.stateCache.GetClusterCache(app.Spec.Destination.Server) + clusterCache, err := ctrl.stateCache.GetClusterCache(destCluster.Server) if err != nil { return nil, fmt.Errorf("error getting cluster cache: %w", err) } @@ -1112,7 +1112,7 @@ func (ctrl *ApplicationController) shouldBeDeleted(app *appv1.Application, obj * } func (ctrl *ApplicationController) getPermittedAppLiveObjects(destCluster *appv1.Cluster, app *appv1.Application, proj *appv1.AppProject, projectClusters func(project string) ([]*appv1.Cluster, error)) (map[kube.ResourceKey]*unstructured.Unstructured, error) { - objsMap, err := ctrl.stateCache.GetManagedLiveObjs(app, []*unstructured.Unstructured{}) + objsMap, err := ctrl.stateCache.GetManagedLiveObjs(destCluster, app, []*unstructured.Unstructured{}) if err != nil { return nil, err } @@ -1229,7 +1229,7 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic return err } - done, err := ctrl.executePostDeleteHooks(app, proj, objsMap, config, logCtx) + done, err := ctrl.executePostDeleteHooks(destCluster, app, proj, objsMap, config, logCtx) if err != nil { return err } @@ -1382,16 +1382,17 @@ func (ctrl *ApplicationController) processRequestedAppOperation(app *appv1.Appli } ts.AddCheckpoint("initial_operation_stage_ms") - if _, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, ctrl.db); err != nil { + destCluster, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, ctrl.db) + if err != nil { state.Phase = synccommon.OperationFailed state.Message = err.Error() } else { - ctrl.appStateManager.SyncAppState(app, state) + ctrl.appStateManager.SyncAppState(destCluster, app, state) } ts.AddCheckpoint("validate_and_sync_app_state_ms") // Check whether application is allowed to use project - _, err := ctrl.getAppProj(app) + _, err = ctrl.getAppProj(app) ts.AddCheckpoint("get_app_proj_ms") if err != nil { state.Phase = synccommon.OperationError @@ -1683,9 +1684,16 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo sources = append(sources, app.Spec.GetSource()) } - compareResult, err := ctrl.appStateManager.CompareAppState(app, project, revisions, sources, + destCluster, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, ctrl.db) + if err != nil { + logCtx.Errorf("Failed to get destination cluster: %v", err) + return + } + + compareResult, err := ctrl.appStateManager.CompareAppState(destCluster, app, project, revisions, sources, refreshType == appv1.RefreshTypeHard, comparisonLevel == CompareWithLatestForceResolve, localManifests, hasMultipleSources, false) + ts.AddCheckpoint("compare_app_state_ms") if goerrors.Is(err, CompareStateRepoError) { @@ -1699,12 +1707,6 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo ctrl.normalizeApplication(origApp, app) ts.AddCheckpoint("normalize_application_ms") - - destCluster, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, ctrl.db) - if err != nil { - logCtx.Errorf("Failed to get destination cluster: %v", err) - return - } tree, err := ctrl.setAppManagedResources(destCluster, app, compareResult) ts.AddCheckpoint("set_app_managed_resources_ms") if err != nil { @@ -2181,11 +2183,11 @@ func (ctrl *ApplicationController) canProcessApp(obj interface{}) bool { } } - cluster, err := ctrl.db.GetCluster(context.Background(), app.Spec.Destination.Server) + destCluster, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, ctrl.db) if err != nil { return ctrl.clusterSharding.IsManagedCluster(nil) } - return ctrl.clusterSharding.IsManagedCluster(cluster) + return ctrl.clusterSharding.IsManagedCluster(destCluster) } func (ctrl *ApplicationController) newApplicationInformerAndLister() (cache.SharedIndexInformer, applisters.ApplicationLister) { diff --git a/controller/appcontroller_test.go b/controller/appcontroller_test.go index 8a385288dd660..a6f543896e2b0 100644 --- a/controller/appcontroller_test.go +++ b/controller/appcontroller_test.go @@ -936,7 +936,7 @@ func TestFinalizeAppDeletion(t *testing.T) { }) require.NoError(t, err) assert.True(t, patched) - objsMap, err := ctrl.stateCache.GetManagedLiveObjs(app, []*unstructured.Unstructured{}) + objsMap, err := ctrl.stateCache.GetManagedLiveObjs(&v1alpha1.Cluster{Server: "test", Name: "test"}, app, []*unstructured.Unstructured{}) if err != nil { require.NoError(t, err) } @@ -2185,7 +2185,7 @@ func TestGetAppHosts(t *testing.T) { })).Return(nil) ctrl.stateCache = mockStateCache - hosts, err := ctrl.getAppHosts(app, []v1alpha1.ResourceNode{{ + hosts, err := ctrl.getAppHosts(&v1alpha1.Cluster{Server: "test", Name: "test"}, app, []v1alpha1.ResourceNode{{ ResourceRef: v1alpha1.ResourceRef{Name: "pod1", Namespace: "default", Kind: kube.PodKind}, Info: []v1alpha1.InfoItem{{ Name: "Host", diff --git a/controller/cache/cache.go b/controller/cache/cache.go index 673485c85779d..3222ceb7215dc 100644 --- a/controller/cache/cache.go +++ b/controller/cache/cache.go @@ -128,7 +128,7 @@ type LiveStateCache interface { // Executes give callback against resources specified by the keys and all its children IterateHierarchyV2(server string, keys []kube.ResourceKey, action func(child appv1.ResourceNode, appName string) bool) error // Returns state of live nodes which correspond for target nodes of specified application. - GetManagedLiveObjs(a *appv1.Application, targetObjs []*unstructured.Unstructured) (map[kube.ResourceKey]*unstructured.Unstructured, error) + GetManagedLiveObjs(destCluster *appv1.Cluster, a *appv1.Application, targetObjs []*unstructured.Unstructured) (map[kube.ResourceKey]*unstructured.Unstructured, error) // IterateResources iterates all resource stored in cache IterateResources(server string, callback func(res *clustercache.Resource, info *ResourceInfo)) error // Returns all top level resources (resources without owner references) of a specified namespace @@ -695,8 +695,8 @@ func (c *liveStateCache) GetNamespaceTopLevelResources(server string, namespace return res, nil } -func (c *liveStateCache) GetManagedLiveObjs(a *appv1.Application, targetObjs []*unstructured.Unstructured) (map[kube.ResourceKey]*unstructured.Unstructured, error) { - clusterInfo, err := c.getSyncedCluster(a.Spec.Destination.Server) +func (c *liveStateCache) GetManagedLiveObjs(destCluster *appv1.Cluster, a *appv1.Application, targetObjs []*unstructured.Unstructured) (map[kube.ResourceKey]*unstructured.Unstructured, error) { + clusterInfo, err := c.getSyncedCluster(destCluster.Server) if err != nil { return nil, fmt.Errorf("failed to get cluster info for %q: %w", a.Spec.Destination.Server, err) } diff --git a/controller/cache/mocks/LiveStateCache.go b/controller/cache/mocks/LiveStateCache.go index 85a4a298ba4c2..f0d700709443e 100644 --- a/controller/cache/mocks/LiveStateCache.go +++ b/controller/cache/mocks/LiveStateCache.go @@ -76,7 +76,7 @@ func (_m *LiveStateCache) GetClustersInfo() []cache.ClusterInfo { } // GetManagedLiveObjs provides a mock function with given fields: a, targetObjs -func (_m *LiveStateCache) GetManagedLiveObjs(a *v1alpha1.Application, targetObjs []*unstructured.Unstructured) (map[kube.ResourceKey]*unstructured.Unstructured, error) { +func (_m *LiveStateCache) GetManagedLiveObjs(destCluster *v1alpha1.Cluster, a *v1alpha1.Application, targetObjs []*unstructured.Unstructured) (map[kube.ResourceKey]*unstructured.Unstructured, error) { ret := _m.Called(a, targetObjs) if len(ret) == 0 { diff --git a/controller/hook.go b/controller/hook.go index 5c391114ab9bb..cff13e2872a71 100644 --- a/controller/hook.go +++ b/controller/hook.go @@ -41,7 +41,7 @@ func isPostDeleteHook(obj *unstructured.Unstructured) bool { return false } -func (ctrl *ApplicationController) executePostDeleteHooks(app *v1alpha1.Application, proj *v1alpha1.AppProject, liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) { +func (ctrl *ApplicationController) executePostDeleteHooks(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, proj *v1alpha1.AppProject, liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) { appLabelKey, err := ctrl.settingsMgr.GetAppInstanceLabelKey() if err != nil { return false, err @@ -51,7 +51,7 @@ func (ctrl *ApplicationController) executePostDeleteHooks(app *v1alpha1.Applicat revisions = append(revisions, src.TargetRevision) } - targets, _, _, err := ctrl.appStateManager.GetRepoObjs(app, app.Spec.GetSources(), appLabelKey, revisions, false, false, false, proj, false) + targets, _, _, err := ctrl.appStateManager.GetRepoObjs(destCluster, app, app.Spec.GetSources(), appLabelKey, revisions, false, false, false, proj, false) if err != nil { return false, err } diff --git a/controller/state.go b/controller/state.go index 6aad8e8465da5..a38e2623ed18d 100644 --- a/controller/state.go +++ b/controller/state.go @@ -69,9 +69,9 @@ type managedResource struct { // AppStateManager defines methods which allow to compare application spec and actual application state. type AppStateManager interface { - CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localObjects []string, hasMultipleSources bool, rollback bool) (*comparisonResult, error) - SyncAppState(app *v1alpha1.Application, state *v1alpha1.OperationState) - GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject, rollback bool) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, bool, error) + CompareAppState(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localObjects []string, hasMultipleSources bool, rollback bool) (*comparisonResult, error) + SyncAppState(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, state *v1alpha1.OperationState) + GetRepoObjs(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject, rollback bool) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, bool, error) } // comparisonResult holds the state of an application after the reconciliation @@ -125,7 +125,7 @@ type appStateManager struct { // task to the repo-server. It returns the list of generated manifests as unstructured // objects. It also returns the full response from all calls to the repo server as the // second argument. -func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject, rollback bool) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, bool, error) { +func (m *appStateManager) GetRepoObjs(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject, rollback bool) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, bool, error) { ts := stats.NewTimingStats() helmRepos, err := m.db.ListHelmRepositories(context.Background()) if err != nil { @@ -168,9 +168,9 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp } ts.AddCheckpoint("build_options_ms") - serverVersion, apiResources, err := m.liveStateCache.GetVersionsInfo(app.Spec.Destination.Server) + serverVersion, apiResources, err := m.liveStateCache.GetVersionsInfo(destCluster.Server) if err != nil { - return nil, nil, false, fmt.Errorf("failed to get cluster version for cluster %q: %w", app.Spec.Destination.Server, err) + return nil, nil, false, fmt.Errorf("failed to get cluster version for cluster %q: %w", destCluster.Server, err) } conn, repoClient, err := m.repoClientset.NewRepoServerClient() if err != nil { @@ -429,7 +429,7 @@ func isManagedNamespace(ns *unstructured.Unstructured, app *v1alpha1.Application // CompareAppState compares application git state to the live app state, using the specified // revision and supplied source. If revision or overrides are empty, then compares against // 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) { +func (m *appStateManager) CompareAppState(destCluster *v1alpha1.Cluster, 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() @@ -470,7 +470,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 conditions := make([]v1alpha1.ApplicationCondition, 0) logCtx := log.WithField("application", app.QualifiedName()) - logCtx.Infof("Comparing app state (cluster: %s, namespace: %s)", app.Spec.Destination.Server, app.Spec.Destination.Namespace) + logCtx.Infof("Comparing app state (cluster: %s, namespace: %s)", destCluster.Server, app.Spec.Destination.Namespace) var targetObjs []*unstructured.Unstructured now := metav1.Now() @@ -490,7 +490,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 } } - targetObjs, manifestInfos, revisionUpdated, err = m.GetRepoObjs(app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project, rollback) + targetObjs, manifestInfos, revisionUpdated, err = m.GetRepoObjs(destCluster, app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project, rollback) if err != nil { targetObjs = make([]*unstructured.Unstructured, 0) msg := fmt.Sprintf("Failed to load target state: %s", err.Error()) @@ -534,7 +534,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 ts.AddCheckpoint("git_ms") var infoProvider kubeutil.ResourceInfoProvider - infoProvider, err = m.liveStateCache.GetClusterCache(app.Spec.Destination.Server) + infoProvider, err = m.liveStateCache.GetClusterCache(destCluster.Server) if err != nil { infoProvider = &resourceInfoProviderStub{} } @@ -547,7 +547,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 for i := len(targetObjs) - 1; i >= 0; i-- { targetObj := targetObjs[i] gvk := targetObj.GroupVersionKind() - if resFilter.IsExcludedResource(gvk.Group, gvk.Kind, app.Spec.Destination.Server) { + if resFilter.IsExcludedResource(gvk.Group, gvk.Kind, destCluster.Server) { targetObjs = append(targetObjs[:i], targetObjs[i+1:]...) conditions = append(conditions, v1alpha1.ApplicationCondition{ Type: v1alpha1.ApplicationConditionExcludedResourceWarning, @@ -565,7 +565,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 } ts.AddCheckpoint("dedup_ms") - liveObjByKey, err := m.liveStateCache.GetManagedLiveObjs(app, targetObjs) + liveObjByKey, err := m.liveStateCache.GetManagedLiveObjs(destCluster, app, targetObjs) if err != nil { liveObjByKey = make(map[kubeutil.ResourceKey]*unstructured.Unstructured) msg := fmt.Sprintf("Failed to load live state: %s", err.Error()) @@ -574,12 +574,6 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 } logCtx.Debugf("Retrieved live manifests") - - destCluster, err := argo.GetDestinationCluster(context.TODO(), app.Spec.Destination, m.db) - if err != nil { - msg := fmt.Sprintf("Failed to get destination cluster %q: %s", app.Spec.Destination.Server, err.Error()) - conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now}) - } // filter out all resources which are not permitted in the application project for k, v := range liveObjByKey { permitted, err := project.IsLiveResourcePermitted(v, destCluster, func(project string) ([]*v1alpha1.Cluster, error) { @@ -693,7 +687,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 diffConfigBuilder.WithIgnoreMutationWebhook(false) } - gvkParser, err := m.getGVKParser(app.Spec.Destination.Server) + gvkParser, err := m.getGVKParser(destCluster.Server) if err != nil { conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionUnknownError, Message: err.Error(), LastTransitionTime: &now}) } @@ -703,7 +697,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 diffConfigBuilder.WithServerSideDiff(serverSideDiff) if serverSideDiff { - resourceOps, cleanup, err := m.getResourceOperations(app.Spec.Destination.Server) + resourceOps, cleanup, err := m.getResourceOperations(destCluster.Server) if err != nil { log.Errorf("CompareAppState error getting resource operations: %s", err) conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionUnknownError, Message: err.Error(), LastTransitionTime: &now}) @@ -797,7 +791,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 resState.Status = v1alpha1.SyncStatusCodeSynced } // set unknown status to all resource that are not permitted in the app project - isNamespaced, err := m.liveStateCache.IsNamespaced(app.Spec.Destination.Server, gvk.GroupKind()) + isNamespaced, err := m.liveStateCache.IsNamespaced(destCluster.Server, gvk.GroupKind()) if !project.IsGroupKindPermitted(gvk.GroupKind(), isNamespaced && err == nil) { resState.Status = v1alpha1.SyncStatusCodeUnknown } diff --git a/controller/state_test.go b/controller/state_test.go index 04f373aa6fd6f..30c57a2d5e03a 100644 --- a/controller/state_test.go +++ b/controller/state_test.go @@ -34,6 +34,8 @@ import ( // TestCompareAppStateEmpty tests comparison when both git and live have no objects func TestCompareAppStateEmpty(t *testing.T) { + t.Parallel() + app := newFakeApp() data := fakeData{ manifestResponse: &apiclient.ManifestResponse{ @@ -49,7 +51,7 @@ func TestCompareAppStateEmpty(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -67,18 +69,18 @@ func TestCompareAppStateRepoError(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) assert.Nil(t, compRes) require.EqualError(t, err, CompareStateRepoError.Error()) // expect to still get compare state error to as inside grace period - compRes, err = ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err = ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) assert.Nil(t, compRes) require.EqualError(t, err, CompareStateRepoError.Error()) time.Sleep(10 * time.Second) // expect to not get error as outside of grace period, but status should be unknown - compRes, err = ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err = ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) assert.NotNil(t, compRes) require.NoError(t, err) assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status) @@ -113,7 +115,7 @@ func TestCompareAppStateNamespaceMetadataDiffers(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -162,7 +164,7 @@ func TestCompareAppStateNamespaceMetadataDiffersToManifest(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -220,7 +222,7 @@ func TestCompareAppStateNamespaceMetadata(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -279,7 +281,7 @@ func TestCompareAppStateNamespaceMetadataIsTheSame(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -307,7 +309,7 @@ func TestCompareAppStateMissing(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -339,7 +341,7 @@ func TestCompareAppStateExtra(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status) @@ -370,7 +372,7 @@ func TestCompareAppStateHook(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -402,7 +404,7 @@ func TestCompareAppStateSkipHook(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -433,7 +435,7 @@ func TestCompareAppStateCompareOptionIgnoreExtraneous(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) @@ -466,7 +468,7 @@ func TestCompareAppStateExtraHook(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) @@ -495,7 +497,7 @@ func TestAppRevisionsSingleSource(t *testing.T) { app := newFakeApp() revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources(), false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources(), false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -535,7 +537,7 @@ func TestAppRevisionsMultiSource(t *testing.T) { app := newFakeMultiSourceApp() revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources(), false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources(), false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -584,7 +586,7 @@ func TestCompareAppStateDuplicatedNamespacedResources(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) @@ -621,7 +623,7 @@ func TestCompareAppStateManagedNamespaceMetadataWithLiveNsDoesNotGetPruned(t *te }, } ctrl := newFakeController(&data, nil) - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, []string{}, app.Spec.Sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, []string{}, app.Spec.Sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) @@ -675,7 +677,7 @@ func TestCompareAppStateWithManifestGeneratePath(t *testing.T) { ctrl := newFakeController(&data, nil) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -711,7 +713,7 @@ func TestSetHealth(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) @@ -748,7 +750,7 @@ func TestPreserveStatusTimestamp(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) @@ -786,7 +788,7 @@ func TestSetHealthSelfReferencedApp(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) @@ -862,7 +864,7 @@ func TestReturnUnknownComparisonStateOnSettingLoadError(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.Equal(t, health.HealthStatusUnknown, compRes.healthStatus.Status) @@ -1004,7 +1006,7 @@ func TestSignedResponseNoSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1031,7 +1033,7 @@ func TestSignedResponseNoSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1063,7 +1065,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1090,7 +1092,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1117,7 +1119,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1144,7 +1146,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1174,7 +1176,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &testProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &testProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1204,7 +1206,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, localManifests, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1234,7 +1236,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1264,7 +1266,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, localManifests, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1768,7 +1770,7 @@ func TestCompareAppStateDefaultRevisionUpdated(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1791,7 +1793,7 @@ func TestCompareAppStateRevisionUpdatedWithHelmSource(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) diff --git a/controller/sync.go b/controller/sync.go index f9414e8390bdc..2f1d0dc6acc81 100644 --- a/controller/sync.go +++ b/controller/sync.go @@ -92,7 +92,7 @@ func (m *appStateManager) getResourceOperations(server string) (kube.ResourceOpe return ops, cleanup, nil } -func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha1.OperationState) { +func (m *appStateManager) SyncAppState(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, state *v1alpha1.OperationState) { // Sync requests might be requested with ambiguous revisions (e.g. master, HEAD, v1.2.3). // This can change meaning when resuming operations (e.g a hook sync). After calculating a // concrete git commit SHA, the SHA is remembered in the status.operationState.syncResult field. @@ -199,7 +199,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha } // ignore error if CompareStateRepoError, this shouldn't happen as noRevisionCache is true - compareResult, err := m.CompareAppState(app, proj, revisions, sources, false, true, syncOp.Manifests, isMultiSourceRevision, rollback) + compareResult, err := m.CompareAppState(destCluster, app, proj, revisions, sources, false, true, syncOp.Manifests, isMultiSourceRevision, rollback) if err != nil && !goerrors.Is(err, CompareStateRepoError) { state.Phase = common.OperationError state.Message = err.Error() @@ -221,7 +221,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha return } - cluster, err := m.db.GetCluster(context.Background(), app.Spec.Destination.Server) + cluster, err := m.db.GetCluster(context.Background(), destCluster.Server) if err != nil { state.Phase = common.OperationError state.Message = err.Error() @@ -422,9 +422,9 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha for _, res := range resState { augmentedMsg, err := argo.AugmentSyncMsg(res, func() ([]kube.APIResourceInfo, error) { if apiVersion == nil { - _, apiVersion, err = m.liveStateCache.GetVersionsInfo(app.Spec.Destination.Server) + _, apiVersion, err = m.liveStateCache.GetVersionsInfo(destCluster.Server) if err != nil { - return nil, fmt.Errorf("failed to get version info from the target cluster %q", app.Spec.Destination.Server) + return nil, fmt.Errorf("failed to get version info from the target cluster %q", destCluster.Server) } } return apiVersion, nil diff --git a/controller/sync_test.go b/controller/sync_test.go index a553fd3e37cf7..b83aa7c9b395c 100644 --- a/controller/sync_test.go +++ b/controller/sync_test.go @@ -50,7 +50,7 @@ func TestPersistRevisionHistory(t *testing.T) { opState := &v1alpha1.OperationState{Operation: v1alpha1.Operation{ Sync: &v1alpha1.SyncOperation{}, }} - ctrl.appStateManager.SyncAppState(app, opState) + ctrl.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, app, opState) // Ensure we record spec.source into sync result assert.Equal(t, app.Spec.GetSource(), opState.SyncResult.Source) @@ -96,7 +96,7 @@ func TestPersistManagedNamespaceMetadataState(t *testing.T) { opState := &v1alpha1.OperationState{Operation: v1alpha1.Operation{ Sync: &v1alpha1.SyncOperation{}, }} - ctrl.appStateManager.SyncAppState(app, opState) + ctrl.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, app, opState) // Ensure we record spec.syncPolicy.managedNamespaceMetadata into sync result assert.Equal(t, app.Spec.SyncPolicy.ManagedNamespaceMetadata, opState.SyncResult.ManagedNamespaceMetadata) } @@ -139,7 +139,7 @@ func TestPersistRevisionHistoryRollback(t *testing.T) { Source: &source, }, }} - ctrl.appStateManager.SyncAppState(app, opState) + ctrl.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, app, opState) // Ensure we record opState's source into sync result assert.Equal(t, source, opState.SyncResult.Source) @@ -182,7 +182,7 @@ func TestSyncComparisonError(t *testing.T) { Sync: &v1alpha1.SyncOperation{}, }} t.Setenv("ARGOCD_GPG_ENABLED", "true") - ctrl.appStateManager.SyncAppState(app, opState) + ctrl.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, opState) conditions := app.Status.GetConditions(map[v1alpha1.ApplicationConditionType]bool{v1alpha1.ApplicationConditionComparisonError: true}) assert.NotEmpty(t, conditions) @@ -249,7 +249,7 @@ func TestAppStateManager_SyncAppState(t *testing.T) { }} // when - f.controller.appStateManager.SyncAppState(f.application, opState) + f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, f.application, opState) // then assert.Equal(t, common.OperationFailed, opState.Phase) @@ -319,7 +319,7 @@ func TestSyncWindowDeniesSync(t *testing.T) { Phase: common.OperationRunning, } // when - f.controller.appStateManager.SyncAppState(f.application, opState) + f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, f.application, opState) // then assert.Equal(t, common.OperationRunning, opState.Phase) @@ -1340,7 +1340,7 @@ func TestSyncWithImpersonate(t *testing.T) { Phase: common.OperationRunning, } // when - f.controller.appStateManager.SyncAppState(f.application, opState) + f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, f.application, opState) // then, app sync should fail with expected error message in operation state assert.Equal(t, common.OperationError, opState.Phase) @@ -1361,7 +1361,7 @@ func TestSyncWithImpersonate(t *testing.T) { Phase: common.OperationRunning, } // when - f.controller.appStateManager.SyncAppState(f.application, opState) + f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, f.application, opState) // then app sync should fail with expected error message in operation state assert.Equal(t, common.OperationError, opState.Phase) @@ -1382,7 +1382,7 @@ func TestSyncWithImpersonate(t *testing.T) { Phase: common.OperationRunning, } // when - f.controller.appStateManager.SyncAppState(f.application, opState) + f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, f.application, opState) // then app sync should not fail assert.Equal(t, common.OperationSucceeded, opState.Phase) @@ -1403,7 +1403,7 @@ func TestSyncWithImpersonate(t *testing.T) { Phase: common.OperationRunning, } // when - f.controller.appStateManager.SyncAppState(f.application, opState) + f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, f.application, opState) // then application sync should pass using the control plane service account assert.Equal(t, common.OperationSucceeded, opState.Phase) diff --git a/util/argo/argo.go b/util/argo/argo.go index e5e3ba4d5acc8..2645b920e4409 100644 --- a/util/argo/argo.go +++ b/util/argo/argo.go @@ -914,7 +914,7 @@ func GetDestinationCluster(ctx context.Context, destination argoappv1.Applicatio if destination.Server != "" { cluster, err := db.GetCluster(ctx, destination.Server) if err != nil { - return cluster, fmt.Errorf("error getting cluster by server %q: %w", destination.Server, err) + return nil, fmt.Errorf("error getting cluster by server %q: %w", destination.Server, err) } return cluster, nil } else if destination.Name != "" { diff --git a/util/webhook/webhook.go b/util/webhook/webhook.go index 03b7770bf9fd9..f0f912753a0ed 100644 --- a/util/webhook/webhook.go +++ b/util/webhook/webhook.go @@ -350,13 +350,13 @@ func getWebUrlRegex(webURL string) (*regexp.Regexp, error) { } func (a *ArgoCDWebhookHandler) storePreviouslyCachedManifests(app *v1alpha1.Application, change changeInfo, trackingMethod string, appInstanceLabelKey string, installationID string) error { - _, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, a.db) + destCluster, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, a.db) if err != nil { return fmt.Errorf("error validating destination: %w", err) } var clusterInfo v1alpha1.ClusterInfo - err = a.serverCache.GetClusterInfo(app.Spec.Destination.Server, &clusterInfo) + err = a.serverCache.GetClusterInfo(destCluster.Server, &clusterInfo) if err != nil { return fmt.Errorf("error getting cluster info: %w", err) }