diff --git a/docs/user/bundling.md b/docs/user/bundling.md index 1b69d0454..15259ea21 100644 --- a/docs/user/bundling.md +++ b/docs/user/bundling.md @@ -22,7 +22,7 @@ re-render the same recipe for whatever pipeline you run: | `helm` (default) | Per-component Helm values + a `deploy.sh` that installs in dependency order. | | `helmfile` | A `helmfile.yaml` release graph. | | `argocd` | Argo CD `Application` manifests (app-of-apps), published from a Git repo (`--repo`). | -| `argocd-helm` | A URL-portable Helm chart app-of-apps; publish location is set at install time. | +| `argocd-helm` | A Helm chart app-of-apps; `repoURL` defaults to the push-target registry — plain `helm install` works with no `--set repoURL` needed. Override with `--set repoURL=oci://mirror` when mirroring. | | `flux` | Flux `HelmRelease` and `Kustomization` manifests. | ```bash diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index 20c10b5b3..d648a5391 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -1867,8 +1867,7 @@ bundles/ Manifest-only components and mixed-component raw manifests are supported by `--deployer argocd-helm` via the path-based Application shape. -**The bundle is URL-portable.** No `--repo` flag is needed (and is ignored if passed with `--deployer argocd-helm`). The same generated bundle bytes can be pushed to any chart-source backend the user chooses — Argo CD pulls from whichever URL the user supplies at install time via `helm install --set repoURL=...`. The publish location is *not* baked into the bundle artifact. - +**The bundle's `repoURL` defaults to the registry it was pushed to.** No `--repo` flag is needed (and is ignored if passed with `--deployer argocd-helm`). When pushed to an OCI registry, the parent namespace is baked into `values.yaml` as the default `repoURL` — a plain `helm install` works with no `--set repoURL` needed. Override with `--set repoURL=oci://mirror` when deploying from a different registry. **Recommended deploy flow:** ```shell diff --git a/pkg/bundler/bundler.go b/pkg/bundler/bundler.go index c5345f2b6..954dd5022 100644 --- a/pkg/bundler/bundler.go +++ b/pkg/bundler/bundler.go @@ -386,6 +386,7 @@ func (b *DefaultBundler) buildDeployer(ctx context.Context, recipeResult *recipe VendorCharts: b.Config.VendorCharts(), ChartName: b.Config.BundleChartName(), AppName: b.Config.AppName(), + OCIParentNamespace: b.Config.OCIParentNamespace(), }, nil case config.DeployerArgoCD: diff --git a/pkg/bundler/config/config.go b/pkg/bundler/config/config.go index 407cfcbae..edcee15a2 100644 --- a/pkg/bundler/config/config.go +++ b/pkg/bundler/config/config.go @@ -240,6 +240,12 @@ type Config struct { // targetRevision` triple resolves against the real artifact. See #1019. bundleChartName string + // ociParentNamespace is the OCI registry + repository path with the chart-name + // segment stripped (e.g. "oci://ghcr.io/nvidia" for "oci://ghcr.io/nvidia/my-bundle:v1"). + // Baked into the argocd-helm bundle's root values.yaml as the default repoURL. + // Empty when --output targets a local directory. See #1342. + ociParentNamespace string + // appName overrides the parent Argo Application's `metadata.name` for // the argocd-helm and argocd deployers. Empty means each deployer // applies its own default ("aicr-stack" / "nvidia-stack"). When two @@ -459,6 +465,12 @@ func (c *Config) BundleChartName() string { return c.bundleChartName } +// OCIParentNamespace returns the OCI parent namespace baked into the +// argocd-helm bundle's root values.yaml as the default repoURL. See #1342. +func (c *Config) OCIParentNamespace() string { + return c.ociParentNamespace +} + // AppName returns the parent Application name override for the argocd-helm // and argocd deployers. Empty means "use the deployer's default". See #1011. func (c *Config) AppName() string { @@ -727,6 +739,15 @@ func WithBundleChartName(name string) Option { } } +// WithOCIParentNamespace sets the OCI parent namespace (registry + repo path +// without the chart segment) baked into the argocd-helm bundle's values.yaml +// as the default repoURL. Empty keeps repoURL as "" (local output). See #1342. +func WithOCIParentNamespace(ns string) Option { + return func(c *Config) { + c.ociParentNamespace = ns + } +} + // WithAppName sets the parent Argo Application's `metadata.name` for the // argocd-helm and argocd deployers. Empty leaves the deployer's default // in place. Required by operators deploying multiple non-overlapping diff --git a/pkg/bundler/deployer/argocdhelm/argocdhelm.go b/pkg/bundler/deployer/argocdhelm/argocdhelm.go index 61a0f6735..30a34a182 100644 --- a/pkg/bundler/deployer/argocdhelm/argocdhelm.go +++ b/pkg/bundler/deployer/argocdhelm/argocdhelm.go @@ -167,6 +167,12 @@ type Generator struct { // is the multi-bundle collision fix — see issue #1011. AppName string + // OCIParentNamespace is the OCI registry + repository path with the + // chart-name segment stripped. When set, written as the default repoURL + // in the bundle's root values.yaml so deploying from the push-target + // registry requires no --set flags. Empty for local-directory output. See #1342. + OCIParentNamespace string + // DynamicValues maps component names to their dynamic value paths. DynamicValues map[string][]string @@ -297,11 +303,12 @@ func (g *Generator) Generate(ctx context.Context, outputDir string) (*deployer.O dynamicOnlyValues[rootValuesAppNameKey] = g.AppName } - // 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] = "" + // Bake the OCI parent namespace as the repoURL default when the bundle was + // pushed to a registry — `helm show values` and a plain `helm install` both + // work without --set flags. For local output, OCIParentNamespace is "" and + // the {{ required }} safety-net is unchanged. See #1342. + repoURLDefault := g.OCIParentNamespace + dynamicOnlyValues[rootValuesRepoURLKey] = repoURLDefault dynamicOnlyValues[rootValuesTargetRevisionKey] = "" valuesPath, valuesSize, err := writeRootValuesFile(dynamicOnlyValues, outputDir) @@ -463,11 +470,13 @@ func (g *Generator) writeStaticValuesAndBuildStubs(outputDir string) ([]string, // issue #1020. const rootValuesInstallHeader = `# Generated by AICR # -# Install-time inputs (required for argocd-helm bundles): +# Install-time inputs (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. +# Pre-filled when pushed to an OCI registry; override +# with --set repoURL=oci://mirror when mirroring. # Example: --set repoURL=oci://ghcr.io/nvidia # # targetRevision Chart version / OCI artifact tag. @@ -1172,9 +1181,9 @@ func (g *Generator) writeReadme(outputDir string) (string, int64, error) { buf.WriteString("# Argo CD Helm Chart Deployment Bundle\n\n") buf.WriteString("This bundle is a Helm chart whose templates generate one Argo CD\n") buf.WriteString("Application per component plus a parent Application that manages\n") - buf.WriteString("the chart deployment as a whole. The bundle is **URL-portable**:\n") - buf.WriteString("the publish location is supplied at install time via `--set\n") - buf.WriteString("repoURL=...`, not baked into the chart bytes.\n\n") + buf.WriteString("the chart deployment as a whole. The bundle's `repoURL` defaults\n") + buf.WriteString("to the registry it was pushed to; override with `--set\n") + buf.WriteString("repoURL=oci://mirror` when deploying from a different registry.\n\n") buf.WriteString("## Deploy\n\n") buf.WriteString("`/` below is the **parent namespace** you\n") diff --git a/pkg/bundler/deployer/argocdhelm/argocdhelm_test.go b/pkg/bundler/deployer/argocdhelm/argocdhelm_test.go index 8d76daa28..9e57b5e02 100644 --- a/pkg/bundler/deployer/argocdhelm/argocdhelm_test.go +++ b/pkg/bundler/deployer/argocdhelm/argocdhelm_test.go @@ -987,6 +987,53 @@ func TestBundleGolden_MixedComponent(t *testing.T) { } } +// TestBundleGolden_OCI_BakesRepoURL verifies that when OCIParentNamespace is +// set the bundle's root values.yaml contains the parent namespace as the +// repoURL default, not an empty string. See #1342. +func TestBundleGolden_OCI_BakesRepoURL(t *testing.T) { + ctx := context.Background() + outputDir := t.TempDir() + + rr := newRecipeResult("v1.0.0", []recipe.ComponentRef{ + { + Name: "cert-manager", + Namespace: "cert-manager", + Chart: "cert-manager", + Version: "v1.20.2", + Type: recipe.ComponentTypeHelm, + Source: "https://charts.jetstack.io", + }, + }) + rr.DeploymentOrder = []string{"cert-manager"} + + const wantNamespace = "oci://ghcr.io/myorg" + + g := &Generator{ + RecipeResult: rr, + ComponentValues: map[string]map[string]any{"cert-manager": {"replicaCount": 1}}, + Version: "v0.0.0-golden", + TargetRevision: "v1.0.0", + OCIParentNamespace: wantNamespace, + } + + if _, err := g.Generate(ctx, outputDir); err != nil { + t.Fatalf("Generate() error = %v", err) + } + + valuesBytes, err := os.ReadFile(filepath.Join(outputDir, "values.yaml")) + if err != nil { + t.Fatalf("read values.yaml: %v", err) + } + values := string(valuesBytes) + + if !strings.Contains(values, `repoURL: `+wantNamespace) { + t.Errorf("values.yaml missing baked repoURL; got:\n%s", values) + } + if strings.Contains(values, "repoURL: \"\"") { + t.Errorf("values.yaml should not contain empty repoURL when OCIParentNamespace is set; got:\n%s", values) + } +} + // TestBundleGolden_ReadinessGate freezes the argocd-helm bundle output when a // component ships a readiness gate. The delegated argocd.Generator emits a // 002--readiness/ local-helm folder after the primary; the argocdhelm diff --git a/pkg/bundler/deployer/argocdhelm/testdata/helm_and_manifest_only/values.yaml b/pkg/bundler/deployer/argocdhelm/testdata/helm_and_manifest_only/values.yaml index 544ea54c6..33ba49aeb 100644 --- a/pkg/bundler/deployer/argocdhelm/testdata/helm_and_manifest_only/values.yaml +++ b/pkg/bundler/deployer/argocdhelm/testdata/helm_and_manifest_only/values.yaml @@ -1,10 +1,12 @@ # Generated by AICR # -# Install-time inputs (required for argocd-helm bundles): +# Install-time inputs (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. +# Pre-filled when pushed to an OCI registry; override +# with --set repoURL=oci://mirror when mirroring. # Example: --set repoURL=oci://ghcr.io/nvidia # # targetRevision Chart version / OCI artifact tag. diff --git a/pkg/bundler/deployer/argocdhelm/testdata/mixed_component/values.yaml b/pkg/bundler/deployer/argocdhelm/testdata/mixed_component/values.yaml index 8091b5a2b..129403d64 100644 --- a/pkg/bundler/deployer/argocdhelm/testdata/mixed_component/values.yaml +++ b/pkg/bundler/deployer/argocdhelm/testdata/mixed_component/values.yaml @@ -1,10 +1,12 @@ # Generated by AICR # -# Install-time inputs (required for argocd-helm bundles): +# Install-time inputs (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. +# Pre-filled when pushed to an OCI registry; override +# with --set repoURL=oci://mirror when mirroring. # Example: --set repoURL=oci://ghcr.io/nvidia # # targetRevision Chart version / OCI artifact tag. diff --git a/pkg/cli/bundle.go b/pkg/cli/bundle.go index 1d378e800..fedadcb78 100644 --- a/pkg/cli/bundle.go +++ b/pkg/cli/bundle.go @@ -20,7 +20,6 @@ import ( "io" "log/slog" "os" - "path" "path/filepath" "strings" @@ -117,6 +116,12 @@ type bundleCmdOptions struct { // deployer's "aicr-bundle" default is used when this is empty. See #1019. bundleChartName string + // ociParentNamespace is the OCI registry + repository path with the + // chart segment stripped; used as the default repoURL baked into the + // argocd-helm bundle's values.yaml. Derived from ociRef when --output + // is an OCI reference; empty for local-directory output. See #1342. + ociParentNamespace string + // appName overrides the parent Argo Application's metadata.name. Empty // means each deployer applies its own default ("aicr-stack" for // argocd-helm, "nvidia-stack" for argocd). See #1011. @@ -207,6 +212,9 @@ func parseBundleCmdOptions(cmd *cli.Command, cfg *appcfg.AICRConfig) (*bundleCmd // resolves against an artifact that doesn't exist in the registry // when the user picks a non-default name. See #1019. opts.bundleChartName = opts.ociRef.ChartName() + // Derive the parent namespace (strip chart segment) so the argocd-helm + // deployer can bake it into values.yaml as the default repoURL. See #1342. + opts.ociParentNamespace = opts.ociRef.ParentNamespace() } else { // Resolve local output path to absolute to ensure consistent behavior // regardless of how the binary is invoked. @@ -837,6 +845,7 @@ func runBundleCmd(ctx context.Context, cmd *cli.Command) error { config.WithOCISourceName(opts.ociSourceName), config.WithFluxNamespace(opts.fluxNamespace), config.WithBundleChartName(opts.bundleChartName), + config.WithOCIParentNamespace(opts.ociParentNamespace), config.WithAppName(opts.appName), ) @@ -1040,21 +1049,12 @@ func printArgoCDHelmOCIInstructions(w io.Writer, ref *oci.Reference) { 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 - } + // Delegate to canonical derivation so baked repoURL and printed hint always match. See #1342. + repoURL := ref.ParentNamespace() fmt.Fprintf(w, "\nargocd-helm bundle pushed: %s\n", chartRef) fmt.Fprintln(w, "\nTo install:") fmt.Fprintf(w, " helm install %s \\\n", chartRef) - fmt.Fprintln(w, " --namespace argocd \\") - fmt.Fprintf(w, " --set repoURL=%s\n", repoURL) + fmt.Fprintln(w, " --namespace argocd") + fmt.Fprintf(w, " # repoURL defaults to %s (override with --set repoURL=oci://mirror if mirroring)\n", repoURL) } diff --git a/pkg/cli/bundle_test.go b/pkg/cli/bundle_test.go index 84c497aba..836682eb0 100644 --- a/pkg/cli/bundle_test.go +++ b/pkg/cli/bundle_test.go @@ -416,7 +416,7 @@ func TestPrintArgoCDHelmOCIInstructions(t *testing.T) { }, wantContain: []string{ "oci://ghcr.io/nvidia/aicr-bundle:v1.0.0", - "--set repoURL=oci://ghcr.io/nvidia", + "# repoURL defaults to oci://ghcr.io/nvidia", "--namespace argocd", }, }, @@ -430,7 +430,7 @@ func TestPrintArgoCDHelmOCIInstructions(t *testing.T) { }, wantContain: []string{ "oci://registry.example.com/team/platform/aicr-bundle:0.42.0", - "--set repoURL=oci://registry.example.com/team/platform", + "# repoURL defaults to oci://registry.example.com/team/platform", }, }, { @@ -443,7 +443,7 @@ func TestPrintArgoCDHelmOCIInstructions(t *testing.T) { }, wantContain: []string{ "oci://localhost:5000/aicr-bundle:dev", - "--set repoURL=oci://localhost:5000", + "# repoURL defaults to oci://localhost:5000", }, }, { diff --git a/pkg/oci/reference.go b/pkg/oci/reference.go index 5d08f2aa1..a23786c19 100644 --- a/pkg/oci/reference.go +++ b/pkg/oci/reference.go @@ -164,6 +164,20 @@ func (r *Reference) ChartName() string { return base } +// ParentNamespace returns the OCI registry + repository path with the +// chart-name segment (last path element) stripped, prefixed with the +// oci:// scheme. Returns "" for non-OCI references. See #1342. +func (r *Reference) ParentNamespace() string { + if !r.IsOCI || r.Repository == "" { + return "" + } + parentPath := path.Dir(r.Repository) + if parentPath == "." || parentPath == "/" { + return URIScheme + r.Registry + } + return URIScheme + r.Registry + "/" + parentPath +} + // WithTag returns a copy of the reference with the specified tag. // For non-OCI references, returns the same reference unchanged. func (r *Reference) WithTag(tag string) *Reference {