Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions internal/controller/argo.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ const (
ParamVpArgoNamespace = "global.vpArgoNamespace"
ParamMultiSourceTargetRevision = "global.multiSourceTargetRevision"
ParamDeletePattern = "global.deletePattern"
ParamVpNewFolderDir = "global.vpNewFolderDir"
)

// ConsoleLink constants
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
77 changes: 68 additions & 9 deletions internal/controller/argo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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))
})
})
Expand All @@ -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))
})
})
Expand All @@ -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")))
})
})
Expand All @@ -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")))
Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down
17 changes: 13 additions & 4 deletions internal/controller/pattern_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
40 changes: 40 additions & 0 deletions internal/controller/pattern_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package controllers
import (
"context"
"os"
"path/filepath"

"github.com/go-git/go-git/v5"
"github.com/go-logr/logr"
Expand Down Expand Up @@ -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"))
})
})
})

Expand Down
7 changes: 7 additions & 0 deletions internal/controller/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
39 changes: 39 additions & 0 deletions internal/controller/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down