diff --git a/applicationset/generators/git.go b/applicationset/generators/git.go index 8d92e3dc30..76cc557f38 100644 --- a/applicationset/generators/git.go +++ b/applicationset/generators/git.go @@ -29,10 +29,10 @@ type GitGenerator struct { } // NewGitGenerator creates a new instance of Git Generator -func NewGitGenerator(repos services.Repos, namespace string) Generator { +func NewGitGenerator(repos services.Repos, controllerNamespace string) Generator { g := &GitGenerator{ repos: repos, - namespace: namespace, + namespace: controllerNamespace, } return g @@ -78,11 +78,11 @@ func (g *GitGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Applic if !strings.Contains(appSet.Spec.Template.Spec.Project, "{{") { project := appSet.Spec.Template.Spec.Project appProject := &argoprojiov1alpha1.AppProject{} - namespace := g.namespace - if namespace == "" { - namespace = appSet.Namespace + controllerNamespace := g.namespace + if controllerNamespace == "" { + controllerNamespace = appSet.Namespace } - if err := client.Get(context.TODO(), types.NamespacedName{Name: project, Namespace: namespace}, appProject); err != nil { + if err := client.Get(context.TODO(), types.NamespacedName{Name: project, Namespace: controllerNamespace}, appProject); err != nil { return nil, fmt.Errorf("error getting project %s: %w", project, err) } // we need to verify the signature on the Git revision if GPG is enabled diff --git a/applicationset/generators/utils.go b/applicationset/generators/utils.go index 23258234ac..43d2b345ed 100644 --- a/applicationset/generators/utils.go +++ b/applicationset/generators/utils.go @@ -10,15 +10,15 @@ import ( "github.com/argoproj/argo-cd/v3/applicationset/services" ) -func GetGenerators(ctx context.Context, c client.Client, k8sClient kubernetes.Interface, namespace string, argoCDService services.Repos, dynamicClient dynamic.Interface, scmConfig SCMConfig) map[string]Generator { +func GetGenerators(ctx context.Context, c client.Client, k8sClient kubernetes.Interface, controllerNamespace string, argoCDService services.Repos, dynamicClient dynamic.Interface, scmConfig SCMConfig) map[string]Generator { terminalGenerators := map[string]Generator{ "List": NewListGenerator(), - "Clusters": NewClusterGenerator(ctx, c, k8sClient, namespace), - "Git": NewGitGenerator(argoCDService, namespace), + "Clusters": NewClusterGenerator(ctx, c, k8sClient, controllerNamespace), + "Git": NewGitGenerator(argoCDService, controllerNamespace), "SCMProvider": NewSCMProviderGenerator(c, scmConfig), - "ClusterDecisionResource": NewDuckTypeGenerator(ctx, dynamicClient, k8sClient, namespace), + "ClusterDecisionResource": NewDuckTypeGenerator(ctx, dynamicClient, k8sClient, controllerNamespace), "PullRequest": NewPullRequestGenerator(c, scmConfig), - "Plugin": NewPluginGenerator(c, namespace), + "Plugin": NewPluginGenerator(c, controllerNamespace), } nestedGenerators := map[string]Generator{ diff --git a/server/applicationset/applicationset.go b/server/applicationset/applicationset.go index 8cdf96b214..57bab3b30c 100644 --- a/server/applicationset/applicationset.go +++ b/server/applicationset/applicationset.go @@ -200,7 +200,7 @@ func (s *Server) Create(ctx context.Context, q *applicationset.ApplicationSetCre } if q.GetDryRun() { - apps, err := s.generateApplicationSetApps(ctx, log.WithField("applicationset", appset.Name), *appset, namespace) + apps, err := s.generateApplicationSetApps(ctx, log.WithField("applicationset", appset.Name), *appset) if err != nil { return nil, fmt.Errorf("unable to generate Applications of ApplicationSet: %w", err) } @@ -260,12 +260,12 @@ func (s *Server) Create(ctx context.Context, q *applicationset.ApplicationSetCre return updated, nil } -func (s *Server) generateApplicationSetApps(ctx context.Context, logEntry *log.Entry, appset v1alpha1.ApplicationSet, namespace string) ([]v1alpha1.Application, error) { +func (s *Server) generateApplicationSetApps(ctx context.Context, logEntry *log.Entry, appset v1alpha1.ApplicationSet) ([]v1alpha1.Application, error) { argoCDDB := s.db scmConfig := generators.NewSCMConfig(s.ScmRootCAPath, s.AllowedScmProviders, s.EnableScmProviders, s.EnableGitHubAPIMetrics, github_app.NewAuthCredentials(argoCDDB.(db.RepoCredsDB)), true) argoCDService := services.NewArgoCDService(s.db, s.GitSubmoduleEnabled, s.repoClientSet, s.EnableNewGitFileGlobbing) - appSetGenerators := generators.GetGenerators(ctx, s.client, s.k8sClient, namespace, argoCDService, s.dynamicClient, scmConfig) + appSetGenerators := generators.GetGenerators(ctx, s.client, s.k8sClient, s.ns, argoCDService, s.dynamicClient, scmConfig) apps, _, err := appsettemplate.GenerateApplications(logEntry, appset, appSetGenerators, &appsetutils.Render{}, s.client) if err != nil { @@ -363,11 +363,15 @@ func (s *Server) Generate(ctx context.Context, q *applicationset.ApplicationSetG if appset == nil { return nil, errors.New("error creating ApplicationSets: ApplicationSets is nil in request") } - namespace := s.appsetNamespaceOrDefault(appset.Namespace) + // The RBAC check needs to be performed against the appset namespace + // However, when trying to generate params, the server namespace needs + // to be passed. + namespace := s.appsetNamespaceOrDefault(appset.Namespace) if !s.isNamespaceEnabled(namespace) { return nil, security.NamespaceNotPermittedError(namespace) } + projectName, err := s.validateAppSet(appset) if err != nil { return nil, fmt.Errorf("error validating ApplicationSets: %w", err) @@ -380,7 +384,16 @@ func (s *Server) Generate(ctx context.Context, q *applicationset.ApplicationSetG logger := log.New() logger.SetOutput(logs) - apps, err := s.generateApplicationSetApps(ctx, logger.WithField("applicationset", appset.Name), *appset, namespace) + // The server namespace will be used in the function + // since this is the exact namespace that is being used + // to generate parameters (especially for git generator). + // + // In case of Git generator, if the namespace is set to + // appset namespace, we'll look for a project in the appset + // namespace that would lead to error when generating params + // for an appset in any namespace feature. + // See https://github.com/argoproj/argo-cd/issues/22942 + apps, err := s.generateApplicationSetApps(ctx, logger.WithField("applicationset", appset.Name), *appset) if err != nil { return nil, fmt.Errorf("unable to generate Applications of ApplicationSet: %w\n%s", err, logs.String()) } diff --git a/server/applicationset/applicationset_test.go b/server/applicationset/applicationset_test.go index f0f21a2b79..aa610f8cd0 100644 --- a/server/applicationset/applicationset_test.go +++ b/server/applicationset/applicationset_test.go @@ -4,6 +4,9 @@ import ( "sort" "testing" + "sigs.k8s.io/controller-runtime/pkg/client" + cr_fake "sigs.k8s.io/controller-runtime/pkg/client/fake" + "github.com/argoproj/gitops-engine/pkg/health" "github.com/argoproj/pkg/v2/sync" "github.com/stretchr/testify/assert" @@ -50,7 +53,7 @@ func fakeCluster() *appsv1.Cluster { } // return an ApplicationServiceServer which returns fake data -func newTestAppSetServer(t *testing.T, objects ...runtime.Object) *Server { +func newTestAppSetServer(t *testing.T, objects ...client.Object) *Server { t.Helper() f := func(enf *rbac.Enforcer) { _ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV) @@ -61,7 +64,7 @@ func newTestAppSetServer(t *testing.T, objects ...runtime.Object) *Server { } // return an ApplicationServiceServer which returns fake data -func newTestNamespacedAppSetServer(t *testing.T, objects ...runtime.Object) *Server { +func newTestNamespacedAppSetServer(t *testing.T, objects ...client.Object) *Server { t.Helper() f := func(enf *rbac.Enforcer) { _ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV) @@ -71,7 +74,7 @@ func newTestNamespacedAppSetServer(t *testing.T, objects ...runtime.Object) *Ser return newTestAppSetServerWithEnforcerConfigure(t, f, scopedNamespaces, objects...) } -func newTestAppSetServerWithEnforcerConfigure(t *testing.T, f func(*rbac.Enforcer), namespace string, objects ...runtime.Object) *Server { +func newTestAppSetServerWithEnforcerConfigure(t *testing.T, f func(*rbac.Enforcer), namespace string, objects ...client.Object) *Server { t.Helper() kubeclientset := fake.NewClientset(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -115,7 +118,11 @@ func newTestAppSetServerWithEnforcerConfigure(t *testing.T, f func(*rbac.Enforce objects = append(objects, defaultProj, myProj) - fakeAppsClientset := apps.NewSimpleClientset(objects...) + runtimeObjects := make([]runtime.Object, len(objects)) + for i := range objects { + runtimeObjects[i] = objects[i] + } + fakeAppsClientset := apps.NewSimpleClientset(runtimeObjects...) factory := appinformer.NewSharedInformerFactoryWithOptions(fakeAppsClientset, 0, appinformer.WithNamespace(namespace), appinformer.WithTweakListOptions(func(_ *metav1.ListOptions) {})) fakeProjLister := factory.Argoproj().V1alpha1().AppProjects().Lister().AppProjects(testNamespace) @@ -138,6 +145,13 @@ func newTestAppSetServerWithEnforcerConfigure(t *testing.T, f func(*rbac.Enforce panic("Timed out waiting for caches to sync") } + scheme := runtime.NewScheme() + err = appsv1.AddToScheme(scheme) + require.NoError(t, err) + err = corev1.AddToScheme(scheme) + require.NoError(t, err) + crClient := cr_fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() + projInformer := factory.Argoproj().V1alpha1().AppProjects().Informer() go projInformer.Run(ctx.Done()) if !k8scache.WaitForCacheSync(ctx.Done(), projInformer.HasSynced) { @@ -148,7 +162,7 @@ func newTestAppSetServerWithEnforcerConfigure(t *testing.T, f func(*rbac.Enforce db, kubeclientset, nil, - nil, + crClient, enforcer, nil, fakeAppsClientset, @@ -640,3 +654,54 @@ func TestResourceTree(t *testing.T) { assert.EqualError(t, err, "namespace 'NOT-ALLOWED' is not permitted") }) } + +func TestAppSet_Generate_Cluster(t *testing.T) { + appSet1 := newTestAppSet(func(appset *appsv1.ApplicationSet) { + appset.Name = "AppSet1" + appset.Spec.Template.Name = "{{name}}" + appset.Spec.Generators = []appsv1.ApplicationSetGenerator{ + { + Clusters: &appsv1.ClusterGenerator{}, + }, + } + }) + + t.Run("Generate in default namespace", func(t *testing.T) { + appSetServer := newTestAppSetServer(t, appSet1) + appsetQuery := applicationset.ApplicationSetGenerateRequest{ + ApplicationSet: appSet1, + } + + res, err := appSetServer.Generate(t.Context(), &appsetQuery) + require.NoError(t, err) + require.Len(t, res.Applications, 2) + assert.Equal(t, "fake-cluster", res.Applications[0].Name) + assert.Equal(t, "in-cluster", res.Applications[1].Name) + }) + + t.Run("Generate in different namespace", func(t *testing.T) { + appSetServer := newTestAppSetServer(t, appSet1) + + appSet1Ns := appSet1.DeepCopy() + appSet1Ns.Namespace = "external-namespace" + appsetQuery := applicationset.ApplicationSetGenerateRequest{ApplicationSet: appSet1Ns} + + res, err := appSetServer.Generate(t.Context(), &appsetQuery) + require.NoError(t, err) + require.Len(t, res.Applications, 2) + assert.Equal(t, "fake-cluster", res.Applications[0].Name) + assert.Equal(t, "in-cluster", res.Applications[1].Name) + }) + + t.Run("Generate in not allowed namespace", func(t *testing.T) { + appSetServer := newTestAppSetServer(t, appSet1) + + appSet1Ns := appSet1.DeepCopy() + appSet1Ns.Namespace = "NOT-ALLOWED" + + appsetQuery := applicationset.ApplicationSetGenerateRequest{ApplicationSet: appSet1Ns} + + _, err := appSetServer.Generate(t.Context(), &appsetQuery) + assert.EqualError(t, err, "namespace 'NOT-ALLOWED' is not permitted") + }) +}