fix(bundler): argocd-helm repoURL UX — surface key, emit install hint

pkg/bundler/deployer/argocdhelm/argocdhelm.go

@@ -90,6 +90,7 @@ import (
 	"github.com/NVIDIA/aicr/pkg/component"
 	"github.com/NVIDIA/aicr/pkg/errors"
 	"github.com/NVIDIA/aicr/pkg/recipe"
+	"github.com/NVIDIA/aicr/pkg/serializer"
 )
 
 // yamlStringTag is the YAML resolved-tag for explicit scalar strings, used
@@ -122,6 +123,16 @@ const DefaultAppName = "aicr-stack"
 // root values.yaml, and any documentation that references it.
 const rootValuesAppNameKey = "appName"
 
+// rootValuesRepoURLKey and rootValuesTargetRevisionKey are surfaced in
+// the root values.yaml with empty defaults so `helm show values`
+// documents them. The parent App template's {{ required }} directive
+// still enforces non-empty at render time — empty strings here only
+// affect documentation surface, not runtime behavior.
+const (
+	rootValuesRepoURLKey        = "repoURL"
+	rootValuesTargetRevisionKey = "targetRevision"
+)
+
 // compile-time interface check
 var _ deployer.Deployer = (*Generator)(nil)
 
@@ -276,7 +287,14 @@ func (g *Generator) Generate(ctx context.Context, outputDir string) (*deployer.O
 		dynamicOnlyValues[rootValuesAppNameKey] = g.AppName
 	}
 
-	valuesPath, valuesSize, err := deployer.WriteValuesFile(dynamicOnlyValues, outputDir, "values.yaml")
+	// Surface repoURL and targetRevision in values.yaml so `helm show
+	// values` documents the install-time inputs. Empty defaults — the
+	// parent App template's {{ required }} directive still enforces
+	// non-empty at render time. See issue #1020.
+	dynamicOnlyValues[rootValuesRepoURLKey] = ""
+	dynamicOnlyValues[rootValuesTargetRevisionKey] = ""
+
+	valuesPath, valuesSize, err := writeRootValuesFile(dynamicOnlyValues, outputDir)
 	if err != nil {
 		return nil, errors.Wrap(errors.ErrCodeInternal, "failed to write root values.yaml", err)
 	}
@@ -428,6 +446,58 @@ func (g *Generator) writeStaticValuesAndBuildStubs(outputDir string) ([]string,
 	return files, totalSize, dynamicOnlyValues, nil
 }
 
+// rootValuesInstallHeader documents the install-time inputs surfaced
+// in the bundle's root values.yaml. The empty defaults below let
+// `helm show values` list the keys; the parent App template's
+// {{ required }} directive enforces non-empty at render time. See
+// issue #1020.
+const rootValuesInstallHeader = `# Generated by AICR
+#
+# Install-time inputs (required for argocd-helm bundles):
+#
+#   repoURL          Parent OCI namespace or Helm chart repo URL — WITHOUT
+#                    the chart name. The parent App template appends
+#                    .Chart.Name itself to assemble the full reference.
+#                    Example: --set repoURL=oci://ghcr.io/nvidia
+#
+#   targetRevision   Chart version / OCI artifact tag.
+#                    Defaults to .Chart.Version when unset at install time.
+#                    Example: --set targetRevision=v1.0.0
+#
+---
+`
+
+// writeRootValuesFile writes the bundle's root values.yaml with a
+// documentation header that explains the required install-time inputs.
+// Parallels deployer.WriteValuesFile, with a richer header tailored to
+// the argocd-helm contract.
+func writeRootValuesFile(values map[string]any, outputDir string) (string, int64, error) {
+	outputPath, err := deployer.SafeJoin(outputDir, "values.yaml")
+	if err != nil {
+		return "", 0, err
+	}
+
+	var buf strings.Builder
+	buf.WriteString(rootValuesInstallHeader)
+
+	if len(values) > 0 {
+		// Deterministic marshal so values.yaml feeding checksums.txt
+		// (and the bundle attestation) is byte-stable across runs.
+		yamlBytes, err := serializer.MarshalYAMLDeterministic(values)
+		if err != nil {
+			return "", 0, errors.PropagateOrWrap(err, errors.ErrCodeInternal, "failed to marshal values")
+		}
+		buf.Write(yamlBytes)
+	}
+
+	content := buf.String()
+	if err := os.WriteFile(outputPath, []byte(content), 0600); err != nil {
+		return "", 0, errors.Wrap(errors.ErrCodeInternal, "failed to write values file", err)
+	}
+
+	return outputPath, int64(len(content)), nil
+}
+
 // parentAppTemplate is the Helm template body for the parent Argo
 // Application. It lives inside the chart's templates/ directory so that
 // the user supplies repoURL and targetRevision at install time via

