fix: change the appset namespace to server namespace when generating appset (#23900)

Signed-off-by: nitishfy <justnitish06@gmail.com>
Co-authored-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com>
This commit is contained in:
Nitish Kumar
2025-09-09 01:17:17 +05:30
committed by GitHub
parent 7829e2c6c1
commit 670d383f69
4 changed files with 99 additions and 21 deletions

View File

@@ -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())
}

View File

@@ -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")
})
}