diff --git a/Makefile b/Makefile index d6f8cdf62d5d8..c1ef27163cc60 100644 --- a/Makefile +++ b/Makefile @@ -486,6 +486,7 @@ start-e2e-local: mod-vendor-local dep-ui-local cli-local BIN_MODE=$(ARGOCD_BIN_MODE) \ ARGOCD_APPLICATION_NAMESPACES=argocd-e2e-external,argocd-e2e-external-2 \ ARGOCD_APPLICATIONSET_CONTROLLER_NAMESPACES=argocd-e2e-external,argocd-e2e-external-2 \ + ARGOCD_APPLICATIONSET_CONTROLLER_TOKENREF_STRICT_MODE=true \ ARGOCD_APPLICATIONSET_CONTROLLER_ALLOWED_SCM_PROVIDERS=http://127.0.0.1:8341,http://127.0.0.1:8342,http://127.0.0.1:8343,http://127.0.0.1:8344 \ ARGOCD_E2E_TEST=true \ goreman -f $(ARGOCD_PROCFILE) start ${ARGOCD_START} diff --git a/applicationset/controllers/applicationset_controller_test.go b/applicationset/controllers/applicationset_controller_test.go index 1b3a225398587..01a90f025b315 100644 --- a/applicationset/controllers/applicationset_controller_test.go +++ b/applicationset/controllers/applicationset_controller_test.go @@ -36,6 +36,7 @@ import ( "github.com/argoproj/argo-cd/v2/applicationset/utils" appsetmetrics "github.com/argoproj/argo-cd/v2/applicationset/metrics" + argocommon "github.com/argoproj/argo-cd/v2/common" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" dbmocks "github.com/argoproj/argo-cd/v2/util/db/mocks" @@ -1150,7 +1151,7 @@ func TestRemoveFinalizerOnInvalidDestination_FinalizerTypes(t *testing.T) { Name: "my-secret", Namespace: "namespace", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, Data: map[string][]byte{ @@ -1306,7 +1307,7 @@ func TestRemoveFinalizerOnInvalidDestination_DestinationTypes(t *testing.T) { Name: "my-secret", Namespace: "namespace", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, Data: map[string][]byte{ @@ -2052,7 +2053,7 @@ func TestValidateGeneratedApplications(t *testing.T) { Name: "my-secret", Namespace: "namespace", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, Data: map[string][]byte{ diff --git a/applicationset/controllers/clustereventhandler.go b/applicationset/controllers/clustereventhandler.go index 66fdebca66a21..363fc03f16694 100644 --- a/applicationset/controllers/clustereventhandler.go +++ b/applicationset/controllers/clustereventhandler.go @@ -14,7 +14,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" - "github.com/argoproj/argo-cd/v2/applicationset/generators" + "github.com/argoproj/argo-cd/v2/common" argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) @@ -50,7 +50,7 @@ type addRateLimitingInterface[T comparable] interface { func (h *clusterSecretEventHandler) queueRelatedAppGenerators(ctx context.Context, q addRateLimitingInterface[reconcile.Request], object client.Object) { // Check for label, lookup all ApplicationSets that might match the cluster, queue them all - if object.GetLabels()[generators.ArgoCDSecretTypeLabel] != generators.ArgoCDSecretTypeCluster { + if object.GetLabels()[common.LabelKeySecretType] != common.LabelValueSecretTypeCluster { return } diff --git a/applicationset/controllers/clustereventhandler_test.go b/applicationset/controllers/clustereventhandler_test.go index 1f73ab36746f2..8af4b1c17d49b 100644 --- a/applicationset/controllers/clustereventhandler_test.go +++ b/applicationset/controllers/clustereventhandler_test.go @@ -4,6 +4,8 @@ import ( "context" "testing" + argocommon "github.com/argoproj/argo-cd/v2/common" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -16,7 +18,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "github.com/argoproj/argo-cd/v2/applicationset/generators" argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) @@ -42,7 +43,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -70,7 +71,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -113,7 +114,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -157,7 +158,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -218,7 +219,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -254,7 +255,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -304,7 +305,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -355,7 +356,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -389,7 +390,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -425,7 +426,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -475,7 +476,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, @@ -526,7 +527,7 @@ func TestClusterEventHandler(t *testing.T) { Namespace: "argocd", Name: "my-secret", Labels: map[string]string{ - generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster, + argocommon.LabelKeySecretType: argocommon.LabelValueSecretTypeCluster, }, }, }, diff --git a/applicationset/controllers/requeue_after_test.go b/applicationset/controllers/requeue_after_test.go index 674a7ff074bcc..c0c039b88faca 100644 --- a/applicationset/controllers/requeue_after_test.go +++ b/applicationset/controllers/requeue_after_test.go @@ -57,7 +57,7 @@ func TestRequeueAfter(t *testing.T) { }, } fakeDynClient := dynfake.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), gvrToListKind, duckType) - scmConfig := generators.NewSCMConfig("", []string{""}, true, nil) + scmConfig := generators.NewSCMConfig("", []string{""}, true, nil, true) terminalGenerators := map[string]generators.Generator{ "List": generators.NewListGenerator(), "Clusters": generators.NewClusterGenerator(k8sClient, ctx, appClientset, "argocd"), diff --git a/applicationset/generators/cluster.go b/applicationset/generators/cluster.go index 14cb52747fd21..100e8e45022c8 100644 --- a/applicationset/generators/cluster.go +++ b/applicationset/generators/cluster.go @@ -15,14 +15,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/argoproj/argo-cd/v2/applicationset/utils" + "github.com/argoproj/argo-cd/v2/common" argoappsetv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) -const ( - ArgoCDSecretTypeLabel = "argocd.argoproj.io/secret-type" - ArgoCDSecretTypeCluster = "cluster" -) - var _ Generator = (*ClusterGenerator)(nil) // ClusterGenerator generates Applications for some or all clusters registered with ArgoCD. @@ -186,7 +182,7 @@ func (g *ClusterGenerator) getSecretsByClusterName(appSetGenerator *argoappsetv1 // List all Clusters: clusterSecretList := &corev1.SecretList{} - selector := metav1.AddLabelToSelector(&appSetGenerator.Clusters.Selector, ArgoCDSecretTypeLabel, ArgoCDSecretTypeCluster) + selector := metav1.AddLabelToSelector(&appSetGenerator.Clusters.Selector, common.LabelKeySecretType, common.LabelValueSecretTypeCluster) secretSelector, err := metav1.LabelSelectorAsSelector(selector) if err != nil { return nil, fmt.Errorf("error converting label selector: %w", err) diff --git a/applicationset/generators/pull_request.go b/applicationset/generators/pull_request.go index 3392480bf419b..f0c2bfaacfcf5 100644 --- a/applicationset/generators/pull_request.go +++ b/applicationset/generators/pull_request.go @@ -139,7 +139,7 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera return nil, fmt.Errorf("error fetching CA certificates from ConfigMap: %w", prErr) } } - token, err := utils.GetSecretRef(ctx, g.client, providerConfig.TokenRef, applicationSetInfo.Namespace) + token, err := utils.GetSecretRef(ctx, g.client, providerConfig.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Secret token: %w", err) } @@ -147,7 +147,7 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera } if generatorConfig.Gitea != nil { providerConfig := generatorConfig.Gitea - token, err := utils.GetSecretRef(ctx, g.client, providerConfig.TokenRef, applicationSetInfo.Namespace) + token, err := utils.GetSecretRef(ctx, g.client, providerConfig.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Secret token: %w", err) } @@ -164,13 +164,13 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera } } if providerConfig.BearerToken != nil { - appToken, err := utils.GetSecretRef(ctx, g.client, providerConfig.BearerToken.TokenRef, applicationSetInfo.Namespace) + appToken, err := utils.GetSecretRef(ctx, g.client, providerConfig.BearerToken.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Secret Bearer token: %w", err) } return pullrequest.NewBitbucketServiceBearerToken(ctx, appToken, providerConfig.API, providerConfig.Project, providerConfig.Repo, g.scmRootCAPath, providerConfig.Insecure, caCerts) } else if providerConfig.BasicAuth != nil { - password, err := utils.GetSecretRef(ctx, g.client, providerConfig.BasicAuth.PasswordRef, applicationSetInfo.Namespace) + password, err := utils.GetSecretRef(ctx, g.client, providerConfig.BasicAuth.PasswordRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Secret token: %w", err) } @@ -182,13 +182,13 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera if generatorConfig.Bitbucket != nil { providerConfig := generatorConfig.Bitbucket if providerConfig.BearerToken != nil { - appToken, err := utils.GetSecretRef(ctx, g.client, providerConfig.BearerToken.TokenRef, applicationSetInfo.Namespace) + appToken, err := utils.GetSecretRef(ctx, g.client, providerConfig.BearerToken.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Secret Bearer token: %w", err) } return pullrequest.NewBitbucketCloudServiceBearerToken(providerConfig.API, appToken, providerConfig.Owner, providerConfig.Repo) } else if providerConfig.BasicAuth != nil { - password, err := utils.GetSecretRef(ctx, g.client, providerConfig.BasicAuth.PasswordRef, applicationSetInfo.Namespace) + password, err := utils.GetSecretRef(ctx, g.client, providerConfig.BasicAuth.PasswordRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Secret token: %w", err) } @@ -199,7 +199,7 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera } if generatorConfig.AzureDevOps != nil { providerConfig := generatorConfig.AzureDevOps - token, err := utils.GetSecretRef(ctx, g.client, providerConfig.TokenRef, applicationSetInfo.Namespace) + token, err := utils.GetSecretRef(ctx, g.client, providerConfig.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Secret token: %w", err) } @@ -219,7 +219,7 @@ func (g *PullRequestGenerator) github(ctx context.Context, cfg *argoprojiov1alph } // always default to token, even if not set (public access) - token, err := utils.GetSecretRef(ctx, g.client, cfg.TokenRef, applicationSetInfo.Namespace) + token, err := utils.GetSecretRef(ctx, g.client, cfg.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Secret token: %w", err) } diff --git a/applicationset/generators/pull_request_test.go b/applicationset/generators/pull_request_test.go index e02e7312b350f..d4eae1602bfda 100644 --- a/applicationset/generators/pull_request_test.go +++ b/applicationset/generators/pull_request_test.go @@ -283,7 +283,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) { "gitea.myorg.com", "bitbucket.myorg.com", "azuredevops.myorg.com", - }, true, nil)) + }, true, nil, true)) applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ ObjectMeta: metav1.ObjectMeta{ @@ -306,7 +306,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) { } func TestSCMProviderDisabled_PRGenerator(t *testing.T) { - generator := NewPullRequestGenerator(nil, NewSCMConfig("", []string{}, false, nil)) + generator := NewPullRequestGenerator(nil, NewSCMConfig("", []string{}, false, nil, true)) applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ ObjectMeta: metav1.ObjectMeta{ diff --git a/applicationset/generators/scm_provider.go b/applicationset/generators/scm_provider.go index 85a2550ae21f9..417b682e50511 100644 --- a/applicationset/generators/scm_provider.go +++ b/applicationset/generators/scm_provider.go @@ -35,14 +35,16 @@ type SCMConfig struct { allowedSCMProviders []string enableSCMProviders bool GitHubApps github_app_auth.Credentials + tokenRefStrictMode bool } -func NewSCMConfig(scmRootCAPath string, allowedSCMProviders []string, enableSCMProviders bool, gitHubApps github_app_auth.Credentials) SCMConfig { +func NewSCMConfig(scmRootCAPath string, allowedSCMProviders []string, enableSCMProviders bool, gitHubApps github_app_auth.Credentials, tokenRefStrictMode bool) SCMConfig { return SCMConfig{ scmRootCAPath: scmRootCAPath, allowedSCMProviders: allowedSCMProviders, enableSCMProviders: enableSCMProviders, GitHubApps: gitHubApps, + tokenRefStrictMode: tokenRefStrictMode, } } @@ -154,7 +156,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("error fetching CA certificates from ConfigMap: %w", scmError) } } - token, err := utils.GetSecretRef(ctx, g.client, providerConfig.TokenRef, applicationSetInfo.Namespace) + token, err := utils.GetSecretRef(ctx, g.client, providerConfig.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Gitlab token: %w", err) } @@ -163,7 +165,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("error initializing Gitlab service: %w", err) } } else if providerConfig.Gitea != nil { - token, err := utils.GetSecretRef(ctx, g.client, providerConfig.Gitea.TokenRef, applicationSetInfo.Namespace) + token, err := utils.GetSecretRef(ctx, g.client, providerConfig.Gitea.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Gitea token: %w", err) } @@ -182,13 +184,13 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha } } if providerConfig.BearerToken != nil { - appToken, err := utils.GetSecretRef(ctx, g.client, providerConfig.BearerToken.TokenRef, applicationSetInfo.Namespace) + appToken, err := utils.GetSecretRef(ctx, g.client, providerConfig.BearerToken.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Secret Bearer token: %w", err) } provider, scmError = scm_provider.NewBitbucketServerProviderBearerToken(ctx, appToken, providerConfig.API, providerConfig.Project, providerConfig.AllBranches, g.scmRootCAPath, providerConfig.Insecure, caCerts) } else if providerConfig.BasicAuth != nil { - password, err := utils.GetSecretRef(ctx, g.client, providerConfig.BasicAuth.PasswordRef, applicationSetInfo.Namespace) + password, err := utils.GetSecretRef(ctx, g.client, providerConfig.BasicAuth.PasswordRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Secret token: %w", err) } @@ -200,7 +202,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("error initializing Bitbucket Server service: %w", scmError) } } else if providerConfig.AzureDevOps != nil { - token, err := utils.GetSecretRef(ctx, g.client, providerConfig.AzureDevOps.AccessTokenRef, applicationSetInfo.Namespace) + token, err := utils.GetSecretRef(ctx, g.client, providerConfig.AzureDevOps.AccessTokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Azure Devops access token: %w", err) } @@ -209,7 +211,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("error initializing Azure Devops service: %w", err) } } else if providerConfig.Bitbucket != nil { - appPassword, err := utils.GetSecretRef(ctx, g.client, providerConfig.Bitbucket.AppPasswordRef, applicationSetInfo.Namespace) + appPassword, err := utils.GetSecretRef(ctx, g.client, providerConfig.Bitbucket.AppPasswordRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Bitbucket cloud appPassword: %w", err) } @@ -283,7 +285,7 @@ func (g *SCMProviderGenerator) githubProvider(ctx context.Context, github *argop ) } - token, err := utils.GetSecretRef(ctx, g.client, github.TokenRef, applicationSetInfo.Namespace) + token, err := utils.GetSecretRef(ctx, g.client, github.TokenRef, applicationSetInfo.Namespace, g.tokenRefStrictMode) if err != nil { return nil, fmt.Errorf("error fetching Github token: %w", err) } diff --git a/applicationset/utils/kubernetes.go b/applicationset/utils/kubernetes.go index f9e90bf1d9f81..b5708bad2ab53 100644 --- a/applicationset/utils/kubernetes.go +++ b/applicationset/utils/kubernetes.go @@ -4,14 +4,18 @@ import ( "context" "fmt" + "github.com/argoproj/argo-cd/v2/common" + corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) +var ErrDisallowedSecretAccess = fmt.Errorf("secret must have label %q=%q", common.LabelKeySecretType, common.LabelValueSecretTypeSCMCreds) + // getSecretRef gets the value of the key for the specified Secret resource. -func GetSecretRef(ctx context.Context, k8sClient client.Client, ref *argoprojiov1alpha1.SecretRef, namespace string) (string, error) { +func GetSecretRef(ctx context.Context, k8sClient client.Client, ref *argoprojiov1alpha1.SecretRef, namespace string, tokenRefStrictMode bool) (string, error) { if ref == nil { return "", nil } @@ -27,6 +31,11 @@ func GetSecretRef(ctx context.Context, k8sClient client.Client, ref *argoprojiov if err != nil { return "", fmt.Errorf("error fetching secret %s/%s: %w", namespace, ref.SecretName, err) } + + if tokenRefStrictMode && secret.GetLabels()[common.LabelKeySecretType] != common.LabelValueSecretTypeSCMCreds { + return "", fmt.Errorf("secret %s/%s is not a valid SCM creds secret: %w", namespace, ref.SecretName, ErrDisallowedSecretAccess) + } + tokenBytes, ok := secret.Data[ref.Key] if !ok { return "", fmt.Errorf("key %q in secret %s/%s not found", ref.Key, namespace, ref.SecretName) diff --git a/applicationset/utils/kubernetes_test.go b/applicationset/utils/kubernetes_test.go index bddda0c473073..d8e86b89b011c 100644 --- a/applicationset/utils/kubernetes_test.go +++ b/applicationset/utils/kubernetes_test.go @@ -67,7 +67,7 @@ func TestGetSecretRef(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - token, err := GetSecretRef(ctx, client, c.ref, c.namespace) + token, err := GetSecretRef(ctx, client, c.ref, c.namespace, false) if c.hasError { require.Error(t, err) } else { diff --git a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go index e1adc4bf71834..345454b7e7a2c 100644 --- a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go +++ b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go @@ -72,6 +72,7 @@ func NewCommand() *cobra.Command { metricsAplicationsetLabels []string enableScmProviders bool webhookParallelism int + tokenRefStrictMode bool ) scheme := runtime.NewScheme() _ = clientgoscheme.AddToScheme(scheme) @@ -163,7 +164,7 @@ func NewCommand() *cobra.Command { argoSettingsMgr := argosettings.NewSettingsManager(ctx, k8sClient, namespace) argoCDDB := db.NewDB(namespace, argoSettingsMgr, k8sClient) - scmConfig := generators.NewSCMConfig(scmRootCAPath, allowedScmProviders, enableScmProviders, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB))) + scmConfig := generators.NewSCMConfig(scmRootCAPath, allowedScmProviders, enableScmProviders, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), tokenRefStrictMode) tlsConfig := apiclient.TLSConfiguration{ DisableTLS: repoServerPlaintext, @@ -249,6 +250,7 @@ func NewCommand() *cobra.Command { command.Flags().StringSliceVar(&allowedScmProviders, "allowed-scm-providers", env.StringsFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ALLOWED_SCM_PROVIDERS", []string{}, ","), "The list of allowed custom SCM provider API URLs. This restriction does not apply to SCM or PR generators which do not accept a custom API URL. (Default: Empty = all)") command.Flags().BoolVar(&enableScmProviders, "enable-scm-providers", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_SCM_PROVIDERS", true), "Enable retrieving information from SCM providers, used by the SCM and PR generators (Default: true)") command.Flags().BoolVar(&dryRun, "dry-run", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_DRY_RUN", false), "Enable dry run mode") + command.Flags().BoolVar(&tokenRefStrictMode, "token-ref-strict-mode", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_TOKENREF_STRICT_MODE", false), fmt.Sprintf("Set to true to require secrets referenced by SCM providers to have the %s=%s label set (Default: false)", common.LabelKeySecretType, common.LabelValueSecretTypeSCMCreds)) command.Flags().BoolVar(&enableProgressiveSyncs, "enable-progressive-syncs", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_PROGRESSIVE_SYNCS", false), "Enable use of the experimental progressive syncs feature.") command.Flags().BoolVar(&enableNewGitFileGlobbing, "enable-new-git-file-globbing", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_NEW_GIT_FILE_GLOBBING", false), "Enable new globbing in Git files generator.") command.Flags().BoolVar(&repoServerPlaintext, "repo-server-plaintext", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_REPO_SERVER_PLAINTEXT", false), "Disable TLS on connections to repo server") diff --git a/common/common.go b/common/common.go index 82776c8c93996..d2e47aa5b1607 100644 --- a/common/common.go +++ b/common/common.go @@ -175,6 +175,8 @@ const ( LabelValueSecretTypeRepository = "repository" // LabelValueSecretTypeRepoCreds indicates a secret type of repository credentials LabelValueSecretTypeRepoCreds = "repo-creds" + // LabelValueSecretTypeSCMCreds indicates a secret type of SCM credentials + LabelValueSecretTypeSCMCreds = "scm-creds" // AnnotationKeyAppInstance is the Argo CD application name is used as the instance name AnnotationKeyAppInstance = "argocd.argoproj.io/tracking-id" diff --git a/docs/operator-manual/applicationset/Appset-Any-Namespace.md b/docs/operator-manual/applicationset/Appset-Any-Namespace.md index b0d684e46b5c0..01efe576d049c 100644 --- a/docs/operator-manual/applicationset/Appset-Any-Namespace.md +++ b/docs/operator-manual/applicationset/Appset-Any-Namespace.md @@ -79,6 +79,29 @@ data: If you do not intend to allow users to use the SCM or PR generators, you can disable them entirely by setting the environment variable `ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_SCM_PROVIDERS` to argocd-cmd-params-cm `applicationsetcontroller.enable.scm.providers` to `false`. +#### `tokenRef` Restrictions + +It is **highly recommended** to enable SCM Providers secrets restrictions to avoid any secrets exfiltration. This +recommendation applies even when AppSets-in-any-namespace is disabled, but is especially important when it is enabled, +since non-Argo-admins may attempt to reference out-of-bounds secrets in the `argocd` namespace from an AppSet +`tokenRef`. + +When this mode is enabled, the referenced secret must have a label `argocd.argoproj.io/secret-type` with value +`scm-creds`. + +To enable this mode, set the `ARGOCD_APPLICATIONSET_CONTROLLER_TOKENREF_STRICT_MODE` environment variable to `true` in the +`argocd-application-controller` deployment. You can do this by adding the following to your `argocd-cmd-paramscm` +ConfigMap: + +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: argocd-cmd-params-cm +data: + applicationsetcontroller.tokenref.strict.mode: "true" +``` + ### Overview In order for an ApplicationSet to be managed and reconciled outside the Argo CD's control plane namespace, two prerequisites must match: diff --git a/docs/operator-manual/argocd-cmd-params-cm.yaml b/docs/operator-manual/argocd-cmd-params-cm.yaml index baba40baba99b..37aaadd12a4d4 100644 --- a/docs/operator-manual/argocd-cmd-params-cm.yaml +++ b/docs/operator-manual/argocd-cmd-params-cm.yaml @@ -246,6 +246,8 @@ data: applicationsetcontroller.webhook.parallelism.limit: "50" # Override the default requeue time for the controller. (default 3m) applicationsetcontroller.requeue.after: "3m" + # Enable strict mode for tokenRef in ApplicationSet resources. When enabled, the referenced secret must have a label `argocd.argoproj.io/secret-type` with value `scm-creds`. + applicationsetcontroller.enable.tokenref.strict.mode: "false" ## Argo CD Notifications Controller Properties # Set the logging level. One of: debug|info|warn|error (default "info") diff --git a/docs/user-guide/annotations-and-labels.md b/docs/user-guide/annotations-and-labels.md index 2b4e9968dcfb4..df5fd278893bb 100644 --- a/docs/user-guide/annotations-and-labels.md +++ b/docs/user-guide/annotations-and-labels.md @@ -21,7 +21,7 @@ ## Labels -| Label key | Target resource(es) | Possible values | Description | -|--------------------------------|---------------------|---------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| argocd.argoproj.io/instance | Application | any | Recommended tracking label to [avoid conflicts with other tools which use `app.kubernetes.io/instance`](../faq.md#why-is-my-app-out-of-sync-even-after-syncing). | -| argocd.argoproj.io/secret-type | Secret | `cluster`, `repository`, `repo-creds` | Identifies certain types of Secrets used by Argo CD. See the [Declarative Setup docs](../operator-manual/declarative-setup.md) for details. | +| Label key | Target resource(es) | Possible values | Description | +|--------------------------------|---------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| argocd.argoproj.io/instance | Application | any | Recommended tracking label to [avoid conflicts with other tools which use `app.kubernetes.io/instance`](../faq.md#why-is-my-app-out-of-sync-even-after-syncing). | +| argocd.argoproj.io/secret-type | Secret | `cluster`, `repository`, `repo-creds`, `scm-creds` | Identifies certain types of Secrets used by Argo CD. See the [Declarative Setup docs](../operator-manual/declarative-setup.md) for details about the first three, and [AppSet-in-any-namespace docs](../operator-manual/applicationset/Appset-Any-Namespace.md) for the last one. | diff --git a/manifests/base/applicationset-controller/argocd-applicationset-controller-deployment.yaml b/manifests/base/applicationset-controller/argocd-applicationset-controller-deployment.yaml index 8886c1587916b..f4df48823a5ff 100644 --- a/manifests/base/applicationset-controller/argocd-applicationset-controller-deployment.yaml +++ b/manifests/base/applicationset-controller/argocd-applicationset-controller-deployment.yaml @@ -103,6 +103,12 @@ spec: key: applicationsetcontroller.enable.progressive.syncs name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_TOKENREF_STRICT_MODE + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.tokenref.strict.mode + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_NEW_GIT_FILE_GLOBBING valueFrom: configMapKeyRef: diff --git a/manifests/core-install.yaml b/manifests/core-install.yaml index cab3ad4da450b..ea6566129bae8 100644 --- a/manifests/core-install.yaml +++ b/manifests/core-install.yaml @@ -22661,6 +22661,12 @@ spec: key: applicationsetcontroller.enable.progressive.syncs name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_TOKENREF_STRICT_MODE + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.tokenref.strict.mode + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_NEW_GIT_FILE_GLOBBING valueFrom: configMapKeyRef: diff --git a/manifests/ha/install.yaml b/manifests/ha/install.yaml index 12c95ce19cf4c..6a33b0d28b65b 100644 --- a/manifests/ha/install.yaml +++ b/manifests/ha/install.yaml @@ -24005,6 +24005,12 @@ spec: key: applicationsetcontroller.enable.progressive.syncs name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_TOKENREF_STRICT_MODE + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.tokenref.strict.mode + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_NEW_GIT_FILE_GLOBBING valueFrom: configMapKeyRef: diff --git a/manifests/ha/namespace-install.yaml b/manifests/ha/namespace-install.yaml index a28f0dd53808b..1897f8a0901f4 100644 --- a/manifests/ha/namespace-install.yaml +++ b/manifests/ha/namespace-install.yaml @@ -1634,6 +1634,12 @@ spec: key: applicationsetcontroller.enable.progressive.syncs name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_TOKENREF_STRICT_MODE + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.tokenref.strict.mode + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_NEW_GIT_FILE_GLOBBING valueFrom: configMapKeyRef: diff --git a/manifests/install.yaml b/manifests/install.yaml index a823cdddb30cb..312f81fb65258 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -23122,6 +23122,12 @@ spec: key: applicationsetcontroller.enable.progressive.syncs name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_TOKENREF_STRICT_MODE + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.tokenref.strict.mode + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_NEW_GIT_FILE_GLOBBING valueFrom: configMapKeyRef: diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index f0f0367c78b56..4f7ffccdbbb95 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -751,6 +751,12 @@ spec: key: applicationsetcontroller.enable.progressive.syncs name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_TOKENREF_STRICT_MODE + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.tokenref.strict.mode + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_NEW_GIT_FILE_GLOBBING valueFrom: configMapKeyRef: diff --git a/server/applicationset/applicationset.go b/server/applicationset/applicationset.go index 5822b32d88558..b5288c71c1509 100644 --- a/server/applicationset/applicationset.go +++ b/server/applicationset/applicationset.go @@ -265,7 +265,7 @@ func (s *Server) Create(ctx context.Context, q *applicationset.ApplicationSetCre func (s *Server) generateApplicationSetApps(ctx context.Context, logEntry *log.Entry, appset v1alpha1.ApplicationSet, namespace string) ([]v1alpha1.Application, error) { argoCDDB := s.db - scmConfig := generators.NewSCMConfig(s.ScmRootCAPath, s.AllowedScmProviders, s.EnableScmProviders, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB))) + scmConfig := generators.NewSCMConfig(s.ScmRootCAPath, s.AllowedScmProviders, s.EnableScmProviders, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), true) getRepository := func(ctx context.Context, url, project string) (*v1alpha1.Repository, error) { return s.db.GetRepository(ctx, url, project) diff --git a/test/e2e/applicationset_test.go b/test/e2e/applicationset_test.go index 5df36d591b1d9..934d1be0177db 100644 --- a/test/e2e/applicationset_test.go +++ b/test/e2e/applicationset_test.go @@ -1,6 +1,7 @@ package e2e import ( + "context" "fmt" "io" "net" @@ -10,6 +11,8 @@ import ( "testing" "time" + "github.com/google/uuid" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -17,6 +20,7 @@ import ( "github.com/argoproj/pkg/rand" + "github.com/argoproj/argo-cd/v2/common" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v2/test/e2e/fixture" @@ -2704,6 +2708,219 @@ func githubPullMockHandler(t *testing.T) func(http.ResponseWriter, *http.Request } } +func TestSimpleSCMProviderGeneratorTokenRefStrictOk(t *testing.T) { + secretName := uuid.New().String() + + ts := testServerWithPort(t, 8341, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + githubSCMMockHandler(t)(w, r) + })) + + ts.Start() + defer ts.Close() + + expectedApp := argov1alpha1.Application{ + TypeMeta: metav1.TypeMeta{ + Kind: application.ApplicationKind, + APIVersion: "argoproj.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "argo-cd-guestbook", + Namespace: fixture.TestNamespace(), + Finalizers: []string{"resources-finalizer.argocd.argoproj.io"}, + }, + Spec: argov1alpha1.ApplicationSpec{ + Project: "default", + Source: &argov1alpha1.ApplicationSource{ + RepoURL: "git@github.com:argoproj/argo-cd.git", + TargetRevision: "master", + Path: "guestbook", + }, + Destination: argov1alpha1.ApplicationDestination{ + Server: "https://kubernetes.default.svc", + Namespace: "guestbook", + }, + }, + } + + // Because you can't &"". + repoMatch := "argo-cd" + + Given(t). + And(func() { + _, err := utils.GetE2EFixtureK8sClient().KubeClientset.CoreV1().Secrets(fixture.TestNamespace()).Create(context.Background(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: fixture.TestNamespace(), + Name: secretName, + Labels: map[string]string{ + common.LabelKeySecretType: common.LabelValueSecretTypeSCMCreds, + }, + }, + Data: map[string][]byte{ + "hello": []byte("world"), + }, + }, metav1.CreateOptions{}) + + assert.NoError(t, err) + }). + // Create an SCMProviderGenerator-based ApplicationSet + When().Create(v1alpha1.ApplicationSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "simple-scm-provider-generator-strict", + }, + Spec: v1alpha1.ApplicationSetSpec{ + Template: v1alpha1.ApplicationSetTemplate{ + ApplicationSetTemplateMeta: v1alpha1.ApplicationSetTemplateMeta{Name: "{{ repository }}-guestbook"}, + Spec: argov1alpha1.ApplicationSpec{ + Project: "default", + Source: &argov1alpha1.ApplicationSource{ + RepoURL: "{{ url }}", + TargetRevision: "{{ branch }}", + Path: "guestbook", + }, + Destination: argov1alpha1.ApplicationDestination{ + Server: "https://kubernetes.default.svc", + Namespace: "guestbook", + }, + }, + }, + Generators: []v1alpha1.ApplicationSetGenerator{ + { + SCMProvider: &v1alpha1.SCMProviderGenerator{ + Github: &v1alpha1.SCMProviderGeneratorGithub{ + Organization: "argoproj", + API: ts.URL, + TokenRef: &argov1alpha1.SecretRef{ + SecretName: secretName, + Key: "hello", + }, + }, + Filters: []v1alpha1.SCMProviderGeneratorFilter{ + { + RepositoryMatch: &repoMatch, + }, + }, + }, + }, + }, + }, + }).Then().Expect(ApplicationsExist([]argov1alpha1.Application{expectedApp})). + When().And(func() { + err := utils.GetE2EFixtureK8sClient().KubeClientset.CoreV1().Secrets(fixture.TestNamespace()).Delete(context.Background(), secretName, metav1.DeleteOptions{}) + assert.NoError(t, err) + }) +} + +func TestSimpleSCMProviderGeneratorTokenRefStrictKo(t *testing.T) { + secretName := uuid.New().String() + + ts := testServerWithPort(t, 8341, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + githubSCMMockHandler(t)(w, r) + })) + + ts.Start() + defer ts.Close() + + expectedApp := argov1alpha1.Application{ + TypeMeta: metav1.TypeMeta{ + Kind: application.ApplicationKind, + APIVersion: "argoproj.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "argo-cd-guestbook", + Namespace: fixture.TestNamespace(), + Finalizers: []string{"resources-finalizer.argocd.argoproj.io"}, + Labels: map[string]string{ + common.LabelKeyAppInstance: "simple-scm-provider-generator-strict-ko", + }, + }, + Spec: argov1alpha1.ApplicationSpec{ + Project: "default", + Source: &argov1alpha1.ApplicationSource{ + RepoURL: "git@github.com:argoproj/argo-cd.git", + TargetRevision: "master", + Path: "guestbook", + }, + Destination: argov1alpha1.ApplicationDestination{ + Server: "https://kubernetes.default.svc", + Namespace: "guestbook", + }, + }, + } + + // Because you can't &"". + repoMatch := "argo-cd" + + Given(t). + And(func() { + _, err := utils.GetE2EFixtureK8sClient().KubeClientset.CoreV1().Secrets(fixture.TestNamespace()).Create(context.Background(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: fixture.TestNamespace(), + Name: secretName, + Labels: map[string]string{ + // Try to exfiltrate cluster secret + common.LabelKeySecretType: common.LabelValueSecretTypeCluster, + }, + }, + Data: map[string][]byte{ + "hello": []byte("world"), + }, + }, metav1.CreateOptions{}) + + assert.NoError(t, err) + }). + // Create an SCMProviderGenerator-based ApplicationSet + When().Create(v1alpha1.ApplicationSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "simple-scm-provider-generator-strict-ko", + }, + Spec: v1alpha1.ApplicationSetSpec{ + Template: v1alpha1.ApplicationSetTemplate{ + ApplicationSetTemplateMeta: v1alpha1.ApplicationSetTemplateMeta{Name: "{{ repository }}-guestbook"}, + Spec: argov1alpha1.ApplicationSpec{ + Project: "default", + Source: &argov1alpha1.ApplicationSource{ + RepoURL: "{{ url }}", + TargetRevision: "{{ branch }}", + Path: "guestbook", + }, + Destination: argov1alpha1.ApplicationDestination{ + Server: "https://kubernetes.default.svc", + Namespace: "guestbook", + }, + }, + }, + Generators: []v1alpha1.ApplicationSetGenerator{ + { + SCMProvider: &v1alpha1.SCMProviderGenerator{ + Github: &v1alpha1.SCMProviderGeneratorGithub{ + Organization: "argoproj", + API: ts.URL, + TokenRef: &argov1alpha1.SecretRef{ + SecretName: secretName, + Key: "hello", + }, + }, + Filters: []v1alpha1.SCMProviderGeneratorFilter{ + { + RepositoryMatch: &repoMatch, + }, + }, + }, + }, + }, + }, + }).Then().Expect(ApplicationsDoNotExist([]argov1alpha1.Application{expectedApp})). + When(). + And(func() { + // app should be listed + output, err := fixture.RunCli("appset", "get", "simple-scm-provider-generator-strict-ko") + require.NoError(t, err) + assert.Contains(t, output, fmt.Sprintf("scm provider: error fetching Github token: secret %s/%s is not a valid SCM creds secret", fixture.TestNamespace(), secretName)) + err2 := utils.GetE2EFixtureK8sClient().KubeClientset.CoreV1().Secrets(fixture.TestNamespace()).Delete(context.Background(), secretName, metav1.DeleteOptions{}) + assert.NoError(t, err2) + }) +} + func TestSimplePullRequestGenerator(t *testing.T) { ts := testServerWithPort(t, 8343, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { githubPullMockHandler(t)(w, r)