diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 103c8909..2562ce2a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -21,6 +21,9 @@ jobs: - name: Run unit tests run: make test + - name: Verify README flag tables + run: make verify-readme + - name: Verify installation run: | mkdir -p helmhome @@ -150,3 +153,46 @@ jobs: - name: helm diff upgrade -C 3 --set replicaCount=2 --install helm-diff ./helm-diff run: helm diff upgrade -C 3 --set replicaCount=2 --install helm-diff ./helm-diff + + # Flux-style setup: the release storage (sh.helm.release.v1.* secrets) lives + # in a separate storage namespace (flux-system) while the workloads are + # deployed into the target namespace (prod-apps). Exercises --storage-namespace, + # HELM_DIFF_STORAGE_NAMESPACE and the three-way-merge pipeline against a + # real cluster. + - name: Install release into prod-apps and move its storage to flux-system + run: | + helm upgrade -i storage-diff ./helm-diff -n prod-apps --create-namespace + kubectl create namespace flux-system + for secret in $(kubectl get secrets -n prod-apps -o name | grep '^secret/sh\.helm\.release\.v1\.storage-diff\.'); do + kubectl get -n prod-apps "${secret}" -o json \ + | jq --arg ns flux-system '.metadata.namespace = $ns | del(.metadata.resourceVersion, .metadata.uid, .metadata.creationTimestamp)' \ + | kubectl apply -f - + kubectl delete -n prod-apps "${secret}" + done + + - name: Verify release storage is only visible in flux-system + run: | + if helm get manifest storage-diff -n prod-apps >/dev/null 2>&1; then + echo "unexpected: release storage still found in prod-apps" >&2 + exit 1 + fi + helm get manifest storage-diff -n flux-system >/dev/null + + - name: helm diff upgrade fails without --storage-namespace + run: | + if helm diff upgrade storage-diff ./helm-diff -n prod-apps >/dev/null 2>&1; then + echo "unexpected: diff succeeded although the release storage is not in prod-apps" >&2 + exit 1 + fi + + - name: helm diff upgrade --storage-namespace + run: helm diff upgrade storage-diff ./helm-diff -n prod-apps --storage-namespace flux-system + + - name: helm diff upgrade with HELM_DIFF_STORAGE_NAMESPACE env var + run: HELM_DIFF_STORAGE_NAMESPACE=flux-system helm diff upgrade storage-diff ./helm-diff -n prod-apps + + - name: helm diff upgrade --storage-namespace --three-way-merge + run: | + set -o pipefail + helm diff upgrade storage-diff ./helm-diff -n prod-apps --storage-namespace flux-system --three-way-merge --set replicaCount=2 | tee /tmp/three-way.out + grep -q 'replicas: 2' /tmp/three-way.out diff --git a/Makefile b/Makefile index ff362c58..941d5318 100644 --- a/Makefile +++ b/Makefile @@ -41,6 +41,15 @@ test: go test -v ./... -coverprofile cover.out -race go tool cover -func cover.out +.PHONY: readme +readme: build + scripts/gen-readme.sh bin/diff + +.PHONY: verify-readme +verify-readme: build + scripts/gen-readme.sh bin/diff + git diff --exit-code README.md + .PHONY: docker-run-release docker-run-release: export pkg=/go/src/github.com/databus23/helm-diff docker-run-release: diff --git a/README.md b/README.md index 968b2f92..722942f9 100644 --- a/README.md +++ b/README.md @@ -156,8 +156,10 @@ Flags: --include-tests enable the diffing of the helm test hooks --insecure-skip-tls-verify skip tls certificate checks for the chart download --install enables diffing of releases that are not yet deployed via Helm (equivalent to --allow-unreleased, added to match "helm upgrade --install" command + --kube-context string name of the kubeconfig context to use --kube-version string Kubernetes version used for Capabilities.KubeVersion --kubeconfig string This flag is ignored, to allow passing of this top level flag to helm + -n, --namespace string namespace to assume the release to be installed into. Defaults to the current kube config namespace. --no-color remove colors from the output. If both --no-color and --color are unspecified, coloring enabled only when the stdout is a term and TERM is not "dumb" --no-hooks disable diffing of hooks --normalize-manifests normalize manifests before running diff to exclude style differences from the output @@ -178,6 +180,7 @@ Flags: --show-secrets do not redact secret values in the output --show-secrets-decoded decode secret values in the output --skip-schema-validation skip validation of the rendered manifests against the Kubernetes OpenAPI schema + --storage-namespace string namespace where the helm release storage (Secret/ConfigMap) is located. Defaults to the target namespace (-n/--namespace) --strip-trailing-cr strip trailing carriage return on input --suppress stringArray allows suppression of the kinds listed in the diff output (can specify multiple, like '--suppress Deployment --suppress Service') --suppress-output-line-regex stringArray a regex to suppress diff output lines that match @@ -187,7 +190,7 @@ Flags: -f, --values valueFiles specify values in a YAML file (can specify multiple) (default []) --version string specify the exact chart version to use. If this is not specified, the latest version is used -Additional help topcis: +Additional help topics: diff Use "diff [command] --help" for more information about a command. @@ -323,6 +326,17 @@ Examples: # Read the flag usage below for more information on --context. HELM_DIFF_OUTPUT_CONTEXT=5 helm diff upgrade my-release datadog/datadog + # Set HELM_DIFF_STORAGE_NAMESPACE=flux-system to + # fetch release manifests/values/hooks from a storage namespace different from the target namespace. + # This is equivalent to specifying the --storage-namespace flag. + HELM_DIFF_STORAGE_NAMESPACE=flux-system helm diff upgrade -n prod-apps my-release datadog/datadog + + # NOTE: The storage namespace separation is not supported in combination with + # HELM_DIFF_USE_UPGRADE_DRY_RUN=true, because rendering then goes through + # `helm upgrade --dry-run`, which resolves the release storage in the target + # namespace. If the release exists only in the storage namespace, keep the + # default `helm template` based rendering instead. + Flags: --allow-unreleased enables diffing of releases that are not yet deployed via Helm -a, --api-versions stringArray Kubernetes api versions used for Capabilities.APIVersions @@ -339,8 +353,10 @@ Flags: --include-tests enable the diffing of the helm test hooks --insecure-skip-tls-verify skip tls certificate checks for the chart download --install enables diffing of releases that are not yet deployed via Helm (equivalent to --allow-unreleased, added to match "helm upgrade --install" command + --kube-context string name of the kubeconfig context to use --kube-version string Kubernetes version used for Capabilities.KubeVersion --kubeconfig string This flag is ignored, to allow passing of this top level flag to helm + -n, --namespace string namespace to assume the release to be installed into. Defaults to the current kube config namespace. --no-hooks disable diffing of hooks --normalize-manifests normalize manifests before running diff to exclude style differences from the output --output string Possible values: diff, simple, template, json, structured, dyff. When set to "template", use the env var HELM_DIFF_TPL to specify the template. (default "diff") @@ -360,6 +376,7 @@ Flags: --show-secrets do not redact secret values in the output --show-secrets-decoded decode secret values in the output --skip-schema-validation skip validation of the rendered manifests against the Kubernetes OpenAPI schema + --storage-namespace string namespace where the helm release storage (Secret/ConfigMap) is located. Defaults to the target namespace (-n/--namespace) --strip-trailing-cr strip trailing carriage return on input --suppress stringArray allows suppression of the kinds listed in the diff output (can specify multiple, like '--suppress Deployment --suppress Service') --suppress-output-line-regex stringArray a regex to suppress diff output lines that match @@ -399,9 +416,11 @@ Flags: -D, --find-renames float32 Enable rename detection if set to any value greater than 0. If specified, the value denotes the maximum fraction of changed content as lines added + removed compared to total lines in a diff for considering it a rename. Only objects of the same Kind are attempted to be matched -h, --help help for release --include-tests enable the diffing of the helm test hooks + --kube-context string name of the kubeconfig context to use --normalize-manifests normalize manifests before running diff to exclude style differences from the output --output string Possible values: diff, simple, template, json, structured, dyff. When set to "template", use the env var HELM_DIFF_TPL to specify the template. (default "diff") --show-secrets do not redact secret values in the output + --show-secrets-decoded decode secret values in the output --strip-trailing-cr strip trailing carriage return on input --suppress stringArray allows suppression of the kinds listed in the diff output (can specify multiple, like '--suppress Deployment --suppress Service') --suppress-output-line-regex stringArray a regex to suppress diff output lines that match @@ -436,15 +455,17 @@ Usage: Flags: -C, --context int output NUM lines of context around changes (default -1) - --show-secrets-decoded decode secret values in the output --detailed-exitcode return a non-zero exit code when there are changes -D, --find-renames float32 Enable rename detection if set to any value greater than 0. If specified, the value denotes the maximum fraction of changed content as lines added + removed compared to total lines in a diff for considering it a rename. Only objects of the same Kind are attempted to be matched -h, --help help for revision --include-tests enable the diffing of the helm test hooks + --kube-context string name of the kubeconfig context to use + -n, --namespace string namespace to assume the release to be installed into. Defaults to the current kube config namespace. --normalize-manifests normalize manifests before running diff to exclude style differences from the output --output string Possible values: diff, simple, template, json, structured, dyff. When set to "template", use the env var HELM_DIFF_TPL to specify the template. (default "diff") --show-secrets do not redact secret values in the output --show-secrets-decoded decode secret values in the output + --storage-namespace string namespace where the helm release storage (Secret/ConfigMap) is located. Defaults to the target namespace (-n/--namespace) --strip-trailing-cr strip trailing carriage return on input --suppress stringArray allows suppression of the kinds listed in the diff output (can specify multiple, like '--suppress Deployment --suppress Service') --suppress-output-line-regex stringArray a regex to suppress diff output lines that match @@ -477,10 +498,13 @@ Flags: -D, --find-renames float32 Enable rename detection if set to any value greater than 0. If specified, the value denotes the maximum fraction of changed content as lines added + removed compared to total lines in a diff for considering it a rename. Only objects of the same Kind are attempted to be matched -h, --help help for rollback --include-tests enable the diffing of the helm test hooks + --kube-context string name of the kubeconfig context to use + -n, --namespace string namespace to assume the release to be installed into. Defaults to the current kube config namespace. --normalize-manifests normalize manifests before running diff to exclude style differences from the output --output string Possible values: diff, simple, template, json, structured, dyff. When set to "template", use the env var HELM_DIFF_TPL to specify the template. (default "diff") --show-secrets do not redact secret values in the output --show-secrets-decoded decode secret values in the output + --storage-namespace string namespace where the helm release storage (Secret/ConfigMap) is located. Defaults to the target namespace (-n/--namespace) --strip-trailing-cr strip trailing carriage return on input --suppress stringArray allows suppression of the kinds listed in the diff output (can specify multiple, like '--suppress Deployment --suppress Service') --suppress-output-line-regex stringArray a regex to suppress diff output lines that match @@ -517,6 +541,15 @@ To run all tests: go test -v ./... ``` +### Updating the flag tables in this README + +The per-command `Flags:` tables above are generated from the actual `--help` +output. After adding or changing a command flag, regenerate them with: +``` +make readme +``` +CI fails if the committed tables do not match the binary (`make verify-readme`). + ## Release Bump `version` in `plugin.yaml`: diff --git a/cmd/helm.go b/cmd/helm.go index 5a3f39df..2d7f36bf 100644 --- a/cmd/helm.go +++ b/cmd/helm.go @@ -428,8 +428,8 @@ func (d *diffCmd) writeExistingValues(f *os.File, all bool) error { if all { args = append(args, "--all") } - if d.namespace != "" { - args = append(args, "--namespace", d.namespace) + if storageNs := d.storage(); storageNs != "" { + args = append(args, "--namespace", storageNs) } if d.kubeContext != "" { args = append(args, "--kube-context", d.kubeContext) diff --git a/cmd/main_test.go b/cmd/main_test.go index 813a9b3c..c49bc6b9 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -3,8 +3,11 @@ package cmd import ( "fmt" "os" + "os/exec" "strings" "testing" + + "github.com/stretchr/testify/require" ) func shouldRunFakeHelm() bool { @@ -17,6 +20,28 @@ func shouldRunFakeHelm() bool { return !strings.HasPrefix(os.Args[1], "-test.") } +// printFakeHelmVersion prints helm version build info, so that the version +// checks in cmd (see getHelmVersion) work against the fake helm. +// The version output can be overridden via HELM_DIFF_FAKE_VERSION_OUTPUT. +func printFakeHelmVersion() { + if v := os.Getenv("HELM_DIFF_FAKE_VERSION_OUTPUT"); v != "" { + fmt.Print(v) + return + } + fmt.Println(`version.BuildInfo{Version:"v3.18.0"}`) +} + +// printFakeHelmOutput prints the output for a fake helm invocation. +// A `helm version` call prints helm version build info; any other +// invocation prints HELM_DIFF_FAKE_OUTPUT. +func printFakeHelmOutput() { + if len(os.Args) > 1 && os.Args[1] == "version" { + printFakeHelmVersion() + return + } + fmt.Print(os.Getenv("HELM_DIFF_FAKE_OUTPUT")) +} + func TestMain(m *testing.M) { if shouldRunFakeHelm() { mode := os.Getenv("HELM_DIFF_FAKE_HELM_MODE") @@ -51,16 +76,51 @@ func TestMain(m *testing.M) { case "capture_args": argsFile := os.Getenv("HELM_DIFF_FAKE_ARGS_FILE") if argsFile != "" { - if err := os.WriteFile(argsFile, []byte(strings.Join(os.Args[1:], " ")), 0644); err != nil { - fmt.Fprintf(os.Stderr, "failed to write fake helm args file %q: %v\n", argsFile, err) - os.Exit(1) + f, err := os.OpenFile(argsFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644) + if err == nil { + _, _ = fmt.Fprintln(f, strings.Join(os.Args[1:], " ")) + _ = f.Close() } } - fmt.Print(os.Getenv("HELM_DIFF_FAKE_OUTPUT")) + printFakeHelmOutput() default: - fmt.Print(os.Getenv("HELM_DIFF_FAKE_OUTPUT")) + printFakeHelmOutput() } os.Exit(0) } os.Exit(m.Run()) } + +func TestFakeHelmVersionOutput(t *testing.T) { + exe, err := os.Executable() + require.NoError(t, err) + + t.Run("custom version output via HELM_DIFF_FAKE_VERSION_OUTPUT", func(t *testing.T) { + t.Setenv("HELM_DIFF_FAKE_HELM", "1") + t.Setenv("HELM_DIFF_FAKE_HELM_MODE", "default") + t.Setenv("HELM_DIFF_FAKE_VERSION_OUTPUT", `version.BuildInfo{Version:"v3.99.0"}`) + + out, err := exec.Command(exe, "version").CombinedOutput() + require.NoError(t, err) + require.Equal(t, `version.BuildInfo{Version:"v3.99.0"}`, string(out)) + }) + + t.Run("default version build info", func(t *testing.T) { + t.Setenv("HELM_DIFF_FAKE_HELM", "1") + t.Setenv("HELM_DIFF_FAKE_HELM_MODE", "capture_args") + + out, err := exec.Command(exe, "version").CombinedOutput() + require.NoError(t, err) + require.Equal(t, "version.BuildInfo{Version:\"v3.18.0\"}\n", string(out)) + }) + + t.Run("non-version invocations print HELM_DIFF_FAKE_OUTPUT", func(t *testing.T) { + t.Setenv("HELM_DIFF_FAKE_HELM", "1") + t.Setenv("HELM_DIFF_FAKE_HELM_MODE", "default") + t.Setenv("HELM_DIFF_FAKE_OUTPUT", "manifest-output") + + out, err := exec.Command(exe, "get", "manifest").CombinedOutput() + require.NoError(t, err) + require.Equal(t, "manifest-output", string(out)) + }) +} diff --git a/cmd/namespaces.go b/cmd/namespaces.go new file mode 100644 index 00000000..fe5e8f20 --- /dev/null +++ b/cmd/namespaces.go @@ -0,0 +1,43 @@ +package cmd + +import ( + "os" + + "github.com/spf13/pflag" +) + +// namespaces groups the target namespace and the storage namespace of a release. +// +// The target namespace is where the release resources are (or would be) deployed +// and where chart templates are rendered into. The storage namespace is where the +// helm release storage (Secret/ConfigMap) is located. GitOps tools like the FluxCD +// HelmRelease controller persist release records in a storage namespace (e.g. +// flux-system) that differs from the target namespace of the workloads. +type namespaces struct { + namespace string // target namespace (-n/--namespace, HELM_NAMESPACE) + storageNamespace string // storage namespace (--storage-namespace, HELM_DIFF_STORAGE_NAMESPACE) +} + +// storage returns the namespace of the helm release storage, falling back to the +// target namespace when no storage namespace was configured. +func (n *namespaces) storage() string { + return resolveStorageNamespace(n.storageNamespace, n.namespace) +} + +// addNamespaceFlags registers the -n/--namespace and --storage-namespace flags on f, +// binding them to n. Both flags default to their respective environment variables +// (HELM_NAMESPACE and HELM_DIFF_STORAGE_NAMESPACE), so a flag passed on the command +// line (including an explicit empty string) always takes precedence over the +// environment. +func addNamespaceFlags(f *pflag.FlagSet, n *namespaces) { + f.StringVarP(&n.namespace, "namespace", "n", os.Getenv("HELM_NAMESPACE"), "namespace to assume the release to be installed into. Defaults to the current kube config namespace.") + f.StringVar(&n.storageNamespace, "storage-namespace", os.Getenv("HELM_DIFF_STORAGE_NAMESPACE"), "namespace where the helm release storage (Secret/ConfigMap) is located. Defaults to the target namespace (-n/--namespace)") +} + +// resolveStorageNamespace returns storageNamespace if non-empty, otherwise falls back to namespace. +func resolveStorageNamespace(storageNamespace, namespace string) string { + if storageNamespace != "" { + return storageNamespace + } + return namespace +} diff --git a/cmd/namespaces_test.go b/cmd/namespaces_test.go new file mode 100644 index 00000000..6367c1ea --- /dev/null +++ b/cmd/namespaces_test.go @@ -0,0 +1,133 @@ +package cmd + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" +) + +func TestNamespacesStorage(t *testing.T) { + cases := []struct { + name string + namespaces namespaces + expected string + }{ + { + name: "storage namespace defaults to target namespace when unset", + namespaces: namespaces{namespace: "target-ns"}, + expected: "target-ns", + }, + { + name: "storage namespace overrides target namespace when set", + namespaces: namespaces{namespace: "target-ns", storageNamespace: "flux-system"}, + expected: "flux-system", + }, + { + name: "both empty returns empty", + namespaces: namespaces{}, + expected: "", + }, + { + name: "storage namespace set with empty target namespace", + namespaces: namespaces{storageNamespace: "flux-system"}, + expected: "flux-system", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, tc.namespaces.storage()) + }) + } +} + +func TestResolveStorageNamespace(t *testing.T) { + cases := []struct { + name string + storageNamespace string + namespace string + expected string + }{ + { + name: "storage namespace set returns storage namespace", + storageNamespace: "flux-system", + namespace: "prod-apps", + expected: "flux-system", + }, + { + name: "storage namespace empty returns target namespace", + storageNamespace: "", + namespace: "prod-apps", + expected: "prod-apps", + }, + { + name: "both empty returns empty", + storageNamespace: "", + namespace: "", + expected: "", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, resolveStorageNamespace(tc.storageNamespace, tc.namespace)) + }) + } +} + +func TestStorageNamespaceEnvVarDefaults(t *testing.T) { + newCommands := map[string]func() *cobra.Command{ + "upgrade": newChartCommand, + "revision": revisionCmd, + "rollback": rollbackCmd, + } + + t.Run("env vars populate flag defaults", func(t *testing.T) { + t.Setenv("HELM_DIFF_STORAGE_NAMESPACE", "env-storage") + t.Setenv("HELM_NAMESPACE", "env-target") + + for name, newCmd := range newCommands { + t.Run(name, func(t *testing.T) { + cmd := newCmd() + storageNs, err := cmd.Flags().GetString("storage-namespace") + require.NoError(t, err) + require.Equal(t, "env-storage", storageNs) + + ns, err := cmd.Flags().GetString("namespace") + require.NoError(t, err) + require.Equal(t, "env-target", ns) + }) + } + }) + + t.Run("explicit flags take precedence over env var defaults", func(t *testing.T) { + t.Setenv("HELM_DIFF_STORAGE_NAMESPACE", "env-storage") + t.Setenv("HELM_NAMESPACE", "env-target") + + cmd := newChartCommand() + err := cmd.ParseFlags([]string{"--storage-namespace", "flag-storage", "-n", "flag-target"}) + require.NoError(t, err) + + storageNs, _ := cmd.Flags().GetString("storage-namespace") + ns, _ := cmd.Flags().GetString("namespace") + require.Equal(t, "flag-storage", storageNs) + require.Equal(t, "flag-target", ns) + }) + + t.Run("empty env vars leave flags empty", func(t *testing.T) { + t.Setenv("HELM_DIFF_STORAGE_NAMESPACE", "") + t.Setenv("HELM_NAMESPACE", "") + + for name, newCmd := range newCommands { + t.Run(name, func(t *testing.T) { + cmd := newCmd() + storageNs, _ := cmd.Flags().GetString("storage-namespace") + require.Empty(t, storageNs) + + ns, _ := cmd.Flags().GetString("namespace") + require.Empty(t, ns) + }) + } + }) +} diff --git a/cmd/revision.go b/cmd/revision.go index 081e6582..87a5adfb 100644 --- a/cmd/revision.go +++ b/cmd/revision.go @@ -14,6 +14,7 @@ import ( type revision struct { release string + namespaces // target namespace (-n/--namespace) and helm release storage namespace (--storage-namespace) kubeContext string detailedExitCode bool revisions []string @@ -68,6 +69,7 @@ func revisionCmd() *cobra.Command { }, } + addNamespaceFlags(revisionCmd.Flags(), &diff.namespaces) revisionCmd.Flags().BoolVar(&diff.detailedExitCode, "detailed-exitcode", false, "return a non-zero exit code when there are changes") revisionCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks") revisionCmd.Flags().BoolVar(&diff.normalizeManifests, "normalize-manifests", false, "normalize manifests before running diff to exclude style differences from the output") @@ -80,27 +82,28 @@ func revisionCmd() *cobra.Command { } func (d *revision) differentiateHelm3() error { - namespace := os.Getenv("HELM_NAMESPACE") + storageNs := d.storage() + targetNs := d.namespace excludes := []string{manifest.Helm3TestHook, manifest.Helm2TestSuccessHook} if d.includeTests { excludes = []string{} } switch len(d.revisions) { case 1: - releaseResponse, err := getRelease(d.release, 0, namespace, d.kubeContext) + releaseResponse, err := getRelease(d.release, 0, storageNs, d.kubeContext) if err != nil { return err } revision, _ := strconv.Atoi(d.revisions[0]) - revisionResponse, err := getRelease(d.release, revision, namespace, d.kubeContext) + revisionResponse, err := getRelease(d.release, revision, storageNs, d.kubeContext) if err != nil { return err } - oldSpecs := manifest.Parse(revisionResponse, namespace, d.normalizeManifests, excludes...) - newSpecs := manifest.Parse(releaseResponse, namespace, d.normalizeManifests, excludes...) + oldSpecs := manifest.Parse(revisionResponse, targetNs, d.normalizeManifests, excludes...) + newSpecs := manifest.Parse(releaseResponse, targetNs, d.normalizeManifests, excludes...) revisionResponse = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation releaseResponse = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation @@ -117,18 +120,18 @@ func (d *revision) differentiateHelm3() error { revision1, revision2 = revision2, revision1 } - revisionResponse1, err := getRelease(d.release, revision1, namespace, d.kubeContext) + revisionResponse1, err := getRelease(d.release, revision1, storageNs, d.kubeContext) if err != nil { return err } - revisionResponse2, err := getRelease(d.release, revision2, namespace, d.kubeContext) + revisionResponse2, err := getRelease(d.release, revision2, storageNs, d.kubeContext) if err != nil { return err } - oldSpecs := manifest.Parse(revisionResponse1, namespace, d.normalizeManifests, excludes...) - newSpecs := manifest.Parse(revisionResponse2, namespace, d.normalizeManifests, excludes...) + oldSpecs := manifest.Parse(revisionResponse1, targetNs, d.normalizeManifests, excludes...) + newSpecs := manifest.Parse(revisionResponse2, targetNs, d.normalizeManifests, excludes...) revisionResponse1 = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation revisionResponse2 = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation diff --git a/cmd/revision_test.go b/cmd/revision_test.go new file mode 100644 index 00000000..0962c871 --- /dev/null +++ b/cmd/revision_test.go @@ -0,0 +1,125 @@ +package cmd + +import ( + "os" + "strings" + "testing" +) + +func TestRevisionCommand_StorageNamespaceFlag(t *testing.T) { + cmd := revisionCmd() + f := cmd.Flags() + + if f.Lookup("storage-namespace") == nil { + t.Fatal("expected flag --storage-namespace to be registered on revisionCmd") + } + + if f.Lookup("namespace") == nil { + t.Fatal("expected flag --namespace to be registered on revisionCmd") + } + + if f.ShorthandLookup("n") == nil { + t.Fatal("expected shorthand flag -n to be registered on revisionCmd") + } + + err := cmd.ParseFlags([]string{"--storage-namespace", "flux-system", "-n", "prod-apps"}) + if err != nil { + t.Fatalf("unexpected error parsing flags: %v", err) + } + + storageNs, err := cmd.Flags().GetString("storage-namespace") + if err != nil || storageNs != "flux-system" { + t.Errorf("expected storage-namespace=flux-system, got %q (err: %v)", storageNs, err) + } + + ns, err := cmd.Flags().GetString("namespace") + if err != nil || ns != "prod-apps" { + t.Errorf("expected namespace=prod-apps, got %q (err: %v)", ns, err) + } +} + +func TestRevisionCommand_Execution_StorageNamespace(t *testing.T) { + manifestYAML := `--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-config + namespace: prod-apps +data: + key: value +` + + t.Run("explicit flag passes storage namespace to helm get", func(t *testing.T) { + argsFile := t.TempDir() + "/args" + setupFakeHelm(t, "capture_args", manifestYAML, argsFile, "") + + cmd := revisionCmd() + cmd.SetArgs([]string{"my-release", "1", "2", "--storage-namespace", "flux-system", "-n", "prod-apps"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error executing revision command: %v", err) + } + + data, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("failed to read fake helm args: %v", err) + } + argsContent := string(data) + + if !strings.Contains(argsContent, "get manifest my-release --revision 1 --namespace flux-system") { + t.Errorf("expected 'helm get manifest' for revision 1 to use --namespace flux-system, got:\n%s", argsContent) + } + if !strings.Contains(argsContent, "get manifest my-release --revision 2 --namespace flux-system") { + t.Errorf("expected 'helm get manifest' for revision 2 to use --namespace flux-system, got:\n%s", argsContent) + } + }) + + t.Run("env var sets storage namespace when flag is omitted", func(t *testing.T) { + argsFile := t.TempDir() + "/args" + setupFakeHelm(t, "capture_args", manifestYAML, argsFile, "") + t.Setenv("HELM_DIFF_STORAGE_NAMESPACE", "flux-system-env") + + cmd := revisionCmd() + cmd.SetArgs([]string{"my-release", "1", "2", "-n", "prod-apps"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error executing revision command: %v", err) + } + + data, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("failed to read fake helm args: %v", err) + } + argsContent := string(data) + + if !strings.Contains(argsContent, "get manifest my-release --revision 1 --namespace flux-system-env") { + t.Errorf("expected 'helm get manifest' to use env var --namespace flux-system-env, got:\n%s", argsContent) + } + }) + + t.Run("defaults to target namespace when storage namespace is omitted", func(t *testing.T) { + argsFile := t.TempDir() + "/args" + setupFakeHelm(t, "capture_args", manifestYAML, argsFile, "") + t.Setenv("HELM_DIFF_STORAGE_NAMESPACE", "") + + cmd := revisionCmd() + cmd.SetArgs([]string{"my-release", "1", "2", "-n", "prod-apps"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error executing revision command: %v", err) + } + + data, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("failed to read fake helm args: %v", err) + } + argsContent := string(data) + + if !strings.Contains(argsContent, "get manifest my-release --revision 1 --namespace prod-apps") { + t.Errorf("expected 'helm get manifest' to fall back to target namespace --namespace prod-apps, got:\n%s", argsContent) + } + }) +} diff --git a/cmd/rollback.go b/cmd/rollback.go index b390b125..8d6aae64 100644 --- a/cmd/rollback.go +++ b/cmd/rollback.go @@ -14,6 +14,7 @@ import ( type rollback struct { release string + namespaces // target namespace (-n/--namespace) and helm release storage namespace (--storage-namespace) kubeContext string detailedExitCode bool revisions []string @@ -58,6 +59,7 @@ func rollbackCmd() *cobra.Command { }, } + addNamespaceFlags(rollbackCmd.Flags(), &diff.namespaces) rollbackCmd.Flags().BoolVar(&diff.detailedExitCode, "detailed-exitcode", false, "return a non-zero exit code when there are changes") rollbackCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks") rollbackCmd.Flags().BoolVar(&diff.normalizeManifests, "normalize-manifests", false, "normalize manifests before running diff to exclude style differences from the output") @@ -70,13 +72,14 @@ func rollbackCmd() *cobra.Command { } func (d *rollback) backcastHelm3() error { - namespace := os.Getenv("HELM_NAMESPACE") + storageNs := d.storage() + targetNs := d.namespace excludes := []string{manifest.Helm3TestHook, manifest.Helm2TestSuccessHook} if d.includeTests { excludes = []string{} } // get manifest of the latest release - releaseResponse, err := getRelease(d.release, 0, namespace, d.kubeContext) + releaseResponse, err := getRelease(d.release, 0, storageNs, d.kubeContext) if err != nil { return err @@ -84,14 +87,14 @@ func (d *rollback) backcastHelm3() error { // get manifest of the release to rollback revision, _ := strconv.Atoi(d.revisions[0]) - revisionResponse, err := getRelease(d.release, revision, namespace, d.kubeContext) + revisionResponse, err := getRelease(d.release, revision, storageNs, d.kubeContext) if err != nil { return err } // create a diff between the current manifest and the version of the manifest that a user is intended to rollback - oldSpecs := manifest.Parse(releaseResponse, namespace, d.normalizeManifests, excludes...) - newSpecs := manifest.Parse(revisionResponse, namespace, d.normalizeManifests, excludes...) + oldSpecs := manifest.Parse(releaseResponse, targetNs, d.normalizeManifests, excludes...) + newSpecs := manifest.Parse(revisionResponse, targetNs, d.normalizeManifests, excludes...) releaseResponse = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation revisionResponse = nil //nolint:ineffassign // nil to allow GC to reclaim raw bytes before diff computation diff --git a/cmd/rollback_test.go b/cmd/rollback_test.go new file mode 100644 index 00000000..89d45abf --- /dev/null +++ b/cmd/rollback_test.go @@ -0,0 +1,125 @@ +package cmd + +import ( + "os" + "strings" + "testing" +) + +func TestRollbackCommand_StorageNamespaceFlag(t *testing.T) { + cmd := rollbackCmd() + f := cmd.Flags() + + if f.Lookup("storage-namespace") == nil { + t.Fatal("expected flag --storage-namespace to be registered on rollbackCmd") + } + + if f.Lookup("namespace") == nil { + t.Fatal("expected flag --namespace to be registered on rollbackCmd") + } + + if f.ShorthandLookup("n") == nil { + t.Fatal("expected shorthand flag -n to be registered on rollbackCmd") + } + + err := cmd.ParseFlags([]string{"--storage-namespace", "flux-system", "-n", "prod-apps"}) + if err != nil { + t.Fatalf("unexpected error parsing flags: %v", err) + } + + storageNs, err := cmd.Flags().GetString("storage-namespace") + if err != nil || storageNs != "flux-system" { + t.Errorf("expected storage-namespace=flux-system, got %q (err: %v)", storageNs, err) + } + + ns, err := cmd.Flags().GetString("namespace") + if err != nil || ns != "prod-apps" { + t.Errorf("expected namespace=prod-apps, got %q (err: %v)", ns, err) + } +} + +func TestRollbackCommand_Execution_StorageNamespace(t *testing.T) { + manifestYAML := `--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-config + namespace: prod-apps +data: + key: value +` + + t.Run("explicit flag passes storage namespace to helm get", func(t *testing.T) { + argsFile := t.TempDir() + "/args" + setupFakeHelm(t, "capture_args", manifestYAML, argsFile, "") + + cmd := rollbackCmd() + cmd.SetArgs([]string{"my-release", "2", "--storage-namespace", "flux-system", "-n", "prod-apps"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error executing rollback command: %v", err) + } + + data, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("failed to read fake helm args: %v", err) + } + argsContent := string(data) + + if !strings.Contains(argsContent, "get manifest my-release --namespace flux-system") { + t.Errorf("expected 'helm get manifest' for latest release to use --namespace flux-system, got:\n%s", argsContent) + } + if !strings.Contains(argsContent, "get manifest my-release --revision 2 --namespace flux-system") { + t.Errorf("expected 'helm get manifest' for revision 2 to use --namespace flux-system, got:\n%s", argsContent) + } + }) + + t.Run("env var sets storage namespace when flag is omitted", func(t *testing.T) { + argsFile := t.TempDir() + "/args" + setupFakeHelm(t, "capture_args", manifestYAML, argsFile, "") + t.Setenv("HELM_DIFF_STORAGE_NAMESPACE", "flux-system-env") + + cmd := rollbackCmd() + cmd.SetArgs([]string{"my-release", "2", "-n", "prod-apps"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error executing rollback command: %v", err) + } + + data, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("failed to read fake helm args: %v", err) + } + argsContent := string(data) + + if !strings.Contains(argsContent, "get manifest my-release --namespace flux-system-env") { + t.Errorf("expected 'helm get manifest' to use env var --namespace flux-system-env, got:\n%s", argsContent) + } + }) + + t.Run("defaults to target namespace when storage namespace is omitted", func(t *testing.T) { + argsFile := t.TempDir() + "/args" + setupFakeHelm(t, "capture_args", manifestYAML, argsFile, "") + t.Setenv("HELM_DIFF_STORAGE_NAMESPACE", "") + + cmd := rollbackCmd() + cmd.SetArgs([]string{"my-release", "2", "-n", "prod-apps"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error executing rollback command: %v", err) + } + + data, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("failed to read fake helm args: %v", err) + } + argsContent := string(data) + + if !strings.Contains(argsContent, "get manifest my-release --namespace prod-apps") { + t.Errorf("expected 'helm get manifest' to fall back to target namespace --namespace prod-apps, got:\n%s", argsContent) + } + }) +} diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 63c8ebdb..e47e1933 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -47,7 +47,7 @@ type diffCmd struct { disableOpenAPIValidation bool enableDNS bool SkipSchemaValidation bool - namespace string // namespace to assume the release to be installed into. Defaults to the current kube config namespace. + namespaces // target namespace (-n/--namespace) and helm release storage namespace (--storage-namespace) valueFiles valueFiles values []string stringValues []string @@ -136,9 +136,7 @@ perform. ` func newChartCommand() *cobra.Command { - diff := diffCmd{ - namespace: os.Getenv("HELM_NAMESPACE"), - } + diff := diffCmd{} unknownFlags := os.Getenv("HELM_DIFF_IGNORE_UNKNOWN_FLAGS") == envTrue cmd := &cobra.Command{ @@ -241,6 +239,7 @@ func newChartCommand() *cobra.Command { f := cmd.Flags() var kubeconfig string f.StringVar(&kubeconfig, "kubeconfig", "", "This flag is ignored, to allow passing of this top level flag to helm") + addNamespaceFlags(f, &diff.namespaces) f.BoolVar(&diff.threeWayMerge, "three-way-merge", false, "use three-way-merge to compute patch and generate diff output") f.StringVar(&diff.kubeContext, "kube-context", "", "name of the kubeconfig context to use") f.StringVar(&diff.chartVersion, "version", "", "specify the exact chart version to use. If this is not specified, the latest version is used") @@ -303,13 +302,13 @@ func (d *diffCmd) runHelm3() error { } if d.clusterAccessAllowed() { - releaseManifest, err = getRelease(d.release, d.revision, d.namespace, d.kubeContext) + releaseManifest, err = getRelease(d.release, d.revision, d.storage(), d.kubeContext) } var newInstall bool if err != nil && strings.Contains(err.Error(), "release: not found") { if d.revision > 0 { - return fmt.Errorf("Failed to get revision %d of release %s in namespace %s: %w", d.revision, d.release, d.namespace, err) + return fmt.Errorf("Failed to get revision %d of release %s in namespace %s: %w", d.revision, d.release, d.storage(), err) } if d.isAllowUnreleased() { newInstall = true @@ -320,7 +319,7 @@ func (d *diffCmd) runHelm3() error { } } if err != nil { - return fmt.Errorf("Failed to get release %s in namespace %s: %w", d.release, d.namespace, err) + return fmt.Errorf("Failed to get release %s in namespace %s: %w", d.release, d.storage(), err) } installManifest, err := d.template(!newInstall) @@ -332,7 +331,11 @@ func (d *diffCmd) runHelm3() error { if d.threeWayMerge || d.takeOwnership { actionConfig = new(action.Configuration) localEnv := prepareEnvSettings(d.kubeContext) - if err := actionConfig.Init(localEnv.RESTClientGetter(), localEnv.Namespace(), os.Getenv("HELM_DRIVER")); err != nil { + storageNs := d.storage() + if storageNs == "" { + storageNs = localEnv.Namespace() + } + if err := actionConfig.Init(localEnv.RESTClientGetter(), storageNs, os.Getenv("HELM_DRIVER")); err != nil { log.Fatalf("%+v", err) } if err := actionConfig.KubeClient.IsReachable(); err != nil { @@ -350,7 +353,7 @@ func (d *diffCmd) runHelm3() error { currentSpecs := make(map[string]*manifest.MappingResult) if !newInstall && d.clusterAccessAllowed() { if !d.noHooks && !d.threeWayMerge { - hooks, err := getHooks(d.release, d.revision, d.namespace, d.kubeContext) + hooks, err := getHooks(d.release, d.revision, d.storage(), d.kubeContext) if err != nil { return err } diff --git a/cmd/upgrade_test.go b/cmd/upgrade_test.go index 60744a54..900cc375 100644 --- a/cmd/upgrade_test.go +++ b/cmd/upgrade_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "slices" + "strings" "testing" ) @@ -233,3 +234,136 @@ func TestValidateRevision(t *testing.T) { }) } } + +func TestUpgradeCommand_StorageNamespaceFlag(t *testing.T) { + cmd := newChartCommand() + f := cmd.Flags() + + if f.Lookup("storage-namespace") == nil { + t.Fatal("expected flag --storage-namespace to be registered") + } + + if f.Lookup("namespace") == nil { + t.Fatal("expected flag --namespace to be registered") + } + + if f.ShorthandLookup("n") == nil { + t.Fatal("expected shorthand flag -n to be registered") + } + + err := cmd.ParseFlags([]string{"--storage-namespace", "flux-system", "-n", "prod-apps"}) + if err != nil { + t.Fatalf("unexpected error parsing flags: %v", err) + } + + storageNs, err := cmd.Flags().GetString("storage-namespace") + if err != nil || storageNs != "flux-system" { + t.Errorf("expected storage-namespace=flux-system, got %q (err: %v)", storageNs, err) + } + + ns, err := cmd.Flags().GetString("namespace") + if err != nil || ns != "prod-apps" { + t.Errorf("expected namespace=prod-apps, got %q (err: %v)", ns, err) + } +} + +func TestUpgradeCommand_Execution_StorageNamespace(t *testing.T) { + manifestYAML := `--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-config + namespace: prod-apps +data: + key: value +` + + t.Run("explicit flag separates storage and target namespace", func(t *testing.T) { + argsFile := t.TempDir() + "/args" + setupFakeHelm(t, "capture_args", manifestYAML, argsFile, "") + + chartDir := t.TempDir() + cmd := newChartCommand() + cmd.SetArgs([]string{"my-release", chartDir, "--storage-namespace", "flux-system", "-n", "prod-apps"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error executing upgrade command: %v", err) + } + + data, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("failed to read fake helm args: %v", err) + } + argsContent := string(data) + + // get manifest should use storage namespace + if !strings.Contains(argsContent, "get manifest my-release --namespace flux-system") { + t.Errorf("expected 'helm get manifest' to use --namespace flux-system, got:\n%s", argsContent) + } + // template should use target namespace + if !strings.Contains(argsContent, "template my-release "+chartDir+" --namespace prod-apps") { + t.Errorf("expected 'helm template' to use --namespace prod-apps, got:\n%s", argsContent) + } + }) + + t.Run("env var sets storage namespace when flag is omitted", func(t *testing.T) { + argsFile := t.TempDir() + "/args" + setupFakeHelm(t, "capture_args", manifestYAML, argsFile, "") + t.Setenv("HELM_DIFF_STORAGE_NAMESPACE", "flux-system-env") + + chartDir := t.TempDir() + cmd := newChartCommand() + cmd.SetArgs([]string{"my-release", chartDir, "-n", "prod-apps"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error executing upgrade command: %v", err) + } + + data, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("failed to read fake helm args: %v", err) + } + argsContent := string(data) + + // get manifest should use storage namespace from env var + if !strings.Contains(argsContent, "get manifest my-release --namespace flux-system-env") { + t.Errorf("expected 'helm get manifest' to use --namespace flux-system-env, got:\n%s", argsContent) + } + // template should use target namespace + if !strings.Contains(argsContent, "template my-release "+chartDir+" --namespace prod-apps") { + t.Errorf("expected 'helm template' to use --namespace prod-apps, got:\n%s", argsContent) + } + }) + + t.Run("defaults to target namespace when storage namespace is omitted", func(t *testing.T) { + argsFile := t.TempDir() + "/args" + setupFakeHelm(t, "capture_args", manifestYAML, argsFile, "") + t.Setenv("HELM_DIFF_STORAGE_NAMESPACE", "") + + chartDir := t.TempDir() + cmd := newChartCommand() + cmd.SetArgs([]string{"my-release", chartDir, "-n", "prod-apps"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error executing upgrade command: %v", err) + } + + data, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("failed to read fake helm args: %v", err) + } + argsContent := string(data) + + // get manifest should use target namespace as fallback + if !strings.Contains(argsContent, "get manifest my-release --namespace prod-apps") { + t.Errorf("expected 'helm get manifest' to fall back to --namespace prod-apps, got:\n%s", argsContent) + } + // template should use target namespace + if !strings.Contains(argsContent, "template my-release "+chartDir+" --namespace prod-apps") { + t.Errorf("expected 'helm template' to use --namespace prod-apps, got:\n%s", argsContent) + } + }) +} diff --git a/scripts/gen-readme.sh b/scripts/gen-readme.sh new file mode 100755 index 00000000..3b09aeca --- /dev/null +++ b/scripts/gen-readme.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# Regenerates the cobra flag tables in README.md from the --help output of the +# diff binary, so the documented flags cannot drift from the actual flags. +# +# Usage: scripts/gen-readme.sh [path-to-diff-binary] (default: bin/diff) +# +# The README contains one flag table per command, in the order listed in +# COMMANDS below. Only the contiguous block of indented flag rows that follows +# each "Flags:" line is rewritten; any surrounding prose is left untouched. +# The script fails if the number of "Flags:" tables found does not match the +# number of commands, so a lost or accidentally added table cannot slip through. +set -euo pipefail + +BIN="${1:-bin/diff}" + +if [ ! -x "${BIN}" ]; then + echo "diff binary not found or not executable at ${BIN}. Run 'make build' first." >&2 + exit 1 +fi + +# The README documents the flags of these commands, in this order. +# The first table belongs to the (deprecated) root command, which carries the +# same flag set as "upgrade". +COMMANDS=("" local upgrade release revision rollback) + +WORKDIR="$(mktemp -d)" +trap 'rm -rf "${WORKDIR}"' EXIT + +# extract_flags prints the indented flag rows of the "Flags:" section of the +# given subcommand's --help output. An empty subcommand selects the root +# command. HELM_NAMESPACE and HELM_DIFF_STORAGE_NAMESPACE are unset so that +# no environment default leaks into the rendered flag defaults. +extract_flags() { + local subcmd="$1" + local args=() + if [ -n "${subcmd}" ]; then + args+=("${subcmd}") + fi + env -u HELM_NAMESPACE -u HELM_DIFF_STORAGE_NAMESPACE "${BIN}" ${args[@]+"${args[@]}"} --help | awk ' + /^Flags:$/ { in_flags = 1; next } + in_flags && /^ / { print; next } + in_flags { exit } + ' +} + +i=0 +for subcmd in "${COMMANDS[@]}"; do + if ! extract_flags "${subcmd}" > "${WORKDIR}/flags_${i}.txt" || [ ! -s "${WORKDIR}/flags_${i}.txt" ]; then + echo "failed to extract flag table for command '${subcmd:-}'" >&2 + exit 1 + fi + i=$((i + 1)) +done + +README="${README:-README.md}" + +awk -v workdir="${WORKDIR}" -v n="${#COMMANDS[@]}" -v readme="${README}" ' + function load_rows(i, line) { + rows[i] = "" + while ((getline line < (workdir "/flags_" i ".txt")) > 0) + rows[i] = rows[i] line "\n" + close(workdir "/flags_" i ".txt") + } + BEGIN { + for (i = 0; i < n; i++) load_rows(i) + } + /^Flags:$/ { total++ } + /^Flags:$/ && idx < n { + print + idx++ + printf "%s", rows[idx - 1] + in_rows = 1 + next + } + in_rows && /^ / { next } + { in_rows = 0; print } + END { + if (total != n || idx != n) { + printf "expected %d flag tables in %s, found %d\n", n, readme, total > "/dev/stderr" + exit 1 + } + } +' "${README}" > "${WORKDIR}/README.new" + +mv "${WORKDIR}/README.new" "${README}" +echo "Regenerated ${#COMMANDS[@]} flag tables in ${README}"