feat(iac): dynamic-input mode on step.iac_provider_plan/apply (infra-admin P2 PR3/12)#842
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…iac_provider_apply Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(review)
Addresses PR3 code review:
- Empty-specs footgun: dynamic specs_from path now rejects zero specs via
len(specs)==0 (ParseResourceSpecs returns a non-nil empty slice for []any{},
which the prior nil check let through → apply over zero specs = destroy-all).
Applied to both step.iac_provider_plan and step.iac_provider_apply; static
path unchanged.
- Test fidelity: rewrote the apply dynamic tests to use the canonical
request_parse wiring (steps.parse-request.body.specs and
steps.parse-request.body.desired_hash) instead of injecting a specs key the
plan step never emits.
- Failure paths: added table-driven Execute-error tests on both steps for
missing path, non-list scalar specs, empty []any specs, and (apply) a
non-string desired_hash (guarded cast, no panic).
- Schema: declared specs_from on plan and specs_from + desired_hash_from on
apply in schema/step_schema_builtins.go; relaxed apply desired_hash from
Required since desired_hash_from is now a valid alternative source.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds dynamic-input support to the built-in IaC provider plan/apply steps so they can resolve operator-edited specs and hashes from the pipeline context at execution time (rather than only from static step config), enabling the infra-admin SPA plan→apply flow while keeping the existing hash-guard behavior.
Changes:
- Extend
step.iac_provider_planwithspecs_from, andstep.iac_provider_applywithspecs_from+desired_hash_from, resolved via dotted context paths atExecutetime. - Enforce mutual exclusivity between static (
specs/desired_hash) and dynamic (*_from) inputs in step factories. - Add dynamic-input test coverage for success and failure paths (missing path, wrong types, empty specs, missing/invalid hash, hash mismatch, secret:// preservation).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| schema/step_schema_builtins.go | Registers specs_from / desired_hash_from fields for builtin IaC plan/apply steps. |
| module/pipeline_step_iac_provider_plan.go | Adds context-resolved specs_from support and updates Execute to use PipelineContext. |
| module/pipeline_step_iac_provider_plan_test.go | Adds tests for specs_from success, exclusivity error, and failure modes. |
| module/pipeline_step_iac_provider_apply.go | Adds context-resolved specs_from + desired_hash_from support and retains the hash-guard. |
| module/pipeline_step_iac_provider_apply_test.go | Adds dynamic-input tests including failure modes, hash mismatch, and secret ref preservation. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
… (Copilot) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR3/12 — dynamic-input mode on
step.iac_provider_plan/apply(the Phase 2 headline fix)Locked plan Tasks 5–6. Makes the infra-admin SPA's plan→apply actually work with operator-edited specs. Today both steps read
specs/desired_hashfrom STATIC step config in their factory closures andExecute(ctx, _ *PipelineContext)ignores the context — so a POST body can never reach them (the shipped scenario 92 hardcoded them; the SPA dynamic path was a static demo).Changes
step.iac_provider_plangains optionalspecs_from;step.iac_provider_applygains optionalspecs_from+desired_hash_from— dotted context paths (e.g.steps.parse-request.body.specs) resolved at Execute via the engine's existingresolveBodyFrom+iac/specparse.ParseResourceSpecs.Execute(ctx, _)→Execute(ctx, pc)._fromfields are unset.provider.Status(), reject mismatch). It is NOT tamper protection (the client supplies both specs+hash); access control is subject-RBAC at the route, per ADR 0016.secret://refs survive verbatim (no resolver on the dynamic path; hash uses the existing no-op env handling).schema/step_schema_builtins.go): the new fields registered sowfctl validateknows them;desired_hashrelaxed from unconditionally-required (now one-ofdesired_hash/desired_hash_from, matching the factory).Review notes (resolved)
len(specs) == 0(not== nil) —ParseResourceSpecs([]any{})returns a non-nil empty slice, so an empty list would otherwise slip through to an apply-over-zero-specs (destructive on some providers).steps.parse-request.body.specswiring (mirroringstep.request_parse's output), not a fictional plan-stepspecskey.Verified:
go build ./...exit 0;module+schema+plugins/alltests pass (incl. static-path, hash-guard, secret-ref, failure paths);golangci-lint --new-from-rev0 issues.🤖 Generated with Claude Code