pkg/bundler/deployer/argocdhelm/testdata/helm_and_manifest_only/values.yaml

@@ -1,4 +1,18 @@
 # Generated by AICR
+#
+# Install-time inputs (required for argocd-helm bundles):
+#
+#   repoURL          Parent OCI namespace or Helm chart repo URL — WITHOUT
+#                    the chart name. The parent App template appends
+#                    .Chart.Name itself to assemble the full reference.
+#                    Example: --set repoURL=oci://ghcr.io/nvidia
+#
+#   targetRevision   Chart version / OCI artifact tag.
+#                    Defaults to .Chart.Version when unset at install time.
+#                    Example: --set targetRevision=v1.0.0
+#
 ---
 certmanager:
   replicaCount: 1
+repoURL: ""
+targetRevision: ""

pkg/bundler/deployer/argocdhelm/testdata/mixed_component/values.yaml

@@ -1,5 +1,19 @@
 # Generated by AICR
+#
+# Install-time inputs (required for argocd-helm bundles):
+#
+#   repoURL          Parent OCI namespace or Helm chart repo URL — WITHOUT
+#                    the chart name. The parent App template appends
+#                    .Chart.Name itself to assemble the full reference.
+#                    Example: --set repoURL=oci://ghcr.io/nvidia
+#
+#   targetRevision   Chart version / OCI artifact tag.
+#                    Defaults to .Chart.Version when unset at install time.
+#                    Example: --set targetRevision=v1.0.0
+#
 ---
 gpuoperator:
   driver:
     version: "580"
+repoURL: ""
+targetRevision: ""

pkg/cli/bundle.go

@@ -20,6 +20,7 @@ import (
 	"io"
 	"log/slog"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -711,6 +712,15 @@ func runBundleCmd(ctx context.Context, cmd *cli.Command) error {
 		if err := pushOCIBundle(ctx, opts, out); err != nil {
 			return err
 		}
+		// argocd-helm is the only deployer that publishes a Helm chart
+		// artifact consumers run `helm install` against; for everyone
+		// else the OCI artifact is a generic AICR bundle with its own
+		// distribution path. Surface the canonical install line so
+		// users don't have to derive it from the registry URL + the
+		// chart's {{ required }} error. See issue #1020.
+		if opts.deployer == config.DeployerArgoCDHelm {
+			printArgoCDHelmOCIInstructions(cmd.Root().Writer, opts.ociRef)
+		}
 	}
 
 	return nil
@@ -823,3 +833,42 @@ func printDeploymentInstructions(w io.Writer, out *result.Output) {
 		}
 	}
 }
+
+// printArgoCDHelmOCIInstructions emits the canonical `helm install` line
+// for an argocd-helm bundle pushed to OCI. The post-#1051 contract is:
+//
+//   - `helm install` targets the full OCI artifact reference
+//     (oci://<registry>/<repo>:<tag>, including the chart name).
+//   - `--set repoURL` carries the PARENT namespace only — the chart's
+//     parent App template appends .Chart.Name itself to assemble the
+//     final reference, so the chart name must NOT be included in
+//     `repoURL`.
+//
+// Skips silently for non-OCI references; the OCI-mode call site in
+// runBundleCmd already gates on opts.ociRef != nil, this guard is
+// defense-in-depth for callers that may construct ociRef differently.
+func printArgoCDHelmOCIInstructions(w io.Writer, ref *oci.Reference) {
+	if ref == nil || !ref.IsOCI {
+		return
+	}
+
+	chartRef := fmt.Sprintf("%s%s/%s:%s", oci.URIScheme, ref.Registry, ref.Repository, ref.Tag)
+
+	// Parent namespace: the registry + repository path with the chart
+	// (last) segment stripped. path.Dir returns "." for a single-segment
+	// repo (e.g., "aicr-bundle"); in that case the parent is just the
+	// registry, with no path component.
+	parentPath := path.Dir(ref.Repository)
+	var repoURL string
+	if parentPath == "." || parentPath == "/" {
+		repoURL = oci.URIScheme + ref.Registry
+	} else {
+		repoURL = oci.URIScheme + ref.Registry + "/" + parentPath
+	}
+
+	fmt.Fprintf(w, "\nargocd-helm bundle pushed: %s\n", chartRef)
+	fmt.Fprintln(w, "\nTo install:")
+	fmt.Fprintf(w, "  helm install <release> %s \\\n", chartRef)
+	fmt.Fprintln(w, "    --namespace argocd \\")
+	fmt.Fprintf(w, "    --set repoURL=%s\n", repoURL)
+}

