From 10b04dad11393ef7701657b5f5e58ed541242885 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Fri, 25 Aug 2023 14:29:08 -0400 Subject: [PATCH 1/8] feat(appset): add option to disable SCM providers entirely (#14246) Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- .../controllers/requeue_after_test.go | 4 +- applicationset/generators/pull_request.go | 22 +++--- .../generators/pull_request_test.go | 36 ++++++++-- applicationset/generators/scm_provider.go | 68 +++++++++++++------ .../generators/scm_provider_test.go | 55 +++++++++++---- .../commands/applicationset_controller.go | 10 +-- .../applicationset/Appset-Any-Namespace.md | 3 +- .../operator-manual/argocd-cmd-params-cm.yaml | 2 + ...-applicationset-controller-deployment.yaml | 6 ++ manifests/core-install.yaml | 6 ++ manifests/ha/install.yaml | 6 ++ manifests/ha/namespace-install.yaml | 6 ++ manifests/install.yaml | 6 ++ manifests/namespace-install.yaml | 6 ++ 14 files changed, 177 insertions(+), 59 deletions(-) diff --git a/applicationset/controllers/requeue_after_test.go b/applicationset/controllers/requeue_after_test.go index da6b0b10b47df..6db6145af5348 100644 --- a/applicationset/controllers/requeue_after_test.go +++ b/applicationset/controllers/requeue_after_test.go @@ -60,9 +60,9 @@ func TestRequeueAfter(t *testing.T) { "List": generators.NewListGenerator(), "Clusters": generators.NewClusterGenerator(k8sClient, ctx, appClientset, "argocd"), "Git": generators.NewGitGenerator(mockServer), - "SCMProvider": generators.NewSCMProviderGenerator(fake.NewClientBuilder().WithObjects(&corev1.Secret{}).Build(), generators.SCMAuthProviders{}, "", []string{""}), + "SCMProvider": generators.NewSCMProviderGenerator(fake.NewClientBuilder().WithObjects(&corev1.Secret{}).Build(), generators.SCMAuthProviders{}, "", []string{""}, true), "ClusterDecisionResource": generators.NewDuckTypeGenerator(ctx, fakeDynClient, appClientset, "argocd"), - "PullRequest": generators.NewPullRequestGenerator(k8sClient, generators.SCMAuthProviders{}, "", []string{""}), + "PullRequest": generators.NewPullRequestGenerator(k8sClient, generators.SCMAuthProviders{}, "", []string{""}, true), } nestedGenerators := map[string]generators.Generator{ diff --git a/applicationset/generators/pull_request.go b/applicationset/generators/pull_request.go index c024f1b723919..f261636831ed4 100644 --- a/applicationset/generators/pull_request.go +++ b/applicationset/generators/pull_request.go @@ -27,14 +27,16 @@ type PullRequestGenerator struct { auth SCMAuthProviders scmRootCAPath string allowedSCMProviders []string + enableSCMProviders bool } -func NewPullRequestGenerator(client client.Client, auth SCMAuthProviders, scmRootCAPath string, allowedScmProviders []string) Generator { +func NewPullRequestGenerator(client client.Client, auth SCMAuthProviders, scmRootCAPath string, allowedScmProviders []string, enableSCMProviders bool) Generator { g := &PullRequestGenerator{ client: client, auth: auth, scmRootCAPath: scmRootCAPath, allowedSCMProviders: allowedScmProviders, + enableSCMProviders: enableSCMProviders, } g.selectServiceProviderFunc = g.selectServiceProvider return g @@ -66,7 +68,7 @@ func (g *PullRequestGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha ctx := context.Background() svc, err := g.selectServiceProviderFunc(ctx, appSetGenerator.PullRequest, applicationSetInfo) if err != nil { - return nil, fmt.Errorf("failed to select pull request service provider: %v", err) + return nil, fmt.Errorf("failed to select pull request service provider: %w", err) } pulls, err := pullrequest.ListPullRequests(ctx, svc, appSetGenerator.PullRequest.Filters) @@ -122,15 +124,15 @@ func (g *PullRequestGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha // selectServiceProvider selects the provider to get pull requests from the configuration func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, generatorConfig *argoprojiov1alpha1.PullRequestGenerator, applicationSetInfo *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) { if generatorConfig.Github != nil { - if !ScmProviderAllowed(applicationSetInfo, generatorConfig.Github.API, g.allowedSCMProviders) { - return nil, fmt.Errorf("scm provider not allowed: %s", generatorConfig.Github.API) + if err := ScmProviderAllowed(applicationSetInfo, generatorConfig.Github.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) } return g.github(ctx, generatorConfig.Github, applicationSetInfo) } if generatorConfig.GitLab != nil { providerConfig := generatorConfig.GitLab - if !ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders) { - return nil, fmt.Errorf("scm provider not allowed: %s", providerConfig.API) + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) } token, err := g.getSecretRef(ctx, providerConfig.TokenRef, applicationSetInfo.Namespace) if err != nil { @@ -140,8 +142,8 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera } if generatorConfig.Gitea != nil { providerConfig := generatorConfig.Gitea - if !ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders) { - return nil, fmt.Errorf("scm provider not allowed: %s", generatorConfig.Gitea.API) + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) } token, err := g.getSecretRef(ctx, providerConfig.TokenRef, applicationSetInfo.Namespace) if err != nil { @@ -151,8 +153,8 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera } if generatorConfig.BitbucketServer != nil { providerConfig := generatorConfig.BitbucketServer - if !ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders) { - return nil, fmt.Errorf("scm provider not allowed: %s", providerConfig.API) + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) } if providerConfig.BasicAuth != nil { password, err := g.getSecretRef(ctx, providerConfig.BasicAuth.PasswordRef, applicationSetInfo.Namespace) diff --git a/applicationset/generators/pull_request_test.go b/applicationset/generators/pull_request_test.go index 72017f522946e..9f4d3d0a9b693 100644 --- a/applicationset/generators/pull_request_test.go +++ b/applicationset/generators/pull_request_test.go @@ -278,7 +278,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) { cases := []struct { name string providerConfig *argoprojiov1alpha1.PullRequestGenerator - expectedError string + expectedError error }{ { name: "Error Github", @@ -287,7 +287,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) { API: "https://myservice.mynamespace.svc.cluster.local", }, }, - expectedError: "failed to select pull request service provider: scm provider not allowed: https://myservice.mynamespace.svc.cluster.local", + expectedError: &ErrDisallowedSCMProvider{}, }, { name: "Error Gitlab", @@ -296,7 +296,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) { API: "https://myservice.mynamespace.svc.cluster.local", }, }, - expectedError: "failed to select pull request service provider: scm provider not allowed: https://myservice.mynamespace.svc.cluster.local", + expectedError: &ErrDisallowedSCMProvider{}, }, { name: "Error Gitea", @@ -305,7 +305,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) { API: "https://myservice.mynamespace.svc.cluster.local", }, }, - expectedError: "failed to select pull request service provider: scm provider not allowed: https://myservice.mynamespace.svc.cluster.local", + expectedError: &ErrDisallowedSCMProvider{}, }, { name: "Error Bitbucket", @@ -314,7 +314,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) { API: "https://myservice.mynamespace.svc.cluster.local", }, }, - expectedError: "failed to select pull request service provider: scm provider not allowed: https://myservice.mynamespace.svc.cluster.local", + expectedError: &ErrDisallowedSCMProvider{}, }, } @@ -330,7 +330,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) { "gitea.myorg.com", "bitbucket.myorg.com", "azuredevops.myorg.com", - }) + }, true) applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ ObjectMeta: metav1.ObjectMeta{ @@ -346,7 +346,29 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) { _, err := pullRequestGenerator.GenerateParams(&applicationSetInfo.Spec.Generators[0], &applicationSetInfo) assert.Error(t, err, "Must return an error") - assert.Equal(t, testCaseCopy.expectedError, err.Error()) + assert.ErrorAs(t, err, testCaseCopy.expectedError) }) } } + +func TestSCMProviderDisabled_PRGenerator(t *testing.T) { + generator := NewPullRequestGenerator(nil, SCMAuthProviders{}, "", []string{}, false) + + applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "set", + }, + Spec: argoprojiov1alpha1.ApplicationSetSpec{ + Generators: []argoprojiov1alpha1.ApplicationSetGenerator{{ + PullRequest: &argoprojiov1alpha1.PullRequestGenerator{ + Github: &argoprojiov1alpha1.PullRequestGeneratorGithub{ + API: "https://myservice.mynamespace.svc.cluster.local", + }, + }, + }}, + }, + } + + _, err := generator.GenerateParams(&applicationSetInfo.Spec.Generators[0], &applicationSetInfo) + assert.ErrorIs(t, err, ErrSCMProvidersDisabled) +} diff --git a/applicationset/generators/scm_provider.go b/applicationset/generators/scm_provider.go index 3d7c930870eaf..0037b29b94c7c 100644 --- a/applicationset/generators/scm_provider.go +++ b/applicationset/generators/scm_provider.go @@ -2,6 +2,7 @@ package generators import ( "context" + "errors" "fmt" "strings" "time" @@ -31,18 +32,20 @@ type SCMProviderGenerator struct { SCMAuthProviders scmRootCAPath string allowedSCMProviders []string + enableSCMProviders bool } type SCMAuthProviders struct { GitHubApps github_app_auth.Credentials } -func NewSCMProviderGenerator(client client.Client, providers SCMAuthProviders, scmRootCAPath string, allowedSCMProviders []string) Generator { +func NewSCMProviderGenerator(client client.Client, providers SCMAuthProviders, scmRootCAPath string, allowedSCMProviders []string, enableSCMProviders bool) Generator { return &SCMProviderGenerator{ client: client, SCMAuthProviders: providers, scmRootCAPath: scmRootCAPath, allowedSCMProviders: allowedSCMProviders, + enableSCMProviders: enableSCMProviders, } } @@ -65,24 +68,49 @@ func (g *SCMProviderGenerator) GetTemplate(appSetGenerator *argoprojiov1alpha1.A return &appSetGenerator.SCMProvider.Template } -func ScmProviderAllowed(applicationSetInfo *argoprojiov1alpha1.ApplicationSet, url string, allowedScmProviders []string) bool { +var ErrSCMProvidersDisabled = errors.New("scm providers are disabled") + +type ErrDisallowedSCMProvider struct { + Provider string + Allowed []string +} + +func NewErrDisallowedSCMProvider(provider string, allowed []string) ErrDisallowedSCMProvider { + return ErrDisallowedSCMProvider{ + Provider: provider, + Allowed: allowed, + } +} + +func (e ErrDisallowedSCMProvider) Error() string { + return fmt.Sprintf("scm provider %q not allowed, must use one of the following: %s", e.Provider, strings.Join(e.Allowed, ", ")) +} + +func ScmProviderAllowed(applicationSetInfo *argoprojiov1alpha1.ApplicationSet, url string, allowedScmProviders []string, enableSCMProviders bool) error { + logEntry := log.WithFields(log.Fields{ + common.SecurityField: common.SecurityMedium, + "applicationset": applicationSetInfo.Name, + "appSetNamespace": applicationSetInfo.Namespace, + }) + + if !enableSCMProviders { + logEntry.Debugf("attempted to use SCM %q, but SCM providers are disabled", url) + return ErrSCMProvidersDisabled + } + if url == "" || len(allowedScmProviders) == 0 { - return true + return nil } for _, allowedScmProvider := range allowedScmProviders { if url == allowedScmProvider { - return true + return nil } } - log.WithFields(log.Fields{ - common.SecurityField: common.SecurityMedium, - "applicationset": applicationSetInfo.Name, - "appSetNamespace": applicationSetInfo.Namespace, - }).Debugf("attempted to use disallowed SCM %q", url) + logEntry.Debugf("attempted to use disallowed SCM %q, must use one of the following: %s", url, strings.Join(allowedScmProviders, ", ")) - return false + return NewErrDisallowedSCMProvider(url, allowedScmProviders) } func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, applicationSetInfo *argoprojiov1alpha1.ApplicationSet) ([]map[string]interface{}, error) { @@ -102,8 +130,8 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha if g.overrideProvider != nil { provider = g.overrideProvider } else if providerConfig.Github != nil { - if !ScmProviderAllowed(applicationSetInfo, providerConfig.Github.API, g.allowedSCMProviders) { - return nil, fmt.Errorf("scm provider not allowed: %s", providerConfig.Github.API) + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Github.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) } var err error provider, err = g.githubProvider(ctx, providerConfig.Github, applicationSetInfo) @@ -111,8 +139,8 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("scm provider: %w", err) } } else if providerConfig.Gitlab != nil { - if !ScmProviderAllowed(applicationSetInfo, providerConfig.Gitlab.API, g.allowedSCMProviders) { - return nil, fmt.Errorf("scm provider not allowed: %s", providerConfig.Gitlab.API) + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Gitlab.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) } token, err := g.getSecretRef(ctx, providerConfig.Gitlab.TokenRef, applicationSetInfo.Namespace) if err != nil { @@ -123,8 +151,8 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("error initializing Gitlab service: %v", err) } } else if providerConfig.Gitea != nil { - if !ScmProviderAllowed(applicationSetInfo, providerConfig.Gitea.API, g.allowedSCMProviders) { - return nil, fmt.Errorf("scm provider not allowed: %s", providerConfig.Gitea.API) + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Gitea.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) } token, err := g.getSecretRef(ctx, providerConfig.Gitea.TokenRef, applicationSetInfo.Namespace) if err != nil { @@ -136,8 +164,8 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha } } else if providerConfig.BitbucketServer != nil { providerConfig := providerConfig.BitbucketServer - if !ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders) { - return nil, fmt.Errorf("scm provider not allowed: %s", providerConfig.API) + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) } var scmError error if providerConfig.BasicAuth != nil { @@ -153,8 +181,8 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("error initializing Bitbucket Server service: %v", scmError) } } else if providerConfig.AzureDevOps != nil { - if !ScmProviderAllowed(applicationSetInfo, providerConfig.AzureDevOps.API, g.allowedSCMProviders) { - return nil, fmt.Errorf("scm provider not allowed: %s", providerConfig.AzureDevOps.API) + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.AzureDevOps.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) } token, err := g.getSecretRef(ctx, providerConfig.AzureDevOps.AccessTokenRef, applicationSetInfo.Namespace) if err != nil { diff --git a/applicationset/generators/scm_provider_test.go b/applicationset/generators/scm_provider_test.go index 4dcb8fdf3ce6f..c438aa8f646fe 100644 --- a/applicationset/generators/scm_provider_test.go +++ b/applicationset/generators/scm_provider_test.go @@ -174,7 +174,7 @@ func TestSCMProviderGenerateParams(t *testing.T) { mockProvider := &scm_provider.MockProvider{ Repos: testCaseCopy.repos, } - scmGenerator := &SCMProviderGenerator{overrideProvider: mockProvider} + scmGenerator := &SCMProviderGenerator{overrideProvider: mockProvider, enableSCMProviders: true} applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ ObjectMeta: metav1.ObjectMeta{ Name: "set", @@ -205,7 +205,7 @@ func TestAllowedSCMProvider(t *testing.T) { cases := []struct { name string providerConfig *argoprojiov1alpha1.SCMProviderGenerator - expectedError string + expectedError error }{ { name: "Error Github", @@ -214,7 +214,7 @@ func TestAllowedSCMProvider(t *testing.T) { API: "https://myservice.mynamespace.svc.cluster.local", }, }, - expectedError: "scm provider not allowed: https://myservice.mynamespace.svc.cluster.local", + expectedError: &ErrDisallowedSCMProvider{}, }, { name: "Error Gitlab", @@ -223,7 +223,7 @@ func TestAllowedSCMProvider(t *testing.T) { API: "https://myservice.mynamespace.svc.cluster.local", }, }, - expectedError: "scm provider not allowed: https://myservice.mynamespace.svc.cluster.local", + expectedError: &ErrDisallowedSCMProvider{}, }, { name: "Error Gitea", @@ -232,7 +232,7 @@ func TestAllowedSCMProvider(t *testing.T) { API: "https://myservice.mynamespace.svc.cluster.local", }, }, - expectedError: "scm provider not allowed: https://myservice.mynamespace.svc.cluster.local", + expectedError: &ErrDisallowedSCMProvider{}, }, { name: "Error Bitbucket", @@ -241,7 +241,7 @@ func TestAllowedSCMProvider(t *testing.T) { API: "https://myservice.mynamespace.svc.cluster.local", }, }, - expectedError: "scm provider not allowed: https://myservice.mynamespace.svc.cluster.local", + expectedError: &ErrDisallowedSCMProvider{}, }, { name: "Error AzureDevops", @@ -250,7 +250,7 @@ func TestAllowedSCMProvider(t *testing.T) { API: "https://myservice.mynamespace.svc.cluster.local", }, }, - expectedError: "scm provider not allowed: https://myservice.mynamespace.svc.cluster.local", + expectedError: &ErrDisallowedSCMProvider{}, }, } @@ -260,13 +260,16 @@ func TestAllowedSCMProvider(t *testing.T) { t.Run(testCaseCopy.name, func(t *testing.T) { t.Parallel() - scmGenerator := &SCMProviderGenerator{allowedSCMProviders: []string{ - "github.myorg.com", - "gitlab.myorg.com", - "gitea.myorg.com", - "bitbucket.myorg.com", - "azuredevops.myorg.com", - }} + scmGenerator := &SCMProviderGenerator{ + allowedSCMProviders: []string{ + "github.myorg.com", + "gitlab.myorg.com", + "gitea.myorg.com", + "bitbucket.myorg.com", + "azuredevops.myorg.com", + }, + enableSCMProviders: true, + } applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ ObjectMeta: metav1.ObjectMeta{ @@ -282,7 +285,29 @@ func TestAllowedSCMProvider(t *testing.T) { _, err := scmGenerator.GenerateParams(&applicationSetInfo.Spec.Generators[0], &applicationSetInfo) assert.Error(t, err, "Must return an error") - assert.Equal(t, testCaseCopy.expectedError, err.Error()) + assert.ErrorAs(t, err, testCaseCopy.expectedError) }) } } + +func TestSCMProviderDisabled_SCMGenerator(t *testing.T) { + generator := &SCMProviderGenerator{enableSCMProviders: false} + + applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "set", + }, + Spec: argoprojiov1alpha1.ApplicationSetSpec{ + Generators: []argoprojiov1alpha1.ApplicationSetGenerator{{ + SCMProvider: &argoprojiov1alpha1.SCMProviderGenerator{ + Github: &argoprojiov1alpha1.SCMProviderGeneratorGithub{ + API: "https://myservice.mynamespace.svc.cluster.local", + }, + }, + }}, + }, + } + + _, err := generator.GenerateParams(&applicationSetInfo.Spec.Generators[0], &applicationSetInfo) + assert.ErrorIs(t, err, ErrSCMProvidersDisabled) +} diff --git a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go index a8bc6451eeb0e..5e1d69a34cec3 100644 --- a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go +++ b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go @@ -63,6 +63,7 @@ func NewCommand() *cobra.Command { maxConcurrentReconciliations int scmRootCAPath string allowedScmProviders []string + enableScmProviders bool ) scheme := runtime.NewScheme() _ = clientgoscheme.AddToScheme(scheme) @@ -105,8 +106,8 @@ func NewCommand() *cobra.Command { // If the applicationset-namespaces contains only one namespace it corresponds to the current namespace if len(applicationSetNamespaces) == 1 { watchedNamespace = (applicationSetNamespaces)[0] - } else if len(allowedScmProviders) == 0 { - log.Error("When enabling applicationset in any namespace using applicationset-namespaces, allowed-scm-providers is required") + } else if enableScmProviders && len(allowedScmProviders) == 0 { + log.Error("When enabling applicationset in any namespace using applicationset-namespaces, you must either set --enable-scm-providers=false or specify --allowed-scm-providers") os.Exit(1) } @@ -160,9 +161,9 @@ func NewCommand() *cobra.Command { "List": generators.NewListGenerator(), "Clusters": generators.NewClusterGenerator(mgr.GetClient(), ctx, k8sClient, namespace), "Git": generators.NewGitGenerator(argoCDService), - "SCMProvider": generators.NewSCMProviderGenerator(mgr.GetClient(), scmAuth, scmRootCAPath, allowedScmProviders), + "SCMProvider": generators.NewSCMProviderGenerator(mgr.GetClient(), scmAuth, scmRootCAPath, allowedScmProviders, enableScmProviders), "ClusterDecisionResource": generators.NewDuckTypeGenerator(ctx, dynamicClient, k8sClient, namespace), - "PullRequest": generators.NewPullRequestGenerator(mgr.GetClient(), scmAuth, scmRootCAPath, allowedScmProviders), + "PullRequest": generators.NewPullRequestGenerator(mgr.GetClient(), scmAuth, scmRootCAPath, allowedScmProviders, enableScmProviders), "Plugin": generators.NewPluginGenerator(mgr.GetClient(), ctx, k8sClient, namespace), } @@ -243,6 +244,7 @@ func NewCommand() *cobra.Command { command.Flags().StringVar(&cmdutil.LogFormat, "logformat", env.StringFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_LOGFORMAT", "text"), "Set the logging format. One of: text|json") command.Flags().StringVar(&cmdutil.LogLevel, "loglevel", env.StringFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_LOGLEVEL", "info"), "Set the logging level. One of: debug|info|warn|error") command.Flags().StringSliceVar(&allowedScmProviders, "allowed-scm-providers", env.StringsFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ALLOWED_SCM_PROVIDERS", []string{}, ","), "The list of allowed scm providers. (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(&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.") diff --git a/docs/operator-manual/applicationset/Appset-Any-Namespace.md b/docs/operator-manual/applicationset/Appset-Any-Namespace.md index 494b36dbdcf36..b216266c6cbd0 100644 --- a/docs/operator-manual/applicationset/Appset-Any-Namespace.md +++ b/docs/operator-manual/applicationset/Appset-Any-Namespace.md @@ -53,7 +53,6 @@ spec: Therefore administrator must restrict the urls of the allowed SCM Providers (example: `https://git.mydomain.com/,https://gitlab.mydomain.com/`) by setting the environment variable `ARGOCD_APPLICATIONSET_CONTROLLER_ALLOWED_SCM_PROVIDERS` to argocd-cmd-params-cm `applicationsetcontroller.allowed.scm.providers`. If another url is used, it will be rejected by the applicationset controller. - For example: ```yaml apiVersion: v1 @@ -66,6 +65,8 @@ data: > Please note url used in the `api` field of the `ApplicationSet` must match the url declared by the Administrator including the protocol +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_ALLOW_SCM_PROVIDERS` to argocd-cmd-params-cm `applicationsetcontroller.allow.scm.providers` to `false`. + ### 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 d4a754f0e44b9..7d38506d0b7ec 100644 --- a/docs/operator-manual/argocd-cmd-params-cm.yaml +++ b/docs/operator-manual/argocd-cmd-params-cm.yaml @@ -184,6 +184,8 @@ data: # sending secrets from `tokenRef`s to disallowed `api` domains. # The url used in the scm generator must exactly match one in the list applicationsetcontroller.allowed.scm.providers: "https://git.example.com/,https://gitlab.example.com/" + # To disable SCM providers entirely (i.e. disable the SCM and PR generators), set this to "false". Default is "true". + applicationsetcontroller.enable.scm.providers: "false" ## Argo CD Notifications Controller Properties # Set the logging level. One of: debug|info|warn|error (default "info") diff --git a/manifests/base/applicationset-controller/argocd-applicationset-controller-deployment.yaml b/manifests/base/applicationset-controller/argocd-applicationset-controller-deployment.yaml index ff7cf84c3e60a..b79fc9788fba9 100644 --- a/manifests/base/applicationset-controller/argocd-applicationset-controller-deployment.yaml +++ b/manifests/base/applicationset-controller/argocd-applicationset-controller-deployment.yaml @@ -139,6 +139,12 @@ spec: name: argocd-cmd-params-cm key: applicationsetcontroller.allowed.scm.providers optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_SCM_PROVIDERS + valueFrom: + configMapKeyRef: + name: argocd-cmd-params-cm + key: applicationsetcontroller.enable.scm.providers + optional: true volumeMounts: - mountPath: /app/config/ssh name: ssh-known-hosts diff --git a/manifests/core-install.yaml b/manifests/core-install.yaml index 0e919238d55a5..f84f703627119 100644 --- a/manifests/core-install.yaml +++ b/manifests/core-install.yaml @@ -18898,6 +18898,12 @@ spec: key: applicationsetcontroller.allowed.scm.providers name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_SCM_PROVIDERS + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.scm.providers + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always name: argocd-applicationset-controller diff --git a/manifests/ha/install.yaml b/manifests/ha/install.yaml index c5f45e932a4eb..7baa9f9a27a4d 100644 --- a/manifests/ha/install.yaml +++ b/manifests/ha/install.yaml @@ -20136,6 +20136,12 @@ spec: key: applicationsetcontroller.allowed.scm.providers name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_SCM_PROVIDERS + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.scm.providers + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always name: argocd-applicationset-controller diff --git a/manifests/ha/namespace-install.yaml b/manifests/ha/namespace-install.yaml index 03e2dd32f2395..0e85a7533e7f8 100644 --- a/manifests/ha/namespace-install.yaml +++ b/manifests/ha/namespace-install.yaml @@ -1636,6 +1636,12 @@ spec: key: applicationsetcontroller.allowed.scm.providers name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_SCM_PROVIDERS + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.scm.providers + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always name: argocd-applicationset-controller diff --git a/manifests/install.yaml b/manifests/install.yaml index b48239ab5c714..69cb0cca5689c 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -19237,6 +19237,12 @@ spec: key: applicationsetcontroller.allowed.scm.providers name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_SCM_PROVIDERS + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.scm.providers + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always name: argocd-applicationset-controller diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index ac244c7ccfe1d..b2ac7d06b4989 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -737,6 +737,12 @@ spec: key: applicationsetcontroller.allowed.scm.providers name: argocd-cmd-params-cm optional: true + - name: ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_SCM_PROVIDERS + valueFrom: + configMapKeyRef: + key: applicationsetcontroller.enable.scm.providers + name: argocd-cmd-params-cm + optional: true image: quay.io/argoproj/argocd:latest imagePullPolicy: Always name: argocd-applicationset-controller From 32887be1d78fd4f79f08213535ac8f5c270819b0 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Fri, 25 Aug 2023 15:02:03 -0400 Subject: [PATCH 2/8] clarify docs Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- .../commands/applicationset_controller.go | 2 +- .../operator-manual/applicationset/Appset-Any-Namespace.md | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go index 5e1d69a34cec3..07c9a904ffb7e 100644 --- a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go +++ b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go @@ -243,7 +243,7 @@ func NewCommand() *cobra.Command { command.Flags().BoolVar(&debugLog, "debug", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_DEBUG", false), "Print debug logs. Takes precedence over loglevel") command.Flags().StringVar(&cmdutil.LogFormat, "logformat", env.StringFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_LOGFORMAT", "text"), "Set the logging format. One of: text|json") command.Flags().StringVar(&cmdutil.LogLevel, "loglevel", env.StringFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_LOGLEVEL", "info"), "Set the logging level. One of: debug|info|warn|error") - command.Flags().StringSliceVar(&allowedScmProviders, "allowed-scm-providers", env.StringsFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ALLOWED_SCM_PROVIDERS", []string{}, ","), "The list of allowed scm providers. (Default: Empty = all)") + command.Flags().StringSliceVar(&allowedScmProviders, "allowed-scm-providers", env.StringsFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ALLOWED_SCM_PROVIDERS", []string{}, ","), "The list of allowed custom SCM provider API domains. This restriction does not apply to non-custom, default SCM provider domains, like github.com. (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(&enableProgressiveSyncs, "enable-progressive-syncs", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_PROGRESSIVE_SYNCS", false), "Enable use of the experimental progressive syncs feature.") diff --git a/docs/operator-manual/applicationset/Appset-Any-Namespace.md b/docs/operator-manual/applicationset/Appset-Any-Namespace.md index b216266c6cbd0..fd0213423c1c2 100644 --- a/docs/operator-manual/applicationset/Appset-Any-Namespace.md +++ b/docs/operator-manual/applicationset/Appset-Any-Namespace.md @@ -63,7 +63,12 @@ data: applicationsetcontroller.allowed.scm.providers: https://git.mydomain.com/,https://gitlab.mydomain.com/ ``` -> Please note url used in the `api` field of the `ApplicationSet` must match the url declared by the Administrator including the protocol +!!! note + Please note url used in the `api` field of the `ApplicationSet` must match the url declared by the Administrator including the protocol + +!!! warning + The allow-list only applies to SCM providers for which the user may configure a custom `api`. Hard-coded provider + URLs are assumed to be allowed. 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_ALLOW_SCM_PROVIDERS` to argocd-cmd-params-cm `applicationsetcontroller.allow.scm.providers` to `false`. From f20cff3581df6e4fdcf61d4472f063aa5dea3de6 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Fri, 25 Aug 2023 15:07:26 -0400 Subject: [PATCH 3/8] more clarification, small refactor Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- applicationset/generators/pull_request.go | 12 ++++--- applicationset/generators/scm_provider.go | 33 +++++++++---------- .../commands/applicationset_controller.go | 2 +- .../applicationset/Appset-Any-Namespace.md | 2 +- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/applicationset/generators/pull_request.go b/applicationset/generators/pull_request.go index f261636831ed4..63d2232c026f2 100644 --- a/applicationset/generators/pull_request.go +++ b/applicationset/generators/pull_request.go @@ -123,15 +123,19 @@ func (g *PullRequestGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha // selectServiceProvider selects the provider to get pull requests from the configuration func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, generatorConfig *argoprojiov1alpha1.PullRequestGenerator, applicationSetInfo *argoprojiov1alpha1.ApplicationSet) (pullrequest.PullRequestService, error) { + if !g.enableSCMProviders { + return nil, ErrSCMProvidersDisabled + } + if generatorConfig.Github != nil { - if err := ScmProviderAllowed(applicationSetInfo, generatorConfig.Github.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + if err := ScmProviderAllowed(applicationSetInfo, generatorConfig.Github.API, g.allowedSCMProviders); err != nil { return nil, fmt.Errorf("scm provider not allowed: %w", err) } return g.github(ctx, generatorConfig.Github, applicationSetInfo) } if generatorConfig.GitLab != nil { providerConfig := generatorConfig.GitLab - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders); err != nil { return nil, fmt.Errorf("scm provider not allowed: %w", err) } token, err := g.getSecretRef(ctx, providerConfig.TokenRef, applicationSetInfo.Namespace) @@ -142,7 +146,7 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera } if generatorConfig.Gitea != nil { providerConfig := generatorConfig.Gitea - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders); err != nil { return nil, fmt.Errorf("scm provider not allowed: %w", err) } token, err := g.getSecretRef(ctx, providerConfig.TokenRef, applicationSetInfo.Namespace) @@ -153,7 +157,7 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera } if generatorConfig.BitbucketServer != nil { providerConfig := generatorConfig.BitbucketServer - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders); err != nil { return nil, fmt.Errorf("scm provider not allowed: %w", err) } if providerConfig.BasicAuth != nil { diff --git a/applicationset/generators/scm_provider.go b/applicationset/generators/scm_provider.go index 0037b29b94c7c..3ad6597af6edb 100644 --- a/applicationset/generators/scm_provider.go +++ b/applicationset/generators/scm_provider.go @@ -86,18 +86,7 @@ func (e ErrDisallowedSCMProvider) Error() string { return fmt.Sprintf("scm provider %q not allowed, must use one of the following: %s", e.Provider, strings.Join(e.Allowed, ", ")) } -func ScmProviderAllowed(applicationSetInfo *argoprojiov1alpha1.ApplicationSet, url string, allowedScmProviders []string, enableSCMProviders bool) error { - logEntry := log.WithFields(log.Fields{ - common.SecurityField: common.SecurityMedium, - "applicationset": applicationSetInfo.Name, - "appSetNamespace": applicationSetInfo.Namespace, - }) - - if !enableSCMProviders { - logEntry.Debugf("attempted to use SCM %q, but SCM providers are disabled", url) - return ErrSCMProvidersDisabled - } - +func ScmProviderAllowed(applicationSetInfo *argoprojiov1alpha1.ApplicationSet, url string, allowedScmProviders []string) error { if url == "" || len(allowedScmProviders) == 0 { return nil } @@ -108,7 +97,11 @@ func ScmProviderAllowed(applicationSetInfo *argoprojiov1alpha1.ApplicationSet, u } } - logEntry.Debugf("attempted to use disallowed SCM %q, must use one of the following: %s", url, strings.Join(allowedScmProviders, ", ")) + log.WithFields(log.Fields{ + common.SecurityField: common.SecurityMedium, + "applicationset": applicationSetInfo.Name, + "appSetNamespace": applicationSetInfo.Namespace, + }).Debugf("attempted to use disallowed SCM %q, must use one of the following: %s", url, strings.Join(allowedScmProviders, ", ")) return NewErrDisallowedSCMProvider(url, allowedScmProviders) } @@ -122,6 +115,10 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, EmptyAppSetGeneratorError } + if !g.enableSCMProviders { + return nil, ErrSCMProvidersDisabled + } + ctx := context.Background() // Create the SCM provider helper. @@ -130,7 +127,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha if g.overrideProvider != nil { provider = g.overrideProvider } else if providerConfig.Github != nil { - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Github.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Github.API, g.allowedSCMProviders); err != nil { return nil, fmt.Errorf("scm provider not allowed: %w", err) } var err error @@ -139,7 +136,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("scm provider: %w", err) } } else if providerConfig.Gitlab != nil { - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Gitlab.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Gitlab.API, g.allowedSCMProviders); err != nil { return nil, fmt.Errorf("scm provider not allowed: %w", err) } token, err := g.getSecretRef(ctx, providerConfig.Gitlab.TokenRef, applicationSetInfo.Namespace) @@ -151,7 +148,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("error initializing Gitlab service: %v", err) } } else if providerConfig.Gitea != nil { - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Gitea.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Gitea.API, g.allowedSCMProviders); err != nil { return nil, fmt.Errorf("scm provider not allowed: %w", err) } token, err := g.getSecretRef(ctx, providerConfig.Gitea.TokenRef, applicationSetInfo.Namespace) @@ -164,7 +161,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha } } else if providerConfig.BitbucketServer != nil { providerConfig := providerConfig.BitbucketServer - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders); err != nil { return nil, fmt.Errorf("scm provider not allowed: %w", err) } var scmError error @@ -181,7 +178,7 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("error initializing Bitbucket Server service: %v", scmError) } } else if providerConfig.AzureDevOps != nil { - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.AzureDevOps.API, g.allowedSCMProviders, g.enableSCMProviders); err != nil { + if err := ScmProviderAllowed(applicationSetInfo, providerConfig.AzureDevOps.API, g.allowedSCMProviders); err != nil { return nil, fmt.Errorf("scm provider not allowed: %w", err) } token, err := g.getSecretRef(ctx, providerConfig.AzureDevOps.AccessTokenRef, applicationSetInfo.Namespace) diff --git a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go index 07c9a904ffb7e..f379a0cd83d37 100644 --- a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go +++ b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go @@ -243,7 +243,7 @@ func NewCommand() *cobra.Command { command.Flags().BoolVar(&debugLog, "debug", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_DEBUG", false), "Print debug logs. Takes precedence over loglevel") command.Flags().StringVar(&cmdutil.LogFormat, "logformat", env.StringFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_LOGFORMAT", "text"), "Set the logging format. One of: text|json") command.Flags().StringVar(&cmdutil.LogLevel, "loglevel", env.StringFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_LOGLEVEL", "info"), "Set the logging level. One of: debug|info|warn|error") - command.Flags().StringSliceVar(&allowedScmProviders, "allowed-scm-providers", env.StringsFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ALLOWED_SCM_PROVIDERS", []string{}, ","), "The list of allowed custom SCM provider API domains. This restriction does not apply to non-custom, default SCM provider domains, like github.com. (Default: Empty = all)") + 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 non-custom, default SCM provider domains, like github.com. (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(&enableProgressiveSyncs, "enable-progressive-syncs", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_PROGRESSIVE_SYNCS", false), "Enable use of the experimental progressive syncs feature.") diff --git a/docs/operator-manual/applicationset/Appset-Any-Namespace.md b/docs/operator-manual/applicationset/Appset-Any-Namespace.md index fd0213423c1c2..66816fe123789 100644 --- a/docs/operator-manual/applicationset/Appset-Any-Namespace.md +++ b/docs/operator-manual/applicationset/Appset-Any-Namespace.md @@ -68,7 +68,7 @@ data: !!! warning The allow-list only applies to SCM providers for which the user may configure a custom `api`. Hard-coded provider - URLs are assumed to be allowed. + URLs (like github.com) are assumed to be allowed. 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_ALLOW_SCM_PROVIDERS` to argocd-cmd-params-cm `applicationsetcontroller.allow.scm.providers` to `false`. From 285ab5850deb7c405e6948f868121a867d36d15e Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Fri, 25 Aug 2023 15:17:05 -0400 Subject: [PATCH 4/8] more clarification Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- .../commands/applicationset_controller.go | 2 +- docs/operator-manual/applicationset/Appset-Any-Namespace.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go index f379a0cd83d37..8ab74a206867a 100644 --- a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go +++ b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go @@ -243,7 +243,7 @@ func NewCommand() *cobra.Command { command.Flags().BoolVar(&debugLog, "debug", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_DEBUG", false), "Print debug logs. Takes precedence over loglevel") command.Flags().StringVar(&cmdutil.LogFormat, "logformat", env.StringFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_LOGFORMAT", "text"), "Set the logging format. One of: text|json") command.Flags().StringVar(&cmdutil.LogLevel, "loglevel", env.StringFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_LOGLEVEL", "info"), "Set the logging level. One of: debug|info|warn|error") - 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 non-custom, default SCM provider domains, like github.com. (Default: Empty = all)") + 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(&enableProgressiveSyncs, "enable-progressive-syncs", env.ParseBoolFromEnv("ARGOCD_APPLICATIONSET_CONTROLLER_ENABLE_PROGRESSIVE_SYNCS", false), "Enable use of the experimental progressive syncs feature.") diff --git a/docs/operator-manual/applicationset/Appset-Any-Namespace.md b/docs/operator-manual/applicationset/Appset-Any-Namespace.md index 66816fe123789..61716414aeb69 100644 --- a/docs/operator-manual/applicationset/Appset-Any-Namespace.md +++ b/docs/operator-manual/applicationset/Appset-Any-Namespace.md @@ -67,8 +67,8 @@ data: Please note url used in the `api` field of the `ApplicationSet` must match the url declared by the Administrator including the protocol !!! warning - The allow-list only applies to SCM providers for which the user may configure a custom `api`. Hard-coded provider - URLs (like github.com) are assumed to be allowed. + The allow-list only applies to SCM providers for which the user may configure a custom `api`. Where an SCM or PR + generator does not accept a custom API URL, the provider is implicitly allowed. 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_ALLOW_SCM_PROVIDERS` to argocd-cmd-params-cm `applicationsetcontroller.allow.scm.providers` to `false`. From 69a62920a58cd907d1b782a912bc9db8c4ae1c08 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Fri, 25 Aug 2023 15:58:16 -0400 Subject: [PATCH 5/8] fix test Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- applicationset/generators/scm_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/applicationset/generators/scm_provider.go b/applicationset/generators/scm_provider.go index 3ad6597af6edb..c44c0f3166e77 100644 --- a/applicationset/generators/scm_provider.go +++ b/applicationset/generators/scm_provider.go @@ -51,7 +51,7 @@ func NewSCMProviderGenerator(client client.Client, providers SCMAuthProviders, s // Testing generator func NewTestSCMProviderGenerator(overrideProvider scm_provider.SCMProviderService) Generator { - return &SCMProviderGenerator{overrideProvider: overrideProvider} + return &SCMProviderGenerator{overrideProvider: overrideProvider, enableSCMProviders: true} } func (g *SCMProviderGenerator) GetRequeueAfter(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator) time.Duration { From c54453f21dc0feefc5a7480774b0f90a6d94a2ab Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Fri, 25 Aug 2023 16:24:21 -0400 Subject: [PATCH 6/8] refactor Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- applicationset/generators/pull_request.go | 15 ++----- applicationset/generators/scm_provider.go | 27 +++++-------- applicationset/generators/scm_utils.go | 5 +++ .../v1alpha1/applicationset_types.go | 39 +++++++++++++++++++ 4 files changed, 56 insertions(+), 30 deletions(-) create mode 100644 applicationset/generators/scm_utils.go diff --git a/applicationset/generators/pull_request.go b/applicationset/generators/pull_request.go index 63d2232c026f2..c1dfd5ed978e9 100644 --- a/applicationset/generators/pull_request.go +++ b/applicationset/generators/pull_request.go @@ -126,18 +126,15 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera if !g.enableSCMProviders { return nil, ErrSCMProvidersDisabled } + if err := ScmProviderAllowed(applicationSetInfo, generatorConfig, g.allowedSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) + } if generatorConfig.Github != nil { - if err := ScmProviderAllowed(applicationSetInfo, generatorConfig.Github.API, g.allowedSCMProviders); err != nil { - return nil, fmt.Errorf("scm provider not allowed: %w", err) - } return g.github(ctx, generatorConfig.Github, applicationSetInfo) } if generatorConfig.GitLab != nil { providerConfig := generatorConfig.GitLab - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders); err != nil { - return nil, fmt.Errorf("scm provider not allowed: %w", err) - } token, err := g.getSecretRef(ctx, providerConfig.TokenRef, applicationSetInfo.Namespace) if err != nil { return nil, fmt.Errorf("error fetching Secret token: %v", err) @@ -146,9 +143,6 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera } if generatorConfig.Gitea != nil { providerConfig := generatorConfig.Gitea - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders); err != nil { - return nil, fmt.Errorf("scm provider not allowed: %w", err) - } token, err := g.getSecretRef(ctx, providerConfig.TokenRef, applicationSetInfo.Namespace) if err != nil { return nil, fmt.Errorf("error fetching Secret token: %v", err) @@ -157,9 +151,6 @@ func (g *PullRequestGenerator) selectServiceProvider(ctx context.Context, genera } if generatorConfig.BitbucketServer != nil { providerConfig := generatorConfig.BitbucketServer - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders); err != nil { - return nil, fmt.Errorf("scm provider not allowed: %w", err) - } if providerConfig.BasicAuth != nil { password, err := g.getSecretRef(ctx, providerConfig.BasicAuth.PasswordRef, applicationSetInfo.Namespace) if err != nil { diff --git a/applicationset/generators/scm_provider.go b/applicationset/generators/scm_provider.go index c44c0f3166e77..42b7789be67f0 100644 --- a/applicationset/generators/scm_provider.go +++ b/applicationset/generators/scm_provider.go @@ -86,7 +86,9 @@ func (e ErrDisallowedSCMProvider) Error() string { return fmt.Sprintf("scm provider %q not allowed, must use one of the following: %s", e.Provider, strings.Join(e.Allowed, ", ")) } -func ScmProviderAllowed(applicationSetInfo *argoprojiov1alpha1.ApplicationSet, url string, allowedScmProviders []string) error { +func ScmProviderAllowed(applicationSetInfo *argoprojiov1alpha1.ApplicationSet, generator SCMGeneratorWithCustomApiUrl, allowedScmProviders []string) error { + url := generator.CustomApiUrl() + if url == "" || len(allowedScmProviders) == 0 { return nil } @@ -119,26 +121,24 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, ErrSCMProvidersDisabled } - ctx := context.Background() - // Create the SCM provider helper. providerConfig := appSetGenerator.SCMProvider + + if err := ScmProviderAllowed(applicationSetInfo, providerConfig, g.allowedSCMProviders); err != nil { + return nil, fmt.Errorf("scm provider not allowed: %w", err) + } + + ctx := context.Background() var provider scm_provider.SCMProviderService if g.overrideProvider != nil { provider = g.overrideProvider } else if providerConfig.Github != nil { - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Github.API, g.allowedSCMProviders); err != nil { - return nil, fmt.Errorf("scm provider not allowed: %w", err) - } var err error provider, err = g.githubProvider(ctx, providerConfig.Github, applicationSetInfo) if err != nil { return nil, fmt.Errorf("scm provider: %w", err) } } else if providerConfig.Gitlab != nil { - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Gitlab.API, g.allowedSCMProviders); err != nil { - return nil, fmt.Errorf("scm provider not allowed: %w", err) - } token, err := g.getSecretRef(ctx, providerConfig.Gitlab.TokenRef, applicationSetInfo.Namespace) if err != nil { return nil, fmt.Errorf("error fetching Gitlab token: %v", err) @@ -148,9 +148,6 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("error initializing Gitlab service: %v", err) } } else if providerConfig.Gitea != nil { - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.Gitea.API, g.allowedSCMProviders); err != nil { - return nil, fmt.Errorf("scm provider not allowed: %w", err) - } token, err := g.getSecretRef(ctx, providerConfig.Gitea.TokenRef, applicationSetInfo.Namespace) if err != nil { return nil, fmt.Errorf("error fetching Gitea token: %v", err) @@ -161,9 +158,6 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha } } else if providerConfig.BitbucketServer != nil { providerConfig := providerConfig.BitbucketServer - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.API, g.allowedSCMProviders); err != nil { - return nil, fmt.Errorf("scm provider not allowed: %w", err) - } var scmError error if providerConfig.BasicAuth != nil { password, err := g.getSecretRef(ctx, providerConfig.BasicAuth.PasswordRef, applicationSetInfo.Namespace) @@ -178,9 +172,6 @@ func (g *SCMProviderGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha return nil, fmt.Errorf("error initializing Bitbucket Server service: %v", scmError) } } else if providerConfig.AzureDevOps != nil { - if err := ScmProviderAllowed(applicationSetInfo, providerConfig.AzureDevOps.API, g.allowedSCMProviders); err != nil { - return nil, fmt.Errorf("scm provider not allowed: %w", err) - } token, err := g.getSecretRef(ctx, providerConfig.AzureDevOps.AccessTokenRef, applicationSetInfo.Namespace) if err != nil { return nil, fmt.Errorf("error fetching Azure Devops access token: %v", err) diff --git a/applicationset/generators/scm_utils.go b/applicationset/generators/scm_utils.go new file mode 100644 index 0000000000000..51ac99d9b7e49 --- /dev/null +++ b/applicationset/generators/scm_utils.go @@ -0,0 +1,5 @@ +package generators + +type SCMGeneratorWithCustomApiUrl interface { + CustomApiUrl() string +} diff --git a/pkg/apis/application/v1alpha1/applicationset_types.go b/pkg/apis/application/v1alpha1/applicationset_types.go index 8d1395be69c46..d4164b2956bf6 100644 --- a/pkg/apis/application/v1alpha1/applicationset_types.go +++ b/pkg/apis/application/v1alpha1/applicationset_types.go @@ -397,6 +397,22 @@ type SCMProviderGenerator struct { // Values contains key/value pairs which are passed directly as parameters to the template Values map[string]string `json:"values,omitempty" protobuf:"bytes,11,name=values"` AWSCodeCommit *SCMProviderGeneratorAWSCodeCommit `json:"awsCodeCommit,omitempty" protobuf:"bytes,12,opt,name=awsCodeCommit"` + // If you add a new SCM provider, update CustomApiUrl below. +} + +func (g *SCMProviderGenerator) CustomApiUrl() string { + if g.Github != nil { + return g.Github.API + } else if g.Gitlab != nil { + return g.Gitlab.API + } else if g.Gitea != nil { + return g.Gitea.API + } else if g.BitbucketServer != nil { + return g.BitbucketServer.API + } else if g.AzureDevOps != nil { + return g.AzureDevOps.API + } + return "" } // SCMProviderGeneratorGitea defines a connection info specific to Gitea. @@ -539,6 +555,29 @@ type PullRequestGenerator struct { Bitbucket *PullRequestGeneratorBitbucket `json:"bitbucket,omitempty" protobuf:"bytes,8,opt,name=bitbucket"` // Additional provider to use and config for it. AzureDevOps *PullRequestGeneratorAzureDevOps `json:"azuredevops,omitempty" protobuf:"bytes,9,opt,name=azuredevops"` + // If you add a new SCM provider, update CustomApiUrl below. +} + +func (p *PullRequestGenerator) CustomApiUrl() string { + if p.Github != nil { + return p.Github.API + } + if p.GitLab != nil { + return p.GitLab.API + } + if p.Gitea != nil { + return p.Gitea.API + } + if p.BitbucketServer != nil { + return p.BitbucketServer.API + } + if p.Bitbucket != nil { + return p.Bitbucket.API + } + if p.AzureDevOps != nil { + return p.AzureDevOps.API + } + return "" } // PullRequestGeneratorGitea defines connection info specific to Gitea. From cb799c288809127dbda046fcee69a5e47adce08d Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Sat, 23 Sep 2023 16:41:58 -0400 Subject: [PATCH 7/8] fix test assertion Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- test/e2e/applicationset_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/e2e/applicationset_test.go b/test/e2e/applicationset_test.go index 415de564566cc..c416278348806 100644 --- a/test/e2e/applicationset_test.go +++ b/test/e2e/applicationset_test.go @@ -19,10 +19,11 @@ import ( argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v2/test/e2e/fixture" + "github.com/stretchr/testify/assert" + . "github.com/argoproj/argo-cd/v2/test/e2e/fixture/applicationsets" "github.com/argoproj/argo-cd/v2/test/e2e/fixture/applicationsets/utils" . "github.com/argoproj/argo-cd/v2/util/errors" - "github.com/stretchr/testify/assert" "github.com/argoproj/argo-cd/v2/pkg/apis/application" ) @@ -1705,7 +1706,7 @@ func TestSCMProviderGeneratorSCMProviderNotAllowed(t *testing.T) { // app should be listed output, err := fixture.RunCli("appset", "get", "scm-provider-generator-scm-provider-not-allowed") assert.NoError(t, err) - assert.Contains(t, output, "scm provider not allowed: http://myservice.mynamespace.svc.cluster.local") + assert.Contains(t, output, "scm provider not allowed") }) } From 02af30c88d57a9ba84eda15283be542d97f6569d Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Mon, 25 Sep 2023 11:18:42 -0400 Subject: [PATCH 8/8] simplify test expectation Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- test/e2e/applicationset_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/applicationset_test.go b/test/e2e/applicationset_test.go index c416278348806..f56f9f552e9f6 100644 --- a/test/e2e/applicationset_test.go +++ b/test/e2e/applicationset_test.go @@ -2113,7 +2113,7 @@ func TestPullRequestGeneratorNotAllowedSCMProvider(t *testing.T) { // app should be listed output, err := fixture.RunCli("appset", "get", "pull-request-generator-not-allowed-scm") assert.NoError(t, err) - assert.Contains(t, output, "failed to select pull request service provider: scm provider not allowed: http://myservice.mynamespace.svc.cluster.local") + assert.Contains(t, output, "scm provider not allowed") }) }