diff --git a/internal/controller/argo.go b/internal/controller/argo.go index 6fbede7c7..8e1deba26 100644 --- a/internal/controller/argo.go +++ b/internal/controller/argo.go @@ -83,6 +83,7 @@ const ( ParamVpArgoNamespace = "global.vpArgoNamespace" ParamMultiSourceTargetRevision = "global.multiSourceTargetRevision" ParamDeletePattern = "global.deletePattern" + ParamVpNewFolderDir = "global.vpNewFolderDir" ) // ConsoleLink constants @@ -751,6 +752,12 @@ func newApplicationParameters(p *api.Pattern) []argoapi.HelmParameter { Name: ParamMultiSourceTargetRevision, Value: getClusterGroupChartVersion(p), }) + if HasVariantsFolderLayout(p.Status.LocalCheckoutPath) { + parameters = append(parameters, argoapi.HelmParameter{ + Name: ParamVpNewFolderDir, + Value: boolTrue, + }) + } for _, extra := range p.Spec.ExtraParameters { if !updateHelmParameter(extra, parameters) { log.Printf("Parameter %q = %q added", extra.Name, extra.Value) @@ -805,15 +812,29 @@ func convertArgoHelmParametersToMap(params []argoapi.HelmParameter) map[string]a return result } -func newApplicationValueFiles(p *api.Pattern, prefix string) []string { - files := []string{ - fmt.Sprintf("%s/values-global.yaml", prefix), - fmt.Sprintf("%s/values-%s.yaml", prefix, p.Spec.ClusterGroupName), - fmt.Sprintf("%s/values-%s.yaml", prefix, p.Status.ClusterPlatform), - fmt.Sprintf("%s/values-%s-%s.yaml", prefix, p.Status.ClusterPlatform, p.Status.ClusterVersion), - fmt.Sprintf("%s/values-%s-%s.yaml", prefix, p.Status.ClusterPlatform, p.Spec.ClusterGroupName), - fmt.Sprintf("%s/values-%s-%s.yaml", prefix, p.Status.ClusterVersion, p.Spec.ClusterGroupName), - fmt.Sprintf("%s/values-%s.yaml", prefix, p.Status.ClusterName), +func newApplicationValueFiles(p *api.Pattern, prefix string, useVariantsDir bool) []string { + variant := p.Spec.ClusterGroupName + var files []string + if useVariantsDir { + files = []string{ + fmt.Sprintf("%s/values-global.yaml", prefix), + fmt.Sprintf("%s/variants/%s/values-%s.yaml", prefix, variant, variant), + fmt.Sprintf("%s/variants/%s/values-%s.yaml", prefix, variant, p.Status.ClusterPlatform), + fmt.Sprintf("%s/variants/%s/values-%s-%s.yaml", prefix, variant, p.Status.ClusterPlatform, p.Status.ClusterVersion), + fmt.Sprintf("%s/variants/%s/values-%s-%s.yaml", prefix, variant, p.Status.ClusterPlatform, variant), + fmt.Sprintf("%s/variants/%s/values-%s-%s.yaml", prefix, variant, p.Status.ClusterVersion, variant), + fmt.Sprintf("%s/variants/%s/values-%s.yaml", prefix, variant, p.Status.ClusterName), + } + } else { + files = []string{ + fmt.Sprintf("%s/values-global.yaml", prefix), + fmt.Sprintf("%s/values-%s.yaml", prefix, variant), + fmt.Sprintf("%s/values-%s.yaml", prefix, p.Status.ClusterPlatform), + fmt.Sprintf("%s/values-%s-%s.yaml", prefix, p.Status.ClusterPlatform, p.Status.ClusterVersion), + fmt.Sprintf("%s/values-%s-%s.yaml", prefix, p.Status.ClusterPlatform, variant), + fmt.Sprintf("%s/values-%s-%s.yaml", prefix, p.Status.ClusterVersion, variant), + fmt.Sprintf("%s/values-%s.yaml", prefix, p.Status.ClusterName), + } } for _, extra := range p.Spec.ExtraValueFiles { @@ -846,7 +867,8 @@ func getSharedValueFiles(p *api.Pattern, prefix string) ([]string, error) { return nil, fmt.Errorf("%s path does not exist", gitDir) } - valueFiles := newApplicationValueFiles(p, gitDir) + useVariantsDir := HasVariantsFolderLayout(gitDir) + valueFiles := newApplicationValueFiles(p, gitDir, useVariantsDir) helmValues, err := mergeHelmValues(valueFiles...) if err != nil { @@ -949,7 +971,8 @@ func commonApplicationSpec(p *api.Pattern, sources []argoapi.ApplicationSource) } func commonApplicationSourceHelm(p *api.Pattern, prefix string) *argoapi.ApplicationSourceHelm { - valueFiles := newApplicationValueFiles(p, prefix) + useVariantsDir := HasVariantsFolderLayout(p.Status.LocalCheckoutPath) + valueFiles := newApplicationValueFiles(p, prefix, useVariantsDir) sharedValueFiles, err := getSharedValueFiles(p, prefix) if err != nil { log.Printf("Could not fetch sharedValueFiles: %s", err) @@ -1130,7 +1153,8 @@ func countVPApplications(p *api.Pattern) (appCount, appSetsCount int, err error) if _, err := os.Stat(gitDir); err != nil { return -1, -1, fmt.Errorf("%s path does not exist", gitDir) } - valueFiles := newApplicationValueFiles(p, gitDir) + useVariantsDir := HasVariantsFolderLayout(gitDir) + valueFiles := newApplicationValueFiles(p, gitDir, useVariantsDir) helmValues, helmErr := mergeHelmValues(valueFiles...) if helmErr != nil { return -2, -2, fmt.Errorf("error reading value file: %s", helmErr) diff --git a/internal/controller/argo_test.go b/internal/controller/argo_test.go index 9ee3696cf..bde5ad00f 100644 --- a/internal/controller/argo_test.go +++ b/internal/controller/argo_test.go @@ -96,7 +96,7 @@ var _ = Describe("Argo Pattern", func() { Path: "common/clustergroup", TargetRevision: pattern.Spec.GitConfig.TargetRevision, Helm: &argoapi.ApplicationSourceHelm{ - ValueFiles: newApplicationValueFiles(pattern, ""), + ValueFiles: newApplicationValueFiles(pattern, "", false), Parameters: newApplicationParameters(pattern), Values: newApplicationValues(pattern), IgnoreMissingValueFiles: true, @@ -167,7 +167,7 @@ var _ = Describe("Argo Pattern", func() { }, *appSource, } - multiSourceArgoApp.Spec.Sources[1].Helm.ValueFiles = newApplicationValueFiles(pattern, "$patternref") + multiSourceArgoApp.Spec.Sources[1].Helm.ValueFiles = newApplicationValueFiles(pattern, "$patternref", false) Expect(newMultiSourceApplication(pattern)).To(Equal(multiSourceArgoApp)) }) }) @@ -191,7 +191,7 @@ var _ = Describe("Argo Pattern", func() { }, *appSource, } - multiSourceArgoApp.Spec.Sources[1].Helm.ValueFiles = newApplicationValueFiles(pattern, "$patternref") + multiSourceArgoApp.Spec.Sources[1].Helm.ValueFiles = newApplicationValueFiles(pattern, "$patternref", false) Expect(newMultiSourceApplication(pattern)).To(Equal(multiSourceArgoApp)) }) }) @@ -200,11 +200,11 @@ var _ = Describe("Argo Pattern", func() { Describe("Testing newApplicationValueFiles function", func() { Context("Default", func() { It("Returns a default set of values", func() { - valueFiles := newApplicationValueFiles(pattern, "") + valueFiles := newApplicationValueFiles(pattern, "", false) Expect(valueFiles).To(Equal(defaultValueFiles)) }) It("Returns a default set of values with prefix", func() { - valueFiles := newApplicationValueFiles(pattern, "myprefix") + valueFiles := newApplicationValueFiles(pattern, "myprefix", false) Expect(valueFiles).To(Equal(prefixArray(defaultValueFiles, "myprefix"))) }) }) @@ -217,13 +217,13 @@ var _ = Describe("Argo Pattern", func() { } }) It("Returns a default set of values and extravaluefiles without prefix", func() { - valueFiles := newApplicationValueFiles(pattern, "") + valueFiles := newApplicationValueFiles(pattern, "", false) Expect(valueFiles).To(Equal(append(defaultValueFiles, "/test1.yaml", "/test2.yaml"))) }) It("Returns a default set of values and extravaluefiles with prefix", func() { - valueFiles := newApplicationValueFiles(pattern, "myprefix") + valueFiles := newApplicationValueFiles(pattern, "myprefix", false) Expect(valueFiles).To(Equal(append(prefixArray(defaultValueFiles, "myprefix"), "myprefix/test1.yaml", "myprefix/test2.yaml"))) @@ -238,18 +238,48 @@ var _ = Describe("Argo Pattern", func() { } }) It("Returns a default set of values and extravaluefiles", func() { - valueFiles := newApplicationValueFiles(pattern, "") + valueFiles := newApplicationValueFiles(pattern, "", false) Expect(valueFiles).To(Equal(append(defaultValueFiles, "/test1.yaml", "/test2.yaml"))) }) It("Returns a default set of values and extravaluefiles with prefix", func() { - valueFiles := newApplicationValueFiles(pattern, "myprefix") + valueFiles := newApplicationValueFiles(pattern, "myprefix", false) Expect(valueFiles).To(Equal(append(prefixArray(defaultValueFiles, "myprefix"), "myprefix/test1.yaml", "myprefix/test2.yaml"))) }) }) + + Context("With variants directory layout", func() { + var variantsDirFiles []string + BeforeEach(func() { + variantsDirFiles = []string{ + "/values-global.yaml", + "/variants/foogroup/values-foogroup.yaml", + "/variants/foogroup/values-AWS.yaml", + "/variants/foogroup/values-AWS-4.12.yaml", + "/variants/foogroup/values-AWS-foogroup.yaml", + "/variants/foogroup/values-4.12-foogroup.yaml", + "/variants/foogroup/values-barcluster.yaml", + } + }) + It("Returns variants dir layout paths without prefix", func() { + valueFiles := newApplicationValueFiles(pattern, "", true) + Expect(valueFiles).To(Equal(variantsDirFiles)) + }) + It("Returns variants dir layout paths with prefix", func() { + valueFiles := newApplicationValueFiles(pattern, "$patternref", true) + Expect(valueFiles).To(Equal(prefixArray(variantsDirFiles, "$patternref"))) + }) + It("Appends extra value files unchanged", func() { + pattern.Spec.ExtraValueFiles = []string{"extra.yaml", "/leading.yaml"} + valueFiles := newApplicationValueFiles(pattern, "", true) + Expect(valueFiles).To(Equal(append(variantsDirFiles, + "/extra.yaml", + "/leading.yaml"))) + }) + }) }) Describe("Argo Helm Functions", func() { @@ -547,6 +577,35 @@ var _ = Describe("Argo Pattern", func() { Value: "0.0.*", }))) }) + + It("Test newApplicationParameters includes vpNewFolderDir when values/ dir exists", func() { + td, err := os.MkdirTemp("", "vp-param-test") + Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(td) + Expect(os.MkdirAll(filepath.Join(td, "variants"), 0755)).To(Succeed()) + pattern.Status.LocalCheckoutPath = td + params := newApplicationParameters(pattern) + found := false + for _, p := range params { + if p.Name == ParamVpNewFolderDir { + Expect(p.Value).To(Equal("true")) + found = true + break + } + } + Expect(found).To(BeTrue()) + }) + + It("Test newApplicationParameters omits vpNewFolderDir when no values/ dir", func() { + td, err := os.MkdirTemp("", "vp-param-test") + Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(td) + pattern.Status.LocalCheckoutPath = td + params := newApplicationParameters(pattern) + for _, p := range params { + Expect(p.Name).ToNot(Equal(ParamVpNewFolderDir)) + } + }) }) Context("Compare Sources", func() { diff --git a/internal/controller/pattern_controller.go b/internal/controller/pattern_controller.go index 437e59d10..7b264ab8c 100644 --- a/internal/controller/pattern_controller.go +++ b/internal/controller/pattern_controller.go @@ -590,11 +590,20 @@ func (r *PatternReconciler) preValidation(input *api.Pattern) error { // Validate that the required values file exists for the cluster group if input.Spec.ClusterGroupName != "" && input.Status.LocalCheckoutPath != "" { - valuesFile := filepath.Join(input.Status.LocalCheckoutPath, - fmt.Sprintf("values-%s.yaml", input.Spec.ClusterGroupName)) + var valuesFile, displayName string + if HasVariantsFolderLayout(input.Status.LocalCheckoutPath) { + valuesFile = filepath.Join(input.Status.LocalCheckoutPath, + "variants", input.Spec.ClusterGroupName, + fmt.Sprintf("values-%s.yaml", input.Spec.ClusterGroupName)) + displayName = fmt.Sprintf("variants/%s/values-%s.yaml", + input.Spec.ClusterGroupName, input.Spec.ClusterGroupName) + } else { + valuesFile = filepath.Join(input.Status.LocalCheckoutPath, + fmt.Sprintf("values-%s.yaml", input.Spec.ClusterGroupName)) + displayName = fmt.Sprintf("values-%s.yaml", input.Spec.ClusterGroupName) + } if _, err := os.Stat(valuesFile); os.IsNotExist(err) { - return fmt.Errorf("required values file not found: values-%s.yaml", - input.Spec.ClusterGroupName) + return fmt.Errorf("required values file not found: %s", displayName) } } diff --git a/internal/controller/pattern_controller_test.go b/internal/controller/pattern_controller_test.go index 838c36fce..0e78e3fb7 100644 --- a/internal/controller/pattern_controller_test.go +++ b/internal/controller/pattern_controller_test.go @@ -19,6 +19,7 @@ package controllers import ( "context" "os" + "path/filepath" "github.com/go-git/go-git/v5" "github.com/go-logr/logr" @@ -333,6 +334,45 @@ var _ = Describe("pattern controller - preValidation", func() { err := reconciler.preValidation(p) Expect(err).ToNot(HaveOccurred()) }) + + It("should pass when variants dir layout exists and variant file is present", func() { + Expect(os.MkdirAll(filepath.Join(tempDir, "variants", "hub"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(tempDir, "variants", "hub", "values-hub.yaml"), + []byte("clusterGroup:\n name: hub\n"), 0600)).To(Succeed()) + + p := &api.Pattern{ + Spec: api.PatternSpec{ + ClusterGroupName: "hub", + GitConfig: api.GitConfig{ + TargetRepo: "https://github.com/test/repo", + }, + }, + Status: api.PatternStatus{ + LocalCheckoutPath: tempDir, + }, + } + err := reconciler.preValidation(p) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should fail when variants dir layout exists but variant file is missing", func() { + Expect(os.MkdirAll(filepath.Join(tempDir, "variants"), 0755)).To(Succeed()) + + p := &api.Pattern{ + Spec: api.PatternSpec{ + ClusterGroupName: "hub", + GitConfig: api.GitConfig{ + TargetRepo: "https://github.com/test/repo", + }, + }, + Status: api.PatternStatus{ + LocalCheckoutPath: tempDir, + }, + } + err := reconciler.preValidation(p) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("required values file not found: variants/hub/values-hub.yaml")) + }) }) }) diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 13e304269..87557e1d6 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -480,6 +480,13 @@ func IsCommonSlimmed(patternPath string) bool { return true } +// HasVariantsFolderLayout returns true if the pattern repo has a top-level "variants" directory, +// indicating the new organized layout for values files. +func HasVariantsFolderLayout(patternPath string) bool { + info, err := os.Stat(filepath.Join(patternPath, "variants")) + return err == nil && info.IsDir() +} + // IntOrZero retrieves an integer value from a map by key. func IntOrZero(secret map[string][]byte, key string) (int64, error) { val, present := secret[key] diff --git a/internal/controller/utils_test.go b/internal/controller/utils_test.go index cfde9d7a0..d95a3a297 100644 --- a/internal/controller/utils_test.go +++ b/internal/controller/utils_test.go @@ -1647,6 +1647,45 @@ var _ = Describe("IsCommonSlimmed", func() { }) }) +var _ = Describe("HasVariantsFolderLayout", func() { + var td string + + BeforeEach(func() { + td = createTempDir("vp-variants-layout-test") + }) + AfterEach(func() { + cleanupTempDir(td) + }) + + Context("when variants/ directory exists", func() { + It("should return true", func() { + err := os.MkdirAll(filepath.Join(td, "variants"), 0755) + Expect(err).ToNot(HaveOccurred()) + Expect(HasVariantsFolderLayout(td)).To(BeTrue()) + }) + }) + + Context("when variants/ directory does not exist", func() { + It("should return false", func() { + Expect(HasVariantsFolderLayout(td)).To(BeFalse()) + }) + }) + + Context("when variants is a file not a directory", func() { + It("should return false", func() { + err := os.WriteFile(filepath.Join(td, "variants"), []byte("not a dir"), 0600) + Expect(err).ToNot(HaveOccurred()) + Expect(HasVariantsFolderLayout(td)).To(BeFalse()) + }) + }) + + Context("when path does not exist", func() { + It("should return false", func() { + Expect(HasVariantsFolderLayout("/nonexistent/path")).To(BeFalse()) + }) + }) +}) + var _ = Describe("IntOrZero", func() { Context("when key exists with valid integer", func() { It("should return the integer value", func() {