From 5ab393ab001b3fbd909d478e98a6709334af3132 Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Tue, 9 Apr 2024 03:20:45 -0500 Subject: [PATCH 1/4] fix: remove duplicate logic for writing image layers to disk concurrently (#2409) ## Description remove duplicate logic for writing image layers to disk concurrently crane does this already in `WriteImage()`: https://github.com/defenseunicorns/zarf/blob/08c92e12a4a2b05d0ea5abe055c7c01ba9964051/src/internal/packager/images/pull.go#L342 https://github.com/google/go-containerregistry/blob/8dadbe76ff8c20d0e509406f04b7eade43baa6c1/pkg/v1/layout/write.go#L333-L340 ## Type of change - [x] Other (security config, docs update, etc) ## Checklist before merging - [x] Test, docs, adr added or updated as needed - [x] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed Co-authored-by: Austin Abro <37223396+AustinAbro321@users.noreply.github.com> --- src/internal/packager/images/pull.go | 164 +-------------------------- 1 file changed, 2 insertions(+), 162 deletions(-) diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index de5ab03ff2..df839e4f07 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -6,10 +6,7 @@ package images import ( "context" - "errors" "fmt" - "io" - "os" "path/filepath" "strings" @@ -28,7 +25,6 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" clayout "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/partial" - "github.com/google/go-containerregistry/pkg/v1/stream" "github.com/moby/moby/client" ) @@ -182,170 +178,16 @@ func (i *ImageConfig) PullAll() ([]ImgInfo, error) { updateText := fmt.Sprintf("Pulling %d images", imageCount) go utils.RenderProgressBarForLocalDirWrite(i.ImagesPath, totalBytes, doneSaving, updateText, updateText) - // Spawn a goroutine for each layer to write it to disk using crane - - layerWritingConcurrency := helpers.NewConcurrencyTools[bool, error](len(processedLayers)) - - defer layerWritingConcurrency.Cancel() - - for _, layer := range processedLayers { - layer := layer - // Function is a combination of https://github.com/google/go-containerregistry/blob/v0.15.2/pkg/v1/layout/write.go#L270-L305 - // and https://github.com/google/go-containerregistry/blob/v0.15.2/pkg/v1/layout/write.go#L198-L262 - // with modifications. This allows us to dedupe layers for all images and write them concurrently. - go func() { - digest, err := layer.Digest() - if errors.Is(err, stream.ErrNotComputed) { - // Allow digest errors, since streams may not have calculated the hash - // yet. Instead, use an empty value, which will be transformed into a - // random file name with `os.CreateTemp` and the final digest will be - // calculated after writing to a temp file and before renaming to the - // final path. - digest = v1.Hash{Algorithm: "sha256", Hex: ""} - } else if err != nil { - layerWritingConcurrency.ErrorChan <- err - return - } - - size, err := layer.Size() - if errors.Is(err, stream.ErrNotComputed) { - // Allow size errors, since streams may not have calculated the size - // yet. Instead, use -1 as a sentinel value meaning that no size - // comparison can be done and any sized blob file should be considered - // valid and not overwritten. - // - // TODO: Provide an option to always overwrite blobs. - size = -1 - } else if err != nil { - layerWritingConcurrency.ErrorChan <- err - return - } - - if layerWritingConcurrency.IsDone() { - return - } - - readCloser, err := layer.Compressed() - if err != nil { - layerWritingConcurrency.ErrorChan <- err - return - } - - // Create the directory for the blob if it doesn't exist - dir := filepath.Join(string(cranePath), "blobs", digest.Algorithm) - if err := helpers.CreateDirectory(dir, os.ModePerm); err != nil { - layerWritingConcurrency.ErrorChan <- err - return - } - - if layerWritingConcurrency.IsDone() { - return - } - - // Check if blob already exists and is the correct size - file := filepath.Join(dir, digest.Hex) - if s, err := os.Stat(file); err == nil && !s.IsDir() && (s.Size() == size || size == -1) { - layerWritingConcurrency.ProgressChan <- true - return - } - - if layerWritingConcurrency.IsDone() { - return - } - - // Write to a temporary file - w, err := os.CreateTemp(dir, digest.Hex) - if err != nil { - layerWritingConcurrency.ErrorChan <- err - return - } - // Delete temp file if an error is encountered before renaming - defer func() { - if err := os.Remove(w.Name()); err != nil && !errors.Is(err, os.ErrNotExist) { - message.Warnf("error removing temporary file after encountering an error while writing blob: %v", err) - } - }() - - defer w.Close() - - if layerWritingConcurrency.IsDone() { - return - } - - // Write to file rename - if n, err := io.Copy(w, readCloser); err != nil { - layerWritingConcurrency.ErrorChan <- err - return - } else if size != -1 && n != size { - layerWritingConcurrency.ErrorChan <- fmt.Errorf("expected blob size %d, but only wrote %d", size, n) - return - } - - if layerWritingConcurrency.IsDone() { - return - } - - // Always close reader before renaming, since Close computes the digest in - // the case of streaming layers. If Close is not called explicitly, it will - // occur in a goroutine that is not guaranteed to succeed before renamer is - // called. When renamer is the layer's Digest method, it can return - // ErrNotComputed. - if err := readCloser.Close(); err != nil { - layerWritingConcurrency.ErrorChan <- err - return - } - - // Always close file before renaming - if err := w.Close(); err != nil { - layerWritingConcurrency.ErrorChan <- err - return - } - - // Rename file based on the final hash - renamePath := filepath.Join(string(cranePath), "blobs", digest.Algorithm, digest.Hex) - os.Rename(w.Name(), renamePath) - - if layerWritingConcurrency.IsDone() { - return - } - - layerWritingConcurrency.ProgressChan <- true - }() - } - - onLayerWritingError := func(err error) error { - // Send a signal to the progress bar that we're done and wait for the thread to finish - doneSaving <- err - <-doneSaving - message.WarnErr(err, "Failed to write image layers, trying again up to 3 times...") - if strings.HasPrefix(err.Error(), "expected blob size") { - message.Warnf("Potential image cache corruption: %s - try clearing cache with \"zarf tools clear-cache\"", err.Error()) - } - return err - } - - if err := layerWritingConcurrency.WaitWithoutProgress(onLayerWritingError); err != nil { - return nil, err - } - imageSavingConcurrency := helpers.NewConcurrencyTools[digestInfo, error](len(refInfoToImage)) defer imageSavingConcurrency.Cancel() // Spawn a goroutine for each image to write it's config and manifest to disk using crane - // All layers should already be in place so this should be extremely fast for refInfo, img := range refInfoToImage { // Create a closure so that we can pass the refInfo and img into the goroutine refInfo, img := refInfo, img go func() { - // Save the image via crane - err := cranePath.WriteImage(img) - - if imageSavingConcurrency.IsDone() { - return - } - - if err != nil { + if err := cranePath.WriteImage(img); err != nil { // Check if the cache has been invalidated, and warn the user if so if strings.HasPrefix(err.Error(), "error writing layer: expected blob size") { message.Warnf("Potential image cache corruption: %s - try clearing cache with \"zarf tools clear-cache\"", err.Error()) @@ -390,15 +232,13 @@ func (i *ImageConfig) PullAll() ([]ImgInfo, error) { } // for every image sequentially append OCI descriptor - for refInfo, img := range refInfoToImage { desc, err := partial.Descriptor(img) if err != nil { return nil, err } - cranePath.AppendDescriptor(*desc) - if err != nil { + if err := cranePath.AppendDescriptor(*desc); err != nil { return nil, err } From 2982c00745b6461740dad1ed719f1cab539053b6 Mon Sep 17 00:00:00 2001 From: Wayne Starr Date: Tue, 9 Apr 2024 07:44:08 -0600 Subject: [PATCH 2/4] feat: add option to skip cosign lookup during find images (#2427) ## Description For larger packages it can take a long time to parse through the cosign lookups, having an option to not do this is nice for faster feedback cycles. ## Related Issue Fixes #N/A ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [X] Test, docs, adr added or updated as needed - [X] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed Co-authored-by: Austin Abro <37223396+AustinAbro321@users.noreply.github.com> --- .../100-cli-commands/zarf_dev_find-images.md | 1 + src/cmd/dev.go | 2 + src/config/lang/english.go | 17 ++++---- src/pkg/packager/prepare.go | 42 ++++++++++--------- src/types/runtime.go | 3 +- 5 files changed, 36 insertions(+), 29 deletions(-) diff --git a/docs/2-the-zarf-cli/100-cli-commands/zarf_dev_find-images.md b/docs/2-the-zarf-cli/100-cli-commands/zarf_dev_find-images.md index 221ee2020a..6f0c335aec 100644 --- a/docs/2-the-zarf-cli/100-cli-commands/zarf_dev_find-images.md +++ b/docs/2-the-zarf-cli/100-cli-commands/zarf_dev_find-images.md @@ -23,6 +23,7 @@ zarf dev find-images [ PACKAGE ] [flags] --kube-version string Override the default helm template KubeVersion when performing a package chart template --registry-url string Override the ###ZARF_REGISTRY### value (default "127.0.0.1:31999") -p, --repo-chart-path string If git repos hold helm charts, often found with gitops tools, specify the chart path, e.g. "/" or "/chart" + --skip-cosign Skip searching for cosign artifacts related to discovered images --why string Prints the source manifest for the specified image ``` diff --git a/src/cmd/dev.go b/src/cmd/dev.go index 92a46e3bcf..068565f7c7 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -301,6 +301,8 @@ func init() { devFindImagesCmd.Flags().StringVar(&pkgConfig.FindImagesOpts.KubeVersionOverride, "kube-version", "", lang.CmdDevFlagKubeVersion) // check which manifests are using this particular image devFindImagesCmd.Flags().StringVar(&pkgConfig.FindImagesOpts.Why, "why", "", lang.CmdDevFlagFindImagesWhy) + // skip searching cosign artifacts in find images + devFindImagesCmd.Flags().BoolVar(&pkgConfig.FindImagesOpts.SkipCosign, "skip-cosign", false, lang.CmdDevFlagFindImagesSkipCosign) defaultRegistry := fmt.Sprintf("%s:%d", helpers.IPV4Localhost, types.ZarfInClusterContainerRegistryNodePort) devFindImagesCmd.Flags().StringVar(&pkgConfig.FindImagesOpts.RegistryURL, "registry-url", defaultRegistry, lang.CmdDevFlagFindImagesRegistry) diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 9c9219a231..658ebab539 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -385,13 +385,14 @@ $ zarf package pull oci://ghcr.io/defenseunicorns/packages/dos-games:1.0.0 -a sk "NOTE: This file must not already exist. If no filename is provided, the config will be written to the current working directory as zarf-config.toml." CmdDevGenerateConfigErr = "Unable to write the config file %s, make sure the file doesn't already exist" - CmdDevFlagExtractPath = `The path inside of an archive to use to calculate the sha256sum (i.e. for use with "files.extractPath")` - CmdDevFlagSet = "Specify package variables to set on the command line (KEY=value). Note, if using a config file, this will be set by [package.create.set]." - CmdDevFlagRepoChartPath = `If git repos hold helm charts, often found with gitops tools, specify the chart path, e.g. "/" or "/chart"` - CmdDevFlagGitAccount = "User or organization name for the git account that the repos are created under." - CmdDevFlagKubeVersion = "Override the default helm template KubeVersion when performing a package chart template" - CmdDevFlagFindImagesRegistry = "Override the ###ZARF_REGISTRY### value" - CmdDevFlagFindImagesWhy = "Prints the source manifest for the specified image" + CmdDevFlagExtractPath = `The path inside of an archive to use to calculate the sha256sum (i.e. for use with "files.extractPath")` + CmdDevFlagSet = "Specify package variables to set on the command line (KEY=value). Note, if using a config file, this will be set by [package.create.set]." + CmdDevFlagRepoChartPath = `If git repos hold helm charts, often found with gitops tools, specify the chart path, e.g. "/" or "/chart"` + CmdDevFlagGitAccount = "User or organization name for the git account that the repos are created under." + CmdDevFlagKubeVersion = "Override the default helm template KubeVersion when performing a package chart template" + CmdDevFlagFindImagesRegistry = "Override the ###ZARF_REGISTRY### value" + CmdDevFlagFindImagesWhy = "Prints the source manifest for the specified image" + CmdDevFlagFindImagesSkipCosign = "Skip searching for cosign artifacts related to discovered images" CmdDevLintShort = "Lints the given package for valid schema and recommended practices" CmdDevLintLong = "Verifies the package schema, checks if any variables won't be evaluated, and checks for unpinned images/repos/files" @@ -477,7 +478,7 @@ $ zarf tools registry digest reg.example.com/stefanprodan/podinfo:6.4.0 CmdToolsGetGitPasswdShort = "[Deprecated] Returns the push user's password for the Git server" CmdToolsGetGitPasswdLong = "[Deprecated] Reads the password for a user with push access to the configured Git server in Zarf State. Note that this command has been replaced by 'zarf tools get-creds git' and will be removed in Zarf v1.0.0." CmdToolsGetGitPasswdDeprecation = "Deprecated: This command has been replaced by 'zarf tools get-creds git' and will be removed in Zarf v1.0.0." - CmdToolsYqExample = ` + CmdToolsYqExample = ` # yq defaults to 'eval' command if no command is specified. See "zarf tools yq eval --help" for more examples. # read the "stuff" node from "myfile.yml" diff --git a/src/pkg/packager/prepare.go b/src/pkg/packager/prepare.go index 96bbe10ac8..407ddcac4c 100644 --- a/src/pkg/packager/prepare.go +++ b/src/pkg/packager/prepare.go @@ -313,29 +313,31 @@ func (p *Packager) findImages() (imgMap map[string][]string, err error) { spinner.Success() - // Handle cosign artifact lookups - if len(imagesMap[component.Name]) > 0 { - var cosignArtifactList []string - spinner := message.NewProgressSpinner("Looking up cosign artifacts for discovered images (0/%d)", len(imagesMap[component.Name])) - defer spinner.Stop() - - for idx, image := range imagesMap[component.Name] { - spinner.Updatef("Looking up cosign artifacts for discovered images (%d/%d)", idx+1, len(imagesMap[component.Name])) - cosignArtifacts, err := utils.GetCosignArtifacts(image) - if err != nil { - message.WarnErrf(err, "Problem looking up cosign artifacts for %s: %s", image, err.Error()) - erroredCosignLookups = append(erroredCosignLookups, image) + if !p.cfg.FindImagesOpts.SkipCosign { + // Handle cosign artifact lookups + if len(imagesMap[component.Name]) > 0 { + var cosignArtifactList []string + spinner := message.NewProgressSpinner("Looking up cosign artifacts for discovered images (0/%d)", len(imagesMap[component.Name])) + defer spinner.Stop() + + for idx, image := range imagesMap[component.Name] { + spinner.Updatef("Looking up cosign artifacts for discovered images (%d/%d)", idx+1, len(imagesMap[component.Name])) + cosignArtifacts, err := utils.GetCosignArtifacts(image) + if err != nil { + message.WarnErrf(err, "Problem looking up cosign artifacts for %s: %s", image, err.Error()) + erroredCosignLookups = append(erroredCosignLookups, image) + } + cosignArtifactList = append(cosignArtifactList, cosignArtifacts...) } - cosignArtifactList = append(cosignArtifactList, cosignArtifacts...) - } - spinner.Success() + spinner.Success() - if len(cosignArtifactList) > 0 { - imagesMap[component.Name] = append(imagesMap[component.Name], cosignArtifactList...) - componentDefinition += fmt.Sprintf(" # Cosign artifacts for images - %s - %s\n", p.cfg.Pkg.Metadata.Name, component.Name) - for _, cosignArtifact := range cosignArtifactList { - componentDefinition += fmt.Sprintf(" - %s\n", cosignArtifact) + if len(cosignArtifactList) > 0 { + imagesMap[component.Name] = append(imagesMap[component.Name], cosignArtifactList...) + componentDefinition += fmt.Sprintf(" # Cosign artifacts for images - %s - %s\n", p.cfg.Pkg.Metadata.Name, component.Name) + for _, cosignArtifact := range cosignArtifactList { + componentDefinition += fmt.Sprintf(" - %s\n", cosignArtifact) + } } } } diff --git a/src/types/runtime.go b/src/types/runtime.go index 1ee40f91a0..4df0299175 100644 --- a/src/types/runtime.go +++ b/src/types/runtime.go @@ -57,7 +57,8 @@ type ZarfFindImagesOptions struct { RepoHelmChartPath string `json:"repoHelmChartPath" jsonschema:"description=Path to the helm chart directory"` KubeVersionOverride string `json:"kubeVersionOverride" jsonschema:"description=Kubernetes version to use for the helm chart"` RegistryURL string `json:"registryURL" jsonschema:"description=Manual override for ###ZARF_REGISTRY###"` - Why string `json:"why" jsonschema:"description=Find the location of the image given as an argument and print it to the console."` + Why string `json:"why" jsonschema:"description=Find the location of the image given as an argument and print it to the console"` + SkipCosign bool `json:"skip-cosign" jsonschema:"description=Optionally skip lookup of cosign artifacts when finding images"` } // ZarfDeployOptions tracks the user-defined preferences during a package deploy. From d4a750e2a0894839916465c19c1d017c99bd0b7e Mon Sep 17 00:00:00 2001 From: Naveen <172697+naveensrinivasan@users.noreply.github.com> Date: Tue, 9 Apr 2024 09:25:35 -0500 Subject: [PATCH 3/4] feat: allow chart deploy overrides ALPHA (#2403) ## Description - This feature allows chart deployment overrides. ## Related Issue Relates to https://github.com/defenseunicorns/zarf/issues/2133 ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [ ] Test, docs, adr added or updated as needed - [ ] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed --------- Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com> Co-authored-by: Lucas Rodriguez Co-authored-by: razzle Co-authored-by: Austin Abro <37223396+AustinAbro321@users.noreply.github.com> --- adr/0023-chart-override.md | 65 ++++++ docs/3-create-a-zarf-package/4-zarf-schema.md | 121 ++++++++-- examples/helm-charts/zarf.yaml | 7 + src/pkg/packager/deploy.go | 53 ++++- src/pkg/packager/deploy_test.go | 218 ++++++++++++++++++ src/types/component.go | 28 ++- zarf.schema.json | 35 +++ 7 files changed, 493 insertions(+), 34 deletions(-) create mode 100644 adr/0023-chart-override.md create mode 100644 src/pkg/packager/deploy_test.go diff --git a/adr/0023-chart-override.md b/adr/0023-chart-override.md new file mode 100644 index 0000000000..d4fb4661c0 --- /dev/null +++ b/adr/0023-chart-override.md @@ -0,0 +1,65 @@ +# 23. Simplifying Helm Chart Value Overrides + +Date: 2024-03-25 + +## Status + +Accepted + + +## Context + +The process of deploying applications with Helm charts in Kubernetes environments often necessitates the customization of chart values to align with specific operational or environmental requirements. The current method for customizing these values—either through manual edits or `###ZARF_VAR_XYZ###`. A more streamlined approach would greatly enhance the deployment experience by offering both flexibility and reliability. + +## Decision + +To address this issue, we propose the introduction of a feature designed to simplify the process of overriding chart values at the time of deployment. This feature would allow users to easily specify overrides for any chart values directly via command-line arguments, eliminating the need to alter the chart's default values file or manage multiple command-line arguments for each override. + +Key aspects of the proposed implementation include: +- Use existing `--set` flags to specify overrides for chart values. +- The ability to list all overrides in a structured and easily understandable format within `zarf.yaml`. +- Ensuring that during deployment, these specified overrides take precedence over the chart's default values, thus facilitating customized deployments without necessitating permanent modifications to the chart. + +## Consequences + +Adopting this feature would lead to several key improvements: +- **Streamlined Configuration Process**: Allowing helm values overrides in the zarf package schema simplifies the user experience by reducing the reliance on extensive custom `###ZARF_VAR_XYZ###` templating and aligning more closely with standard Helm practices + +Ultimately, this feature is aimed at enhancing the deployment workflow by offering a straightforward and efficient means of customizing Helm chart deployments via command-line inputs. + +## Example Configuration + +Below is an example of how the `zarf.yaml` configuration file might be structured to utilize the new override feature for Helm chart values: + +```yaml +kind: ZarfPackageConfig +metadata: + name: helm-charts + description: Example showcasing multiple ways to deploy helm charts + version: 0.0.1 + +components: + - name: demo-helm-charts + required: true + charts: + - name: podinfo-local + version: 6.4.0 + namespace: podinfo-from-local-chart + localPath: chart + valuesFiles: + - values.yaml + variables: + - name: REPLICA_COUNT + description: "Override the number of pod replicas" + path: replicaCount +``` +This configuration allows for the specification of default values and descriptions for variables that can be overridden at deployment time. The variables section under each chart specifies the variables that can be overridden, along with a path that indicates where in the values file the variable is located. + +### Command Line Example + +To override the `REPLICA_COUNT` variable at deployment time, the following command can be used: + +```bash +zarf package deploy zarf-package-helm-charts-arm64-0.0.1.tar.zst --set REPLICA_COUNT=5 +``` +This command demonstrates how users can easily customize their Helm chart deployments by specifying overrides for chart values directly via command-line arguments, in line with the proposed feature. diff --git a/docs/3-create-a-zarf-package/4-zarf-schema.md b/docs/3-create-a-zarf-package/4-zarf-schema.md index e4ff7eb157..48acf1650a 100644 --- a/docs/3-create-a-zarf-package/4-zarf-schema.md +++ b/docs/3-create-a-zarf-package/4-zarf-schema.md @@ -1185,6 +1185,95 @@ Must be one of: +
+ + variables + +  +
+ + ## components > charts > variables + +**Description:** [alpha] List of variables to set in the Helm chart + +| | | +| -------- | ------- | +| **Type** | `array` | + +![Min Items: N/A](https://img.shields.io/badge/Min%20Items%3A%20N/A-gold) +![Max Items: N/A](https://img.shields.io/badge/Max%20Items%3A%20N/A-gold) +![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) +![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) + + ### ZarfChartVariable + +| | | +| ------------------------- | -------------------------------------------------------------------------------------------------------- | +| **Type** | `object` | +| **Additional properties** | [![Not allowed](https://img.shields.io/badge/Not%20allowed-red)](# "Additional Properties not allowed.") | +| **Defined in** | #/definitions/ZarfChartVariable | + +
+ + name * + +  +
+ +![Required](https://img.shields.io/badge/Required-red) + +**Description:** The name of the variable + +| | | +| -------- | -------- | +| **Type** | `string` | + +| Restrictions | | +| --------------------------------- | ----------------------------------------------------------------------------- | +| **Must match regular expression** | ```^[A-Z0-9_]+$``` [Test](https://regex101.com/?regex=%5E%5BA-Z0-9_%5D%2B%24) | + +
+
+ +
+ + description * + +  +
+ +![Required](https://img.shields.io/badge/Required-red) + +**Description:** A brief description of what the variable controls + +| | | +| -------- | -------- | +| **Type** | `string` | + +
+
+ +
+ + path * + +  +
+ +![Required](https://img.shields.io/badge/Required-red) + +**Description:** The path within the Helm chart values where this variable applies + +| | | +| -------- | -------- | +| **Type** | `string` | + +
+
+ +
+
+ @@ -1208,7 +1297,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### ZarfDataInjection + ### ZarfDataInjection | | | | ------------------------- | -------------------------------------------------------------------------------------------------------- | @@ -1371,7 +1460,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### ZarfFile + ### ZarfFile | | | | ------------------------- | -------------------------------------------------------------------------------------------------------- | @@ -1465,7 +1554,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### symlinks items + ### symlinks items | | | | -------- | -------- | @@ -1511,7 +1600,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### images items + ### images items | | | | -------- | -------- | @@ -1538,7 +1627,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### repos items + ### repos items | | | | -------- | -------- | @@ -1633,7 +1722,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### valuesFiles items + ### valuesFiles items | | | | -------- | -------- | @@ -1676,7 +1765,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### fluxPatchFiles items + ### fluxPatchFiles items | | | | -------- | -------- | @@ -1824,7 +1913,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### env items + ### env items | | | | -------- | -------- | @@ -1939,7 +2028,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### ZarfComponentAction + ### ZarfComponentAction | | | | ------------------------- | -------------------------------------------------------------------------------------------------------- | @@ -2029,7 +2118,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### env items + ### env items | | | | -------- | -------- | @@ -2094,7 +2183,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### ZarfComponentActionSetVariable + ### ZarfComponentActionSetVariable | | | | ------------------------- | -------------------------------------------------------------------------------------------------------- | @@ -2448,7 +2537,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### ZarfComponentAction + ### ZarfComponentAction | | | | ------------------------- | -------------------------------------------------------------------------------------------------------- | @@ -2479,7 +2568,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### ZarfComponentAction + ### ZarfComponentAction | | | | ------------------------- | -------------------------------------------------------------------------------------------------------- | @@ -2510,7 +2599,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### ZarfComponentAction + ### ZarfComponentAction | | | | ------------------------- | -------------------------------------------------------------------------------------------------------- | @@ -2590,7 +2679,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### ZarfPackageConstant + ### ZarfPackageConstant | | | | ------------------------- | -------------------------------------------------------------------------------------------------------- | @@ -2709,7 +2798,7 @@ Must be one of: ![Item unicity: False](https://img.shields.io/badge/Item%20unicity%3A%20False-gold) ![Additional items: N/A](https://img.shields.io/badge/Additional%20items%3A%20N/A-gold) - ### ZarfPackageVariable + ### ZarfPackageVariable | | | | ------------------------- | -------------------------------------------------------------------------------------------------------- | diff --git a/examples/helm-charts/zarf.yaml b/examples/helm-charts/zarf.yaml index d6481b334d..4c8cbd7040 100644 --- a/examples/helm-charts/zarf.yaml +++ b/examples/helm-charts/zarf.yaml @@ -16,6 +16,12 @@ components: localPath: chart valuesFiles: - values.yaml + # Variables are used to override the default values in the chart + # This can be overridden by the user at deployment time with the `--set` flag + variables: + - name: REPLICA_COUNT + description: "Override the number of pod replicas" + path: replicaCount - name: podinfo-oci version: 6.4.0 @@ -77,3 +83,4 @@ components: name: cool-release-name-podinfo namespace: podinfo-from-repo condition: available + diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index 7ec050fd4f..67eccff4db 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -547,10 +547,46 @@ func (p *Packager) pushReposToRepository(reposPath string, repos []string) error return nil } +// generateValuesOverrides creates a map containing overrides for chart values based on the given inputs. +// It prioritizes the overrides in this order: +// 1. Variables defined in the ZarfComponent, including both Set Variables and Chart Variables. +// 2. Deployment options specified through DeployOpts. +// This function is useful for customizing the deployment of a Helm chart by programmatically setting chart values. +// +// Returns: +// - A map containing the final set of value overrides for the chart, where keys are the variable names. +func generateValuesOverrides(chartVariables []types.ZarfChartVariable, + setVariableMap map[string]*types.ZarfSetVariable, + deployOpts types.ZarfDeployOptions, + componentName, chartName string) (map[string]any, error) { + + valuesOverrides := make(map[string]any) + chartOverrides := make(map[string]any) + + for _, variable := range chartVariables { + if setVar, ok := setVariableMap[variable.Name]; ok && setVar != nil { + // Use the variable's path as a key to ensure unique entries for variables with the same name but different paths. + if err := helpers.MergePathAndValueIntoMap(chartOverrides, variable.Path, setVar.Value); err != nil { + return nil, fmt.Errorf("unable to merge path and value into map: %w", err) + } + } + } + + // Apply any direct overrides specified in the deployment options for this component and chart + if componentOverrides, ok := deployOpts.ValuesOverridesMap[componentName]; ok { + if chartSpecificOverrides, ok := componentOverrides[chartName]; ok { + valuesOverrides = chartSpecificOverrides + } + } + + // Merge chartOverrides into valuesOverrides to ensure all overrides are applied. + // This corrects the logic to ensure that chartOverrides and valuesOverrides are merged correctly. + return helpers.MergeMapRecursive(valuesOverrides, chartOverrides), nil +} + // Install all Helm charts and raw k8s manifests into the k8s cluster. func (p *Packager) installChartAndManifests(componentPaths *layout.ComponentPaths, component types.ZarfComponent) (installedCharts []types.InstalledChart, err error) { for _, chart := range component.Charts { - // zarf magic for the value file for idx := range chart.ValuesFiles { chartValueName := helm.StandardValuesName(componentPaths.Values, chart, idx) @@ -558,13 +594,14 @@ func (p *Packager) installChartAndManifests(componentPaths *layout.ComponentPath return installedCharts, err } } - - // TODO (@WSTARR): Currently this logic is library-only and is untested while it is in an experimental state - it may eventually get added as shorthand in Zarf Variables though - var valuesOverrides map[string]any - if componentChartValuesOverrides, ok := p.cfg.DeployOpts.ValuesOverridesMap[component.Name]; ok { - if chartValuesOverrides, ok := componentChartValuesOverrides[chart.Name]; ok { - valuesOverrides = chartValuesOverrides - } + // this is to get the overrides for the chart from the commandline and + // the chart variables set in the ZarfComponent and as well as DeployOpts set from the Library user. + // The order of precedence is as follows: + // 1. Set Variables and Chart Variables from the ZarfComponent + // 2. DeployOpts + valuesOverrides, err := generateValuesOverrides(chart.Variables, p.cfg.SetVariableMap, p.cfg.DeployOpts, component.Name, chart.Name) + if err != nil { + return installedCharts, err } helmCfg := helm.New( diff --git a/src/pkg/packager/deploy_test.go b/src/pkg/packager/deploy_test.go new file mode 100644 index 0000000000..2b235b9c56 --- /dev/null +++ b/src/pkg/packager/deploy_test.go @@ -0,0 +1,218 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +package packager + +import ( + "reflect" + "testing" + + "github.com/defenseunicorns/zarf/src/types" +) + +func TestGenerateValuesOverrides(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + chartVariables []types.ZarfChartVariable + setVariableMap map[string]*types.ZarfSetVariable + deployOpts types.ZarfDeployOptions + componentName string + chartName string + want map[string]any + }{ + { + name: "Empty inputs", + chartVariables: []types.ZarfChartVariable{}, + setVariableMap: map[string]*types.ZarfSetVariable{}, + deployOpts: types.ZarfDeployOptions{}, + componentName: "", + chartName: "", + want: map[string]any{}, + }, + { + name: "Single variable", + chartVariables: []types.ZarfChartVariable{{Name: "testVar", Path: "testVar"}}, + setVariableMap: map[string]*types.ZarfSetVariable{"testVar": {Value: "testValue"}}, + deployOpts: types.ZarfDeployOptions{}, + componentName: "testComponent", + chartName: "testChart", + want: map[string]any{"testVar": "testValue"}, + }, + { + name: "Non-matching setVariable", + chartVariables: []types.ZarfChartVariable{{Name: "expectedVar", Path: "path.to.expectedVar"}}, + setVariableMap: map[string]*types.ZarfSetVariable{"unexpectedVar": {Value: "unexpectedValue"}}, + deployOpts: types.ZarfDeployOptions{}, + componentName: "testComponent", + chartName: "testChart", + want: map[string]any{}, + }, + { + name: "Nested 3 level setVariableMap", + chartVariables: []types.ZarfChartVariable{ + {Name: "level1.level2.level3Var", Path: "level1.level2.level3Var"}, + }, + setVariableMap: map[string]*types.ZarfSetVariable{ + "level1.level2.level3Var": {Value: "nestedValue"}, + }, + deployOpts: types.ZarfDeployOptions{}, + componentName: "nestedComponent", + chartName: "nestedChart", + want: map[string]any{ + "level1": map[string]any{ + "level2": map[string]any{ + "level3Var": "nestedValue", + }, + }, + }, + }, + { + name: "Multiple variables with nested and non-nested paths, distinct values", + chartVariables: []types.ZarfChartVariable{ + {Name: "NESTED_VAR_LEVEL2", Path: "nestedVar.level2"}, + {Name: "simpleVar", Path: "simpleVar"}, + }, + setVariableMap: map[string]*types.ZarfSetVariable{ + "NESTED_VAR_LEVEL2": {Value: "distinctNestedValue"}, + "simpleVar": {Value: "distinctSimpleValue"}, + }, + deployOpts: types.ZarfDeployOptions{}, + componentName: "mixedComponent", + chartName: "mixedChart", + want: map[string]any{ + "nestedVar": map[string]any{ + "level2": "distinctNestedValue", + }, + "simpleVar": "distinctSimpleValue", + }, + }, + { + name: "Values override test", + chartVariables: []types.ZarfChartVariable{ + {Name: "overrideVar", Path: "path"}, + }, + setVariableMap: map[string]*types.ZarfSetVariable{ + "path": {Value: "overrideValue"}, + }, + deployOpts: types.ZarfDeployOptions{ + ValuesOverridesMap: map[string]map[string]map[string]any{ + "testComponent": { + "testChart": { + "path": "deployOverrideValue", + }, + }, + }, + }, + componentName: "testComponent", + chartName: "testChart", + want: map[string]any{ + "path": "deployOverrideValue", + }, + }, + { + name: "Missing variable in setVariableMap but present in ValuesOverridesMap", + chartVariables: []types.ZarfChartVariable{ + {Name: "missingVar", Path: "missingVarPath"}, + }, + setVariableMap: map[string]*types.ZarfSetVariable{}, + deployOpts: types.ZarfDeployOptions{ + ValuesOverridesMap: map[string]map[string]map[string]any{ + "testComponent": { + "testChart": { + "missingVarPath": "overrideValue", + }, + }, + }, + }, + componentName: "testComponent", + chartName: "testChart", + want: map[string]any{ + "missingVarPath": "overrideValue", + }, + }, + { + name: "Non-existent component or chart", + chartVariables: []types.ZarfChartVariable{{Name: "someVar", Path: "someVar"}}, + setVariableMap: map[string]*types.ZarfSetVariable{"someVar": {Value: "value"}}, + deployOpts: types.ZarfDeployOptions{ + ValuesOverridesMap: map[string]map[string]map[string]any{ + "nonExistentComponent": { + "nonExistentChart": { + "someVar": "overrideValue", + }, + }, + }, + }, + componentName: "actualComponent", + chartName: "actualChart", + want: map[string]any{"someVar": "value"}, + }, + { + name: "Variable in setVariableMap but not in chartVariables", + chartVariables: []types.ZarfChartVariable{}, + setVariableMap: map[string]*types.ZarfSetVariable{ + "orphanVar": {Value: "orphanValue"}, + }, + deployOpts: types.ZarfDeployOptions{}, + componentName: "orphanComponent", + chartName: "orphanChart", + want: map[string]any{}, + }, + { + name: "Empty ValuesOverridesMap with non-empty setVariableMap and chartVariables", + chartVariables: []types.ZarfChartVariable{ + {Name: "var1", Path: "path.to.var1"}, + {Name: "var2", Path: "path.to.var2"}, + {Name: "var3", Path: "path.to3.var3"}, + }, + setVariableMap: map[string]*types.ZarfSetVariable{ + "var1": {Value: "value1"}, + "var2": {Value: "value2"}, + "var3": {Value: "value3"}, + }, + deployOpts: types.ZarfDeployOptions{ + ValuesOverridesMap: map[string]map[string]map[string]any{}, + }, + componentName: "componentWithVars", + chartName: "chartWithVars", + want: map[string]any{ + "path": map[string]any{ + "to": map[string]any{ + "var1": "value1", + "var2": "value2", + }, + "to3": map[string]any{ + "var3": "value3", + }, + }, + }, + }, + { + name: "Empty chartVariables and non-empty setVariableMap", + chartVariables: []types.ZarfChartVariable{}, + setVariableMap: map[string]*types.ZarfSetVariable{ + "var1": {Value: "value1"}, + "var2": {Value: "value2"}, + }, + deployOpts: types.ZarfDeployOptions{}, + componentName: "componentWithVars", + chartName: "chartWithVars", + want: map[string]any{}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got, err := generateValuesOverrides(tt.chartVariables, tt.setVariableMap, tt.deployOpts, tt.componentName, tt.chartName) + if err != nil { + t.Errorf("%s: generateValuesOverrides() error = %v", tt.name, err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("%s: generateValuesOverrides() got = %v, want %v", tt.name, got, tt.want) + } + }) + } +} diff --git a/src/types/component.go b/src/types/component.go index 028f25202c..eaeed14bd8 100644 --- a/src/types/component.go +++ b/src/types/component.go @@ -105,16 +105,24 @@ type ZarfFile struct { // ZarfChart defines a helm chart to be deployed. type ZarfChart struct { - Name string `json:"name" jsonschema:"description=The name of the chart within Zarf; note that this must be unique and does not need to be the same as the name in the chart repo"` - Version string `json:"version,omitempty" jsonschema:"description=The version of the chart to deploy; for git-based charts this is also the tag of the git repo by default (when not using the '@' syntax for 'repos')"` - URL string `json:"url,omitempty" jsonschema:"example=OCI registry: oci://ghcr.io/stefanprodan/charts/podinfo,example=helm chart repo: https://stefanprodan.github.io/podinfo,example=git repo: https://github.com/stefanprodan/podinfo (note the '@' syntax for 'repos' is supported here too)" jsonschema_description:"The URL of the OCI registry, chart repository, or git repo where the helm chart is stored"` - RepoName string `json:"repoName,omitempty" jsonschema:"description=The name of a chart within a Helm repository (defaults to the Zarf name of the chart)"` - GitPath string `json:"gitPath,omitempty" jsonschema:"description=(git repo only) The sub directory to the chart within a git repo,example=charts/your-chart"` - LocalPath string `json:"localPath,omitempty" jsonschema:"description=The path to a local chart's folder or .tgz archive"` - Namespace string `json:"namespace" jsonschema:"description=The namespace to deploy the chart to"` - ReleaseName string `json:"releaseName,omitempty" jsonschema:"description=The name of the Helm release to create (defaults to the Zarf name of the chart)"` - NoWait bool `json:"noWait,omitempty" jsonschema:"description=Whether to not wait for chart resources to be ready before continuing"` - ValuesFiles []string `json:"valuesFiles,omitempty" jsonschema:"description=List of local values file paths or remote URLs to include in the package; these will be merged together when deployed"` + Name string `json:"name" jsonschema:"description=The name of the chart within Zarf; note that this must be unique and does not need to be the same as the name in the chart repo"` + Version string `json:"version,omitempty" jsonschema:"description=The version of the chart to deploy; for git-based charts this is also the tag of the git repo by default (when not using the '@' syntax for 'repos')"` + URL string `json:"url,omitempty" jsonschema:"example=OCI registry: oci://ghcr.io/stefanprodan/charts/podinfo,example=helm chart repo: https://stefanprodan.github.io/podinfo,example=git repo: https://github.com/stefanprodan/podinfo (note the '@' syntax for 'repos' is supported here too)" jsonschema_description:"The URL of the OCI registry, chart repository, or git repo where the helm chart is stored"` + RepoName string `json:"repoName,omitempty" jsonschema:"description=The name of a chart within a Helm repository (defaults to the Zarf name of the chart)"` + GitPath string `json:"gitPath,omitempty" jsonschema:"description=(git repo only) The sub directory to the chart within a git repo,example=charts/your-chart"` + LocalPath string `json:"localPath,omitempty" jsonschema:"description=The path to a local chart's folder or .tgz archive"` + Namespace string `json:"namespace" jsonschema:"description=The namespace to deploy the chart to"` + ReleaseName string `json:"releaseName,omitempty" jsonschema:"description=The name of the Helm release to create (defaults to the Zarf name of the chart)"` + NoWait bool `json:"noWait,omitempty" jsonschema:"description=Whether to not wait for chart resources to be ready before continuing"` + ValuesFiles []string `json:"valuesFiles,omitempty" jsonschema:"description=List of local values file paths or remote URLs to include in the package; these will be merged together when deployed"` + Variables []ZarfChartVariable `json:"variables,omitempty" jsonschema:"description=[alpha] List of variables to set in the Helm chart"` +} + +// ZarfChartVariable represents a variable that can be set for a Helm chart overrides. +type ZarfChartVariable struct { + Name string `json:"name" jsonschema:"description=The name of the variable,pattern=^[A-Z0-9_]+$"` + Description string `json:"description" jsonschema:"description=A brief description of what the variable controls"` + Path string `json:"path" jsonschema:"description=The path within the Helm chart values where this variable applies"` } // ZarfManifest defines raw manifests Zarf will deploy as a helm chart. diff --git a/zarf.schema.json b/zarf.schema.json index f61482b7a4..69f4aae9d0 100644 --- a/zarf.schema.json +++ b/zarf.schema.json @@ -256,6 +256,41 @@ }, "type": "array", "description": "List of local values file paths or remote URLs to include in the package; these will be merged together when deployed" + }, + "variables": { + "items": { + "$schema": "http://json-schema.org/draft-04/schema#", + "$ref": "#/definitions/ZarfChartVariable" + }, + "type": "array", + "description": "[alpha] List of variables to set in the Helm chart" + } + }, + "additionalProperties": false, + "type": "object", + "patternProperties": { + "^x-": {} + } + }, + "ZarfChartVariable": { + "required": [ + "name", + "description", + "path" + ], + "properties": { + "name": { + "pattern": "^[A-Z0-9_]+$", + "type": "string", + "description": "The name of the variable" + }, + "description": { + "type": "string", + "description": "A brief description of what the variable controls" + }, + "path": { + "type": "string", + "description": "The path within the Helm chart values where this variable applies" } }, "additionalProperties": false, From 02389274c61b59385ac339d6d690a148878427f9 Mon Sep 17 00:00:00 2001 From: razzle Date: Tue, 9 Apr 2024 10:11:58 -0500 Subject: [PATCH 4/4] chore: update pull_request_template.md (#2428) ## Description As we now use `commitlint` on PR titles, there is no need to have the checklist in the PR description for the type of change. ## Related Issue Fixes N/A ## Checklist before merging - [ ] Test, docs, adr added or updated as needed - [ ] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed --- .github/pull_request_template.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 2064021e2b..370d4faa9a 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -8,12 +8,6 @@ Fixes # Relates to # -## Type of change - -- [ ] Bug fix (non-breaking change which fixes an issue) -- [ ] New feature (non-breaking change which adds functionality) -- [ ] Other (security config, docs update, etc) - ## Checklist before merging - [ ] Test, docs, adr added or updated as needed