diff --git a/.github/workflows/ci-build.yaml b/.github/workflows/ci-build.yaml index f3dd848044408..bf0b650faa35e 100644 --- a/.github/workflows/ci-build.yaml +++ b/.github/workflows/ci-build.yaml @@ -323,6 +323,8 @@ jobs: NODE_ENV: production NODE_ONLINE_ENV: online HOST_ARCH: amd64 + # If we're on the master branch, set the codecov token so that we upload bundle analysis + CODECOV_TOKEN: ${{ github.ref == 'refs/heads/master' && secrets.CODECOV_TOKEN || '' }} working-directory: ui/ - name: Run ESLint run: yarn lint diff --git a/.golangci.yaml b/.golangci.yaml index 2351f11e0fecc..8a1e59816ad27 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -4,6 +4,10 @@ issues: - SA5011 max-issues-per-linter: 0 max-same-issues: 0 + exclude-rules: + - path: '(.+)_test\.go' + linters: + - unparam linters: enable: - errcheck @@ -17,6 +21,7 @@ linters: - misspell - staticcheck - testifylint + - unparam - unused - whitespace linters-settings: diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go index ed64b4167d89d..a246b33563360 100644 --- a/applicationset/controllers/applicationset_controller.go +++ b/applicationset/controllers/applicationset_controller.go @@ -242,20 +242,8 @@ func (r *ApplicationSetReconciler) Reconcile(ctx context.Context, req ctrl.Reque if r.EnableProgressiveSyncs { // trigger appropriate application syncs if RollingSync strategy is enabled - if progressiveSyncsStrategyEnabled(&applicationSetInfo, "RollingSync") { - validApps, err = r.syncValidApplications(logCtx, &applicationSetInfo, appSyncMap, appMap, validApps) - if err != nil { - _ = r.setApplicationSetStatusCondition(ctx, - &applicationSetInfo, - argov1alpha1.ApplicationSetCondition{ - Type: argov1alpha1.ApplicationSetConditionErrorOccurred, - Message: err.Error(), - Reason: argov1alpha1.ApplicationSetReasonSyncApplicationError, - Status: argov1alpha1.ApplicationSetConditionStatusTrue, - }, parametersGenerated, - ) - return ctrl.Result{}, err - } + if progressiveSyncsRollingSyncStrategyEnabled(&applicationSetInfo) { + validApps = r.syncValidApplications(logCtx, &applicationSetInfo, appSyncMap, appMap, validApps) } } @@ -859,12 +847,9 @@ func (r *ApplicationSetReconciler) removeOwnerReferencesOnDeleteAppSet(ctx conte } func (r *ApplicationSetReconciler) performProgressiveSyncs(ctx context.Context, logCtx *log.Entry, appset argov1alpha1.ApplicationSet, applications []argov1alpha1.Application, desiredApplications []argov1alpha1.Application, appMap map[string]argov1alpha1.Application) (map[string]bool, error) { - appDependencyList, appStepMap, err := r.buildAppDependencyList(logCtx, appset, desiredApplications) - if err != nil { - return nil, fmt.Errorf("failed to build app dependency list: %w", err) - } + appDependencyList, appStepMap := r.buildAppDependencyList(logCtx, appset, desiredApplications) - _, err = r.updateApplicationSetApplicationStatus(ctx, logCtx, &appset, applications, appStepMap) + _, err := r.updateApplicationSetApplicationStatus(ctx, logCtx, &appset, applications, appStepMap) if err != nil { return nil, fmt.Errorf("failed to update applicationset app status: %w", err) } @@ -874,34 +859,27 @@ func (r *ApplicationSetReconciler) performProgressiveSyncs(ctx context.Context, logCtx.Infof("step %v: %+v", i+1, step) } - appSyncMap, err := r.buildAppSyncMap(ctx, appset, appDependencyList, appMap) - if err != nil { - return nil, fmt.Errorf("failed to build app sync map: %w", err) - } - + appSyncMap := r.buildAppSyncMap(appset, appDependencyList, appMap) logCtx.Infof("Application allowed to sync before maxUpdate?: %+v", appSyncMap) - _, err = r.updateApplicationSetApplicationStatusProgress(ctx, logCtx, &appset, appSyncMap, appStepMap, appMap) + _, err = r.updateApplicationSetApplicationStatusProgress(ctx, logCtx, &appset, appSyncMap, appStepMap) if err != nil { return nil, fmt.Errorf("failed to update applicationset application status progress: %w", err) } - _, err = r.updateApplicationSetApplicationStatusConditions(ctx, &appset) - if err != nil { - return nil, fmt.Errorf("failed to update applicationset application status conditions: %w", err) - } + _ = r.updateApplicationSetApplicationStatusConditions(ctx, &appset) return appSyncMap, nil } // this list tracks which Applications belong to each RollingUpdate step -func (r *ApplicationSetReconciler) buildAppDependencyList(logCtx *log.Entry, applicationSet argov1alpha1.ApplicationSet, applications []argov1alpha1.Application) ([][]string, map[string]int, error) { +func (r *ApplicationSetReconciler) buildAppDependencyList(logCtx *log.Entry, applicationSet argov1alpha1.ApplicationSet, applications []argov1alpha1.Application) ([][]string, map[string]int) { if applicationSet.Spec.Strategy == nil || applicationSet.Spec.Strategy.Type == "" || applicationSet.Spec.Strategy.Type == "AllAtOnce" { - return [][]string{}, map[string]int{}, nil + return [][]string{}, map[string]int{} } steps := []argov1alpha1.ApplicationSetRolloutStep{} - if progressiveSyncsStrategyEnabled(&applicationSet, "RollingSync") { + if progressiveSyncsRollingSyncStrategyEnabled(&applicationSet) { steps = applicationSet.Spec.Strategy.RollingSync.Steps } @@ -942,7 +920,7 @@ func (r *ApplicationSetReconciler) buildAppDependencyList(logCtx *log.Entry, app } } - return appDependencyList, appStepMap, nil + return appDependencyList, appStepMap } func labelMatchedExpression(logCtx *log.Entry, val string, matchExpression argov1alpha1.ApplicationMatchExpression) bool { @@ -966,7 +944,7 @@ func labelMatchedExpression(logCtx *log.Entry, val string, matchExpression argov } // this map is used to determine which stage of Applications are ready to be updated in the reconciler loop -func (r *ApplicationSetReconciler) buildAppSyncMap(ctx context.Context, applicationSet argov1alpha1.ApplicationSet, appDependencyList [][]string, appMap map[string]argov1alpha1.Application) (map[string]bool, error) { +func (r *ApplicationSetReconciler) buildAppSyncMap(applicationSet argov1alpha1.ApplicationSet, appDependencyList [][]string, appMap map[string]argov1alpha1.Application) map[string]bool { appSyncMap := map[string]bool{} syncEnabled := true @@ -1003,11 +981,11 @@ func (r *ApplicationSetReconciler) buildAppSyncMap(ctx context.Context, applicat } } - return appSyncMap, nil + return appSyncMap } func appSyncEnabledForNextStep(appset *argov1alpha1.ApplicationSet, app argov1alpha1.Application, appStatus argov1alpha1.ApplicationSetApplicationStatus) bool { - if progressiveSyncsStrategyEnabled(appset, "RollingSync") { + if progressiveSyncsRollingSyncStrategyEnabled(appset) { // we still need to complete the current step if the Application is not yet Healthy or there are still pending Application changes return isApplicationHealthy(app) && appStatus.Status == "Healthy" } @@ -1015,16 +993,8 @@ func appSyncEnabledForNextStep(appset *argov1alpha1.ApplicationSet, app argov1al return true } -func progressiveSyncsStrategyEnabled(appset *argov1alpha1.ApplicationSet, strategyType string) bool { - if appset.Spec.Strategy == nil || appset.Spec.Strategy.Type != strategyType { - return false - } - - if strategyType == "RollingSync" && appset.Spec.Strategy.RollingSync == nil { - return false - } - - return true +func progressiveSyncsRollingSyncStrategyEnabled(appset *argov1alpha1.ApplicationSet) bool { + return appset.Spec.Strategy != nil && appset.Spec.Strategy.RollingSync != nil && appset.Spec.Strategy.Type == "RollingSync" } func isApplicationHealthy(app argov1alpha1.Application) bool { @@ -1082,7 +1052,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con } appOutdated := false - if progressiveSyncsStrategyEnabled(applicationSet, "RollingSync") { + if progressiveSyncsRollingSyncStrategyEnabled(applicationSet) { appOutdated = syncStatusString == "OutOfSync" } @@ -1148,7 +1118,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatus(ctx con } // check Applications that are in Waiting status and promote them to Pending if needed -func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress(ctx context.Context, logCtx *log.Entry, applicationSet *argov1alpha1.ApplicationSet, appSyncMap map[string]bool, appStepMap map[string]int, appMap map[string]argov1alpha1.Application) ([]argov1alpha1.ApplicationSetApplicationStatus, error) { +func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress(ctx context.Context, logCtx *log.Entry, applicationSet *argov1alpha1.ApplicationSet, appSyncMap map[string]bool, appStepMap map[string]int) ([]argov1alpha1.ApplicationSetApplicationStatus, error) { now := metav1.Now() appStatuses := make([]argov1alpha1.ApplicationSetApplicationStatus, 0, len(applicationSet.Status.ApplicationStatus)) @@ -1159,7 +1129,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress totalCountMap := []int{} length := 0 - if progressiveSyncsStrategyEnabled(applicationSet, "RollingSync") { + if progressiveSyncsRollingSyncStrategyEnabled(applicationSet) { length = len(applicationSet.Spec.Strategy.RollingSync.Steps) } for s := 0; s < length; s++ { @@ -1171,7 +1141,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress for _, appStatus := range applicationSet.Status.ApplicationStatus { totalCountMap[appStepMap[appStatus.Application]] += 1 - if progressiveSyncsStrategyEnabled(applicationSet, "RollingSync") { + if progressiveSyncsRollingSyncStrategyEnabled(applicationSet) { if appStatus.Status == "Pending" || appStatus.Status == "Progressing" { updateCountMap[appStepMap[appStatus.Application]] += 1 } @@ -1181,7 +1151,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress for _, appStatus := range applicationSet.Status.ApplicationStatus { maxUpdateAllowed := true maxUpdate := &intstr.IntOrString{} - if progressiveSyncsStrategyEnabled(applicationSet, "RollingSync") { + if progressiveSyncsRollingSyncStrategyEnabled(applicationSet) { maxUpdate = applicationSet.Spec.Strategy.RollingSync.Steps[appStepMap[appStatus.Application]].MaxUpdate } @@ -1225,7 +1195,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusProgress return appStatuses, nil } -func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusConditions(ctx context.Context, applicationSet *argov1alpha1.ApplicationSet) ([]argov1alpha1.ApplicationSetCondition, error) { +func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusConditions(ctx context.Context, applicationSet *argov1alpha1.ApplicationSet) []argov1alpha1.ApplicationSetCondition { appSetProgressing := false for _, appStatus := range applicationSet.Status.ApplicationStatus { if appStatus.Status != "Healthy" { @@ -1264,7 +1234,7 @@ func (r *ApplicationSetReconciler) updateApplicationSetApplicationStatusConditio ) } - return applicationSet.Status.Conditions, nil + return applicationSet.Status.Conditions } func findApplicationStatusIndex(appStatuses []argov1alpha1.ApplicationSetApplicationStatus, application string) int { @@ -1374,7 +1344,7 @@ func (r *ApplicationSetReconciler) setAppSetApplicationStatus(ctx context.Contex return nil } -func (r *ApplicationSetReconciler) syncValidApplications(logCtx *log.Entry, applicationSet *argov1alpha1.ApplicationSet, appSyncMap map[string]bool, appMap map[string]argov1alpha1.Application, validApps []argov1alpha1.Application) ([]argov1alpha1.Application, error) { +func (r *ApplicationSetReconciler) syncValidApplications(logCtx *log.Entry, applicationSet *argov1alpha1.ApplicationSet, appSyncMap map[string]bool, appMap map[string]argov1alpha1.Application, validApps []argov1alpha1.Application) []argov1alpha1.Application { rolloutApps := []argov1alpha1.Application{} for i := range validApps { pruneEnabled := false @@ -1395,15 +1365,15 @@ func (r *ApplicationSetReconciler) syncValidApplications(logCtx *log.Entry, appl // check appSyncMap to determine which Applications are ready to be updated and which should be skipped if appSyncMap[validApps[i].Name] && appMap[validApps[i].Name].Status.Sync.Status == "OutOfSync" && appSetStatusPending { logCtx.Infof("triggering sync for application: %v, prune enabled: %v", validApps[i].Name, pruneEnabled) - validApps[i], _ = syncApplication(validApps[i], pruneEnabled) + validApps[i] = syncApplication(validApps[i], pruneEnabled) } rolloutApps = append(rolloutApps, validApps[i]) } - return rolloutApps, nil + return rolloutApps } // used by the RollingSync Progressive Sync strategy to trigger a sync of a particular Application resource -func syncApplication(application argov1alpha1.Application, prune bool) (argov1alpha1.Application, error) { +func syncApplication(application argov1alpha1.Application, prune bool) argov1alpha1.Application { operation := argov1alpha1.Operation{ InitiatedBy: argov1alpha1.OperationInitiator{ Username: "applicationset-controller", @@ -1429,7 +1399,7 @@ func syncApplication(application argov1alpha1.Application, prune bool) (argov1al } application.Operation = &operation - return application, nil + return application } func getOwnsHandlerPredicates(enableProgressiveSyncs bool) predicate.Funcs { diff --git a/applicationset/controllers/applicationset_controller_test.go b/applicationset/controllers/applicationset_controller_test.go index 006ae1cc34998..dba734c8410b5 100644 --- a/applicationset/controllers/applicationset_controller_test.go +++ b/applicationset/controllers/applicationset_controller_test.go @@ -3666,8 +3666,7 @@ func TestBuildAppDependencyList(t *testing.T) { KubeClientset: kubeclientset, } - appDependencyList, appStepMap, err := r.buildAppDependencyList(log.NewEntry(log.StandardLogger()), cc.appSet, cc.apps) - require.NoError(t, err, "expected no errors, but errors occurred") + appDependencyList, appStepMap := r.buildAppDependencyList(log.NewEntry(log.StandardLogger()), cc.appSet, cc.apps) assert.Equal(t, cc.expectedList, appDependencyList, "expected appDependencyList did not match actual") assert.Equal(t, cc.expectedStepMap, appStepMap, "expected appStepMap did not match actual") }) @@ -4257,8 +4256,7 @@ func TestBuildAppSyncMap(t *testing.T) { KubeClientset: kubeclientset, } - appSyncMap, err := r.buildAppSyncMap(context.TODO(), cc.appSet, cc.appDependencyList, cc.appMap) - require.NoError(t, err, "expected no errors, but errors occurred") + appSyncMap := r.buildAppSyncMap(cc.appSet, cc.appDependencyList, cc.appMap) assert.Equal(t, cc.expectedMap, appSyncMap, "expected appSyncMap did not match actual") }) } @@ -5800,7 +5798,7 @@ func TestUpdateApplicationSetApplicationStatusProgress(t *testing.T) { KubeClientset: kubeclientset, } - appStatuses, err := r.updateApplicationSetApplicationStatusProgress(context.TODO(), log.NewEntry(log.StandardLogger()), &cc.appSet, cc.appSyncMap, cc.appStepMap, cc.appMap) + appStatuses, err := r.updateApplicationSetApplicationStatusProgress(context.TODO(), log.NewEntry(log.StandardLogger()), &cc.appSet, cc.appSyncMap, cc.appStepMap) // opt out of testing the LastTransitionTime is accurate for i := range appStatuses { diff --git a/cmd/argocd/commands/app_resource_test.go b/cmd/argocd/commands/app_resource_test.go index 5b85f96050109..4a6cd50d6800f 100644 --- a/cmd/argocd/commands/app_resource_test.go +++ b/cmd/argocd/commands/app_resource_test.go @@ -36,7 +36,7 @@ func TestPrintTreeViewAppResources(t *testing.T) { buf := &bytes.Buffer{} w := tabwriter.NewWriter(buf, 0, 0, 2, ' ', 0) - printTreeViewAppResourcesNotOrphaned(nodeMapping, mapParentToChild, parentNode, false, false, w) + printTreeViewAppResourcesNotOrphaned(nodeMapping, mapParentToChild, parentNode, w) if err := w.Flush(); err != nil { t.Fatal(err) } @@ -77,7 +77,7 @@ func TestPrintTreeViewDetailedAppResources(t *testing.T) { buf := &bytes.Buffer{} w := tabwriter.NewWriter(buf, 0, 0, 2, ' ', 0) - printDetailedTreeViewAppResourcesNotOrphaned(nodeMapping, mapParentToChild, parentNode, false, false, w) + printDetailedTreeViewAppResourcesNotOrphaned(nodeMapping, mapParentToChild, parentNode, w) if err := w.Flush(); err != nil { t.Fatal(err) } diff --git a/cmd/argocd/commands/app_resources.go b/cmd/argocd/commands/app_resources.go index c1fc1dfc82f2a..f86ab0669661b 100644 --- a/cmd/argocd/commands/app_resources.go +++ b/cmd/argocd/commands/app_resources.go @@ -175,25 +175,25 @@ func parentChildInfo(nodes []v1alpha1.ResourceNode) (map[string]v1alpha1.Resourc return mapUidToNode, mapParentToChild, parentNode } -func printDetailedTreeViewAppResourcesNotOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, orphaned bool, listAll bool, w *tabwriter.Writer) { +func printDetailedTreeViewAppResourcesNotOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, w *tabwriter.Writer) { for uid := range parentNodes { detailedTreeViewAppResourcesNotOrphaned("", nodeMapping, parentChildMapping, nodeMapping[uid], w) } } -func printDetailedTreeViewAppResourcesOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, orphaned bool, listAll bool, w *tabwriter.Writer) { +func printDetailedTreeViewAppResourcesOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, w *tabwriter.Writer) { for uid := range parentNodes { detailedTreeViewAppResourcesOrphaned("", nodeMapping, parentChildMapping, nodeMapping[uid], w) } } -func printTreeViewAppResourcesNotOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, orphaned bool, listAll bool, w *tabwriter.Writer) { +func printTreeViewAppResourcesNotOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, w *tabwriter.Writer) { for uid := range parentNodes { treeViewAppResourcesNotOrphaned("", nodeMapping, parentChildMapping, nodeMapping[uid], w) } } -func printTreeViewAppResourcesOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, orphaned bool, listAll bool, w *tabwriter.Writer) { +func printTreeViewAppResourcesOrphaned(nodeMapping map[string]v1alpha1.ResourceNode, parentChildMapping map[string][]string, parentNodes map[string]struct{}, w *tabwriter.Writer) { for uid := range parentNodes { treeViewAppResourcesOrphaned("", nodeMapping, parentChildMapping, nodeMapping[uid], w) } @@ -206,24 +206,24 @@ func printResources(listAll bool, orphaned bool, appResourceTree *v1alpha1.Appli if !orphaned || listAll { mapUidToNode, mapParentToChild, parentNode := parentChildInfo(appResourceTree.Nodes) - printDetailedTreeViewAppResourcesNotOrphaned(mapUidToNode, mapParentToChild, parentNode, orphaned, listAll, w) + printDetailedTreeViewAppResourcesNotOrphaned(mapUidToNode, mapParentToChild, parentNode, w) } if orphaned || listAll { mapUidToNode, mapParentToChild, parentNode := parentChildInfo(appResourceTree.OrphanedNodes) - printDetailedTreeViewAppResourcesOrphaned(mapUidToNode, mapParentToChild, parentNode, orphaned, listAll, w) + printDetailedTreeViewAppResourcesOrphaned(mapUidToNode, mapParentToChild, parentNode, w) } } else if output == "tree" { fmt.Fprintf(w, "GROUP\tKIND\tNAMESPACE\tNAME\tORPHANED\n") if !orphaned || listAll { mapUidToNode, mapParentToChild, parentNode := parentChildInfo(appResourceTree.Nodes) - printTreeViewAppResourcesNotOrphaned(mapUidToNode, mapParentToChild, parentNode, orphaned, listAll, w) + printTreeViewAppResourcesNotOrphaned(mapUidToNode, mapParentToChild, parentNode, w) } if orphaned || listAll { mapUidToNode, mapParentToChild, parentNode := parentChildInfo(appResourceTree.OrphanedNodes) - printTreeViewAppResourcesOrphaned(mapUidToNode, mapParentToChild, parentNode, orphaned, listAll, w) + printTreeViewAppResourcesOrphaned(mapUidToNode, mapParentToChild, parentNode, w) } } else { headers := []interface{}{"GROUP", "KIND", "NAMESPACE", "NAME", "ORPHANED"} diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 1af3a64e16b86..ec51d85a9a638 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -1957,11 +1957,18 @@ func (ctrl *ApplicationController) autoSync(app *appv1.Application, syncStatus * } } + selfHeal := app.Spec.SyncPolicy.Automated.SelfHeal + // Multi-Source Apps with selfHeal disabled should not trigger an autosync if + // the last sync revision and the new sync revision is the same. + if app.Spec.HasMultipleSources() && !selfHeal && reflect.DeepEqual(app.Status.Sync.Revisions, syncStatus.Revisions) { + logCtx.Infof("Skipping auto-sync: selfHeal disabled and sync caused by object update") + return nil, 0 + } + desiredCommitSHA := syncStatus.Revision desiredCommitSHAsMS := syncStatus.Revisions alreadyAttempted, attemptPhase := alreadyAttemptedSync(app, desiredCommitSHA, desiredCommitSHAsMS, app.Spec.HasMultipleSources()) ts.AddCheckpoint("already_attempted_sync_ms") - selfHeal := app.Spec.SyncPolicy.Automated.SelfHeal op := appv1.Operation{ Sync: &appv1.SyncOperation{ Revision: desiredCommitSHA, diff --git a/controller/appcontroller_test.go b/controller/appcontroller_test.go index c023bf17510e6..513400a8c0b65 100644 --- a/controller/appcontroller_test.go +++ b/controller/appcontroller_test.go @@ -563,6 +563,42 @@ func TestAutoSync(t *testing.T) { assert.False(t, app.Operation.Sync.Prune) } +func TestMultiSourceSelfHeal(t *testing.T) { + // Simulate OutOfSync caused by object change in cluster + // So our Sync Revisions and SyncStatus Revisions should deep equal + t.Run("ClusterObjectChangeShouldNotTriggerAutoSync", func(t *testing.T) { + app := newFakeMultiSourceApp() + app.Spec.SyncPolicy.Automated.SelfHeal = false + app.Status.Sync.Revisions = []string{"z", "x", "v"} + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) + syncStatus := v1alpha1.SyncStatus{ + Status: v1alpha1.SyncStatusCodeOutOfSync, + Revisions: []string{"z", "x", "v"}, + } + cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{{Name: "guestbook-1", Kind: kube.DeploymentKind, Status: v1alpha1.SyncStatusCodeOutOfSync}}) + assert.Nil(t, cond) + app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{}) + require.NoError(t, err) + assert.Nil(t, app.Operation) + }) + + t.Run("NewRevisionChangeShouldTriggerAutoSync", func(t *testing.T) { + app := newFakeMultiSourceApp() + app.Spec.SyncPolicy.Automated.SelfHeal = false + app.Status.Sync.Revisions = []string{"a", "b", "c"} + ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil) + syncStatus := v1alpha1.SyncStatus{ + Status: v1alpha1.SyncStatusCodeOutOfSync, + Revisions: []string{"z", "x", "v"}, + } + cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{{Name: "guestbook-1", Kind: kube.DeploymentKind, Status: v1alpha1.SyncStatusCodeOutOfSync}}) + assert.Nil(t, cond) + app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{}) + require.NoError(t, err) + assert.NotNil(t, app.Operation) + }) +} + func TestAutoSyncNotAllowEmpty(t *testing.T) { app := newFakeApp() app.Spec.SyncPolicy.Automated.Prune = true diff --git a/controller/state.go b/controller/state.go index a9a3be4bdd6b8..8c6b24c643965 100644 --- a/controller/state.go +++ b/controller/state.go @@ -591,7 +591,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 } // No need to care about the return value here, we just want the modified managedNs - _, err = syncNamespace(m.resourceTracking, appLabelKey, trackingMethod, app.Name, app.Spec.SyncPolicy)(managedNs, liveObj) + _, err = syncNamespace(app.Spec.SyncPolicy)(managedNs, liveObj) if err != nil { conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now}) failedToLoadObjs = true diff --git a/controller/sync.go b/controller/sync.go index 9cd4b1f0c6e93..ecfeec709ee3c 100644 --- a/controller/sync.go +++ b/controller/sync.go @@ -324,7 +324,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha } if syncOp.SyncOptions.HasOption("CreateNamespace=true") { - opts = append(opts, sync.WithNamespaceModifier(syncNamespace(m.resourceTracking, appLabelKey, trackingMethod, app.Name, app.Spec.SyncPolicy))) + opts = append(opts, sync.WithNamespaceModifier(syncNamespace(app.Spec.SyncPolicy))) } syncCtx, cleanup, err := sync.NewSyncContext( diff --git a/controller/sync_namespace.go b/controller/sync_namespace.go index 9578dc8651322..43e0dc6170f48 100644 --- a/controller/sync_namespace.go +++ b/controller/sync_namespace.go @@ -5,12 +5,11 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/argoproj/argo-cd/v2/util/argo" ) // syncNamespace determine if Argo CD should create and/or manage the namespace // where the application will be deployed. -func syncNamespace(resourceTracking argo.ResourceTracking, appLabelKey string, trackingMethod v1alpha1.TrackingMethod, appName string, syncPolicy *v1alpha1.SyncPolicy) func(m, l *unstructured.Unstructured) (bool, error) { +func syncNamespace(syncPolicy *v1alpha1.SyncPolicy) func(m *unstructured.Unstructured, l *unstructured.Unstructured) (bool, error) { // This function must return true for the managed namespace to be synced. return func(managedNs, liveNs *unstructured.Unstructured) (bool, error) { if managedNs == nil { diff --git a/controller/sync_namespace_test.go b/controller/sync_namespace_test.go index 7e60b0d287789..0124d99532b91 100644 --- a/controller/sync_namespace_test.go +++ b/controller/sync_namespace_test.go @@ -8,9 +8,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" - "github.com/argoproj/argo-cd/v2/common" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/argoproj/argo-cd/v2/util/argo" ) func createFakeNamespace(uid string, resourceVersion string, labels map[string]string, annotations map[string]string) *unstructured.Unstructured { @@ -250,7 +248,7 @@ func Test_shouldNamespaceSync(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual, err := syncNamespace(argo.NewResourceTracking(), common.LabelKeyAppInstance, argo.TrackingMethodAnnotation, "some-app", tt.syncPolicy)(tt.managedNs, tt.liveNs) + actual, err := syncNamespace(tt.syncPolicy)(tt.managedNs, tt.liveNs) require.NoError(t, err) if tt.managedNs != nil { diff --git a/hack/gen-resources/generators/cluster_generator.go b/hack/gen-resources/generators/cluster_generator.go index 6f125723c35ef..c07343f15c6d5 100644 --- a/hack/gen-resources/generators/cluster_generator.go +++ b/hack/gen-resources/generators/cluster_generator.go @@ -161,7 +161,7 @@ func (cg *ClusterGenerator) getClusterServerUri(namespace string, releaseSuffix return "https://" + pod.Status.PodIP + ":8443", nil } -func (cg *ClusterGenerator) retrieveClusterUri(namespace, releaseSuffix string) (string, error) { +func (cg *ClusterGenerator) retrieveClusterUri(namespace, releaseSuffix string) string { for i := 0; i < 10; i++ { log.Printf("Attempting to get cluster uri") uri, err := cg.getClusterServerUri(namespace, releaseSuffix) @@ -170,9 +170,9 @@ func (cg *ClusterGenerator) retrieveClusterUri(namespace, releaseSuffix string) time.Sleep(10 * time.Second) continue } - return uri, nil + return uri } - return "", nil + return "" } func (cg *ClusterGenerator) generate(i int, opts *util.GenerateOpts) error { @@ -208,11 +208,7 @@ func (cg *ClusterGenerator) generate(i int, opts *util.GenerateOpts) error { log.Print("Get cluster server uri") - uri, err := cg.retrieveClusterUri(namespace, releaseSuffix) - if err != nil { - return err - } - + uri := cg.retrieveClusterUri(namespace, releaseSuffix) log.Printf("Cluster server uri is %s", uri) log.Print("Create cluster") diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index d530f1a6e0c15..a729ca1b1af51 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -2042,7 +2042,7 @@ func (s *Service) GetAppDetails(ctx context.Context, q *apiclient.RepoServerAppD return err } case v1alpha1.ApplicationSourceTypePlugin: - if err := populatePluginAppDetails(ctx, res, opContext.appPath, repoRoot, q, s.gitCredsStore, s.initConstants.CMPTarExcludedGlobs); err != nil { + if err := populatePluginAppDetails(ctx, res, opContext.appPath, repoRoot, q, s.initConstants.CMPTarExcludedGlobs); err != nil { return fmt.Errorf("failed to populate plugin app details: %w", err) } } @@ -2196,7 +2196,7 @@ func populateKustomizeAppDetails(res *apiclient.RepoAppDetailsResponse, q *apicl return nil } -func populatePluginAppDetails(ctx context.Context, res *apiclient.RepoAppDetailsResponse, appPath string, repoPath string, q *apiclient.RepoServerAppDetailsQuery, store git.CredsStore, tarExcludedGlobs []string) error { +func populatePluginAppDetails(ctx context.Context, res *apiclient.RepoAppDetailsResponse, appPath string, repoPath string, q *apiclient.RepoServerAppDetailsQuery, tarExcludedGlobs []string) error { res.Plugin = &apiclient.PluginAppSpec{} envVars := []string{ diff --git a/server/application/application.go b/server/application/application.go index 0640bbaa13ebb..3972ee9dc6608 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -2444,7 +2444,7 @@ func (s *Server) RunResourceAction(ctx context.Context, q *application.ResourceA // the dry-run for relevant apply/delete operation would have to be invoked as well. for _, impactedResource := range newObjects { newObj := impactedResource.UnstructuredObj - err := s.verifyResourcePermitted(ctx, app, proj, newObj) + err := s.verifyResourcePermitted(app, proj, newObj) if err != nil { return nil, err } @@ -2537,7 +2537,7 @@ func (s *Server) patchResource(ctx context.Context, config *rest.Config, liveObj return &application.ApplicationResponse{}, nil } -func (s *Server) verifyResourcePermitted(ctx context.Context, app *appv1.Application, proj *appv1.AppProject, obj *unstructured.Unstructured) error { +func (s *Server) verifyResourcePermitted(app *appv1.Application, proj *appv1.AppProject, obj *unstructured.Unstructured) error { permitted, err := proj.IsResourcePermitted(schema.GroupKind{Group: obj.GroupVersionKind().Group, Kind: obj.GroupVersionKind().Kind}, obj.GetNamespace(), app.Spec.Destination, func(project string) ([]*appv1.Cluster, error) { clusters, err := s.db.GetProjectClusters(context.TODO(), project) if err != nil { diff --git a/server/application/terminal.go b/server/application/terminal.go index 0f454467ec5df..e6ddc6d832df3 100644 --- a/server/application/terminal.go +++ b/server/application/terminal.go @@ -33,29 +33,32 @@ import ( type terminalHandler struct { appLister applisters.ApplicationLister db db.ArgoDB - enf *rbac.Enforcer cache *servercache.Cache appResourceTreeFn func(ctx context.Context, app *appv1.Application) (*appv1.ApplicationTree, error) allowedShells []string namespace string enabledNamespaces []string sessionManager *util_session.SessionManager + terminalOptions *TerminalOptions +} + +type TerminalOptions struct { + DisableAuth bool + Enf *rbac.Enforcer } // NewHandler returns a new terminal handler. -func NewHandler(appLister applisters.ApplicationLister, namespace string, enabledNamespaces []string, db db.ArgoDB, enf *rbac.Enforcer, cache *servercache.Cache, - appResourceTree AppResourceTreeFn, allowedShells []string, sessionManager *util_session.SessionManager, -) *terminalHandler { +func NewHandler(appLister applisters.ApplicationLister, namespace string, enabledNamespaces []string, db db.ArgoDB, cache *servercache.Cache, appResourceTree AppResourceTreeFn, allowedShells []string, sessionManager *sessionmgr.SessionManager, terminalOptions *TerminalOptions) *terminalHandler { return &terminalHandler{ appLister: appLister, db: db, - enf: enf, cache: cache, appResourceTreeFn: appResourceTree, allowedShells: allowedShells, namespace: namespace, enabledNamespaces: enabledNamespaces, sessionManager: sessionManager, + terminalOptions: terminalOptions, } } @@ -146,12 +149,12 @@ func (s *terminalHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { ctx := r.Context() appRBACName := security.RBACName(s.namespace, project, appNamespace, app) - if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplications, rbacpolicy.ActionGet, appRBACName); err != nil { + if err := s.terminalOptions.Enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplications, rbacpolicy.ActionGet, appRBACName); err != nil { http.Error(w, err.Error(), http.StatusUnauthorized) return } - if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceExec, rbacpolicy.ActionCreate, appRBACName); err != nil { + if err := s.terminalOptions.Enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceExec, rbacpolicy.ActionCreate, appRBACName); err != nil { http.Error(w, err.Error(), http.StatusUnauthorized) return } @@ -229,7 +232,7 @@ func (s *terminalHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { fieldLog.Info("terminal session starting") - session, err := newTerminalSession(ctx, w, r, nil, s.sessionManager, appRBACName, s.enf) + session, err := newTerminalSession(ctx, w, r, nil, s.sessionManager, appRBACName, s.terminalOptions) if err != nil { http.Error(w, "Failed to start terminal session", http.StatusBadRequest) return diff --git a/server/application/websocket.go b/server/application/websocket.go index d6057cae7957f..86c85749d803b 100644 --- a/server/application/websocket.go +++ b/server/application/websocket.go @@ -8,10 +8,8 @@ import ( "sync" "time" - "github.com/argoproj/argo-cd/v2/server/rbacpolicy" - "github.com/argoproj/argo-cd/v2/util/rbac" - "github.com/argoproj/argo-cd/v2/common" + "github.com/argoproj/argo-cd/v2/server/rbacpolicy" httputil "github.com/argoproj/argo-cd/v2/util/http" util_session "github.com/argoproj/argo-cd/v2/util/session" @@ -46,7 +44,7 @@ type terminalSession struct { sessionManager *util_session.SessionManager token *string appRBACName string - enf *rbac.Enforcer + terminalOpts *TerminalOptions } // getToken get auth token from web socket request @@ -56,7 +54,7 @@ func getToken(r *http.Request) (string, error) { } // newTerminalSession create terminalSession -func newTerminalSession(ctx context.Context, w http.ResponseWriter, r *http.Request, responseHeader http.Header, sessionManager *util_session.SessionManager, appRBACName string, enf *rbac.Enforcer) (*terminalSession, error) { +func newTerminalSession(ctx context.Context, w http.ResponseWriter, r *http.Request, responseHeader http.Header, sessionManager *util_session.SessionManager, appRBACName string, terminalOpts *TerminalOptions) (*terminalSession, error) { token, err := getToken(r) if err != nil { return nil, err @@ -75,7 +73,7 @@ func newTerminalSession(ctx context.Context, w http.ResponseWriter, r *http.Requ sessionManager: sessionManager, token: &token, appRBACName: appRBACName, - enf: enf, + terminalOpts: terminalOpts, } return session, nil } @@ -141,7 +139,7 @@ func (t *terminalSession) validatePermissions(p []byte) (int, error) { Operation: "stdout", Data: "Permission denied", }) - if err := t.enf.EnforceErr(t.ctx.Value("claims"), rbacpolicy.ResourceApplications, rbacpolicy.ActionGet, t.appRBACName); err != nil { + if err := t.terminalOpts.Enf.EnforceErr(t.ctx.Value("claims"), rbacpolicy.ResourceApplications, rbacpolicy.ActionGet, t.appRBACName); err != nil { err = t.wsConn.WriteMessage(websocket.TextMessage, permissionDeniedMessage) if err != nil { log.Errorf("permission denied message err: %v", err) @@ -149,7 +147,7 @@ func (t *terminalSession) validatePermissions(p []byte) (int, error) { return copy(p, EndOfTransmission), permissionDeniedErr } - if err := t.enf.EnforceErr(t.ctx.Value("claims"), rbacpolicy.ResourceExec, rbacpolicy.ActionCreate, t.appRBACName); err != nil { + if err := t.terminalOpts.Enf.EnforceErr(t.ctx.Value("claims"), rbacpolicy.ResourceExec, rbacpolicy.ActionCreate, t.appRBACName); err != nil { err = t.wsConn.WriteMessage(websocket.TextMessage, permissionDeniedMessage) if err != nil { log.Errorf("permission denied message err: %v", err) @@ -159,8 +157,12 @@ func (t *terminalSession) validatePermissions(p []byte) (int, error) { return 0, nil } -// Read called in a loop from remotecommand as long as the process is running -func (t *terminalSession) Read(p []byte) (int, error) { +func (t *terminalSession) performValidationsAndReconnect(p []byte) (int, error) { + // In disable auth mode, no point verifying the token or validating permissions + if t.terminalOpts.DisableAuth { + return 0, nil + } + // check if token still valid _, newToken, err := t.sessionManager.VerifyToken(*t.token) // err in case if token is revoked, newToken in case if refresh happened @@ -168,13 +170,21 @@ func (t *terminalSession) Read(p []byte) (int, error) { // need to send reconnect code in case if token was refreshed return t.reconnect() } - - // validate permissions code, err := t.validatePermissions(p) if err != nil { return code, err } + return 0, nil +} + +// Read called in a loop from remotecommand as long as the process is running +func (t *terminalSession) Read(p []byte) (int, error) { + code, err := t.performValidationsAndReconnect(p) + if err != nil { + return code, err + } + t.readLock.Lock() _, message, err := t.wsConn.ReadMessage() t.readLock.Unlock() diff --git a/server/application/websocket_test.go b/server/application/websocket_test.go index 5b6f903d48f27..0d048a1727d1b 100644 --- a/server/application/websocket_test.go +++ b/server/application/websocket_test.go @@ -85,6 +85,45 @@ func TestReconnect(t *testing.T) { assert.Equal(t, ReconnectMessage, message.Data) } +func testServerConnection(t *testing.T, testFunc func(w http.ResponseWriter, r *http.Request), expectPermissionDenied bool) { + s := httptest.NewServer(http.HandlerFunc(testFunc)) + defer s.Close() + + u := "ws" + strings.TrimPrefix(s.URL, "http") + + // Connect to the server + ws, _, err := websocket.DefaultDialer.Dial(u, nil) + require.NoError(t, err) + + defer ws.Close() + if expectPermissionDenied { + _, p, _ := ws.ReadMessage() + + var message TerminalMessage + + err = json.Unmarshal(p, &message) + + require.NoError(t, err) + assert.Equal(t, "Permission denied", message.Data) + } +} + +func TestVerifyAndReconnectDisableAuthTrue(t *testing.T) { + validate := func(w http.ResponseWriter, r *http.Request) { + ts := newTestTerminalSession(w, r) + // Currently testing only the usecase of disableAuth: true since the disableAuth: false case + // requires a valid token to be passed in the request. + // Note that running with disableAuth: false will surprisingly succeed as well, because + // the underlying token nil pointer dereference is swallowed in a location I didn't find, + // or even swallowed by the test framework. + ts.terminalOpts = &TerminalOptions{DisableAuth: true} + code, err := ts.performValidationsAndReconnect([]byte{}) + assert.Equal(t, 0, code) + require.NoError(t, err) + } + testServerConnection(t, validate, false) +} + func TestValidateWithAdminPermissions(t *testing.T) { validate := func(w http.ResponseWriter, r *http.Request) { enf := newEnforcer() @@ -94,7 +133,7 @@ func TestValidateWithAdminPermissions(t *testing.T) { return true }) ts := newTestTerminalSession(w, r) - ts.enf = enf + ts.terminalOpts = &TerminalOptions{Enf: enf} ts.appRBACName = "test" // nolint:staticcheck ts.ctx = context.WithValue(context.Background(), "claims", &jwt.MapClaims{"groups": []string{"admin"}}) @@ -102,16 +141,7 @@ func TestValidateWithAdminPermissions(t *testing.T) { require.NoError(t, err) } - s := httptest.NewServer(http.HandlerFunc(validate)) - defer s.Close() - - u := "ws" + strings.TrimPrefix(s.URL, "http") - - // Connect to the server - ws, _, err := websocket.DefaultDialer.Dial(u, nil) - require.NoError(t, err) - - defer ws.Close() + testServerConnection(t, validate, false) } func TestValidateWithoutPermissions(t *testing.T) { @@ -123,7 +153,7 @@ func TestValidateWithoutPermissions(t *testing.T) { return false }) ts := newTestTerminalSession(w, r) - ts.enf = enf + ts.terminalOpts = &TerminalOptions{Enf: enf} ts.appRBACName = "test" // nolint:staticcheck ts.ctx = context.WithValue(context.Background(), "claims", &jwt.MapClaims{"groups": []string{"test"}}) @@ -132,23 +162,5 @@ func TestValidateWithoutPermissions(t *testing.T) { assert.Equal(t, permissionDeniedErr.Error(), err.Error()) } - s := httptest.NewServer(http.HandlerFunc(validate)) - defer s.Close() - - u := "ws" + strings.TrimPrefix(s.URL, "http") - - // Connect to the server - ws, _, err := websocket.DefaultDialer.Dial(u, nil) - require.NoError(t, err) - - defer ws.Close() - - _, p, _ := ws.ReadMessage() - - var message TerminalMessage - - err = json.Unmarshal(p, &message) - - require.NoError(t, err) - assert.Equal(t, "Permission denied", message.Data) + testServerConnection(t, validate, true) } diff --git a/server/applicationset/applicationset.go b/server/applicationset/applicationset.go index a66173384e8c1..23bf93344d6d3 100644 --- a/server/applicationset/applicationset.go +++ b/server/applicationset/applicationset.go @@ -185,7 +185,7 @@ func (s *Server) Create(ctx context.Context, q *applicationset.ApplicationSetCre return nil, fmt.Errorf("error creating ApplicationSets: ApplicationSets is nil in request") } - projectName, err := s.validateAppSet(ctx, appset) + projectName, err := s.validateAppSet(appset) if err != nil { return nil, fmt.Errorf("error validating ApplicationSets: %w", err) } @@ -363,10 +363,10 @@ func (s *Server) ResourceTree(ctx context.Context, q *applicationset.Application return nil, err } - return s.buildApplicationSetTree(ctx, a) + return s.buildApplicationSetTree(a) } -func (s *Server) buildApplicationSetTree(ctx context.Context, a *v1alpha1.ApplicationSet) (*v1alpha1.ApplicationSetTree, error) { +func (s *Server) buildApplicationSetTree(a *v1alpha1.ApplicationSet) (*v1alpha1.ApplicationSetTree, error) { var tree v1alpha1.ApplicationSetTree gvk := v1alpha1.ApplicationSetSchemaGroupVersionKind @@ -393,7 +393,7 @@ func (s *Server) buildApplicationSetTree(ctx context.Context, a *v1alpha1.Applic return &tree, nil } -func (s *Server) validateAppSet(ctx context.Context, appset *v1alpha1.ApplicationSet) (string, error) { +func (s *Server) validateAppSet(appset *v1alpha1.ApplicationSet) (string, error) { if appset == nil { return "", fmt.Errorf("ApplicationSet cannot be validated for nil value") } diff --git a/server/server.go b/server/server.go index 2222b7b3df87f..41474240a40e2 100644 --- a/server/server.go +++ b/server/server.go @@ -737,7 +737,7 @@ func (a *ArgoCDServer) rbacPolicyLoader(ctx context.Context) { scopes = make([]string, 0) err := yaml.Unmarshal([]byte(scopesStr), &scopes) if err != nil { - return err + return fmt.Errorf("error unmarshalling scopes: %w", err) } } @@ -950,7 +950,7 @@ func (a *ArgoCDServer) translateGrpcCookieHeader(ctx context.Context, w http.Res token := sessionResp.Token err := a.setTokenCookie(token, w) if err != nil { - return err + return fmt.Errorf("error setting token cookie from session response: %w", err) } } else if md, ok := runtime.ServerMetadataFromContext(ctx); ok { renewToken := md.HeaderMD[renewTokenKey] @@ -970,7 +970,7 @@ func (a *ArgoCDServer) setTokenCookie(token string, w http.ResponseWriter) error } cookies, err := httputil.MakeCookieMetadata(common.AuthCookieName, token, flags...) if err != nil { - return err + return fmt.Errorf("error creating cookie metadata: %w", err) } for _, cookie := range cookies { w.Header().Add("Set-Cookie", cookie) @@ -1041,7 +1041,9 @@ func (a *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWebHandl } mux.Handle("/api/", handler) - terminal := application.NewHandler(a.appLister, a.Namespace, a.ApplicationNamespaces, a.db, a.enf, a.Cache, appResourceTreeFn, a.settings.ExecShells, a.sessionMgr). + terminalOpts := application.TerminalOptions{DisableAuth: a.ArgoCDServerOpts.DisableAuth, Enf: a.enf} + + terminal := application.NewHandler(a.appLister, a.Namespace, a.ApplicationNamespaces, a.db, a.Cache, appResourceTreeFn, a.settings.ExecShells, a.sessionMgr, &terminalOpts). WithFeatureFlagMiddleware(a.settingsMgr.GetSettings) th := util_session.WithAuthMiddleware(a.DisableAuth, a.sessionMgr, terminal) mux.Handle("/terminal", th) diff --git a/test/container/Dockerfile b/test/container/Dockerfile index 924592905330f..ad4995ecc8cfe 100644 --- a/test/container/Dockerfile +++ b/test/container/Dockerfile @@ -10,7 +10,7 @@ FROM docker.io/library/node:22.5.1@sha256:86915971d2ce1548842315fcce7cda0da59319 FROM docker.io/library/golang:1.23@sha256:a400dc7e0a82fb3ffef3736d38edf42c3cc36140d5427d65a19c422d07ba60d9 as golang -FROM docker.io/library/registry:2.8@sha256:79b29591e1601a73f03fcd413e655b72b9abfae5a23f1ad2e883d4942fbb4351 as registry +FROM docker.io/library/registry:2.8@sha256:12120425f07de11a1b899e418d4b0ea174c8d4d572d45bdb640f93bc7ca06a3d as registry FROM docker.io/bitnami/kubectl:1.30@sha256:dc190b7c6b87ba74eabff4e1e78758eed95ebb97b5a186bbdaf2b43625f9a62a as kubectl diff --git a/ui-test/package.json b/ui-test/package.json index b5a788d37a5c7..36169fd3f5eab 100644 --- a/ui-test/package.json +++ b/ui-test/package.json @@ -14,12 +14,12 @@ "dependencies": { "@types/selenium-webdriver": "^4.1.23", "assert": "^2.1.0", - "chromedriver": "^126.0.5", + "chromedriver": "^127.0.3", "selenium-webdriver": "^4.21.0" }, "devDependencies": { "@types/mocha": "^10.0.6", - "@types/node": "^22.1.0", + "@types/node": "^22.3.0", "dotenv": "^16.4.5", "mocha": "^10.4.0", "prettier": "^2.8.8", diff --git a/ui-test/yarn.lock b/ui-test/yarn.lock index 65f7a8c0a21ba..f34f539eb9d27 100644 --- a/ui-test/yarn.lock +++ b/ui-test/yarn.lock @@ -38,12 +38,12 @@ resolved "https://registry.yarnpkg.com/@types/mocha/-/mocha-10.0.6.tgz#818551d39113081048bdddbef96701b4e8bb9d1b" integrity sha512-dJvrYWxP/UcXm36Qn36fxhUKu8A/xMRXVT2cliFF1Z7UA9liG5Psj3ezNSZw+5puH2czDXRLcXQxf8JbJt0ejg== -"@types/node@*", "@types/node@^22.1.0": - version "22.1.0" - resolved "https://registry.yarnpkg.com/@types/node/-/node-22.1.0.tgz#6d6adc648b5e03f0e83c78dc788c2b037d0ad94b" - integrity sha512-AOmuRF0R2/5j1knA3c6G3HOk523Ga+l+ZXltX8SF1+5oqcXijjfTd8fY3XRZqSihEu9XhtQnKYLmkFaoxgsJHw== +"@types/node@*", "@types/node@^22.3.0": + version "22.3.0" + resolved "https://registry.yarnpkg.com/@types/node/-/node-22.3.0.tgz#7f8da0e2b72c27c4f9bd3cb5ef805209d04d4f9e" + integrity sha512-nrWpWVaDZuaVc5X84xJ0vNrLvomM205oQyLsRt7OHNZbSHslcWsvgFR7O7hire2ZonjLrWBbedmotmIlJDVd6g== dependencies: - undici-types "~6.13.0" + undici-types "~6.18.2" "@types/selenium-webdriver@^4.1.23": version "4.1.23" @@ -153,10 +153,10 @@ available-typed-arrays@^1.0.2: dependencies: array-filter "^1.0.0" -axios@^1.6.7: - version "1.7.1" - resolved "https://registry.yarnpkg.com/axios/-/axios-1.7.1.tgz#522145622a09dfaf49359837db9649ff245a35b9" - integrity sha512-+LV37nQcd1EpFalkXksWNBiA17NZ5m5/WspmHGmZmdx1qBOg/VNq/c4eRJiA9VQQHBOs+N0ZhhdU10h2TyNK7Q== +axios@^1.7.4: + version "1.7.4" + resolved "https://registry.yarnpkg.com/axios/-/axios-1.7.4.tgz#4c8ded1b43683c8dd362973c393f3ede24052aa2" + integrity sha512-DukmaFRnY6AzAALSH4J2M3k6PkaC+MfaAGdEERRWcC9q3/TWQwLpHR8ZRLKTdQ3aBDL64EdluRDjJqKw+BPZEw== dependencies: follow-redirects "^1.15.6" form-data "^4.0.0" @@ -262,13 +262,13 @@ chokidar@3.5.3: optionalDependencies: fsevents "~2.3.2" -chromedriver@^126.0.5: - version "126.0.5" - resolved "https://registry.yarnpkg.com/chromedriver/-/chromedriver-126.0.5.tgz#5db6f463c7557e3a76b7fe7fb73aaf2f06fd07e6" - integrity sha512-xXVxwxd8CJ6yg2KEvFqLQi7V0RvF78xFnLB+xo9g9MoJNHMQccD7b4OWaxtKDy5RXrMgQ6Jb6vUN3SjTYXHLEQ== +chromedriver@^127.0.3: + version "127.0.3" + resolved "https://registry.yarnpkg.com/chromedriver/-/chromedriver-127.0.3.tgz#33abca5924eb809e0e6ef2dd30eaa8cf7dba58d4" + integrity sha512-trUHkFt0n7jGzNOgkO1srOJfz50kKyAGJ016PyV0hrtyKNIGnOC9r3Jlssz19UoEjSzI/1g2shEiIFtDbBYVaw== dependencies: "@testim/chrome-version" "^1.1.4" - axios "^1.6.7" + axios "^1.7.4" compare-versions "^6.1.0" extract-zip "^2.0.1" proxy-agent "^6.4.0" @@ -1489,10 +1489,10 @@ typescript@^5.5.4: resolved "https://registry.yarnpkg.com/typescript/-/typescript-5.5.4.tgz#d9852d6c82bad2d2eda4fd74a5762a8f5909e9ba" integrity sha512-Mtq29sKDAEYP7aljRgtPOpTvOfbwRWlS6dPRzwjdE+C0R4brX/GUyhHSecbHMFLNBLcJIPt9nl9yG5TZ1weH+Q== -undici-types@~6.13.0: - version "6.13.0" - resolved "https://registry.yarnpkg.com/undici-types/-/undici-types-6.13.0.tgz#e3e79220ab8c81ed1496b5812471afd7cf075ea5" - integrity sha512-xtFJHudx8S2DSoujjMd1WeWvn7KKWFRESZTMeL1RptAYERu29D6jphMjjY+vn96jvN3kVPDNxU/E13VTaXj6jg== +undici-types@~6.18.2: + version "6.18.2" + resolved "https://registry.yarnpkg.com/undici-types/-/undici-types-6.18.2.tgz#8b678cf939d4fc9ec56be3c68ed69c619dee28b0" + integrity sha512-5ruQbENj95yDYJNS3TvcaxPMshV7aizdv/hWYjGIKoANWKjhWNBsr2YEuYZKodQulB1b8l7ILOuDQep3afowQQ== universalify@^2.0.0: version "2.0.1" diff --git a/ui/package.json b/ui/package.json index b61d9e2cd1c3e..6068aea91f914 100644 --- a/ui/package.json +++ b/ui/package.json @@ -69,6 +69,7 @@ "@babel/preset-env": "^7.7.1", "@babel/preset-react": "^7.18.6", "@babel/preset-typescript": "^7.7.2", + "@codecov/webpack-plugin": "^0.0.1-beta.10", "@eslint/js": "^9.1.1", "@types/classnames": "^2.2.3", "@types/cookie": "^0.5.1", diff --git a/ui/src/app/applications/components/application-parameters/application-parameters.scss b/ui/src/app/applications/components/application-parameters/application-parameters.scss index e49945dc85324..86a3af27e24f5 100644 --- a/ui/src/app/applications/components/application-parameters/application-parameters.scss +++ b/ui/src/app/applications/components/application-parameters/application-parameters.scss @@ -64,6 +64,10 @@ .select { padding-bottom: 0; + + .select__value{ + min-height: 28px; + } } .row.application-retry-options { diff --git a/ui/src/app/webpack.config.js b/ui/src/app/webpack.config.js index 5b68c0a28d0f7..31641decb58fa 100644 --- a/ui/src/app/webpack.config.js +++ b/ui/src/app/webpack.config.js @@ -3,6 +3,7 @@ const MonacoWebpackPlugin = require('monaco-editor-webpack-plugin'); const CopyWebpackPlugin = require('copy-webpack-plugin'); const HtmlWebpackPlugin = require('html-webpack-plugin'); +const {codecovWebpackPlugin} = require("@codecov/webpack-plugin"); const webpack = require('webpack'); const isProd = process.env.NODE_ENV === 'production'; @@ -94,7 +95,12 @@ const config = { new MonacoWebpackPlugin({ // https://github.com/microsoft/monaco-editor-webpack-plugin#options languages: ['yaml'] - }) + }), + codecovWebpackPlugin({ + enableBundleAnalysis: process.env.CODECOV_TOKEN !== undefined, + bundleName: "argo-cd-ui", + uploadToken: process.env.CODECOV_TOKEN, + }), ], devServer: { compress: false, diff --git a/ui/yarn.lock b/ui/yarn.lock index 1a70aa578f028..88d6001cc9d41 100644 --- a/ui/yarn.lock +++ b/ui/yarn.lock @@ -1312,6 +1312,24 @@ resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39" integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw== +"@codecov/bundler-plugin-core@^0.0.1-beta.10": + version "0.0.1-beta.10" + resolved "https://registry.yarnpkg.com/@codecov/bundler-plugin-core/-/bundler-plugin-core-0.0.1-beta.10.tgz#8e19916db3ef164ae3ea0cab78b540b377ba7635" + integrity sha512-fOgy02gc0Z0ipKVe8QqN7mcmzQYjHb2UzT6RtHR+tqyYwpe1r1Kg5E/pbIOXKLaJGXK+AaOX23qtgp6kvUn+iA== + dependencies: + chalk "4.1.2" + semver "^7.5.4" + unplugin "^1.10.1" + zod "^3.22.4" + +"@codecov/webpack-plugin@^0.0.1-beta.10": + version "0.0.1-beta.10" + resolved "https://registry.yarnpkg.com/@codecov/webpack-plugin/-/webpack-plugin-0.0.1-beta.10.tgz#74b5d8b05775c39ec8acb82bd921ea60f472be1e" + integrity sha512-wGh4YEy05HdNa64zRbqv20t7gq24mqTf0T3YUr6XzEd7DLHvm3QcTtUOz8Q8eekMKtvS0CiH3UHmBIAQyPSTZw== + dependencies: + "@codecov/bundler-plugin-core" "^0.0.1-beta.10" + unplugin "^1.10.1" + "@cspotcode/source-map-support@^0.8.0": version "0.8.1" resolved "https://registry.yarnpkg.com/@cspotcode/source-map-support/-/source-map-support-0.8.1.tgz#00629c35a688e05a88b1cda684fb9d5e73f000a1" @@ -2668,6 +2686,11 @@ acorn@^8.1.0, acorn@^8.11.3, acorn@^8.4.1, acorn@^8.5.0, acorn@^8.7.1, acorn@^8. resolved "https://registry.yarnpkg.com/acorn/-/acorn-8.11.3.tgz#71e0b14e13a4ec160724b38fb7b0f233b1b81d7a" integrity sha512-Y9rRfJG5jcKOE0CLisYbojUjIrIEE7AGMzA/Sm4BslANhbS+cDMpgBdcPT91oJ7OuJ9hYJBx59RjbhxVnrF8Xg== +acorn@^8.12.1: + version "8.12.1" + resolved "https://registry.yarnpkg.com/acorn/-/acorn-8.12.1.tgz#71616bdccbe25e27a54439e0046e89ca76df2248" + integrity sha512-tcpGyI9zbizT9JbV6oYE477V6mTlXvvi0T0G3SNIYE2apm/G5huBa1+K89VGeovbg+jycCrfhl3ADxErOuO6Jg== + add@^2.0.6: version "2.0.6" resolved "https://registry.yarnpkg.com/add/-/add-2.0.6.tgz#248f0a9f6e5a528ef2295dbeec30532130ae2235" @@ -3360,6 +3383,14 @@ caniuse-lite@^1.0.30001587: resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001620.tgz#78bb6f35b8fe315b96b8590597094145d0b146b4" integrity sha512-WJvYsOjd1/BYUY6SNGUosK9DUidBPDTnOARHp3fSmFO1ekdxaY6nKRttEVrfMmYi80ctS0kz1wiWmm14fVc3ew== +chalk@4.1.2: + version "4.1.2" + resolved "https://registry.yarnpkg.com/chalk/-/chalk-4.1.2.tgz#aac4e2b7734a740867aeb16bf02aad556a1e7a01" + integrity sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA== + dependencies: + ansi-styles "^4.1.0" + supports-color "^7.1.0" + chalk@^1.1.3: version "1.1.3" resolved "https://registry.yarnpkg.com/chalk/-/chalk-1.1.3.tgz#a8115c55e4a702fe4d150abd3872822a7e09fc98" @@ -3408,6 +3439,21 @@ char-regex@^1.0.2: optionalDependencies: fsevents "~2.3.2" +chokidar@^3.6.0: + version "3.6.0" + resolved "https://registry.yarnpkg.com/chokidar/-/chokidar-3.6.0.tgz#197c6cc669ef2a8dc5e7b4d97ee4e092c3eb0d5b" + integrity sha512-7VT13fmjotKpGipCW9JEQAusEPE+Ei8nl6/g4FBAmIm0GOOLMua9NDDo/DWp0ZAxCr3cPq5ZpBqmPAQgDda2Pw== + dependencies: + anymatch "~3.1.2" + braces "~3.0.2" + glob-parent "~5.1.2" + is-binary-path "~2.1.0" + is-glob "~4.0.1" + normalize-path "~3.0.0" + readdirp "~3.6.0" + optionalDependencies: + fsevents "~2.3.2" + chownr@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/chownr/-/chownr-2.0.0.tgz#15bfbe53d2eab4cf70f18a8cd68ebe5b3cb1dece" @@ -10117,6 +10163,16 @@ unpipe@1.0.0, unpipe@~1.0.0: resolved "https://registry.yarnpkg.com/unpipe/-/unpipe-1.0.0.tgz#b2bf4ee8514aae6165b4817829d21b2ef49904ec" integrity sha512-pjy2bYhSsufwWlKwPc+l3cN7+wuJlK6uz0YdJEOlQDbl6jo/YlPi4mb8agUkVC8BF7V8NuzeyPNqRksA3hztKQ== +unplugin@^1.10.1: + version "1.12.1" + resolved "https://registry.yarnpkg.com/unplugin/-/unplugin-1.12.1.tgz#dc5834dc9337f47ddb7cf4cbbb9b8dac07e5bea4" + integrity sha512-aXEH9c5qi3uYZHo0niUtxDlT9ylG/luMW/dZslSCkbtC31wCyFkmM0kyoBBh+Grhn7CL+/kvKLfN61/EdxPxMQ== + dependencies: + acorn "^8.12.1" + chokidar "^3.6.0" + webpack-sources "^3.2.3" + webpack-virtual-modules "^0.6.2" + update-browserslist-db@^1.0.13: version "1.0.16" resolved "https://registry.yarnpkg.com/update-browserslist-db/-/update-browserslist-db-1.0.16.tgz#f6d489ed90fb2f07d67784eb3f53d7891f736356" @@ -10356,6 +10412,11 @@ webpack-sources@^3.2.3: resolved "https://registry.yarnpkg.com/webpack-sources/-/webpack-sources-3.2.3.tgz#2d4daab8451fd4b240cc27055ff6a0c2ccea0cde" integrity sha512-/DyMEOrDgLKKIG0fmvtz+4dUX/3Ghozwgm6iPp8KRhvn+eQf9+Q7GWxVNMk3+uCPWfdXYC4ExGBckIXdFEfH1w== +webpack-virtual-modules@^0.6.2: + version "0.6.2" + resolved "https://registry.yarnpkg.com/webpack-virtual-modules/-/webpack-virtual-modules-0.6.2.tgz#057faa9065c8acf48f24cb57ac0e77739ab9a7e8" + integrity sha512-66/V2i5hQanC51vBQKPH4aI8NMAcBW59FVBs+rC7eGHupMyfn34q7rZIE+ETlJ+XTevqfUhVVBgSUNSW2flEUQ== + webpack@^5.84.1: version "5.84.1" resolved "https://registry.yarnpkg.com/webpack/-/webpack-5.84.1.tgz#d4493acdeca46b26ffc99d86d784cabfeb925a15" @@ -10636,3 +10697,8 @@ yocto-queue@^0.1.0: version "0.1.0" resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-0.1.0.tgz#0294eb3dee05028d31ee1a5fa2c556a6aaf10a1b" integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q== + +zod@^3.22.4: + version "3.23.8" + resolved "https://registry.yarnpkg.com/zod/-/zod-3.23.8.tgz#e37b957b5d52079769fb8097099b592f0ef4067d" + integrity sha512-XBx9AXhXktjUqnepgTiE5flcKIYWi/rme0Eaj+5Y0lftuGBq+jyRu/md4WnuxqgP1ubdpNCsYEYPxrzVHD8d6g== diff --git a/util/argo/argo.go b/util/argo/argo.go index 5e0ce1839aaf5..37c7bbc1c0733 100644 --- a/util/argo/argo.go +++ b/util/argo/argo.go @@ -509,7 +509,7 @@ func ValidateDestination(ctx context.Context, dest *argoappv1.ApplicationDestina return nil } -func validateSourcePermissions(ctx context.Context, source argoappv1.ApplicationSource, proj *argoappv1.AppProject, project string, hasMultipleSources bool) []argoappv1.ApplicationCondition { +func validateSourcePermissions(source argoappv1.ApplicationSource, hasMultipleSources bool) []argoappv1.ApplicationCondition { var conditions []argoappv1.ApplicationCondition if hasMultipleSources { if source.RepoURL == "" || (source.Path == "" && source.Chart == "" && source.Ref == "") { @@ -545,7 +545,7 @@ func ValidatePermissions(ctx context.Context, spec *argoappv1.ApplicationSpec, p if spec.HasMultipleSources() { for _, source := range spec.Sources { - condition := validateSourcePermissions(ctx, source, proj, spec.Project, spec.HasMultipleSources()) + condition := validateSourcePermissions(source, spec.HasMultipleSources()) if len(condition) > 0 { conditions = append(conditions, condition...) return conditions, nil @@ -559,7 +559,7 @@ func ValidatePermissions(ctx context.Context, spec *argoappv1.ApplicationSpec, p } } } else { - conditions = validateSourcePermissions(ctx, spec.GetSource(), proj, spec.Project, spec.HasMultipleSources()) + conditions = validateSourcePermissions(spec.GetSource(), spec.HasMultipleSources()) if len(conditions) > 0 { return conditions, nil } diff --git a/util/argo/resource_tracking.go b/util/argo/resource_tracking.go index 476f739c13924..312637104a01c 100644 --- a/util/argo/resource_tracking.go +++ b/util/argo/resource_tracking.go @@ -65,7 +65,7 @@ func IsOldTrackingMethod(trackingMethod string) bool { return trackingMethod == "" || trackingMethod == string(TrackingMethodLabel) } -func (rt *resourceTracking) getAppInstanceValue(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod) *AppInstanceValue { +func (rt *resourceTracking) getAppInstanceValue(un *unstructured.Unstructured) *AppInstanceValue { appInstanceAnnotation, err := argokube.GetAppInstanceAnnotation(un, common.AnnotationKeyAppInstance) if err != nil { return nil @@ -80,7 +80,7 @@ func (rt *resourceTracking) getAppInstanceValue(un *unstructured.Unstructured, k // GetAppName retrieve application name base on tracking method func (rt *resourceTracking) GetAppName(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod) string { retrieveAppInstanceValue := func() string { - value := rt.getAppInstanceValue(un, key, trackingMethod) + value := rt.getAppInstanceValue(un) if value != nil { return value.ApplicationName } @@ -112,7 +112,7 @@ func (rt *resourceTracking) GetAppName(un *unstructured.Unstructured, key string func (rt *resourceTracking) GetAppInstance(un *unstructured.Unstructured, key string, trackingMethod v1alpha1.TrackingMethod) *AppInstanceValue { switch trackingMethod { case TrackingMethodAnnotation, TrackingMethodAnnotationAndLabel: - return rt.getAppInstanceValue(un, key, trackingMethod) + return rt.getAppInstanceValue(un) default: return nil } diff --git a/util/kube/kube.go b/util/kube/kube.go index 5ea4394b726f0..a982460e331e6 100644 --- a/util/kube/kube.go +++ b/util/kube/kube.go @@ -21,7 +21,7 @@ func IsValidResourceName(name string) bool { // SetAppInstanceLabel the recommended app.kubernetes.io/instance label against an unstructured object // Uses the legacy labeling if environment variable is set func SetAppInstanceLabel(target *unstructured.Unstructured, key, val string) error { - labels, _, err := nestedNullableStringMap(target.Object, "metadata", "labels") + labels, err := nestedNullableStringMap(target.Object, "metadata", "labels") if err != nil { return fmt.Errorf("failed to get labels from target object %s %s/%s: %w", target.GroupVersionKind().String(), target.GetNamespace(), target.GetName(), err) } @@ -100,7 +100,7 @@ func SetAppInstanceLabel(target *unstructured.Unstructured, key, val string) err // SetAppInstanceAnnotation the recommended app.kubernetes.io/instance annotation against an unstructured object // Uses the legacy labeling if environment variable is set func SetAppInstanceAnnotation(target *unstructured.Unstructured, key, val string) error { - annotations, _, err := nestedNullableStringMap(target.Object, "metadata", "annotations") + annotations, err := nestedNullableStringMap(target.Object, "metadata", "annotations") if err != nil { return fmt.Errorf("failed to get annotations from target object %s %s/%s: %w", target.GroupVersionKind().String(), target.GetNamespace(), target.GetName(), err) } @@ -115,7 +115,7 @@ func SetAppInstanceAnnotation(target *unstructured.Unstructured, key, val string // GetAppInstanceAnnotation returns the application instance name from annotation func GetAppInstanceAnnotation(un *unstructured.Unstructured, key string) (string, error) { - annotations, _, err := nestedNullableStringMap(un.Object, "metadata", "annotations") + annotations, err := nestedNullableStringMap(un.Object, "metadata", "annotations") if err != nil { return "", fmt.Errorf("failed to get annotations from target object %s %s/%s: %w", un.GroupVersionKind().String(), un.GetNamespace(), un.GetName(), err) } @@ -127,7 +127,7 @@ func GetAppInstanceAnnotation(un *unstructured.Unstructured, key string) (string // GetAppInstanceLabel returns the application instance name from labels func GetAppInstanceLabel(un *unstructured.Unstructured, key string) (string, error) { - labels, _, err := nestedNullableStringMap(un.Object, "metadata", "labels") + labels, err := nestedNullableStringMap(un.Object, "metadata", "labels") if err != nil { return "", fmt.Errorf("failed to get labels for %s %s/%s: %w", un.GroupVersionKind().String(), un.GetNamespace(), un.GetName(), err) } @@ -139,7 +139,7 @@ func GetAppInstanceLabel(un *unstructured.Unstructured, key string) (string, err // RemoveLabel removes label with the specified name func RemoveLabel(un *unstructured.Unstructured, key string) error { - labels, _, err := nestedNullableStringMap(un.Object, "metadata", "labels") + labels, err := nestedNullableStringMap(un.Object, "metadata", "labels") if err != nil { return fmt.Errorf("failed to get labels for %s %s/%s: %w", un.GroupVersionKind().String(), un.GetNamespace(), un.GetName(), err) } @@ -163,14 +163,14 @@ func RemoveLabel(un *unstructured.Unstructured, key string) error { // nestedNullableStringMap returns a copy of map[string]string value of a nested field. // Returns false if value is not found and an error if not one of map[string]interface{} or nil, or contains non-string values in the map. -func nestedNullableStringMap(obj map[string]interface{}, fields ...string) (map[string]string, bool, error) { +func nestedNullableStringMap(obj map[string]interface{}, fields ...string) (map[string]string, error) { var m map[string]string val, found, err := unstructured.NestedFieldNoCopy(obj, fields...) if err != nil { - return nil, found, err + return nil, err } if found && val != nil { - return unstructured.NestedStringMap(obj, fields...) + m, _, err = unstructured.NestedStringMap(obj, fields...) } - return m, found, err + return m, err } diff --git a/util/manifeststream/stream.go b/util/manifeststream/stream.go index 9243b735fc53c..dd02f2570d975 100644 --- a/util/manifeststream/stream.go +++ b/util/manifeststream/stream.go @@ -179,7 +179,7 @@ func ReceiveManifestFileStream(ctx context.Context, receiver RepoStreamReceiver, } metadata := header2.GetMetadata() - tgzFile, err := receiveFile(ctx, receiver, metadata.GetChecksum(), destDir, maxTarSize) + tgzFile, err := receiveFile(ctx, receiver, metadata.GetChecksum(), maxTarSize) if err != nil { return nil, nil, fmt.Errorf("error receiving tgz file: %w", err) } @@ -197,7 +197,7 @@ func ReceiveManifestFileStream(ctx context.Context, receiver RepoStreamReceiver, // receiveFile will receive the file from the gRPC stream and save it in the dst folder. // Returns error if checksum doesn't match the one provided in the fileMetadata. // It is responsibility of the caller to close the returned file. -func receiveFile(ctx context.Context, receiver RepoStreamReceiver, checksum, dst string, maxSize int64) (*os.File, error) { +func receiveFile(ctx context.Context, receiver RepoStreamReceiver, checksum string, maxSize int64) (*os.File, error) { hasher := sha256.New() tmpDir, err := files.CreateTempDir("") if err != nil { diff --git a/util/oidc/oidc.go b/util/oidc/oidc.go index e56448e48b638..5708532061d6a 100644 --- a/util/oidc/oidc.go +++ b/util/oidc/oidc.go @@ -404,7 +404,7 @@ func (a *ClientApp) HandleCallback(w http.ResponseWriter, r *http.Request) { } sub := jwtutil.StringField(claims, "sub") err = a.clientCache.Set(&cache.Item{ - Key: formatAccessTokenCacheKey(AccessTokenCachePrefix, sub), + Key: formatAccessTokenCacheKey(sub), Object: encToken, CacheActionOpts: cache.CacheActionOpts{ Expiration: getTokenExpiration(claims), @@ -557,7 +557,7 @@ func (a *ClientApp) GetUserInfo(actualClaims jwt.MapClaims, issuerURL, userInfoP var encClaims []byte // in case we got it in the cache, we just return the item - clientCacheKey := formatUserInfoResponseCacheKey(UserInfoResponseCachePrefix, sub) + clientCacheKey := formatUserInfoResponseCacheKey(sub) if err := a.clientCache.Get(clientCacheKey, &encClaims); err == nil { claimsRaw, err := crypto.Decrypt(encClaims, a.encryptionKey) if err != nil { @@ -575,7 +575,7 @@ func (a *ClientApp) GetUserInfo(actualClaims jwt.MapClaims, issuerURL, userInfoP // check if the accessToken for the user is still present var encAccessToken []byte - err := a.clientCache.Get(formatAccessTokenCacheKey(AccessTokenCachePrefix, sub), &encAccessToken) + err := a.clientCache.Get(formatAccessTokenCacheKey(sub), &encAccessToken) // without an accessToken we can't query the user info endpoint // thus the user needs to reauthenticate for argocd to get a new accessToken if errors.Is(err, cache.ErrCacheMiss) { @@ -684,11 +684,11 @@ func getTokenExpiration(claims jwt.MapClaims) time.Duration { } // formatUserInfoResponseCacheKey returns the key which is used to store userinfo of user in cache -func formatUserInfoResponseCacheKey(prefix, sub string) string { +func formatUserInfoResponseCacheKey(sub string) string { return fmt.Sprintf("%s_%s", UserInfoResponseCachePrefix, sub) } // formatAccessTokenCacheKey returns the key which is used to store the accessToken of a user in cache -func formatAccessTokenCacheKey(prefix, sub string) string { - return fmt.Sprintf("%s_%s", prefix, sub) +func formatAccessTokenCacheKey(sub string) string { + return fmt.Sprintf("%s_%s", AccessTokenCachePrefix, sub) } diff --git a/util/oidc/oidc_test.go b/util/oidc/oidc_test.go index c7a47bcdfb3de..8332acaf98885 100644 --- a/util/oidc/oidc_test.go +++ b/util/oidc/oidc_test.go @@ -639,7 +639,7 @@ func TestGetUserInfo(t *testing.T) { expectError bool }{ { - key: formatUserInfoResponseCacheKey(UserInfoResponseCachePrefix, "randomUser"), + key: formatUserInfoResponseCacheKey("randomUser"), expectError: true, }, }, @@ -654,7 +654,7 @@ func TestGetUserInfo(t *testing.T) { encrypt bool }{ { - key: formatAccessTokenCacheKey(AccessTokenCachePrefix, "randomUser"), + key: formatAccessTokenCacheKey("randomUser"), value: "FakeAccessToken", encrypt: true, }, @@ -673,7 +673,7 @@ func TestGetUserInfo(t *testing.T) { expectError bool }{ { - key: formatUserInfoResponseCacheKey(UserInfoResponseCachePrefix, "randomUser"), + key: formatUserInfoResponseCacheKey("randomUser"), expectError: true, }, }, @@ -688,7 +688,7 @@ func TestGetUserInfo(t *testing.T) { encrypt bool }{ { - key: formatAccessTokenCacheKey(AccessTokenCachePrefix, "randomUser"), + key: formatAccessTokenCacheKey("randomUser"), value: "FakeAccessToken", encrypt: true, }, @@ -707,7 +707,7 @@ func TestGetUserInfo(t *testing.T) { expectError bool }{ { - key: formatUserInfoResponseCacheKey(UserInfoResponseCachePrefix, "randomUser"), + key: formatUserInfoResponseCacheKey("randomUser"), expectError: true, }, }, @@ -730,7 +730,7 @@ func TestGetUserInfo(t *testing.T) { encrypt bool }{ { - key: formatAccessTokenCacheKey(AccessTokenCachePrefix, "randomUser"), + key: formatAccessTokenCacheKey("randomUser"), value: "FakeAccessToken", encrypt: true, }, @@ -749,7 +749,7 @@ func TestGetUserInfo(t *testing.T) { expectError bool }{ { - key: formatUserInfoResponseCacheKey(UserInfoResponseCachePrefix, "randomUser"), + key: formatUserInfoResponseCacheKey("randomUser"), expectError: true, }, }, @@ -782,7 +782,7 @@ func TestGetUserInfo(t *testing.T) { expectError bool }{ { - key: formatUserInfoResponseCacheKey(UserInfoResponseCachePrefix, "randomUser"), + key: formatUserInfoResponseCacheKey("randomUser"), value: "{\"groups\":[\"githubOrg:engineers\"]}", expectEncrypted: true, expectError: false, @@ -809,7 +809,7 @@ func TestGetUserInfo(t *testing.T) { encrypt bool }{ { - key: formatAccessTokenCacheKey(AccessTokenCachePrefix, "randomUser"), + key: formatAccessTokenCacheKey("randomUser"), value: "FakeAccessToken", encrypt: true, },