diff --git a/cli/internal/cli/install.go b/cli/internal/cli/install.go index f8afceb..f79a1a7 100644 --- a/cli/internal/cli/install.go +++ b/cli/internal/cli/install.go @@ -34,9 +34,9 @@ func newInstallCommand(commandContext *commandContext) *cobra.Command { "equal version to heal drift, and refuses a newer or major-mismatched\n" + "in-cluster version with actionable guidance. Use --dry-run to preview the\n" + "action without changing the cluster.\n\n" + - "The default operator image is digest-pinned to the version this CLI\n" + - "embeds; the supported way to change the operator version is to upgrade the\n" + - "CLI. The -f/--set/--set-string flags customize OPERATIONAL chart values\n" + + "The default operator image is pinned to the version this CLI embeds (the\n" + + "chart's appVersion); the supported way to change the operator version is to\n" + + "upgrade the CLI. The -f/--set/--set-string flags customize OPERATIONAL chart values\n" + "(replicas, resources, scheduling, logging, metrics, and so on). -f reads\n" + "YAML values files (repeatable, later files win); --set and --set-string\n" + "take inline key=value overrides in Helm syntax (--set-string forces string\n" + @@ -45,9 +45,8 @@ func newInstallCommand(commandContext *commandContext) *cobra.Command { "--set-string by argument order the way Helm does). These override values are\n" + "validated against the chart's schema, so a bad value fails fast (under\n" + "--dry-run too). Because user values deep-merge over the defaults, a --set\n" + - "image.tag is ineffective (the chart renders repository@digest and the pinned\n" + - "digest wins), but a --set image.digest or image.repository WILL repoint the\n" + - "operator image and is not a supported configuration.", + "image.tag, image.repository, or image.digest WILL repoint the operator\n" + + "image and is not a supported configuration.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { runtimeConfig, err := loadRuntimeConfig(commandContext.viper) @@ -85,12 +84,11 @@ func newInstallCommand(commandContext *commandContext) *cobra.Command { return err } - // Model A: the pinned typed Image stays, and the user overrides ride - // in Extra, which ToHelmValues deep-merges on top. The embedded - // digest shadows a --set image.tag (the chart renders - // repository@digest), so the default install stays digest-pinned for - // operational knobs; image.digest/image.repository overrides do still win - // through the merge and are an unsupported configuration, not blocked here. + // The typed default pins the manager image to the chart's appVersion + // (repository:appVersion), and the user overrides ride in Extra, which + // ToHelmValues deep-merges on top. image.tag/image.repository/image.digest + // overrides do repoint the operator image through the merge and are an + // unsupported configuration, not blocked here. vals := operator.Default() vals.Extra = userOverrides diff --git a/cli/internal/cli/install_test.go b/cli/internal/cli/install_test.go index bc17020..c16d0aa 100644 --- a/cli/internal/cli/install_test.go +++ b/cli/internal/cli/install_test.go @@ -510,16 +510,16 @@ func TestInstallOverridesPreserveImagePins(t *testing.T) { var stdout, stderr bytes.Buffer installer, _, run := newInstallRoot(t, &stdout, &stderr) - // Model A invariant: user overrides ride in Extra and never clobber the - // typed Default() pins. With a --set override present, the spec still carries - // the digest-pinned manager image byte-for-byte from Default(). + // User overrides ride in Extra and never clobber the typed Default() image. + // With a --set override present, the spec still carries the appVersion-pinned + // manager image byte-for-byte from Default() (repository set, no digest/tag). spec := captureSpec(installer) require.NoError(t, run("install", "--set", "replicaCount=2")) wantDefault := operator.Default() assert.Equal(t, wantDefault.Image, spec.Values.Image) - assert.NotEmpty(t, spec.Values.Image.Digest) + assert.Empty(t, spec.Values.Image.Digest, "default image is appVersion-pinned, not digest-pinned") // The override is isolated to Extra. assert.Equal(t, map[string]any{"replicaCount": int64(2)}, spec.Values.Extra) } diff --git a/cli/internal/operator/ssa/render_test.go b/cli/internal/operator/ssa/render_test.go index c13e42c..fcbd9db 100644 --- a/cli/internal/operator/ssa/render_test.go +++ b/cli/internal/operator/ssa/render_test.go @@ -8,6 +8,7 @@ import ( "github.com/meigma/yacd/cli/internal/operator" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "helm.sh/helm/v3/pkg/chart/loader" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -55,21 +56,15 @@ func TestRenderDefaultObjectSet(t *testing.T) { assert.Equal(t, want, got, "rendered object set must be exactly the expected 12") } -// TestRenderDigestPinnedImages asserts the default render is digest-pinned: the -// manager container image carries the manager digest. -func TestRenderDigestPinnedImages(t *testing.T) { - const wantManagerImage = "ghcr.io/meigma/yacd@sha256:5d53ca824dacad39c482dc93edfd2db4a65d5803f43dce5b18b1a7482b0f8e21" +// TestRenderDefaultImageUsesAppVersionTag asserts the default render pins the +// manager image to the chart's appVersion (repository:appVersion). The expected +// tag is read from the embedded chart so the test stays correct across release +// bumps rather than hardcoding a version. +func TestRenderDefaultImageUsesAppVersionTag(t *testing.T) { + wantManagerImage := "ghcr.io/meigma/yacd:" + embeddedChartAppVersion(t) - deployment := findObject(t, renderDefault(t, installNamespace), "Deployment", "yacd-controller-manager") - - containers, found, err := unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "containers") - require.NoError(t, err) - require.True(t, found, "deployment must declare containers") - require.Len(t, containers, 1) - - manager, ok := containers[0].(map[string]any) - require.True(t, ok) - assert.Equal(t, wantManagerImage, manager["image"], "manager image is digest-pinned") + got := managerImage(t, renderDefault(t, installNamespace)) + assert.Equal(t, wantManagerImage, got, "manager image defaults to repository:appVersion") } // TestRenderPresenceOfCoreObjects double-checks the metrics Service and the @@ -109,28 +104,62 @@ func TestRenderOverrideReachesDeployment(t *testing.T) { assert.EqualValues(t, 2, replicas, "override replicaCount must reach the rendered Deployment") } -// TestRenderImageTagOverrideKeepsDigestPin proves the headline Model-A invariant -// end to end through the renderer: with a user --set image.tag override present -// (folded through Extra exactly as the install path does), the manager container -// image is STILL the embedded digest, because the chart renders repository@digest -// and Default()'s digest survives the deep-merge. This guards against a future -// ToHelmValues merge change silently letting a tag override repoint the image. -func TestRenderImageTagOverrideKeepsDigestPin(t *testing.T) { - const wantManagerImage = "ghcr.io/meigma/yacd@sha256:5d53ca824dacad39c482dc93edfd2db4a65d5803f43dce5b18b1a7482b0f8e21" +// TestRenderImageDigestOverridePinsDigest proves an explicit image.digest +// override still produces a digest-pinned render: the chart's yacd.image helper +// prefers digest over tag, so a deliberate digest pin remains available even +// though the default now tracks the chart appVersion. +func TestRenderImageDigestOverridePinsDigest(t *testing.T) { + const digest = "sha256:5d53ca824dacad39c482dc93edfd2db4a65d5803f43dce5b18b1a7482b0f8e21" + const wantManagerImage = "ghcr.io/meigma/yacd@" + digest + + objs, err := renderWithExtra(t, map[string]any{"image": map[string]any{"digest": digest}}) + require.NoError(t, err) + + assert.Equal(t, wantManagerImage, managerImage(t, objs), + "an explicit image.digest override pins the manager image to repository@digest") +} + +// TestRenderImageTagOverrideRepointsImage proves a --set image.tag override now +// repoints the manager image to repository:tag. With the default install no +// longer digest-pinned, a tag override is honored rather than shadowed; it is an +// unsupported configuration, not blocked here. +func TestRenderImageTagOverrideRepointsImage(t *testing.T) { + const wantManagerImage = "ghcr.io/meigma/yacd:v9.9.9" objs, err := renderWithExtra(t, map[string]any{"image": map[string]any{"tag": "v9.9.9"}}) require.NoError(t, err) + assert.Equal(t, wantManagerImage, managerImage(t, objs), + "a --set image.tag override repoints the manager image") +} + +// embeddedChartAppVersion loads the embedded operator chart and returns its +// declared appVersion, so image-pin assertions track the chart rather than a +// hardcoded release. +func embeddedChartAppVersion(t *testing.T) string { + t.Helper() + files, err := bufferedFiles(charts.OperatorChart) + require.NoError(t, err) + ch, err := loader.LoadFiles(files) + require.NoError(t, err) + require.NotEmpty(t, ch.Metadata.AppVersion, "chart must declare an appVersion") + return ch.Metadata.AppVersion +} + +// managerImage returns the single manager container image string from a rendered +// object set, failing the test if the Deployment or container is missing. +func managerImage(t *testing.T, objs []*unstructured.Unstructured) string { + t.Helper() deployment := findObject(t, objs, "Deployment", "yacd-controller-manager") containers, found, err := unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "containers") require.NoError(t, err) require.True(t, found, "deployment must declare containers") require.Len(t, containers, 1) - manager, ok := containers[0].(map[string]any) require.True(t, ok) - assert.Equal(t, wantManagerImage, manager["image"], - "a --set image.tag override must be shadowed by the surviving digest pin") + image, ok := manager["image"].(string) + require.True(t, ok, "manager container must declare an image string") + return image } // TestRenderSchemaInvalidOverrideFails proves a schema-violating override fails diff --git a/cli/internal/operator/values.go b/cli/internal/operator/values.go index 61f91aa..55eaf52 100644 --- a/cli/internal/operator/values.go +++ b/cli/internal/operator/values.go @@ -1,17 +1,12 @@ package operator -// Default image references that pin the operator install to the published -// release. They reproduce the digest pinning the render script used to inject -// via --set-string image.digest, moved into Go so the default install stays -// digest-pinned and tamper-evident, offline. When cutting a new operator -// release, bump these alongside charts/yacd/Chart.yaml appVersion. -const ( - // defaultManagerRepository is the published operator manager image. - defaultManagerRepository = "ghcr.io/meigma/yacd" - - // defaultManagerDigest pins the manager image to the v0.1.1 release digest. - defaultManagerDigest = "sha256:5d53ca824dacad39c482dc93edfd2db4a65d5803f43dce5b18b1a7482b0f8e21" -) +// defaultManagerRepository is the published operator manager image. The default +// install sets only the repository and leaves the tag and digest unset, so the +// chart renders repository:appVersion (its own Chart.yaml appVersion). Operator +// and CLI release together under one version, so that appVersion tag always +// resolves to the matching, already-published manager image. The supported way +// to change the operator version is to upgrade the CLI. +const defaultManagerRepository = "ghcr.io/meigma/yacd" // Image identifies a container image by repository plus an optional tag or // digest. When Digest is set it wins over Tag (matching the chart's @@ -122,18 +117,16 @@ func mergeValues(dst, src map[string]any) { } } -// Default returns the pinned, offline baseline that reproduces today's install: -// the digest-pinned manager image, leader election on, secure metrics, and -// json/info logging. It mirrors the chart's own defaults (which already set -// secure metrics, leader election on, and json/info logging) and only adds the -// release digest, so the rendered Deployment is byte-equivalent to a -// `helm template … --set-string image.digest=…` render at the chart's default -// values. +// Default returns the baseline install: the manager image pinned to the chart's +// appVersion (repository:appVersion, the operator release this CLI ships with), +// leader election on, secure metrics, and json/info logging. It sets only the +// image repository; leaving the tag and digest empty lets the chart default the +// tag to its appVersion, so the rendered Deployment matches a plain +// `helm template` at the chart's default values. func Default() Values { return Values{ Image: Image{ Repository: defaultManagerRepository, - Digest: defaultManagerDigest, }, } } diff --git a/cli/internal/operator/values_test.go b/cli/internal/operator/values_test.go index 548bb78..5d9e788 100644 --- a/cli/internal/operator/values_test.go +++ b/cli/internal/operator/values_test.go @@ -11,19 +11,22 @@ import ( "helm.sh/helm/v3/pkg/chartutil" ) -// TestDefaultPinsReleaseDigests proves the offline default install stays -// digest-pinned to the published manager release. -func TestDefaultPinsReleaseDigests(t *testing.T) { +// TestDefaultUsesAppVersionTag proves the default install sets only the image +// repository, leaving tag and digest empty so the chart renders +// repository:appVersion (the operator release this CLI ships with). +func TestDefaultUsesAppVersionTag(t *testing.T) { values := Default() assert.Equal(t, defaultManagerRepository, values.Image.Repository) - assert.Equal(t, defaultManagerDigest, values.Image.Digest) - assert.Empty(t, values.Image.Tag, "digest pin leaves tag empty") + assert.Empty(t, values.Image.Digest, "default leaves the digest empty so the chart uses its appVersion tag") + assert.Empty(t, values.Image.Tag, "default leaves the tag empty so the chart defaults it to appVersion") helmValues := values.ToHelmValues() image, ok := helmValues["image"].(map[string]any) require.True(t, ok, "image sub-tree must be present") - assert.Equal(t, defaultManagerDigest, image["digest"]) + assert.Equal(t, defaultManagerRepository, image["repository"]) + _, hasDigest := image["digest"] + assert.False(t, hasDigest, "empty digest is omitted so the chart's appVersion tag applies") _, hasTag := image["tag"] assert.False(t, hasTag, "empty tag is omitted so the chart default applies") }