diff --git a/internal/controller/pattern_controller.go b/internal/controller/pattern_controller.go index f6f21910d..2292d916a 100644 --- a/internal/controller/pattern_controller.go +++ b/internal/controller/pattern_controller.go @@ -244,7 +244,7 @@ func (r *PatternReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return result, argoErr } - // Copy the bootstrap secret to the namespaced argo namespace + // Copy the bootstrap secret to the clusterwide argo namespace if qualifiedInstance.Spec.GitConfig.TokenSecret != "" { if err = r.copyAuthGitSecret(qualifiedInstance.Spec.GitConfig.TokenSecretNamespace, qualifiedInstance.Spec.GitConfig.TokenSecret, getClusterWideArgoNamespace(), "vp-private-repo-credentials"); err != nil { @@ -280,11 +280,21 @@ func (r *PatternReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return result, appErr } - // Copy the bootstrap secret to the namespaced argo namespace + // Copy the bootstrap secret to the per-pattern namespaced argo namespace. + // Under singleArgoCD the application lives in the clusterwide ArgoCD namespace + // (which already received the secret above), so skip this copy. if qualifiedInstance.Spec.GitConfig.TokenSecret != "" { - if err = r.copyAuthGitSecret(qualifiedInstance.Spec.GitConfig.TokenSecretNamespace, - qualifiedInstance.Spec.GitConfig.TokenSecret, applicationName(qualifiedInstance), "vp-private-repo-credentials"); err != nil { - return r.actionPerformed(qualifiedInstance, "copying clusterwide git auth secret to namespaced argo", err) + singleArgo := false + if mergedValues, valErr := getPatternMergedValues(qualifiedInstance); valErr == nil { + if v := getGlobalValue("singleArgoCD", mergedValues); v != nil { + singleArgo = v == true || v == boolTrue + } + } + if !singleArgo { + if err = r.copyAuthGitSecret(qualifiedInstance.Spec.GitConfig.TokenSecretNamespace, + qualifiedInstance.Spec.GitConfig.TokenSecret, applicationName(qualifiedInstance), "vp-private-repo-credentials"); err != nil { + return r.actionPerformed(qualifiedInstance, "copying git auth secret to namespaced argo", err) + } } } // Perform validation of the site values file(s) diff --git a/internal/controller/pattern_controller_test.go b/internal/controller/pattern_controller_test.go index 2f2f122eb..472ef7854 100644 --- a/internal/controller/pattern_controller_test.go +++ b/internal/controller/pattern_controller_test.go @@ -32,6 +32,7 @@ import ( operatorclient "github.com/openshift/client-go/operator/clientset/versioned/fake" olmclient "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/fake" gomock "go.uber.org/mock/gomock" + "gopkg.in/yaml.v3" kubeclient "k8s.io/client-go/kubernetes/fake" @@ -619,6 +620,134 @@ var _ = Describe("pattern controller - buildPatternManifest helpers", func() { }) }) +var _ = Describe("pattern controller - singleArgoCD secret copy", func() { + const ( + srcNamespace = "openshift-operators" + srcSecretName = "private-repo" + destSecretName = "vp-private-repo-credentials" //nolint:gosec + ) + + var reconciler *PatternReconciler + + createSourceSecret := func(r *PatternReconciler) { + srcSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: srcSecretName, + Namespace: srcNamespace, + }, + Data: map[string][]byte{ + "sshPrivateKey": []byte("fake-key"), + "url": []byte("git@github.com:example/repo.git"), + "type": []byte("git"), + }, + } + _, err := r.fullClient.CoreV1().Secrets(srcNamespace).Create( + context.TODO(), srcSecret, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + } + + secretExistsIn := func(r *PatternReconciler, ns string) bool { + _, err := r.fullClient.CoreV1().Secrets(ns).Get( + context.TODO(), destSecretName, metav1.GetOptions{}) + return err == nil + } + + writeValuesGlobal := func(dir string, singleArgoCD any) { + Expect(os.MkdirAll(dir, 0755)).To(Succeed()) + content := map[string]any{"global": map[string]any{}} + if singleArgoCD != nil { + content["global"] = map[string]any{"singleArgoCD": singleArgoCD} + } + data, err := yaml.Marshal(content) + Expect(err).NotTo(HaveOccurred()) + Expect(os.WriteFile(filepath.Join(dir, "values-global.yaml"), data, 0600)).To(Succeed()) + } + + BeforeEach(func() { + nsOperators := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} + reconciler = newFakeReconciler(nsOperators, buildPatternManifest()) + createSourceSecret(reconciler) + activeArgoNamespace = ApplicationNamespace + activeArgoName = ClusterWideArgoName + }) + + Context("singleArgoCD is true", func() { + It("skips copying the secret to the application-name namespace", func() { + patternDir := filepath.Join(tempDir, "single-argo-test") + writeValuesGlobal(patternDir, true) + + p := buildPatternManifest() + enabled := true + p.Spec.MultiSourceConfig.Enabled = &enabled + p.Spec.ClusterGroupName = "standalone" + p.Spec.GitConfig.TokenSecret = srcSecretName + p.Spec.GitConfig.TokenSecretNamespace = srcNamespace + p.Status.LocalCheckoutPath = patternDir + + singleArgo := false + mergedValues, err := getPatternMergedValues(p) + Expect(err).NotTo(HaveOccurred()) + if v := getGlobalValue("singleArgoCD", mergedValues); v != nil { + singleArgo = v == true || v == "true" + } + Expect(singleArgo).To(BeTrue()) + + appNS := applicationName(p) + Expect(appNS).To(Equal("foo-standalone")) + Expect(secretExistsIn(reconciler, appNS)).To(BeFalse()) + }) + }) + + Context("singleArgoCD is false or absent (multi-Argo)", func() { + It("copies the secret to the application-name namespace", func() { + patternDir := filepath.Join(tempDir, "multi-argo-test") + writeValuesGlobal(patternDir, nil) + + p := buildPatternManifest() + enabled := true + p.Spec.MultiSourceConfig.Enabled = &enabled + p.Spec.ClusterGroupName = "standalone" + p.Spec.GitConfig.TokenSecret = srcSecretName + p.Spec.GitConfig.TokenSecretNamespace = srcNamespace + p.Status.LocalCheckoutPath = patternDir + + singleArgo := false + mergedValues, err := getPatternMergedValues(p) + Expect(err).NotTo(HaveOccurred()) + if v := getGlobalValue("singleArgoCD", mergedValues); v != nil { + singleArgo = v == true || v == "true" + } + Expect(singleArgo).To(BeFalse()) + + appNS := applicationName(p) + err = reconciler.copyAuthGitSecret(srcNamespace, srcSecretName, appNS, destSecretName) + Expect(err).NotTo(HaveOccurred()) + Expect(secretExistsIn(reconciler, appNS)).To(BeTrue()) + }) + }) + + Context("legacy ArgoCD namespace", func() { + BeforeEach(func() { + activeArgoNamespace = LegacyApplicationNamespace + activeArgoName = LegacyClusterWideArgoName + }) + + AfterEach(func() { + activeArgoNamespace = ApplicationNamespace + activeArgoName = ClusterWideArgoName + }) + + It("application-name namespace differs from legacy argo namespace", func() { + p := buildPatternManifest() + p.Spec.ClusterGroupName = "standalone" + appNS := applicationName(p) + Expect(appNS).To(Equal("foo-standalone")) + Expect(appNS).NotTo(Equal(getClusterWideArgoNamespace())) + Expect(getClusterWideArgoNamespace()).To(Equal(LegacyApplicationNamespace)) + }) + }) +}) + var _ = Describe("pattern controller - reconciler creation", func() { It("should create a reconciler with all required clients", func() { nsOperators := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} diff --git a/internal/controller/values.go b/internal/controller/values.go index 8fc576159..a6cb0a980 100644 --- a/internal/controller/values.go +++ b/internal/controller/values.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" + api "github.com/hybrid-cloud-patterns/patterns-operator/api/v1alpha1" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/engine" @@ -147,3 +148,41 @@ func countApplicationsAndSets(a any) (appCount, appSetsCount int) { } return applicationCount, applicationSetsCount } + +func getPatternMergedValues(p *api.Pattern) (map[string]any, error) { + gitDir := p.Status.LocalCheckoutPath + if _, err := os.Stat(gitDir); err != nil { + return nil, fmt.Errorf("%s path does not exist", gitDir) + } + + useVariantsDir := HasVariantsFolderLayout(gitDir) + valueFiles := newApplicationValueFiles(p, gitDir, useVariantsDir) + + mergedValues, err := mergeHelmValues(valueFiles...) + if err != nil { + return nil, fmt.Errorf("could not merge value files: %w", err) + } + + extraParams := convertArgoHelmParametersToMap(newApplicationParameters(p)) + mergedValues = chartutil.CoalesceTables(extraParams, mergedValues) + + return mergedValues, nil +} + +func getGlobalValue(key string, values map[string]any) any { //nolint:unparam + global, ok := values["global"] + if !ok { + return nil + } + + globalMap, ok := global.(map[string]any) + if !ok { + return nil + } + + v, ok := globalMap[key] + if !ok { + return nil + } + return v +} diff --git a/internal/controller/values_test.go b/internal/controller/values_test.go index f029a293d..c3c1abf4b 100644 --- a/internal/controller/values_test.go +++ b/internal/controller/values_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" + api "github.com/hybrid-cloud-patterns/patterns-operator/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "gopkg.in/yaml.v3" @@ -405,3 +406,85 @@ var _ = Describe("CountApplicationsAndSets", func() { }) }) }) + +var _ = Describe("getGlobalValue", func() { + It("returns the value when global key exists", func() { + values := map[string]any{ + "global": map[string]any{"singleArgoCD": true}, + } + Expect(getGlobalValue("singleArgoCD", values)).To(BeTrue()) + }) + + It("returns nil when global key is missing", func() { + values := map[string]any{ + "global": map[string]any{"otherKey": "val"}, + } + Expect(getGlobalValue("singleArgoCD", values)).To(BeNil()) + }) + + It("returns nil when global section is absent", func() { + values := map[string]any{"clusterGroup": map[string]any{"foo": "bar"}} + Expect(getGlobalValue("singleArgoCD", values)).To(BeNil()) + }) + + It("returns nil when global is not a map", func() { + values := map[string]any{"global": "notAMap"} + Expect(getGlobalValue("singleArgoCD", values)).To(BeNil()) + }) + + It("returns string value for string-typed booleans", func() { + values := map[string]any{ + "global": map[string]any{"singleArgoCD": "true"}, + } + Expect(getGlobalValue("singleArgoCD", values)).To(Equal("true")) + }) +}) + +var _ = Describe("getPatternMergedValues", func() { + It("merges values-global.yaml and extraParameters", func() { + patternDir := filepath.Join(tempDir, "merge-test") + Expect(os.MkdirAll(patternDir, 0755)).To(Succeed()) + + createTempValueFileAt(patternDir, "values-global.yaml", map[string]any{ + "global": map[string]any{ + "singleArgoCD": true, + "pattern": "from-file", + }, + }) + + p := &api.Pattern{} + enabled := true + p.Spec.MultiSourceConfig.Enabled = &enabled + p.Name = "test-pattern" + p.Spec.ClusterGroupName = "default" + p.Status.LocalCheckoutPath = patternDir + + merged, err := getPatternMergedValues(p) + Expect(err).NotTo(HaveOccurred()) + + global, ok := merged["global"].(map[string]any) + Expect(ok).To(BeTrue()) + Expect(global["singleArgoCD"]).To(BeTrue()) + // extraParameters override file values for global.pattern + Expect(global["pattern"]).To(Equal("test-pattern")) + }) + + It("returns error when checkout path does not exist", func() { + p := &api.Pattern{} + p.Status.LocalCheckoutPath = "/nonexistent/path" + + _, err := getPatternMergedValues(p) + Expect(err).To(HaveOccurred()) + }) +}) + +func createTempValueFileAt(dir, name string, content any) string { + filePath := filepath.Join(dir, name) + data, err := yaml.Marshal(content) + Expect(err).NotTo(HaveOccurred()) + + err = os.WriteFile(filePath, data, 0600) + Expect(err).NotTo(HaveOccurred()) + + return filePath +}