From b5fed382e9bcd8f2ebb563a3e33808776012e23e Mon Sep 17 00:00:00 2001 From: Mohammad Abdolirad Date: Sun, 27 Sep 2026 17:28:04 +0200 Subject: [PATCH 1/2] remove offline planning - drop deployah plan --offline and RenderOffline - render test fixtures without a production API - keep plan cluster-aware --- README.md | 15 +- ...-environment-resolution-and-inheritance.md | 2 +- ...plan-requires-release-and-cluster-state.md | 13 +- docs/cli/deployah_plan.md | 3 +- docs/custom-manifests-and-crds.md | 14 +- internal/cmd/deploy/deploy_test.go | 4 - internal/cmd/plan/plan.go | 73 +------- internal/cmd/plan/plan_test.go | 174 ++++++++++-------- internal/cmd/root_dir_test.go | 6 +- internal/extras/load.go | 5 +- internal/extras/load_test.go | 4 +- internal/helm/chart_crd_test.go | 30 ++- internal/helm/dest_test.go | 29 --- internal/helm/generate_env_test.go | 32 +--- internal/helm/generate_monitor_test.go | 10 +- internal/helm/generate_schedule_test.go | 19 +- internal/helm/generate_task_test.go | 9 +- internal/helm/helm.go | 4 +- internal/helm/identity_test.go | 118 +++++------- internal/helm/render.go | 28 +-- internal/helm/render_test.go | 21 +++ internal/k8s/tlscert.go | 8 +- internal/plan/resource.go | 12 -- internal/plan/view/human_semantic_test.go | 30 +-- internal/session/interfaces.go | 7 - internal/session/session_test.go | 20 -- internal/testing/doc.go | 3 +- internal/testing/helmfixture/render.go | 88 +++++++++ internal/testing/helmfixture/render_test.go | 86 +++++++++ internal/testing/plan_scenarios.go | 17 +- internal/testing/schema_validate.go | 10 +- internal/testing/types.go | 36 ++-- 32 files changed, 452 insertions(+), 478 deletions(-) create mode 100644 internal/testing/helmfixture/render.go create mode 100644 internal/testing/helmfixture/render_test.go diff --git a/README.md b/README.md index e745d2c..5b9ab47 100644 --- a/README.md +++ b/README.md @@ -275,14 +275,15 @@ The spec is a smaller surface than the Kubernetes API on purpose, so step 2 adds fields you did not write. You can read the result before anything is applied: ```sh -deployah plan --offline --raw --yaml +deployah plan --raw --yaml ``` -`--offline` renders without touching a cluster, `--raw` prints raw Kubernetes -field paths instead of the compact Deployah vocabulary, and `--yaml` shows -changed fields as YAML blocks. For the resolved hostname, TLS mode, context, -and runtime environment, use `deployah resolve ` (also offline, -`--output json` for machine-readable output). +`deployah plan` renders the chart and compares it with the release on your +cluster, so it needs cluster access. `--raw` prints raw Kubernetes field paths +instead of the compact Deployah vocabulary, and `--yaml` shows changed fields +as YAML blocks. For the resolved hostname, TLS mode, context, and runtime +environment without a cluster, use `deployah resolve ` +(`--output json` for machine-readable output). For how Deployah compares to similar tools (DevSpace, Werf, Score, Epinio, Kubero), see [docs/comparison.md](docs/comparison.md). @@ -422,7 +423,7 @@ These work with every command: | `deployah validate ` | Also load the platform file and check the resolved configuration for that environment. | | `deployah resolve ` | Preview the fully resolved hostname, TLS mode, context, and runtime environment, offline. Prints FileValues and ExplicitValues; do not treat the output as secret-safe CI output. Use `--output json` for machine-readable output. | | `deployah resolve --environments` | List every environment from both files: where it is registered, its context (or the kubeconfig fallback), domains, and overrides. | -| `deployah plan ` | Inspect changes for an environment, without applying anything. Extra manifests from `.deployah/manifests/` appear in the diff; CRD files from `.deployah/crds/` are listed with Helm's install-only lifecycle, not applied. Use `--offline` to render with no cluster access, `--raw` for raw Kubernetes field paths instead of the compact Deployah vocabulary, `--yaml` to show changed fields as YAML blocks, `--drift` to also compare against live cluster state, `--detailed-exitcode` to exit 2 when changes are pending, or `--output json` for CI. | +| `deployah plan ` | Inspect changes for an environment, without applying anything. It needs cluster access. Extra manifests from `.deployah/manifests/` appear in the diff; CRD files from `.deployah/crds/` are listed with Helm's install-only lifecycle, not applied. Use `--raw` for raw Kubernetes field paths instead of the compact Deployah vocabulary, `--yaml` to show changed fields as YAML blocks, `--drift` to also compare against live cluster state, `--detailed-exitcode` to exit 2 when changes are pending, or `--output json` for CI. | | `deployah deploy ` | Deploy your project. Deployah validates the spec, runs deploy guards, then always runs a Helm install for a new release or a Helm upgrade for an existing one. Use `--skip-crds` to skip installing [chart CRDs](docs/custom-manifests-and-crds.md#helm-crd-lifecycle) on a fresh Helm install (a CRD added after that first install is not installed by a later deploy), `--explain` to print the resolution report first, `--force-hostname-change` to bypass the hostname guard, or `--resize-volumes` to grow [persistence](docs/workloads.md#growing-volumes) sizes. | | `deployah run ` | Run a spec task as a one-off Job. Wait is the default; `--detach` returns after create. `--count` / `--parallelism` override fanout for that run. | | `deployah status ` | Show the status of a deployed project. Use `--detailed` for pod details, `-e` for an environment. | diff --git a/docs/adr/0002-runtime-environment-resolution-and-inheritance.md b/docs/adr/0002-runtime-environment-resolution-and-inheritance.md index 3045cad..2e7fc13 100644 --- a/docs/adr/0002-runtime-environment-resolution-and-inheritance.md +++ b/docs/adr/0002-runtime-environment-resolution-and-inheritance.md @@ -70,4 +70,4 @@ layer, not how streams merge. done" inside the resolver. - Tasks cannot inherit from other tasks. Share a component, or repeat `envFile` / `env:`. -- Invalid keys fail every env-selecting command, including offline plan. +- Invalid keys fail every env-selecting command. diff --git a/docs/adr/0010-semantic-plan-requires-release-and-cluster-state.md b/docs/adr/0010-semantic-plan-requires-release-and-cluster-state.md index 99bc0a9..cc064c3 100644 --- a/docs/adr/0010-semantic-plan-requires-release-and-cluster-state.md +++ b/docs/adr/0010-semantic-plan-requires-release-and-cluster-state.md @@ -14,15 +14,14 @@ resource consequences. ## Decision A semantic plan requires enough release state to construct Previous -and enough cluster state to construct Live. There is no offline -semantic plan in the target architecture. +and enough cluster state to construct Live. Deployah does not support +offline semantic planning. When that information is unavailable, the planner must not fabricate Previous, Live, Drift, HelmAction, or resource consequences. -A Desired-only render or validation capability may exist separately. -Rendering is not semantic planning. This ADR does not freeze a command -name for that capability. +Cluster-independent validation and configuration resolution are +separate capabilities. They are not semantic planning. ## Consequences @@ -33,5 +32,5 @@ name for that capability. ### Negative -- Operators without release and cluster access cannot get a semantic - plan. They need the separate render capability. +- Operators without release and cluster access cannot obtain a + semantic plan. diff --git a/docs/cli/deployah_plan.md b/docs/cli/deployah_plan.md index e1bce55..9b8251f 100644 --- a/docs/cli/deployah_plan.md +++ b/docs/cli/deployah_plan.md @@ -14,8 +14,7 @@ deployah plan [flags] ```text --detailed-exitcode Exit 2 when the plan has pending changes, 0 when it does not, 1 on error (for CI) - --drift Detect drift between the rendered manifests and the live cluster state (requires cluster access; not compatible with --offline) - --offline Render and validate the chart without contacting the cluster + --drift Detect drift between the rendered manifests and the live cluster state --output string Output format (default "text") --raw Show raw Kubernetes field paths instead of the compact Deployah vocabulary --show-secrets Reveal masked secret values in text output (requires an interactive terminal; refused with --output json) diff --git a/docs/custom-manifests-and-crds.md b/docs/custom-manifests-and-crds.md index 1e349b5..f4ea344 100644 --- a/docs/custom-manifests-and-crds.md +++ b/docs/custom-manifests-and-crds.md @@ -71,7 +71,7 @@ spec: ```sh deployah plan prod # extras appear in the plan diff -deployah deploy prod -y # Helm install processes chart CRDs, then the release +deployah deploy prod # Helm install processes chart CRDs, then the release ``` ## Labels and annotations @@ -126,12 +126,12 @@ later. reject invalid or unsupported content. - An extra that collides with a generated chart object fails the render (Deployah will not overwrite chart resources). -- Custom resource kinds must be known to Deployah's built-in table or - live cluster discovery. A small offline allowlist covers common - operator APIs (cert-manager and prometheus-operator). Deployah does - not read `.deployah/crds/` to learn a custom resource's type or - scope. With `deployah plan --offline`, unknown kinds are allowed so - you can still preview; scope defaults to namespaced. +- Custom resource kinds must be known to Deployah's built-in scope table + or live cluster discovery. The built-in table includes common operator + APIs such as cert-manager and prometheus-operator. Deployah does not + read `.deployah/crds/` to learn a custom resource's type or scope. + When no REST config is available, unknown kinds are accepted and + default to namespaced. - Helm hook annotations (`helm.sh/hook`, `helm.sh/hook-weight`, `helm.sh/hook-delete-policy`) are not supported on custom manifests. Use a Deployah `preDeploy` or `postDeploy` task for deploy hooks. diff --git a/internal/cmd/deploy/deploy_test.go b/internal/cmd/deploy/deploy_test.go index 7be0616..e40b803 100644 --- a/internal/cmd/deploy/deploy_test.go +++ b/internal/cmd/deploy/deploy_test.go @@ -92,10 +92,6 @@ func (s *stubHelmClient) RenderManifests(context.Context, *spec.ResolvedSpec, po return s.renderResult, cleanup, nil } -func (s *stubHelmClient) RenderOffline(context.Context, *spec.ResolvedSpec, postrenderer.PostRenderer, []extras.RawFile) (*render.RenderResult, func(), error) { - panic("unexpected RenderOffline call") -} - func (s *stubHelmClient) DeleteRelease(context.Context, string, string, bool) error { panic("unexpected DeleteRelease call") } diff --git a/internal/cmd/plan/plan.go b/internal/cmd/plan/plan.go index a174332..fbeee11 100644 --- a/internal/cmd/plan/plan.go +++ b/internal/cmd/plan/plan.go @@ -25,7 +25,6 @@ import ( "deployah.dev/deployah/internal/cmd/cmdopts" "deployah.dev/deployah/internal/drift" "deployah.dev/deployah/internal/extras" - "deployah.dev/deployah/internal/k8s" "deployah.dev/deployah/internal/session" "deployah.dev/deployah/internal/spec" @@ -44,7 +43,6 @@ var outputFormats = []string{outputFormatText, outputFormatJSON} type Options struct { Environment string `nabat:"environment"` Drift bool `nabat:"drift"` - Offline bool `nabat:"offline"` ShowSecrets bool `nabat:"show-secrets"` Raw bool `nabat:"raw"` YAML bool `nabat:"yaml"` @@ -58,8 +56,7 @@ func Register(app *nabat.App) { nabat.WithDescription("Inspect changes for an environment"), nabat.WithLongDescription("Render the chart for an environment and compare it with the last successful Helm release. With --drift, also compare the rendered manifests with live cluster state. Plan is read-only and never applies anything."), nabat.WithArg("environment", "", nabat.WithRequired(), nabat.WithUsage("Environment to plan for"), nabat.WithPrompt("Environment", "", nabat.WithHint("e.g. prod, staging"))), - nabat.WithFlag("drift", false, nabat.WithUsage("Detect drift between the rendered manifests and the live cluster state (requires cluster access; not compatible with --offline)")), - nabat.WithFlag("offline", false, nabat.WithUsage("Render and validate the chart without contacting the cluster")), + nabat.WithFlag("drift", false, nabat.WithUsage("Detect drift between the rendered manifests and the live cluster state")), nabat.WithFlag("show-secrets", false, nabat.WithUsage("Reveal masked secret values in text output (requires an interactive terminal; refused with --output json)")), nabat.WithFlag("raw", false, nabat.WithUsage("Show raw Kubernetes field paths instead of the compact Deployah vocabulary")), nabat.WithFlag("yaml", false, nabat.WithUsage("Show changed fields as YAML blocks instead of a single line")), @@ -70,9 +67,6 @@ func Register(app *nabat.App) { # Inspect changes for production deployah plan production -# Render and validate the chart without touching the cluster -deployah plan production --offline - # Machine-readable output for CI deployah plan production --output json @@ -99,12 +93,6 @@ func validateOptions(c *nabat.Context) error { if opts.ShowSecrets && !c.IsInteractive() { return errors.New("--show-secrets requires an interactive terminal") } - if opts.Offline && opts.OutputFormat == outputFormatJSON { - return errors.New("--offline has no diff to report as JSON; drop --output json") - } - if opts.Drift && opts.Offline { - return errors.New("--drift requires cluster access; it cannot be used with --offline") - } return nil } @@ -143,65 +131,12 @@ func runPlan(c *nabat.Context) error { return fmt.Errorf("resolution failed: %w", err) } - if opts.Offline { - return runOffline(c, sess, platform, manifest, opts, resolvedSpec) - } - return runOnline(c, sess, platform, manifest, opts, resolvedSpec) -} - -// runOffline renders the chart without contacting the cluster and prints a -// resource count instead of a diff: there is no reachable release history to -// compare against. -func runOffline(c *nabat.Context, sess *session.Session, platform *spec.PlatformConfig, manifest *spec.Spec, opts *Options, resolvedSpec *spec.ResolvedSpec) error { - cluster, err := sess.Target(c, opts.Environment) - if err != nil { - return fmt.Errorf("target cluster: %w", err) - } - helmClient, err := cluster.Helm() - if err != nil { - return fmt.Errorf("helm client: %w", err) - } - - // Offline mode never contacts the cluster, so any self-signed TLS cert - // is generated fresh (nil client) rather than fetched/reused -- a - // deliberate offline generation, not cmdopts.MaterializeSelfSignedTLS's - // fail-closed path for an online command that couldn't build a client. - if resolvedSpec != nil { - if tlsErr := k8s.MaterializeSelfSignedTLS(c, nil, "", resolvedSpec); tlsErr != nil { - return fmt.Errorf("materialize self-signed TLS: %w", tlsErr) - } - } - - bundle, err := extras.LoadFromSpec(sess.Workspace().SpecPath(), manifest, platform, opts.Environment, cluster.Namespace(), nil) - if err != nil { - return fmt.Errorf("load extras: %w", err) - } - postRenderer := bundle.PostRendererFor() - - result, cleanup, err := helmClient.RenderOffline(c, resolvedSpec, postRenderer, bundle.CRDs) - if cleanup != nil { - defer cleanup() - } - if err != nil { - return fmt.Errorf("render manifests: %w", err) - } - - count, err := planengine.CountResources(result.Manifest) - if err != nil { - return fmt.Errorf("count rendered resources: %w", err) - } - - c.Println(fmt.Sprintf("Rendered %d resources for environment '%s' (no cluster comparison).", count, opts.Environment)) - for _, d := range bundle.CRDDocs { - c.Println(fmt.Sprintf(" %s/%s", d.Kind, d.Name)) - } - c.Println("validation: OK") - return nil + return executePlan(c, sess, platform, manifest, opts, resolvedSpec) } -// runOnline renders the chart, diffs it against the last successful +// executePlan renders the chart, diffs it against the last successful // release, and displays the resulting plan. -func runOnline(c *nabat.Context, sess *session.Session, platform *spec.PlatformConfig, manifest *spec.Spec, opts *Options, resolvedSpec *spec.ResolvedSpec) error { +func executePlan(c *nabat.Context, sess *session.Session, platform *spec.PlatformConfig, manifest *spec.Spec, opts *Options, resolvedSpec *spec.ResolvedSpec) error { cluster, err := sess.Target(c, opts.Environment) if err != nil { return fmt.Errorf("target cluster: %w", err) diff --git a/internal/cmd/plan/plan_test.go b/internal/cmd/plan/plan_test.go index fc84228..b2678f0 100644 --- a/internal/cmd/plan/plan_test.go +++ b/internal/cmd/plan/plan_test.go @@ -17,8 +17,10 @@ package plan import ( "context" "encoding/json" + "errors" "os" "path/filepath" + "strings" "testing" "time" @@ -27,6 +29,8 @@ import ( "helm.sh/helm/v4/pkg/postrenderer" "helm.sh/helm/v4/pkg/release/common" "k8s.io/apimachinery/pkg/labels" + "nabat.dev/nabat" + "nabat.dev/nabat/nabattest" "deployah.dev/deployah/internal/extras" "deployah.dev/deployah/internal/helm" @@ -41,8 +45,8 @@ import ( ) // stubHelmClient implements [session.HelmClient] for plan command tests. -// Only the methods runOnline and runOffline actually call are wired; every -// other method panics if invoked unexpectedly, matching the pattern in +// Only the methods executePlan actually calls are wired; every other method +// panics if invoked unexpectedly, matching the pattern in // internal/cmd/deploy/deploy_test.go. type stubHelmClient struct { reachableErr error @@ -50,9 +54,6 @@ type stubHelmClient struct { renderResult *render.RenderResult renderErr error - offlineResult *render.RenderResult - offlineErr error - history []*v1.Release historyErr error } @@ -66,13 +67,6 @@ func (s *stubHelmClient) RenderManifests(context.Context, *spec.ResolvedSpec, po return s.renderResult, func() {}, nil } -func (s *stubHelmClient) RenderOffline(context.Context, *spec.ResolvedSpec, postrenderer.PostRenderer, []extras.RawFile) (*render.RenderResult, func(), error) { - if s.offlineErr != nil { - return nil, nil, s.offlineErr - } - return s.offlineResult, func() {}, nil -} - func (s *stubHelmClient) GetReleaseHistory(context.Context, string, string) ([]*v1.Release, error) { if s.historyErr != nil { return nil, s.historyErr @@ -202,11 +196,11 @@ func renderResult(manifest string) *render.RenderResult { } } -// TestRunOnline covers the common online plan paths: fresh install, image +// TestExecutePlan covers the common plan paths: fresh install, image // bump, add/remove, no-op, detailed-exitcode, secret masking, and failed- -// latest-revision warnings. Drift stream discipline and --offline keep -// their own tests because they need different IO / session wiring. -func TestRunOnline(t *testing.T) { +// latest-revision warnings. Drift stream discipline keeps its own test +// because it needs different IO wiring. +func TestExecutePlan(t *testing.T) { t.Parallel() tests := []struct { @@ -303,7 +297,7 @@ func TestRunOnline(t *testing.T) { opts := testOptions() opts.DetailedExitCode = tt.detailed - err := runOnline(h.Context, sess, nil, testManifest(), opts, testResolved(nil)) + err := executePlan(h.Context, sess, nil, testManifest(), opts, testResolved(nil)) if tt.wantErrIs != nil { require.Error(t, err) assert.ErrorIs(t, err, tt.wantErrIs) @@ -321,10 +315,10 @@ func TestRunOnline(t *testing.T) { } } -// TestRunOnline_DriftOnFreshInstall_NoStdoutFootprint verifies --drift on a +// TestExecutePlan_DriftOnFreshInstall_NoStdoutFootprint verifies --drift on a // fresh install prints its explanation to stderr (via checkDrift's // c.Info), not stdout, and never touches the cluster's REST config. -func TestRunOnline_DriftOnFreshInstall_NoStdoutFootprint(t *testing.T) { +func TestExecutePlan_DriftOnFreshInstall_NoStdoutFootprint(t *testing.T) { t.Parallel() stub := &stubHelmClient{ historyErr: helm.ErrReleaseNotFound, @@ -336,7 +330,7 @@ func TestRunOnline_DriftOnFreshInstall_NoStdoutFootprint(t *testing.T) { opts := testOptions() opts.Drift = true - err := runOnline(h.Context, sess, nil, testManifest(), opts, testResolved(nil)) + err := executePlan(h.Context, sess, nil, testManifest(), opts, testResolved(nil)) require.NoError(t, err, "checkDrift must short-circuit cleanly without a working cluster config") assert.NotContains(t, h.Stdout.String(), "Drift (cluster changed outside deployah):", @@ -347,26 +341,6 @@ func TestRunOnline_DriftOnFreshInstall_NoStdoutFootprint(t *testing.T) { "the explanation belongs on stderr, via c.Info") } -// TestRunOffline_RendersResourceCount verifies --offline reports a resource -// count instead of a diff and never contacts release history. -func TestRunOffline_RendersResourceCount(t *testing.T) { - t.Parallel() - stub := &stubHelmClient{ - offlineResult: renderResult(deploymentV1 + "---\n" + configMap), - } - sess := sessionWithStub(stub) - h := nabatctx.New(t, "test") - h.Context.SetContext(session.WithContext(h.Context.Context(), sess)) - - opts := testOptions() - opts.Offline = true - err := runOffline(h.Context, sess, nil, testManifest(), opts, nil) - - require.NoError(t, err) - assert.Contains(t, h.Stdout.String(), "Rendered 2 resources for environment 'production' (no cluster comparison).") - assert.Contains(t, h.Stdout.String(), "validation: OK") -} - func writePlanExtras(t *testing.T, dir, relative, content string) { t.Helper() path := filepath.Join(dir, relative) @@ -379,13 +353,13 @@ func writePlanCRDFile(t *testing.T, dir, name, body string) { writePlanExtras(t, dir, filepath.Join(".deployah", "crds", name), body) } -func TestRunOffline_LoadExtrasError(t *testing.T) { +func TestExecutePlan_LoadExtrasError(t *testing.T) { t.Parallel() dir := t.TempDir() specPath := filepath.Join(dir, "deployah.yaml") writePlanExtras(t, dir, "deployah.yaml", "apiVersion: deployah.dev/v1-alpha.4\nproject: web\n") writePlanExtras(t, dir, ".deployah/manifests/bad.yaml", "not: [valid") - stub := &stubHelmClient{offlineResult: renderResult(deploymentV1)} + stub := &stubHelmClient{renderResult: renderResult(deploymentV1)} sess := session.New( session.WithSpecPath(specPath), session.WithHelmFactory(func(*target.Target, session.HelmConfig) (session.HelmClient, error) { @@ -393,39 +367,15 @@ func TestRunOffline_LoadExtrasError(t *testing.T) { }), ) h := nabatctx.New(t, "test") - opts := testOptions() - opts.Offline = true - err := runOffline(h.Context, sess, nil, testManifest(), opts, nil) + err := executePlan(h.Context, sess, nil, testManifest(), testOptions(), testResolved(nil)) require.Error(t, err) - assert.Contains(t, err.Error(), "load extras") + assert.ErrorContains(t, err, "load extras") } const planCRDBody = "kind: CustomResourceDefinition\nmetadata:\n name: widgets.example.com\n" -func TestRunOffline_PrintsCRDs(t *testing.T) { - t.Parallel() - dir := t.TempDir() - specPath := filepath.Join(dir, "deployah.yaml") - writePlanExtras(t, dir, "deployah.yaml", "apiVersion: deployah.dev/v1-alpha.4\nproject: web\n") - writePlanCRDFile(t, dir, "widget.yaml", planCRDBody) - stub := &stubHelmClient{offlineResult: renderResult(deploymentV1)} - sess := session.New( - session.WithSpecPath(specPath), - session.WithHelmFactory(func(*target.Target, session.HelmConfig) (session.HelmClient, error) { - return stub, nil - }), - ) - h := nabatctx.New(t, "test") - opts := testOptions() - opts.Offline = true - - err := runOffline(h.Context, sess, nil, testManifest(), opts, nil) - require.NoError(t, err) - assert.Contains(t, h.Stdout.String(), "CustomResourceDefinition/widgets.example.com") -} - -func TestRunOnline_PrintsCRDs(t *testing.T) { +func TestExecutePlan_PrintsCRDs(t *testing.T) { t.Parallel() dir := t.TempDir() specPath := filepath.Join(dir, "deployah.yaml") @@ -444,7 +394,7 @@ func TestRunOnline_PrintsCRDs(t *testing.T) { h := nabatctx.New(t, "test") h.Context.SetContext(session.WithContext(h.Context.Context(), sess)) - err := runOnline(h.Context, sess, nil, testManifest(), testOptions(), testResolved(nil)) + err := executePlan(h.Context, sess, nil, testManifest(), testOptions(), testResolved(nil)) require.NoError(t, err) got := h.Stdout.String() assert.Contains(t, got, "+ CustomResourceDefinition/widgets.example.com") @@ -453,7 +403,7 @@ func TestRunOnline_PrintsCRDs(t *testing.T) { assert.NotContains(t, got, "CRD files to process") } -func TestRunOnline_PrintsCRDsOnUpgrade(t *testing.T) { +func TestExecutePlan_PrintsCRDsOnUpgrade(t *testing.T) { t.Parallel() tests := []struct { name string @@ -498,7 +448,7 @@ func TestRunOnline_PrintsCRDsOnUpgrade(t *testing.T) { h := nabatctx.New(t, "test") h.Context.SetContext(session.WithContext(h.Context.Context(), sess)) - err := runOnline(h.Context, sess, nil, testManifest(), testOptions(), testResolved(nil)) + err := executePlan(h.Context, sess, nil, testManifest(), testOptions(), testResolved(nil)) require.NoError(t, err) got := h.Stdout.String() assert.Contains(t, got, "CustomResourceDefinition/widgets.example.com") @@ -509,7 +459,7 @@ func TestRunOnline_PrintsCRDsOnUpgrade(t *testing.T) { } } -func TestRunOnline_JSONStdoutUnmarshalsWithCRDs(t *testing.T) { +func TestExecutePlan_JSONStdoutUnmarshalsWithCRDs(t *testing.T) { t.Parallel() tests := []struct { name string @@ -546,7 +496,7 @@ func TestRunOnline_JSONStdoutUnmarshalsWithCRDs(t *testing.T) { opts := testOptions() opts.OutputFormat = outputFormatJSON - err := runOnline(h.Context, sess, nil, testManifest(), opts, testResolved(nil)) + err := executePlan(h.Context, sess, nil, testManifest(), opts, testResolved(nil)) require.NoError(t, err) stdout := h.Stdout.String() var doc map[string]any @@ -603,7 +553,7 @@ func TestOutputPlan_JSONSkipCRDsStdoutUnmarshals(t *testing.T) { assert.NotContains(t, entry, "api_version") } -func TestRunOnline_DetailedExitCode_ChartCRDs(t *testing.T) { +func TestExecutePlan_DetailedExitCode_ChartCRDs(t *testing.T) { t.Parallel() tests := []struct { name string @@ -642,7 +592,7 @@ func TestRunOnline_DetailedExitCode_ChartCRDs(t *testing.T) { h.Context.SetContext(session.WithContext(h.Context.Context(), sess)) opts := testOptions() opts.DetailedExitCode = true - err := runOnline(h.Context, sess, nil, testManifest(), opts, testResolved(nil)) + err := executePlan(h.Context, sess, nil, testManifest(), opts, testResolved(nil)) if tc.wantErrIs != nil { require.ErrorIs(t, err, tc.wantErrIs) return @@ -651,3 +601,75 @@ func TestRunOnline_DetailedExitCode_ChartCRDs(t *testing.T) { }) } } + +// TestPlanFlags checks that plan help lists the remaining flags and that +// --offline is rejected before any command setup runs. +func TestPlanFlags(t *testing.T) { + t.Parallel() + + h := nabatctx.New(t, "deployah") + Register(h.App) + err := nabattest.Run(t, h.App, []string{"plan", "--help"}) + require.NoError(t, err) + help := h.Stdout.String() + h.Stderr.String() + + helpTests := []struct { + text string + want bool + }{ + {text: "--drift", want: true}, + {text: "--raw", want: true}, + {text: "--yaml", want: true}, + {text: "--output", want: true}, + {text: "--show-secrets", want: true}, + {text: "--detailed-exitcode", want: true}, + {text: "--offline", want: false}, + {text: "not compatible", want: false}, + {text: "without contacting the cluster", want: false}, + } + for _, tt := range helpTests { + t.Run(tt.text, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, strings.Contains(help, tt.text)) + }) + } + + runTests := []struct { + name string + args []string + prerunErr error + wantErr string + wantPreRun bool + }{ + { + name: "unknown offline flag", + args: []string{"plan", "prod", "--offline"}, + wantErr: "unknown flag: --offline", + wantPreRun: false, + }, + { + name: "recognized plan reaches prerun", + args: []string{"plan", "prod"}, + prerunErr: errors.New("stop after prerun"), + wantErr: "stop after prerun", + wantPreRun: true, + }, + } + for _, tt := range runTests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + preRunCalled := false + app := nabatctx.New(t, "deployah") + require.NoError(t, app.App.OnPreRun(func(*nabat.Context) error { + preRunCalled = true + return tt.prerunErr + })) + Register(app.App) + runErr := nabattest.Run(t, app.App, tt.args) + require.Error(t, runErr) + assert.ErrorContains(t, runErr, tt.wantErr) + assert.Equal(t, tt.wantPreRun, preRunCalled) + assert.Empty(t, app.Stdout.String()) + }) + } +} diff --git a/internal/cmd/root_dir_test.go b/internal/cmd/root_dir_test.go index 995b257..819f328 100644 --- a/internal/cmd/root_dir_test.go +++ b/internal/cmd/root_dir_test.go @@ -53,8 +53,8 @@ func TestWithDirResolvesSpec(t *testing.T) { { name: "literal image without env file", dir: "literal", - args: []string{"plan", "dev", "--offline"}, - wantStdout: "validation: OK", + args: []string{"resolve", "dev"}, + wantStdout: "Environment: dev", }, } @@ -139,7 +139,7 @@ func TestResolveSpec_Error(t *testing.T) { }{ { name: "missing required substitution variable", - args: []string{"plan", "dev", "--offline"}, + args: []string{"plan", "dev"}, opts: []nabattest.RunOption{nabattest.WithDir(testpath.Dir(t, "testdata", "cwd", "missing-var"))}, wantErr: "variable", }, diff --git a/internal/extras/load.go b/internal/extras/load.go index 13007f5..3adcfb6 100644 --- a/internal/extras/load.go +++ b/internal/extras/load.go @@ -59,9 +59,8 @@ type LoadConfig struct { ReleaseNamespace string // Scope resolves namespaced vs cluster-scoped. Required. Scope ScopeResolver - // Offline is true when cluster discovery is unavailable (plan --offline - // or missing rest config). Unknown types are then allowed; scope - // defaults to namespaced. + // Offline is true when no REST config is available. Unknown types are + // then allowed and default to namespaced scope. Offline bool } diff --git a/internal/extras/load_test.go b/internal/extras/load_test.go index 81cca1b..2099d0c 100644 --- a/internal/extras/load_test.go +++ b/internal/extras/load_test.go @@ -596,8 +596,8 @@ metadata: assert.Contains(t, err.Error(), "install that API on the cluster first") } -// TestLoad_OfflineAllowsUnknownType lets plan --offline load custom -// resources without discovery (scope defaults to namespaced). +// TestLoad_OfflineAllowsUnknownType loads custom resources without +// discovery when Offline is set (scope defaults to namespaced). func TestLoad_OfflineAllowsUnknownType(t *testing.T) { t.Parallel() dir := t.TempDir() diff --git a/internal/helm/chart_crd_test.go b/internal/helm/chart_crd_test.go index 82c0a58..0831a2a 100644 --- a/internal/helm/chart_crd_test.go +++ b/internal/helm/chart_crd_test.go @@ -27,6 +27,8 @@ import ( "deployah.dev/deployah/internal/extras" "deployah.dev/deployah/internal/spec" + + chart "helm.sh/helm/v4/pkg/chart/v2" ) func resolvedForChartCRDs(t *testing.T) *spec.ResolvedSpec { @@ -408,7 +410,7 @@ func TestPrepareChart_MaterializeFailureRemovesCopyLeavesBacking(t *testing.T) { assert.Zero(t, leftovers, "failed materialization must RemoveAll the chart copy") } -func TestRenderOffline_ChartCRDObjectsMatchPassedFiles(t *testing.T) { +func TestPrepareAndLoadChart_CRDObjectsMatchPassedFiles(t *testing.T) { t.Parallel() client, err := NewClient(WithNamespace("default")) require.NoError(t, err) @@ -417,15 +419,33 @@ func TestRenderOffline_ChartCRDObjectsMatchPassedFiles(t *testing.T) { {Path: "widgets.yaml", Raw: []byte(widgetCRD("widgets.example.com", ""))}, {Path: "types.yaml", Raw: []byte(widgetCRD("gadgets.example.com", ""))}, } - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, crds) + ch, chartPath, cleanup, err := client.prepareAndLoadChart(t.Context(), resolved, crds) if cleanup != nil { t.Cleanup(cleanup) } require.NoError(t, err) - require.NotEmpty(t, result.ChartPath) + require.NotEmpty(t, chartPath) - ch, err := loader.Load(result.ChartPath) - require.NoError(t, err) + loaded, loadErr := loader.Load(chartPath) + require.NoError(t, loadErr) + + charts := []struct { + name string + ch *chart.Chart + }{ + {name: "prepared", ch: ch}, + {name: "reloaded", ch: loaded}, + } + for _, tt := range charts { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assertPassedCRDFiles(t, tt.ch) + }) + } +} + +func assertPassedCRDFiles(t *testing.T, ch *chart.Chart) { + t.Helper() got := ch.CRDObjects() require.Len(t, got, 2) byName := make(map[string]string, len(got)) diff --git a/internal/helm/dest_test.go b/internal/helm/dest_test.go index 1b4c747..a188e7c 100644 --- a/internal/helm/dest_test.go +++ b/internal/helm/dest_test.go @@ -22,7 +22,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "deployah.dev/deployah/internal/spec" "deployah.dev/deployah/internal/target" diskcached "k8s.io/client-go/discovery/cached/disk" @@ -200,8 +199,6 @@ func TestNewClient_UnconfiguredGetterIgnoresAmbientKubeconfig(t *testing.T) { {name: "no getter", opts: []Option{WithNamespace("default")}}, {name: "nil getter", opts: []Option{WithRESTClientGetter(nil), WithNamespace("default")}}, } - m := envServiceSpec(t.TempDir(), spec.StringMap{"LOG_LEVEL": "debug"}) - resolved := resolveChart(t, m, "dev") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -215,36 +212,10 @@ func TestNewClient_UnconfiguredGetterIgnoresAmbientKubeconfig(t *testing.T) { err = client.IsReachable() require.Error(t, err) assert.ErrorIs(t, err, ErrDestinationNotConfigured) - - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } - require.NotNil(t, result) - assert.Equal(t, "default", result.Namespace) }) } } -func TestNewClient_OfflineRenderWithoutKubeconfig(t *testing.T) { - t.Setenv("KUBECONFIG", filepath.Join(t.TempDir(), "missing-kubeconfig")) - t.Setenv("HOME", t.TempDir()) - - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - - m := envServiceSpec(t.TempDir(), spec.StringMap{"LOG_LEVEL": "debug"}) - resolved := resolveChart(t, m, "dev") - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } - require.NotNil(t, result) - assert.Equal(t, "default", result.Namespace) -} - func writeDestKubeconfig(t *testing.T, content string) string { t.Helper() path := filepath.Join(t.TempDir(), "kubeconfig") diff --git a/internal/helm/generate_env_test.go b/internal/helm/generate_env_test.go index 23ca9ec..ff58b07 100644 --- a/internal/helm/generate_env_test.go +++ b/internal/helm/generate_env_test.go @@ -62,13 +62,7 @@ func TestRenderDeployment_EnvConfigMapAndOverlappingEnv(t *testing.T) { assert.Equal(t, map[string]string{"LOG_LEVEL": "info", "REGION": "eu"}, resolved.Components["api"].Runtime.FileValues) assert.Equal(t, map[string]string{"LOG_LEVEL": "debug", "NOTE": "{{ .Release.Name }}"}, resolved.Components["api"].Runtime.ExplicitValues) - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } + result := renderFreshInstall(t, resolved, nil) cm := findRenderedConfigMap(t, result.Manifest, "-api-env") assert.Equal(t, map[string]string{"LOG_LEVEL": "info", "REGION": "eu"}, cm.Data) @@ -305,13 +299,7 @@ func TestHelmHook_EnvConfigMapWeightAndDeletePolicy(t *testing.T) { assert.Equal(t, 0, resolved.Tasks["migrate"].HookWeight) assert.Equal(t, 1, resolved.Tasks["seed"].HookWeight) - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } + result := renderFreshInstall(t, resolved, nil) migrateCM := hookBySuffix(t, result, "ConfigMap", "-migrate-env") seedCM := hookBySuffix(t, result, "ConfigMap", "-seed-env") @@ -346,13 +334,7 @@ func TestHelmHook_EnvConfigMapWeightFollowsJob(t *testing.T) { rt.HookWeight = 4 resolved.Tasks["migrate"] = rt - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } + result := renderFreshInstall(t, resolved, nil) cm := hookBySuffix(t, result, "ConfigMap", "-migrate-env") job := hookBySuffix(t, result, "Job", "-migrate") @@ -426,13 +408,7 @@ func envServiceSpec(dir string, env spec.StringMap) *spec.Spec { func renderEnvManifest(t *testing.T, m *spec.Spec, env string) *render.RenderResult { t.Helper() resolved := resolveChart(t, m, env) - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } + result := renderFreshInstall(t, resolved, nil) return result } diff --git a/internal/helm/generate_monitor_test.go b/internal/helm/generate_monitor_test.go index 2d9a9a4..f0873fe 100644 --- a/internal/helm/generate_monitor_test.go +++ b/internal/helm/generate_monitor_test.go @@ -25,7 +25,7 @@ import ( "deployah.dev/deployah/internal/spec" ) -func TestRenderOffline_EnabledMetricsAlwaysRenderMonitors(t *testing.T) { +func TestRender_EnabledMetricsAlwaysRenderMonitors(t *testing.T) { t.Parallel() tests := []struct { @@ -94,13 +94,7 @@ func TestRenderOffline_EnabledMetricsAlwaysRenderMonitors(t *testing.T) { resolved, _, err := spec.Resolve(manifest, platform, spec.NormalizeEnv("dev"), spec.SubstitutionReport{}) require.NoError(t, err) - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } + result := renderFreshInstall(t, resolved, nil) kinds := renderedResourceKinds(t, result.Manifest) for _, kind := range tc.wantKinds { diff --git a/internal/helm/generate_schedule_test.go b/internal/helm/generate_schedule_test.go index c284a81..f459d26 100644 --- a/internal/helm/generate_schedule_test.go +++ b/internal/helm/generate_schedule_test.go @@ -340,11 +340,7 @@ func scheduledTaskNamed(name string) *spec.Spec { func renderScheduledCronJob(t *testing.T, manifest *spec.Spec, env, taskName string) *batchv1.CronJob { t.Helper() - result, cleanup, err := renderOfflineScheduled(t, manifest, env) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } + result := renderFreshInstall(t, resolveChart(t, manifest, env), nil) for _, h := range result.Hooks { if h != nil && strings.Contains(h.Manifest, "kind: CronJob") { t.Fatalf("scheduled CronJob must not be a Helm hook") @@ -353,19 +349,6 @@ func renderScheduledCronJob(t *testing.T, manifest *spec.Spec, env, taskName str return cronJobFromManifest(t, result.Manifest, taskName) } -func renderOfflineScheduled(t *testing.T, manifest *spec.Spec, env string) (*render.RenderResult, func(), error) { - t.Helper() - if manifest.SpecDir == "" { - manifest.SpecDir = t.TempDir() - } - require.NoError(t, spec.FillSpecWithDefaults(manifest, spec.CurrentManifestVersion)) - resolved, _, err := spec.Resolve(manifest, nil, spec.NormalizeEnv(env), spec.SubstitutionReport{}) - require.NoError(t, err) - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - return client.RenderOffline(t.Context(), resolved, nil, nil) -} - func mustRenderScheduled(t *testing.T, manifest *spec.Spec, env, releaseName, kubeVersion string) *render.RenderResult { t.Helper() result, err := renderScheduled(t, manifest, env, releaseName, kubeVersion) diff --git a/internal/helm/generate_task_test.go b/internal/helm/generate_task_test.go index 454f61c..b021e0e 100644 --- a/internal/helm/generate_task_test.go +++ b/internal/helm/generate_task_test.go @@ -549,14 +549,7 @@ func resolveChart(t *testing.T, manifest *spec.Spec, env string) *spec.ResolvedS func renderHookJob(t *testing.T, manifest *spec.Spec, env, taskName string) *batchv1.Job { t.Helper() resolved := resolveChart(t, manifest, env) - - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } + result := renderFreshInstall(t, resolved, nil) suffix := "-" + taskName for _, h := range result.Hooks { diff --git a/internal/helm/helm.go b/internal/helm/helm.go index a0fe917..11fd012 100644 --- a/internal/helm/helm.go +++ b/internal/helm/helm.go @@ -149,8 +149,8 @@ func NewClient(opts ...Option) (*Client, error) { return c, nil } -// Namespace returns the release namespace Helm will use for installs and -// offline renders. It comes from [WithNamespace], or "default" when empty. +// Namespace returns the release namespace configured for Helm operations. +// It comes from [WithNamespace], or "default" when empty. func (c *Client) Namespace() string { if c == nil || c.namespace == "" { return "default" diff --git a/internal/helm/identity_test.go b/internal/helm/identity_test.go index 4890577..d04b725 100644 --- a/internal/helm/identity_test.go +++ b/internal/helm/identity_test.go @@ -70,43 +70,30 @@ func TestReleaseIdentity_WildcardUsesMapKey(t *testing.T) { func TestReleaseIdentity_RejectsNil(t *testing.T) { t.Parallel() - _, _, err := releaseIdentity(nil) - require.Error(t, err) - assert.ErrorContains(t, err, "render requires resolved spec") - - _, _, err = releaseIdentity(&spec.ResolvedSpec{}) - require.Error(t, err) - assert.ErrorContains(t, err, "render requires resolved spec source") - - _, _, err = releaseIdentity(&spec.ResolvedSpec{ - Spec: &spec.Spec{Project: "shop"}, - }) - require.Error(t, err) - assert.ErrorContains(t, err, "spec.Resolve") -} - -func TestRenderOffline_RejectsNilResolved(t *testing.T) { - t.Parallel() - - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - - _, _, err = client.RenderOffline(t.Context(), nil, nil, nil) - require.Error(t, err) - assert.ErrorContains(t, err, "render requires resolved spec") - - _, _, err = client.RenderOffline(t.Context(), &spec.ResolvedSpec{}, nil, nil) - require.Error(t, err) - assert.ErrorContains(t, err, "render requires resolved spec source") - - _, _, err = client.RenderOffline(t.Context(), &spec.ResolvedSpec{ - Spec: &spec.Spec{Project: "shop"}, - }, nil, nil) - require.Error(t, err) - assert.ErrorContains(t, err, "spec.Resolve") + tests := []struct { + name string + resolved *spec.ResolvedSpec + wantErr string + }{ + {name: "nil", wantErr: "render requires resolved spec"}, + {name: "empty", resolved: &spec.ResolvedSpec{}, wantErr: "render requires resolved spec source"}, + { + name: "missing resolve", + resolved: &spec.ResolvedSpec{Spec: &spec.Spec{Project: "shop"}}, + wantErr: "spec.Resolve", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + _, _, err := releaseIdentity(tt.resolved) + require.Error(t, err) + assert.ErrorContains(t, err, tt.wantErr) + }) + } } -func TestRenderOffline_ChartAndReleaseAgree(t *testing.T) { +func TestRender_ChartAndReleaseAgree(t *testing.T) { t.Parallel() m := envServiceSpec(t.TempDir(), spec.StringMap{"LOG_LEVEL": "debug"}) @@ -117,13 +104,7 @@ func TestRenderOffline_ChartAndReleaseAgree(t *testing.T) { wantName, wantLabels, err := releaseIdentity(resolved) require.NoError(t, err) - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } + result := renderFreshInstall(t, resolved, nil) assert.Equal(t, wantName, result.ReleaseName) assert.Equal(t, GenerateReleaseName(resolved.Spec.Project, resolved.Env.Original), result.ReleaseName) @@ -161,7 +142,7 @@ func TestPrepareChart_UsesResolvedEnvironment(t *testing.T) { assert.Equal(t, "staging", labels[spec.LabelEnvironment]) } -func TestRenderOffline_WildcardEnvironmentLabels(t *testing.T) { +func TestRender_WildcardEnvironmentLabels(t *testing.T) { t.Parallel() m := envServiceSpec(t.TempDir(), spec.StringMap{"LOG_LEVEL": "debug"}) @@ -176,13 +157,7 @@ func TestRenderOffline_WildcardEnvironmentLabels(t *testing.T) { assert.Equal(t, "review", labels["deployah.dev/environment"]) require.Empty(t, validation.IsValidLabelValue(labels["deployah.dev/environment"])) - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) - require.NoError(t, err) - if cleanup != nil { - t.Cleanup(cleanup) - } + result := renderFreshInstall(t, resolved, nil) assert.Equal(t, name, result.ReleaseName) dep := findRenderedDeployment(t, result.Manifest, "-api") @@ -193,7 +168,7 @@ func TestRenderOffline_WildcardEnvironmentLabels(t *testing.T) { assert.Equal(t, name, dep.Labels[spec.LabelInstance]) } -func TestRenderOffline_WildcardHookJobLabels(t *testing.T) { +func TestRender_WildcardHookJobLabels(t *testing.T) { t.Parallel() m := taskSpec() @@ -206,7 +181,7 @@ func TestRenderOffline_WildcardHookJobLabels(t *testing.T) { assert.Equal(t, GenerateReleaseName("shop", "review/pr-123"), job.Labels[spec.LabelInstance]) } -func TestRenderOffline_WildcardCronJobLabels(t *testing.T) { +func TestRender_WildcardCronJobLabels(t *testing.T) { t.Parallel() m := scheduledRenderSpec(spec.Task{ @@ -233,20 +208,27 @@ func assertLogicalEnvironmentLabel(t *testing.T, labels map[string]string) { func TestInstallApp_RejectsNilResolved(t *testing.T) { t.Parallel() - client, err := NewClient(WithNamespace("default")) - require.NoError(t, err) - - err = client.InstallApp(t.Context(), false, nil, nil, nil, false) - require.Error(t, err) - assert.ErrorContains(t, err, "render requires resolved spec") - - err = client.InstallApp(t.Context(), false, &spec.ResolvedSpec{}, nil, nil, false) - require.Error(t, err) - assert.ErrorContains(t, err, "render requires resolved spec source") - - err = client.InstallApp(t.Context(), false, &spec.ResolvedSpec{ - Spec: &spec.Spec{Project: "shop"}, - }, nil, nil, false) - require.Error(t, err) - assert.ErrorContains(t, err, "spec.Resolve") + tests := []struct { + name string + resolved *spec.ResolvedSpec + wantErr string + }{ + {name: "nil", wantErr: "render requires resolved spec"}, + {name: "empty", resolved: &spec.ResolvedSpec{}, wantErr: "render requires resolved spec source"}, + { + name: "missing resolve", + resolved: &spec.ResolvedSpec{Spec: &spec.Spec{Project: "shop"}}, + wantErr: "spec.Resolve", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + client, err := NewClient(WithNamespace("default")) + require.NoError(t, err) + err = client.InstallApp(t.Context(), false, tt.resolved, nil, nil, false) + require.Error(t, err) + assert.ErrorContains(t, err, tt.wantErr) + }) + } } diff --git a/internal/helm/render.go b/internal/helm/render.go index 1bb6985..2444ea0 100644 --- a/internal/helm/render.go +++ b/internal/helm/render.go @@ -32,8 +32,7 @@ import ( ) // RenderManifests renders the chart from [spec.ResolvedSpec] client-side. -// The cluster must be reachable. Use [Client.RenderOffline] when there is -// no Kubernetes API access. Callers must run the returned cleanup func. +// The cluster must be reachable. Callers must run the returned cleanup func. func (c *Client) RenderManifests(ctx context.Context, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*render.RenderResult, func(), error) { result, _, cleanup, err := c.RenderManifestsWithPrep(ctx, resolved, postRenderer, crds) return result, cleanup, err @@ -77,31 +76,6 @@ func (c *Client) RenderManifestsWithPrep(ctx context.Context, resolved *spec.Res return result, prep, cleanup, nil } -// RenderOffline renders the chart from [spec.ResolvedSpec] as a fresh -// install without Kubernetes API access. A nil or unresolved spec is an -// error. Callers must run the returned cleanup func. -func (c *Client) RenderOffline(ctx context.Context, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (result *render.RenderResult, cleanup func(), err error) { - releaseName, labels, err := releaseIdentity(resolved) - if err != nil { - return nil, nil, err - } - - ch, chartPath, cleanup, err := c.prepareAndLoadChart(ctx, resolved, crds) - if err != nil { - return nil, nil, err - } - - values := map[string]any{} - - result, err = c.renderInstall(ctx, releaseName, ch, values, labels, postRenderer) - if err != nil { - cleanup() - return nil, nil, err - } - result.ChartPath = chartPath - return result, cleanup, nil -} - // prepareAndLoadChart generates or caches the Helm chart from resolved // and loads it. crds are written into the returned copy. Cleanup removes // the temp dir unless WithDebug(true). diff --git a/internal/helm/render_test.go b/internal/helm/render_test.go index 0b63129..6b5efbd 100644 --- a/internal/helm/render_test.go +++ b/internal/helm/render_test.go @@ -23,10 +23,31 @@ import ( "helm.sh/helm/v4/pkg/storage" "helm.sh/helm/v4/pkg/storage/driver" + "deployah.dev/deployah/internal/extras" + "deployah.dev/deployah/internal/render" + "deployah.dev/deployah/internal/spec" + chartcommon "helm.sh/helm/v4/pkg/chart/common" kubefake "helm.sh/helm/v4/pkg/kube/fake" ) +// renderFreshInstall renders resolved as a client-side fresh install in +// namespace "default" without release lookup or Kubernetes access. +func renderFreshInstall(t *testing.T, resolved *spec.ResolvedSpec, crds []extras.RawFile) *render.RenderResult { + t.Helper() + client, err := NewClient(WithNamespace("default")) + require.NoError(t, err) + releaseName, labels, err := releaseIdentity(resolved) + require.NoError(t, err) + ch, chartPath, cleanup, err := client.prepareAndLoadChart(t.Context(), resolved, crds) + require.NoError(t, err) + t.Cleanup(cleanup) + result, err := client.renderInstall(t.Context(), releaseName, ch, map[string]any{}, labels, nil) + require.NoError(t, err) + result.ChartPath = chartPath + return result +} + // newTestConfiguration builds an *action.Configuration with an in-memory // release store and a distinguishable "real" KubeClient/Capabilities pair, // so restore behavior can be verified without a real cluster. diff --git a/internal/k8s/tlscert.go b/internal/k8s/tlscert.go index e141d7c..5e554df 100644 --- a/internal/k8s/tlscert.go +++ b/internal/k8s/tlscert.go @@ -102,8 +102,9 @@ func reusableCert(secret *corev1.Secret, fqdn string) (ok bool, certPEM, keyPEM } // GenerateSelfSignedCert generates a fresh self-signed PEM cert/key pair for -// fqdn without any cluster access. Used for the plan --offline render, where -// there is no cluster to fetch a reusable secret from. +// fqdn without any cluster access. [EnsureSelfSignedCert] uses it when no +// reusable Secret exists, and [MaterializeSelfSignedTLS] uses it when given +// a nil client. func GenerateSelfSignedCert(fqdn string) (certPEM, keyPEM []byte, err error) { certPEM, keyPEM, err = certutil.GenerateSelfSignedCertKeyWithOptions(certutil.SelfSignedCertKeyOptions{ Host: fqdn, @@ -136,8 +137,7 @@ func HasSelfSignedComponents(resolved *spec.ResolvedSpec) bool { // component whose TLSMode is selfSigned. Call it once per CLI invocation, // before any chart render, so the plan render, apply-time verification // render, and real apply all see identical certificate bytes. Pass a nil -// client to force offline generation (no cluster access), as plan --offline -// does. +// client to generate fresh certificates without cluster access. func MaterializeSelfSignedTLS(ctx context.Context, client kubernetes.Interface, namespace string, resolved *spec.ResolvedSpec) error { if resolved == nil { return nil diff --git a/internal/plan/resource.go b/internal/plan/resource.go index 3b1555c..b658b5e 100644 --- a/internal/plan/resource.go +++ b/internal/plan/resource.go @@ -93,18 +93,6 @@ func parseResources(manifest string) ([]resourceDoc, error) { return out, nil } -// CountResources reports how many Kubernetes resources a rendered manifest -// contains, using the same parsing [ComputeDiff] uses. `deployah plan -// --offline` has no prior release to diff against, so it reports this count -// instead of a per-resource change list. -func CountResources(manifest string) (int, error) { - docs, err := parseResources(manifest) - if err != nil { - return 0, err - } - return len(docs), nil -} - // ResourceYAML is one Kubernetes resource extracted from a rendered // manifest, re-encoded as a standalone single-document YAML string. type ResourceYAML struct { diff --git a/internal/plan/view/human_semantic_test.go b/internal/plan/view/human_semantic_test.go index 279f154..c6e2322 100644 --- a/internal/plan/view/human_semantic_test.go +++ b/internal/plan/view/human_semantic_test.go @@ -37,6 +37,7 @@ import ( "deployah.dev/deployah/internal/predict" "deployah.dev/deployah/internal/render" "deployah.dev/deployah/internal/spec" + "deployah.dev/deployah/internal/testing/helmfixture" v1 "helm.sh/helm/v4/pkg/release/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -49,7 +50,7 @@ const ( ) // TestWriteHuman_SemanticPlan is the canonical human output of -// RenderOffline -> prediction -> BuildSemanticPlan -> WriteHuman. +// fixture render -> prediction -> BuildSemanticPlan -> WriteHuman. // human_all_actions.golden stays the synthetic generic renderer contract. func TestWriteHuman_SemanticPlan(t *testing.T) { t.Parallel() @@ -256,9 +257,8 @@ func scheduleTask(schedule string, command ...string) spec.Task { func semanticUpgradePlan(t *testing.T, previous, current *spec.Spec) semantic.Plan { t.Helper() - client := productHelmClient(t) prevResolved := resolveProductSpec(t, previous) - prevResult := renderOffline(t, client, prevResolved) + prevResult := renderDesired(t, prevResolved) prevRelease := previousReleaseFromRender(t, prevResult) cluster := newFakeCluster() @@ -270,9 +270,9 @@ func semanticUpgradePlan(t *testing.T, previous, current *spec.Spec) semantic.Pl storePredictedLive(cluster, installPlan.Changes) currResolved := resolveProductSpec(t, current) - currResult := renderOffline(t, client, currResolved) + currResult := renderDesired(t, currResolved) return buildSemanticPlan(t, &fakeBuildClient{ - result: offlineUpgradeResult(currResult, 2), + result: upgradeResultFrom(currResult, 2), prep: helm.ReleasePrep{ Operation: helm.OperationUpgrade, Current: prevRelease, @@ -282,13 +282,6 @@ func semanticUpgradePlan(t *testing.T, previous, current *spec.Spec) semantic.Pl }, cluster, currResolved) } -func productHelmClient(t *testing.T) *helm.Client { - t.Helper() - client, err := helm.NewClient(helm.WithNamespace(productNamespace)) - require.NoError(t, err) - return client -} - func resolveProductSpec(t *testing.T, manifest *spec.Spec) *spec.ResolvedSpec { t.Helper() if manifest.SpecDir == "" { @@ -300,23 +293,20 @@ func resolveProductSpec(t *testing.T, manifest *spec.Spec) *spec.ResolvedSpec { return resolved } -// offlineUpgradeResult copies an offline install render and sets the +// upgradeResultFrom copies a fresh-install fixture render and sets the // upgrade flags [plan.BuildSemanticPlan] checks. Manifest and Hooks stay -// the [helm.Client.RenderOffline] output; that API cannot emit an upgrade -// dry-run. -func offlineUpgradeResult(result *render.RenderResult, revision int) *render.RenderResult { +// the fresh-install fixture render. +func upgradeResultFrom(result *render.RenderResult, revision int) *render.RenderResult { out := *result out.IsUpgrade = true out.Revision = revision return &out } -func renderOffline(t *testing.T, client *helm.Client, resolved *spec.ResolvedSpec) *render.RenderResult { +func renderDesired(t *testing.T, resolved *spec.ResolvedSpec) *render.RenderResult { t.Helper() - result, cleanup, err := client.RenderOffline(t.Context(), resolved, nil, nil) + result, err := helmfixture.Render(t, productNamespace, resolved, nil, nil) require.NoError(t, err) - require.NotNil(t, cleanup) - t.Cleanup(cleanup) return result } diff --git a/internal/session/interfaces.go b/internal/session/interfaces.go index f4458ea..4198b74 100644 --- a/internal/session/interfaces.go +++ b/internal/session/interfaces.go @@ -50,13 +50,6 @@ type HelmClient interface { // postRenderer, when non-nil, is applied to the rendered manifests. RenderManifests(ctx context.Context, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*render.RenderResult, func(), error) - // RenderOffline renders the chart from [spec.ResolvedSpec] as a fresh - // install, without any Kubernetes API access. The caller must run the - // returned cleanup func once done with the result's ChartPath. - // crds are written into the per-invocation chart copy under crds/. - // postRenderer, when non-nil, is applied to the rendered manifests. - RenderOffline(ctx context.Context, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*render.RenderResult, func(), error) - // DeleteRelease uninstalls a Helm release. When wait is true the call // blocks until all resources are fully removed using the legacy polling // strategy with foreground cascade deletion. diff --git a/internal/session/session_test.go b/internal/session/session_test.go index 6db036e..9b797f2 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -101,26 +101,6 @@ func (m *MockHelmClient) RenderManifests(ctx context.Context, resolved *spec.Res return result, cleanup, nil } -// RenderOffline implements [HelmClient]. -func (m *MockHelmClient) RenderOffline(ctx context.Context, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*render.RenderResult, func(), error) { - args := m.Called(ctx, resolved, postRenderer, crds) - if err := args.Error(2); err != nil { - return nil, func() {}, err - } - if args.Get(0) == nil { - return nil, func() {}, errors.New("mock: render result not set") - } - result, ok := args.Get(0).(*render.RenderResult) - if !ok { - return nil, func() {}, fmt.Errorf("unexpected mock return type %T", args.Get(0)) - } - cleanup, cleanupOK := args.Get(1).(func()) - if !cleanupOK || cleanup == nil { - cleanup = func() {} - } - return result, cleanup, nil -} - // DeleteRelease implements [HelmClient]. func (m *MockHelmClient) DeleteRelease(ctx context.Context, project, environment string, wait bool) error { args := m.Called(ctx, project, environment, wait) diff --git a/internal/testing/doc.go b/internal/testing/doc.go index 6c3de74..c073a83 100644 --- a/internal/testing/doc.go +++ b/internal/testing/doc.go @@ -18,5 +18,6 @@ // files, expected Kubernetes output, and optional e2e.yaml Kind fixtures. // [DiscoverScenarios] finds them; [IntegrationTestSuite] loads a spec, // generates a chart, renders templates, and compares results to golden -// files. [LoadE2EFixture] decodes e2e.yaml. +// files. [LoadE2EFixture] decodes e2e.yaml. Fixture charts are rendered by +// the helmfixture subpackage. package testing diff --git a/internal/testing/helmfixture/render.go b/internal/testing/helmfixture/render.go new file mode 100644 index 0000000..6df8ec0 --- /dev/null +++ b/internal/testing/helmfixture/render.go @@ -0,0 +1,88 @@ +// Copyright 2025 The Deployah Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package helmfixture is test infrastructure for rendering charts without +// Kubernetes access. +package helmfixture + +import ( + "fmt" + "os" + "testing" + "time" + + "helm.sh/helm/v4/pkg/action" + "helm.sh/helm/v4/pkg/chart/v2/loader" + "helm.sh/helm/v4/pkg/postrenderer" + + "deployah.dev/deployah/internal/extras" + "deployah.dev/deployah/internal/helm" + "deployah.dev/deployah/internal/render" + "deployah.dev/deployah/internal/spec" + + v1 "helm.sh/helm/v4/pkg/release/v1" +) + +// chartCacheTTL is how long a prepared chart may be reused during one test. +const chartCacheTTL = time.Hour + +// Render renders resolved as a fresh Helm install without Kubernetes access, +// for test fixtures. The chart comes from [helm.PrepareChart]; Helm renders +// it with a client-only install dry run in namespace. Cleanup removes the +// per-render chart copy when tb finishes. The cache backing directory stays +// in the temp dir. +func Render(tb testing.TB, namespace string, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*render.RenderResult, error) { + tb.Helper() + + chartPath, err := helm.PrepareChart(tb.Context(), resolved, helm.NewChartCache(chartCacheTTL), crds) + if err != nil { + return nil, fmt.Errorf("prepare chart: %w", err) + } + tb.Cleanup(func() { + if removeErr := os.RemoveAll(chartPath); removeErr != nil { + tb.Errorf("remove chart dir %s: %v", chartPath, removeErr) + } + }) + + ch, err := loader.Load(chartPath) + if err != nil { + return nil, fmt.Errorf("load chart: %w", err) + } + + releaseName := helm.GenerateReleaseName(resolved.Spec.Project, resolved.Env.Original) + install := action.NewInstall(action.NewConfiguration()) + install.ReleaseName = releaseName + install.Namespace = namespace + install.DryRunStrategy = action.DryRunClient + install.DisableOpenAPIValidation = true + install.PostRenderer = postRenderer + + rel, err := install.RunWithContext(tb.Context(), ch, map[string]any{}) + if err != nil { + return nil, fmt.Errorf("helm install dry run: %w", err) + } + v1rel, ok := rel.(*v1.Release) + if !ok { + return nil, fmt.Errorf("unexpected helm release type %T", rel) + } + return &render.RenderResult{ + ReleaseName: releaseName, + Namespace: install.Namespace, + Manifest: v1rel.Manifest, + Hooks: v1rel.Hooks, + IsUpgrade: false, + Revision: 1, + ChartPath: chartPath, + }, nil +} diff --git a/internal/testing/helmfixture/render_test.go b/internal/testing/helmfixture/render_test.go new file mode 100644 index 0000000..0bc13c8 --- /dev/null +++ b/internal/testing/helmfixture/render_test.go @@ -0,0 +1,86 @@ +// Copyright 2025 The Deployah Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package helmfixture_test + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "deployah.dev/deployah/internal/helm" + "deployah.dev/deployah/internal/spec" + "deployah.dev/deployah/internal/testing/helmfixture" +) + +func TestRender_FreshInstall(t *testing.T) { + t.Parallel() + + manifest := &spec.Spec{ + APIVersion: spec.CurrentManifestVersion, + Project: "shop", + SpecDir: t.TempDir(), + Environments: map[string]spec.Environment{ + "dev": {}, + }, + Components: map[string]spec.Component{ + "api": { + Role: spec.ComponentRoleService, + Image: "ghcr.io/acme/shop:1.2.3", + Port: 8080, + }, + }, + } + require.NoError(t, spec.FillSpecWithDefaults(manifest, spec.CurrentManifestVersion)) + resolved, _, err := spec.Resolve(manifest, nil, spec.NormalizeEnv("dev"), spec.SubstitutionReport{}) + require.NoError(t, err) + + result, err := helmfixture.Render(t, "default", resolved, nil, nil) + require.NoError(t, err) + assert.Equal(t, helm.GenerateReleaseName("shop", "dev"), result.ReleaseName) + assert.Equal(t, "default", result.Namespace) + assert.False(t, result.IsUpgrade) + assert.Equal(t, 1, result.Revision) + _, statErr := os.Stat(result.ChartPath) + require.NoError(t, statErr) + assert.Contains(t, result.Manifest, "kind: Deployment") +} + +func TestRender_RejectsUnresolved(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + resolved *spec.ResolvedSpec + wantErr string + }{ + {name: "nil", wantErr: "render requires resolved spec"}, + {name: "empty", resolved: &spec.ResolvedSpec{}, wantErr: "render requires resolved spec source"}, + { + name: "missing resolve", + resolved: &spec.ResolvedSpec{Spec: &spec.Spec{Project: "shop"}}, + wantErr: "spec.Resolve", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + _, err := helmfixture.Render(t, "default", tt.resolved, nil, nil) + require.Error(t, err) + assert.ErrorContains(t, err, tt.wantErr) + }) + } +} diff --git a/internal/testing/plan_scenarios.go b/internal/testing/plan_scenarios.go index 1a4daa0..2f6f044 100644 --- a/internal/testing/plan_scenarios.go +++ b/internal/testing/plan_scenarios.go @@ -28,10 +28,10 @@ import ( "go.yaml.in/yaml/v3" "deployah.dev/deployah/internal/extras" - "deployah.dev/deployah/internal/helm" "deployah.dev/deployah/internal/k8s" "deployah.dev/deployah/internal/plan" "deployah.dev/deployah/internal/spec" + "deployah.dev/deployah/internal/testing/helmfixture" v1 "helm.sh/helm/v4/pkg/release/v1" ) @@ -216,7 +216,8 @@ func readRawManifestFile(t *testing.T, path string) (manifestSide, bool) { return manifestSide{Manifest: string(data)}, true } -// renderManifestFile loads dir/filename and renders it offline. +// renderManifestFile loads dir/filename and renders it without Kubernetes +// access. func renderManifestFile(t *testing.T, dir, filename string) manifestSide { t.Helper() ctx := t.Context() @@ -241,18 +242,10 @@ func renderManifestFile(t *testing.T, dir, filename string) manifestSide { require.NoError(t, resolveErr) require.NoError(t, k8s.MaterializeSelfSignedTLS(ctx, nil, "", resolved)) - // Pin the release namespace so goldens stay stable regardless of - // HELM_NAMESPACE or the ambient kubeconfig context. - client, err := helm.NewClient(helm.WithNamespace("default")) - require.NoError(t, err) - - bundle, loadErr := extras.LoadFromSpec(specPath, manifest, platform, envName, client.Namespace(), nil) + bundle, loadErr := extras.LoadFromSpec(specPath, manifest, platform, envName, fixtureNamespace, nil) require.NoError(t, loadErr) - result, cleanup, err := client.RenderOffline(ctx, resolved, bundle.PostRendererFor(), bundle.CRDs) - if cleanup != nil { - t.Cleanup(cleanup) - } + result, err := helmfixture.Render(t, fixtureNamespace, resolved, bundle.PostRendererFor(), bundle.CRDs) require.NoError(t, err) return manifestSide{ diff --git a/internal/testing/schema_validate.go b/internal/testing/schema_validate.go index 42ee04f..f608ad7 100644 --- a/internal/testing/schema_validate.go +++ b/internal/testing/schema_validate.go @@ -30,12 +30,10 @@ import ( // - ServiceMonitor, PodMonitor, PrometheusRule: Prometheus Operator CRDs. // - ClusterWidget, IdempotentWidget, LifecycleWidget: fixture CRDs used // by extras/e2e scenarios. -// - HorizontalPodAutoscaler: [helm.Client.RenderOffline] has no live -// cluster, so Capabilities.KubeVersion falls back to Helm's pre-1.23 -// default, making the chart select the removed autoscaling/v2beta1 API -// instead of autoscaling/v2. This affects `deployah plan --offline` for -// any autoscaling component; the real fix belongs in the render -// pipeline (a modern default KubeVersion), not here. +// - HorizontalPodAutoscaler: fixture rendering runs a Helm client-only +// install with no live cluster, so Capabilities.KubeVersion uses Helm's +// default and the chart picks the removed autoscaling/v2beta1 API +// instead of autoscaling/v2. var unregisteredSchemeKinds = []string{ "ServiceMonitor", "PodMonitor", diff --git a/internal/testing/types.go b/internal/testing/types.go index 824ec0e..cdf2c90 100644 --- a/internal/testing/types.go +++ b/internal/testing/types.go @@ -15,7 +15,6 @@ package testing import ( - "context" "errors" "fmt" "io" @@ -31,9 +30,9 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "deployah.dev/deployah/internal/extras" - "deployah.dev/deployah/internal/helm" "deployah.dev/deployah/internal/k8s" "deployah.dev/deployah/internal/spec" + "deployah.dev/deployah/internal/testing/helmfixture" yamlutil "k8s.io/apimachinery/pkg/util/yaml" ) @@ -41,6 +40,10 @@ import ( // TestScenariosDir is the directory containing integration test scenarios. var TestScenariosDir = getTestScenariosDir() +// fixtureNamespace is the release namespace used when rendering scenario +// charts, so goldens stay stable without a kubeconfig context. +const fixtureNamespace = "default" + // getTestScenariosDir finds the test scenarios directory func getTestScenariosDir() string { // Try different possible locations relative to the test file @@ -223,10 +226,10 @@ func (suite *IntegrationTestSuite) loadAndResolve(t *testing.T, scenario TestSce return nil, "", nil, platform, err } - // Materialize self-signed TLS certs offline (no cluster access), - // matching `deployah plan --offline`. Without this, a selfSigned - // expose scenario fails render with "certificate not materialized - // before render". + // Materialize self-signed TLS certs without cluster access so selfSigned + // expose scenarios can render. Without this, a selfSigned expose + // scenario fails render with "certificate not materialized before + // render". if tlsErr := k8s.MaterializeSelfSignedTLS(ctx, nil, "", resolved); tlsErr != nil { return nil, "", nil, platform, tlsErr } @@ -234,31 +237,20 @@ func (suite *IntegrationTestSuite) loadAndResolve(t *testing.T, scenario TestSce return manifest, envName, resolved, platform, nil } -// renderChart renders manifest/environment through Helm's real template -// engine via [helm.Client.RenderOffline] (no cluster access, matching -// `deployah plan --offline`), returning the resulting Kubernetes objects. -// Extra manifests from .deployah/ under testDir are appended via the Helm +// renderChart renders fixture manifests without Kubernetes access via +// [helmfixture.Render], returning the resulting Kubernetes objects. Extra +// manifests from .deployah/ under testDir are appended via the Helm // post-renderer. func (suite *IntegrationTestSuite) renderChart(t *testing.T, testDir string, manifest *spec.Spec, environment string, resolved *spec.ResolvedSpec, platform *spec.PlatformConfig) ([]unstructured.Unstructured, error) { t.Helper() - // Pin the release namespace so goldens stay stable regardless of - // HELM_NAMESPACE or the ambient kubeconfig context. - client, err := helm.NewClient(helm.WithNamespace("default")) - if err != nil { - return nil, fmt.Errorf("create helm client: %w", err) - } - specPath := filepath.Join(testDir, "deployah.yaml") - bundle, loadErr := extras.LoadFromSpec(specPath, manifest, platform, environment, client.Namespace(), nil) + bundle, loadErr := extras.LoadFromSpec(specPath, manifest, platform, environment, fixtureNamespace, nil) if loadErr != nil { return nil, fmt.Errorf("load extras: %w", loadErr) } - result, cleanup, err := client.RenderOffline(context.Background(), resolved, bundle.PostRendererFor(), bundle.CRDs) - if cleanup != nil { - t.Cleanup(cleanup) - } + result, err := helmfixture.Render(t, fixtureNamespace, resolved, bundle.PostRendererFor(), bundle.CRDs) if err != nil { return nil, fmt.Errorf("render chart: %w", err) } From 3f73f90b439d7e84055670209e8e57cc8d437abd Mon Sep 17 00:00:00 2001 From: Mohammad Abdolirad Date: Sun, 27 Sep 2026 19:51:47 +0200 Subject: [PATCH 2/2] drop the helmfixture test package - render scenario fixtures from internal/testing - keep the semantic render local to the view tests - remove the deleted-flag test and fix the TLS comment --- internal/cmd/plan/plan_test.go | 76 ---------------- internal/k8s/tlscert.go | 8 +- internal/plan/view/human_semantic_test.go | 50 +++++++++-- internal/testing/doc.go | 3 +- .../render.go => fixture_render.go} | 27 +++--- internal/testing/helmfixture/render_test.go | 86 ------------------- internal/testing/plan_scenarios.go | 3 +- internal/testing/types.go | 10 +-- 8 files changed, 67 insertions(+), 196 deletions(-) rename internal/testing/{helmfixture/render.go => fixture_render.go} (69%) delete mode 100644 internal/testing/helmfixture/render_test.go diff --git a/internal/cmd/plan/plan_test.go b/internal/cmd/plan/plan_test.go index b2678f0..fd2a269 100644 --- a/internal/cmd/plan/plan_test.go +++ b/internal/cmd/plan/plan_test.go @@ -17,10 +17,8 @@ package plan import ( "context" "encoding/json" - "errors" "os" "path/filepath" - "strings" "testing" "time" @@ -29,8 +27,6 @@ import ( "helm.sh/helm/v4/pkg/postrenderer" "helm.sh/helm/v4/pkg/release/common" "k8s.io/apimachinery/pkg/labels" - "nabat.dev/nabat" - "nabat.dev/nabat/nabattest" "deployah.dev/deployah/internal/extras" "deployah.dev/deployah/internal/helm" @@ -601,75 +597,3 @@ func TestExecutePlan_DetailedExitCode_ChartCRDs(t *testing.T) { }) } } - -// TestPlanFlags checks that plan help lists the remaining flags and that -// --offline is rejected before any command setup runs. -func TestPlanFlags(t *testing.T) { - t.Parallel() - - h := nabatctx.New(t, "deployah") - Register(h.App) - err := nabattest.Run(t, h.App, []string{"plan", "--help"}) - require.NoError(t, err) - help := h.Stdout.String() + h.Stderr.String() - - helpTests := []struct { - text string - want bool - }{ - {text: "--drift", want: true}, - {text: "--raw", want: true}, - {text: "--yaml", want: true}, - {text: "--output", want: true}, - {text: "--show-secrets", want: true}, - {text: "--detailed-exitcode", want: true}, - {text: "--offline", want: false}, - {text: "not compatible", want: false}, - {text: "without contacting the cluster", want: false}, - } - for _, tt := range helpTests { - t.Run(tt.text, func(t *testing.T) { - t.Parallel() - assert.Equal(t, tt.want, strings.Contains(help, tt.text)) - }) - } - - runTests := []struct { - name string - args []string - prerunErr error - wantErr string - wantPreRun bool - }{ - { - name: "unknown offline flag", - args: []string{"plan", "prod", "--offline"}, - wantErr: "unknown flag: --offline", - wantPreRun: false, - }, - { - name: "recognized plan reaches prerun", - args: []string{"plan", "prod"}, - prerunErr: errors.New("stop after prerun"), - wantErr: "stop after prerun", - wantPreRun: true, - }, - } - for _, tt := range runTests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - preRunCalled := false - app := nabatctx.New(t, "deployah") - require.NoError(t, app.App.OnPreRun(func(*nabat.Context) error { - preRunCalled = true - return tt.prerunErr - })) - Register(app.App) - runErr := nabattest.Run(t, app.App, tt.args) - require.Error(t, runErr) - assert.ErrorContains(t, runErr, tt.wantErr) - assert.Equal(t, tt.wantPreRun, preRunCalled) - assert.Empty(t, app.Stdout.String()) - }) - } -} diff --git a/internal/k8s/tlscert.go b/internal/k8s/tlscert.go index 5e554df..50962b3 100644 --- a/internal/k8s/tlscert.go +++ b/internal/k8s/tlscert.go @@ -134,10 +134,10 @@ func HasSelfSignedComponents(resolved *spec.ResolvedSpec) bool { } // MaterializeSelfSignedTLS fills TLSCertPEM/TLSKeyPEM on every resolved -// component whose TLSMode is selfSigned. Call it once per CLI invocation, -// before any chart render, so the plan render, apply-time verification -// render, and real apply all see identical certificate bytes. Pass a nil -// client to generate fresh certificates without cluster access. +// component whose TLSMode is selfSigned. Call it once per CLI invocation +// before chart rendering so every render and any subsequent Helm operation +// in that invocation sees the same certificate bytes. Pass a nil client to +// generate fresh certificates without cluster access. func MaterializeSelfSignedTLS(ctx context.Context, client kubernetes.Interface, namespace string, resolved *spec.ResolvedSpec) error { if resolved == nil { return nil diff --git a/internal/plan/view/human_semantic_test.go b/internal/plan/view/human_semantic_test.go index c6e2322..0f71dbd 100644 --- a/internal/plan/view/human_semantic_test.go +++ b/internal/plan/view/human_semantic_test.go @@ -21,9 +21,12 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "helm.sh/helm/v4/pkg/action" + "helm.sh/helm/v4/pkg/chart/v2/loader" "helm.sh/helm/v4/pkg/postrenderer" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -37,7 +40,6 @@ import ( "deployah.dev/deployah/internal/predict" "deployah.dev/deployah/internal/render" "deployah.dev/deployah/internal/spec" - "deployah.dev/deployah/internal/testing/helmfixture" v1 "helm.sh/helm/v4/pkg/release/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -257,8 +259,9 @@ func scheduleTask(schedule string, command ...string) spec.Task { func semanticUpgradePlan(t *testing.T, previous, current *spec.Spec) semantic.Plan { t.Helper() + cache := helm.NewChartCache(time.Hour) prevResolved := resolveProductSpec(t, previous) - prevResult := renderDesired(t, prevResolved) + prevResult := renderDesired(t, cache, prevResolved) prevRelease := previousReleaseFromRender(t, prevResult) cluster := newFakeCluster() @@ -270,7 +273,7 @@ func semanticUpgradePlan(t *testing.T, previous, current *spec.Spec) semantic.Pl storePredictedLive(cluster, installPlan.Changes) currResolved := resolveProductSpec(t, current) - currResult := renderDesired(t, currResolved) + currResult := renderDesired(t, cache, currResolved) return buildSemanticPlan(t, &fakeBuildClient{ result: upgradeResultFrom(currResult, 2), prep: helm.ReleasePrep{ @@ -303,11 +306,46 @@ func upgradeResultFrom(result *render.RenderResult, revision int) *render.Render return &out } -func renderDesired(t *testing.T, resolved *spec.ResolvedSpec) *render.RenderResult { +// renderDesired renders resolved as a fresh install in productNamespace +// without Kubernetes access. cache is shared by the previous and current +// renders in one test. The chart comes from [helm.PrepareChart]; Helm +// renders it with a client-only install dry run. This stays in the view +// tests because internal/testing and this package both register -update. +func renderDesired(t *testing.T, cache *helm.ChartCache, resolved *spec.ResolvedSpec) *render.RenderResult { t.Helper() - result, err := helmfixture.Render(t, productNamespace, resolved, nil, nil) + + chartPath, err := helm.PrepareChart(t.Context(), resolved, cache, nil) + require.NoError(t, err) + t.Cleanup(func() { + if removeErr := os.RemoveAll(chartPath); removeErr != nil { + t.Errorf("remove chart dir %s: %v", chartPath, removeErr) + } + }) + + ch, err := loader.Load(chartPath) require.NoError(t, err) - return result + + releaseName := helm.GenerateReleaseName(resolved.Spec.Project, resolved.Env.Original) + install := action.NewInstall(action.NewConfiguration()) + install.ReleaseName = releaseName + install.Namespace = productNamespace + install.DryRunStrategy = action.DryRunClient + install.DisableOpenAPIValidation = true + + rel, err := install.RunWithContext(t.Context(), ch, map[string]any{}) + require.NoError(t, err) + v1rel, ok := rel.(*v1.Release) + require.True(t, ok, "unexpected helm release type %T", rel) + + return &render.RenderResult{ + ReleaseName: releaseName, + Namespace: install.Namespace, + Manifest: v1rel.Manifest, + Hooks: v1rel.Hooks, + IsUpgrade: false, + Revision: 1, + ChartPath: chartPath, + } } func previousReleaseFromRender(t *testing.T, result *render.RenderResult) *v1.Release { diff --git a/internal/testing/doc.go b/internal/testing/doc.go index c073a83..6c3de74 100644 --- a/internal/testing/doc.go +++ b/internal/testing/doc.go @@ -18,6 +18,5 @@ // files, expected Kubernetes output, and optional e2e.yaml Kind fixtures. // [DiscoverScenarios] finds them; [IntegrationTestSuite] loads a spec, // generates a chart, renders templates, and compares results to golden -// files. [LoadE2EFixture] decodes e2e.yaml. Fixture charts are rendered by -// the helmfixture subpackage. +// files. [LoadE2EFixture] decodes e2e.yaml. package testing diff --git a/internal/testing/helmfixture/render.go b/internal/testing/fixture_render.go similarity index 69% rename from internal/testing/helmfixture/render.go rename to internal/testing/fixture_render.go index 6df8ec0..7ee6101 100644 --- a/internal/testing/helmfixture/render.go +++ b/internal/testing/fixture_render.go @@ -1,4 +1,4 @@ -// Copyright 2025 The Deployah Authors +// Copyright 2026 The Deployah Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -12,9 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package helmfixture is test infrastructure for rendering charts without -// Kubernetes access. -package helmfixture +package testing import ( "fmt" @@ -34,18 +32,19 @@ import ( v1 "helm.sh/helm/v4/pkg/release/v1" ) -// chartCacheTTL is how long a prepared chart may be reused during one test. -const chartCacheTTL = time.Hour +// fixtureChartCache reuses prepared charts for the test process. +// [helm.PrepareChart] returns a per-render copy; callers delete only that +// copy. The cache keeps its backing directories. +var fixtureChartCache = helm.NewChartCache(time.Hour) -// Render renders resolved as a fresh Helm install without Kubernetes access, -// for test fixtures. The chart comes from [helm.PrepareChart]; Helm renders -// it with a client-only install dry run in namespace. Cleanup removes the -// per-render chart copy when tb finishes. The cache backing directory stays -// in the temp dir. -func Render(tb testing.TB, namespace string, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*render.RenderResult, error) { +// renderFixtureChart renders resolved as a fresh Helm install in +// [fixtureNamespace] without Kubernetes access. The chart comes from +// [helm.PrepareChart]. Helm renders it with a client-only install dry run. +// Cleanup removes the per-render chart copy when tb finishes. +func renderFixtureChart(tb testing.TB, resolved *spec.ResolvedSpec, postRenderer postrenderer.PostRenderer, crds []extras.RawFile) (*render.RenderResult, error) { tb.Helper() - chartPath, err := helm.PrepareChart(tb.Context(), resolved, helm.NewChartCache(chartCacheTTL), crds) + chartPath, err := helm.PrepareChart(tb.Context(), resolved, fixtureChartCache, crds) if err != nil { return nil, fmt.Errorf("prepare chart: %w", err) } @@ -63,7 +62,7 @@ func Render(tb testing.TB, namespace string, resolved *spec.ResolvedSpec, postRe releaseName := helm.GenerateReleaseName(resolved.Spec.Project, resolved.Env.Original) install := action.NewInstall(action.NewConfiguration()) install.ReleaseName = releaseName - install.Namespace = namespace + install.Namespace = fixtureNamespace install.DryRunStrategy = action.DryRunClient install.DisableOpenAPIValidation = true install.PostRenderer = postRenderer diff --git a/internal/testing/helmfixture/render_test.go b/internal/testing/helmfixture/render_test.go deleted file mode 100644 index 0bc13c8..0000000 --- a/internal/testing/helmfixture/render_test.go +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright 2025 The Deployah Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package helmfixture_test - -import ( - "os" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "deployah.dev/deployah/internal/helm" - "deployah.dev/deployah/internal/spec" - "deployah.dev/deployah/internal/testing/helmfixture" -) - -func TestRender_FreshInstall(t *testing.T) { - t.Parallel() - - manifest := &spec.Spec{ - APIVersion: spec.CurrentManifestVersion, - Project: "shop", - SpecDir: t.TempDir(), - Environments: map[string]spec.Environment{ - "dev": {}, - }, - Components: map[string]spec.Component{ - "api": { - Role: spec.ComponentRoleService, - Image: "ghcr.io/acme/shop:1.2.3", - Port: 8080, - }, - }, - } - require.NoError(t, spec.FillSpecWithDefaults(manifest, spec.CurrentManifestVersion)) - resolved, _, err := spec.Resolve(manifest, nil, spec.NormalizeEnv("dev"), spec.SubstitutionReport{}) - require.NoError(t, err) - - result, err := helmfixture.Render(t, "default", resolved, nil, nil) - require.NoError(t, err) - assert.Equal(t, helm.GenerateReleaseName("shop", "dev"), result.ReleaseName) - assert.Equal(t, "default", result.Namespace) - assert.False(t, result.IsUpgrade) - assert.Equal(t, 1, result.Revision) - _, statErr := os.Stat(result.ChartPath) - require.NoError(t, statErr) - assert.Contains(t, result.Manifest, "kind: Deployment") -} - -func TestRender_RejectsUnresolved(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - resolved *spec.ResolvedSpec - wantErr string - }{ - {name: "nil", wantErr: "render requires resolved spec"}, - {name: "empty", resolved: &spec.ResolvedSpec{}, wantErr: "render requires resolved spec source"}, - { - name: "missing resolve", - resolved: &spec.ResolvedSpec{Spec: &spec.Spec{Project: "shop"}}, - wantErr: "spec.Resolve", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - _, err := helmfixture.Render(t, "default", tt.resolved, nil, nil) - require.Error(t, err) - assert.ErrorContains(t, err, tt.wantErr) - }) - } -} diff --git a/internal/testing/plan_scenarios.go b/internal/testing/plan_scenarios.go index 2f6f044..5cc8f54 100644 --- a/internal/testing/plan_scenarios.go +++ b/internal/testing/plan_scenarios.go @@ -31,7 +31,6 @@ import ( "deployah.dev/deployah/internal/k8s" "deployah.dev/deployah/internal/plan" "deployah.dev/deployah/internal/spec" - "deployah.dev/deployah/internal/testing/helmfixture" v1 "helm.sh/helm/v4/pkg/release/v1" ) @@ -245,7 +244,7 @@ func renderManifestFile(t *testing.T, dir, filename string) manifestSide { bundle, loadErr := extras.LoadFromSpec(specPath, manifest, platform, envName, fixtureNamespace, nil) require.NoError(t, loadErr) - result, err := helmfixture.Render(t, fixtureNamespace, resolved, bundle.PostRendererFor(), bundle.CRDs) + result, err := renderFixtureChart(t, resolved, bundle.PostRendererFor(), bundle.CRDs) require.NoError(t, err) return manifestSide{ diff --git a/internal/testing/types.go b/internal/testing/types.go index cdf2c90..d0188fc 100644 --- a/internal/testing/types.go +++ b/internal/testing/types.go @@ -32,7 +32,6 @@ import ( "deployah.dev/deployah/internal/extras" "deployah.dev/deployah/internal/k8s" "deployah.dev/deployah/internal/spec" - "deployah.dev/deployah/internal/testing/helmfixture" yamlutil "k8s.io/apimachinery/pkg/util/yaml" ) @@ -237,10 +236,9 @@ func (suite *IntegrationTestSuite) loadAndResolve(t *testing.T, scenario TestSce return manifest, envName, resolved, platform, nil } -// renderChart renders fixture manifests without Kubernetes access via -// [helmfixture.Render], returning the resulting Kubernetes objects. Extra -// manifests from .deployah/ under testDir are appended via the Helm -// post-renderer. +// renderChart renders fixture manifests without Kubernetes access, +// returning the resulting Kubernetes objects. Extra manifests from +// .deployah/ under testDir are appended via the Helm post-renderer. func (suite *IntegrationTestSuite) renderChart(t *testing.T, testDir string, manifest *spec.Spec, environment string, resolved *spec.ResolvedSpec, platform *spec.PlatformConfig) ([]unstructured.Unstructured, error) { t.Helper() @@ -250,7 +248,7 @@ func (suite *IntegrationTestSuite) renderChart(t *testing.T, testDir string, man return nil, fmt.Errorf("load extras: %w", loadErr) } - result, err := helmfixture.Render(t, fixtureNamespace, resolved, bundle.PostRendererFor(), bundle.CRDs) + result, err := renderFixtureChart(t, resolved, bundle.PostRendererFor(), bundle.CRDs) if err != nil { return nil, fmt.Errorf("render chart: %w", err) }