pkg/cli/bundle_test.go

@@ -15,13 +15,15 @@
 package cli
 
 import (
+	"bytes"
 	"context"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
 	"github.com/NVIDIA/aicr/pkg/bundler/attestation"
+	"github.com/NVIDIA/aicr/pkg/oci"
 )
 
 // TestParseOutputTarget is now in pkg/oci/reference_test.go
@@ -311,3 +313,111 @@ func TestParseBundleCmdOptions_AppName(t *testing.T) {
 		}
 	})
 }
+
+// TestPrintArgoCDHelmOCIInstructions exercises the post-#1051 install-hint
+// contract: `helm install` against the full OCI artifact reference, and
+// `--set repoURL` carrying only the parent namespace (chart name omitted
+// — the chart template appends .Chart.Name itself). See issue #1020.
+func TestPrintArgoCDHelmOCIInstructions(t *testing.T) {
+	tests := []struct {
+		name        string
+		ref         *oci.Reference
+		wantContain []string
+		wantSkip    bool // true means no output expected
+	}{
+		{
+			name: "registry with nested namespace",
+			ref: &oci.Reference{
+				IsOCI:      true,
+				Registry:   "ghcr.io",
+				Repository: "nvidia/aicr-bundle",
+				Tag:        "v1.0.0",
+			},
+			wantContain: []string{
+				"oci://ghcr.io/nvidia/aicr-bundle:v1.0.0",
+				"--set repoURL=oci://ghcr.io/nvidia",
+				"--namespace argocd",
+			},
+		},
+		{
+			name: "deeply nested namespace",
+			ref: &oci.Reference{
+				IsOCI:      true,
+				Registry:   "registry.example.com",
+				Repository: "team/platform/aicr-bundle",
+				Tag:        "0.42.0",
+			},
+			wantContain: []string{
+				"oci://registry.example.com/team/platform/aicr-bundle:0.42.0",
+				"--set repoURL=oci://registry.example.com/team/platform",
+			},
+		},
+		{
+			name: "single-segment repo: parent collapses to registry-only",
+			ref: &oci.Reference{
+				IsOCI:      true,
+				Registry:   "localhost:5000",
+				Repository: "aicr-bundle",
+				Tag:        "dev",
+			},
+			wantContain: []string{
+				"oci://localhost:5000/aicr-bundle:dev",
+				"--set repoURL=oci://localhost:5000",
+			},
+		},
+		{
+			name:     "nil ref skips silently",
+			ref:      nil,
+			wantSkip: true,
+		},
+		{
+			name: "non-OCI ref skips silently",
+			ref: &oci.Reference{
+				IsOCI:     false,
+				LocalPath: "./bundle",
+			},
+			wantSkip: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var buf bytes.Buffer
+			printArgoCDHelmOCIInstructions(&buf, tt.ref)
+			got := buf.String()
+
+			if tt.wantSkip {
+				if got != "" {
+					t.Errorf("expected no output, got: %q", got)
+				}
+				return
+			}
+			for _, want := range tt.wantContain {
+				if !strings.Contains(got, want) {
+					t.Errorf("output missing %q\nfull output:\n%s", want, got)
+				}
+			}
+			// Sanity check: the repoURL line must NOT include the chart
+			// name segment — the chart template appends .Chart.Name itself,
+			// and a chart-name-bearing repoURL would cause double-append at
+			// render time.
+			if tt.ref != nil && tt.ref.IsOCI {
+				chartName := tt.ref.ChartName()
+				if chartName != "" {
+					// The repoURL line is the last one; isolate it to avoid
+					// false positives from the chartRef line above.
+					var repoURLLine string
+					for line := range strings.SplitSeq(got, "\n") {
+						if strings.Contains(line, "--set repoURL=") {
+							repoURLLine = line
+							break
+						}
+					}
+					if strings.Contains(repoURLLine, "/"+chartName) {
+						t.Errorf("repoURL must not include chart name %q; got line: %q", chartName, repoURLLine)
+					}
+				}
+			}
+		})
+	}
+}