Repository navigation
fix(kubernetes): single-file GitOps delivery and Kustomize metadata.name exemption - #2874
Merged
Merged
Conversation
…e objects from metadata.name The `git` provision target always treated its configured `path` as a directory, fanning out one auto-named file per manifest even when the path named an exact file (e.g. `kustomization.yaml`) — creating a directory by that name instead. Kustomize's own `Kustomization`/`Component` objects were also rejected by Atmos's structural validator for lacking `metadata.name`, even though Kustomize's own schema (and its own field-enforcement checks) never requires one. - Add a `split` tri-state on git provision targets: explicit `true`/`false` wins, otherwise inferred from whether `path`'s last segment looks like a manifest filename. `split: false` merges rendered manifests into a single file at the exact path instead of a directory. - Exempt Kustomize's own `Kustomization`/`Component` kinds (matched against their own vendored `sigs.k8s.io/kustomize/api/types` constants) from the `metadata.name` presence check; add an explicit `validate: false` component flag as a general override for the apply/deploy auto-gate and the standalone `validate` command. - Document the new `split` and `validate` fields, and add a full walkthrough for generating a Kustomize component/patch for GitOps delivery.
Required release docs for the split/validate provision-target fix: a problem-first blog post walking through the Kustomize component/GitOps pattern, and a new shipped milestone on the Extensibility roadmap initiative (with a corrected progress percentage).
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
August 5, 2026 17:07
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Erik Osterman (Cloud Posse) (osterman)
added
the
minor
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Contributor
📝 WalkthroughWalkthroughThis change adds Kustomize-aware Kubernetes validation controls, configurable Git manifest splitting, deterministic multi-document YAML output, and shared Git operation errors with provider stderr. ChangesKubernetes validation and configuration propagation
Manifest and Git delivery
Git diagnostics and documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant KubernetesRender
participant GitProvisionTarget
participant writeArtifact
participant MergeYAMLDocuments
participant GitRepository
KubernetesRender->>GitProvisionTarget: rendered artifacts
GitProvisionTarget->>writeArtifact: resolved split mode
writeArtifact->>MergeYAMLDocuments: sorted documents
MergeYAMLDocuments-->>writeArtifact: multi-document YAML
writeArtifact->>GitRepository: write target file or directory
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/component/kubernetes/executor.go`:
- Around line 261-265: Update the validation flow around
resolveComponentValidateEnabled, resolveValidateOptions, and runValidate so
validation options are resolved before the validate:false check and --server
requests still call runValidate against the live cluster. Restrict the skip
result to offline structural validation only, preserving existing behavior
otherwise, and add a regression test covering validate:false with --server.
In `@website/blog/2026-08-05-kustomize-gitops-delivery.mdx`:
- Around line 8-10: Update the Kustomize filename explanation in the blog
content to say that a remote base or component must contain a recognized
reserved kustomization file name, not only kustomization.yaml. Use Kustomize as
the context and mention the supported filenames kustomization.yaml,
kustomization.yml, and Kustomization, while keeping kustomization.yaml as the
example. Preserve the existing point that the name is fixed by Kustomize and not
configurable.
In `@website/docs/stacks/components/kubernetes.mdx`:
- Around line 239-244: Update the Kubernetes delivery-mode documentation near
the `path` and `split` explanation to state that an unset `split` infers
single-file delivery when `path` ends in `.yaml`, `.yml`, or `.json`; otherwise
it defaults to directory delivery. Clarify that users should set `split`
explicitly when they need to override this path-based inference.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 07caf576-d657-4dff-a351-701802edd092
📒 Files selected for processing (15)
pkg/component/kubernetes/executor.gopkg/component/kubernetes/executor_test.gopkg/component/kubernetes/render.gopkg/component/kubernetes/validate.gopkg/component/kubernetes/validate_test.gopkg/datafetcher/schema/stacks/stack-config/1.0.jsonpkg/provisioner/target/git/git.gopkg/provisioner/target/git/git_test.gopkg/provisioner/target/manifest.gopkg/provisioner/target/manifest_test.gowebsite/blog/2026-08-05-kustomize-gitops-delivery.mdxwebsite/docs/cli/commands/kubernetes/kubernetes-deploy.mdxwebsite/docs/cli/commands/kubernetes/kubernetes-validate.mdxwebsite/docs/stacks/components/kubernetes.mdxwebsite/src/data/roadmap.js
Address CodeRabbit review on #2874: - The validate:false short-circuit returned before resolving --server, so `atmos kubernetes validate --server` never reached the live cluster for a component with validate:false. Resolve validate options first and only skip the offline structural check; --server still runs runServerValidate. Adds a regression test. - Blog post overclaimed kustomization.yaml as the only recognized filename; Kustomize also accepts kustomization.yml and Kustomization (confirmed against the vendored dependency). - Applied CodeRabbit's suggested wording clarifying the split-unset path-extension inference in the stack config docs.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2874 +/- ##
========================================
Coverage 82.77% 82.77%
========================================
Files 1861 1863 +2
Lines 180480 180590 +110
========================================
+ Hits 149390 149485 +95
- Misses 23304 23313 +9
- Partials 7786 7792 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Member
Author
|
CodeRabbit (@coderabbitai) full review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/component/kubernetes/validate_test.go (1)
107-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a table-driven test for validation-option resolution.
These assertions test five input scenarios. Put them in one table to follow the repository test convention.
As per coding guidelines, “Use table-driven tests for testing multiple scenarios in Go.”
Proposed refactor.
func TestResolveComponentValidateEnabled(t *testing.T) { - assert.True(t, resolveComponentValidateEnabled(nil), "unset defaults to enabled") - assert.True(t, resolveComponentValidateEnabled(map[string]any{}), "unset defaults to enabled") - assert.True(t, resolveComponentValidateEnabled(map[string]any{"validate": true})) - assert.False(t, resolveComponentValidateEnabled(map[string]any{"validate": false})) - assert.True(t, resolveComponentValidateEnabled(map[string]any{"validate": "false"}), "non-bool values are ignored, defaulting to enabled") + tests := []struct { + name string + componentSection map[string]any + want bool + }{ + {"nil defaults to enabled", nil, true}, + {"empty defaults to enabled", map[string]any{}, true}, + {"true enables validation", map[string]any{"validate": true}, true}, + {"false disables validation", map[string]any{"validate": false}, false}, + {"non-boolean defaults to enabled", map[string]any{"validate": "false"}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, resolveComponentValidateEnabled(tt.componentSection)) + }) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/component/kubernetes/validate_test.go` around lines 107 - 113, Refactor TestResolveComponentValidateEnabled into a table-driven test covering the existing five inputs and expected results, including descriptive case names and messages where useful. Iterate over the cases with the repository’s standard subtest pattern while preserving the current validation-option behavior assertions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@website/blog/2026-08-05-kustomize-gitops-delivery.mdx`:
- Around line 57-58: Update
website/blog/2026-08-05-kustomize-gitops-delivery.mdx:57-58,
website/docs/stacks/components/kubernetes.mdx:270-271, and
pkg/provisioner/target/git/git_test.go:250-252 to use
kustomize.config.k8s.io/v1alpha1 for Component fixtures. Update
website/docs/cli/commands/kubernetes/kubernetes-validate.mdx:106-113 to document
Kustomization as v1beta1 and Component as v1alpha1 separately, removing the
wildcard API-version description.
---
Nitpick comments:
In `@pkg/component/kubernetes/validate_test.go`:
- Around line 107-113: Refactor TestResolveComponentValidateEnabled into a
table-driven test covering the existing five inputs and expected results,
including descriptive case names and messages where useful. Iterate over the
cases with the repository’s standard subtest pattern while preserving the
current validation-option behavior assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 98b624ea-72ff-4b64-addf-5fbc73840f88
📒 Files selected for processing (15)
pkg/component/kubernetes/executor.gopkg/component/kubernetes/executor_test.gopkg/component/kubernetes/render.gopkg/component/kubernetes/validate.gopkg/component/kubernetes/validate_test.gopkg/datafetcher/schema/stacks/stack-config/1.0.jsonpkg/provisioner/target/git/git.gopkg/provisioner/target/git/git_test.gopkg/provisioner/target/manifest.gopkg/provisioner/target/manifest_test.gowebsite/blog/2026-08-05-kustomize-gitops-delivery.mdxwebsite/docs/cli/commands/kubernetes/kubernetes-deploy.mdxwebsite/docs/cli/commands/kubernetes/kubernetes-validate.mdxwebsite/docs/stacks/components/kubernetes.mdxwebsite/src/data/roadmap.js
… gap Address CodeRabbit full-review findings on #2874: - All Component examples/fixtures used kustomize.config.k8s.io/v1beta1, which is Kustomization's version, not Component's (v1alpha1). Since isKustomizeConfigObject matches exact (apiVersion, kind) pairs, the examples never actually got the metadata.name exemption they claimed. Fixed in the blog post, the kubernetes.mdx walkthrough, and test fixtures; kubernetes-validate.mdx now documents both exact pairs instead of a kustomize.config.k8s.io/* wildcard. - Added TestWriteArtifactSingleFileModeWriteFailure, closing the patch coverage gap Codecov flagged on writeSingleArtifactFile's two new error branches (MkdirAll/WriteFile failure), mirroring the existing split=true failure test.
… schema gap
A field-test pass over the Kustomize GitOps delivery feature found four
real bugs, all fixed here:
- validate: false on a Kubernetes component was silently dropped during
stack processing and never reached any command: internal/exec's
Kubernetes comp-assembly copied provider/paths/manifests/render but
never validate. Threaded it through the full 3-layer merge (global ->
base component -> instance) via the same key-presence-safe
mergeComponentAnySection pattern paths/manifests already use (not
provider's zero-value pattern, which would be unsafe for a bool), plus
the matching --affected diffing and base-component cache entries.
- Git target errors (atmos kubernetes deploy/apply --target <git>) were
always opaque ("git clone (exit 128)", no cause) because the git
provisioner target never captured subprocess stderr, unlike the atmos
git command family. Moved the capture-and-hint machinery from cmd/git
into exported pkg/git symbols (CaptureStderr, WrapOperationError) so
both share one implementation, and wired it into the provisioner's
clone/commit/push calls.
- components.kubernetes.<name>.validate: false failed schema validation
("additionalProperties 'validate' not allowed') because the repo has
three hand-maintained copies of the stack-manifest JSON schema and the
original fix only patched one; atmos describe stacks/validate stacks
enforce a different copy. Patched the copy that's actually enforced and
added a regression test.
- A successful git-target delivery printed nothing at all, unlike cluster
apply and validate. Added a confirmation message.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
pkg/git/errors.go (1)
29-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd performance tracking to
CaptureStderr.
CaptureStderris an exported operation helper. Adddefer perf.Track(nil, "git.CaptureStderr")()and the required blank line.As per coding guidelines, “Add
defer perf.Track(atmosConfig, "pkg.FuncName")()plus a blank line to public functions” unless an explicit exemption applies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/git/errors.go` around lines 29 - 41, Update the exported CaptureStderr function to defer perf.Track(nil, "git.CaptureStderr")() at its start, adding the required blank line after the tracking statement and importing the perf package if necessary.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/exec/describe_affected_components.go`:
- Line 593: Update the comparison loop around cfg.ValidateSectionName to detect
changes when the local stack removes validate, even if the remote section still
exists; compare section presence before applying the absent-section skip, while
preserving value comparison for present sections. Add a regression test covering
removal of validate: false and confirming the component is reported as affected.
In `@internal/exec/stack_processor_merge_test.go`:
- Around line 739-779: The merge tests around mergeComponentConfigurations
currently omit the GlobalKubernetesValidate layer. Add focused cases covering
global validate true, global validate false, and precedence across global, base,
and component values, including explicit component and base overrides where
applicable; configure GlobalKubernetesValidate on the test AtmosConfig and
assert the resulting cfg.ValidateSectionName value or absence.
In `@internal/exec/stack_processor_utils.go`:
- Line 2792: Update the Base component validate comment in the surrounding stack
processor utility code to end with a period, preserving its existing wording.
In `@pkg/component/kubernetes/provision_test.go`:
- Around line 105-145: Strengthen TestDeliverApplyPrintsSuccessConfirmation by
asserting that uiOutput also contains the delivered object-count text “delivered
1 Kubernetes object(s)”, while retaining the existing target-name assertion.
In `@pkg/datafetcher/schema_condition_validation_test.go`:
- Around line 196-200: Update the "website" entry in the schemas map within the
relevant test to load the actual website schema file instead of calling
loadWebsiteSchemaBytes, which currently returns the embedded schema. Reuse the
existing website schema file-loading helper or path used elsewhere in the test
package, while leaving the embedded and stack-config entries unchanged.
In `@pkg/git/errors_test.go`:
- Around line 44-70: Refactor the test doubles around stubNonSwappableProvider
so it no longer inherits SwapStderr: introduce a shared base provider without
that method, embed it in both stubSwappableProvider and
stubNonSwappableProvider, and define SwapStderr only on stubSwappableProvider.
Keep TestCaptureStderr_NonSwappableProviderRunsUnmodified exercising the
fallback path.
In `@pkg/provisioner/target/git/git_test.go`:
- Around line 489-498: Remove the exec.LookPath check and all Git CLI setup from
TestDeliverIntegrationCloneErrorSurfacesStderrAndHint. Use the package-level
provider function hook to install a deterministic test double that returns the
clone error and writes representative stderr, then exercise the existing
delivery flow and restore the hook afterward; keep the regression test
unconditional.
---
Nitpick comments:
In `@pkg/git/errors.go`:
- Around line 29-41: Update the exported CaptureStderr function to defer
perf.Track(nil, "git.CaptureStderr")() at its start, adding the required blank
line after the tracking statement and importing the perf package if necessary.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ce2d2d82-9ab2-4223-a7f9-62362f8c481e
📒 Files selected for processing (22)
cmd/git/executor.gointernal/exec/describe_affected_components.gointernal/exec/stack_processor_cache.gointernal/exec/stack_processor_merge.gointernal/exec/stack_processor_merge_test.gointernal/exec/stack_processor_process_stacks.gointernal/exec/stack_processor_process_stacks_helpers.gointernal/exec/stack_processor_process_stacks_helpers_extraction.gointernal/exec/stack_processor_process_stacks_helpers_inheritance.gointernal/exec/stack_processor_utils.gopkg/component/kubernetes/provision.gopkg/component/kubernetes/provision_test.gopkg/component/kubernetes/validate.gopkg/config/const.gopkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/datafetcher/schema_condition_validation_test.gopkg/datafetcher/schema_section_coverage_test.gopkg/git/errors.gopkg/git/errors_test.gopkg/provisioner/target/git/git.gopkg/provisioner/target/git/git_test.gopkg/schema/schema.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/component/kubernetes/validate.go
- describe_affected_components: detect removal of component-level validate:false (local absent, remote still set), not just value changes, via a new sectionPresent locator check; add regression coverage. - stack_processor_merge_test: cover the GlobalKubernetesValidate layer (global true/false, and precedence against base/component). - provision_test: assert the delivered object count, not just the target name, in the success-confirmation message. - stack_processor_utils: add missing period to a comment (godot). - pkg/git/errors_test: fix stubNonSwappableProvider embedding stubSwappableProvider, which promoted SwapStderr and made the "fallback" test exercise the swappable path instead; split into a shared SwapStderr-less base plus a swappable-only type, with a compile/runtime guard against regressing this again. - pkg/provisioner/target/git: replace the real-git-dependent clone-error regression test with a deterministic provider double, installed via a new package-level newProvider hook (mirrors this codebase's established function-hook testability convention); the test can no longer be skipped when git is unavailable. Skipped: the "website" schema test case intentionally reuses the embedded schema bytes (no separate website schema file is committed anywhere in this repo; the website copy is generated from the embedded schema at build time) -- this is a pre-existing, documented convention every other test in that file already follows, not specific to the new test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
August 6, 2026 23:23 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Andriy Knysh (aknysh)
temporarily deployed
to
screengrabs
August 7, 2026 13:56 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Andriy Knysh (aknysh)
approved these changes
Aug 7, 2026
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Aug 7, 2026
Merging origin/main (which landed #2874, a Kubernetes validate/GitOps fix) into this branch combined cleanly at the text level but broke compilation: this branch had already changed mergeComponentConfigurations to return (map[string]any, ComponentDeferredContexts, error), but #2874 added a new finalComponentValidate error path and 7 test call sites still using the old 2-value (map[string]any, error) signature — a silent semantic merge conflict CI's PR-preview build caught (all 5 failing jobs shared this one root cause). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Aug 7, 2026
- Fix a stale signing.mode: auto reference in the atmos-git skill's GitOps guidance (the config example above it already uses the real flat commit.signing: auto field; per CodeRabbit review on #2905). - Close Codecov patch-coverage gaps flagged on the same PR by adding real behavioral tests for the new error-propagation and remote-URL-sync branches in reconcile/syncRemoteURL, executeKubernetesOperation, and parseConfig/Deliver/Fetch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
These changes were released in v1.226.0-rc.2. |
Marko Petrovic (gitbluf)
pushed a commit
to gitbluf/atmos
that referenced
this pull request
Aug 10, 2026
…very (cloudposse#2905) * fix(kubernetes): close gaps found field-testing Kustomize GitOps delivery (cloudposse#2874) A field-test pass against the merged Kustomize/git-delivery pipeline (validate:false, git error surfacing, single-file delivery, metadata.name exemption) found several confirmed gaps and fixed them: - validate: "false" (a quoted string) was silently ignored by `atmos kubernetes validate/apply/deploy`, leaving validation enabled with no warning; it now fails closed with a clear error. - provision.targets.<name>.split had no type enforcement in the runtime JSON Schema (only in the docs-facing copy), so a bad value like split: "yes" passed `atmos validate stacks` and was silently dropped at runtime; the schema is now synced and the git target fails closed too. - Flipping a git delivery target between directory and single-file mode now warns before the unconditional RemoveAll that replaces whatever is at the managed path. - The managed git workdir cache never reconciled with a changed git.repositories.<name>.uri; reconcile now syncs the local remote URL. - The DNS-1123 invalid-name error embeds a regex with '[', ']', '(', ')' and no spaces, which the CLI's markdown renderer both mangled and hard-wrapped; it's now backtick-fenced as a code span. - Fixed a copy-pasted config example in the atmos-git skill doc (nested signing: {mode: auto} instead of the real flat signing: auto field) that fails to parse if used as-is. - Documented that `atmos kubernetes validate --server` fails on a manifest set that creates its own namespace and delivers into it in the same batch (inherent to server-side dry-run semantics), even though apply/deploy of the same objects succeeds. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): remediate 7 Dependabot alerts (js-yaml, mermaid) Bumps the existing pnpm overrides for two transitive website dependencies to their patched versions, closing 7 open Dependabot alerts (2 high, 4 medium, 1 low), all npm/transitive: - js-yaml 3.x -> 3.15.1, 4.x -> 4.3.1 (GHSA-5p4m-2wfm-xmqj, quadratic CPU consumption in !!omap resolution) - mermaid -> 11.16.1 (GHSA-rhh3-jpg6-66xh, GHSA-c4c3-pg64-4m4v, GHSA-6x64-9x62-f2gx, GHSA-3rrr-jr9j-h3q3, GHSA-2v8p-3f2j-5mp7) No CodeQL alerts were open. All fixes stay within the same major version, so none are blocked by dependabot.yml's major-bump ignore policy. Verified via `pnpm install` (lockfile now resolves only the patched versions) and `npm run build`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(kubernetes): address CodeRabbit findings on PR cloudposse#2874 - Fix a stale signing.mode: auto reference in the atmos-git skill's GitOps guidance (the config example above it already uses the real flat commit.signing: auto field; per CodeRabbit review on cloudposse#2905). - Close Codecov patch-coverage gaps flagged on the same PR by adding real behavioral tests for the new error-propagation and remote-URL-sync branches in reconcile/syncRemoteURL, executeKubernetesOperation, and parseConfig/Deliver/Fetch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(kubernetes): use an invalid object name to prove validate-section precedence TestRunOperationApplyPropagatesValidateSectionError and its validate-op mirror used an already-valid ConfigMap name, so they couldn't distinguish "validate-section resolved first" from "structural check first, but this object happens to pass" -- both orderings return the same error with a valid object. Switching to an invalid name means only the correct precedence still returns ErrKubernetesValidateSectionInvalid. Per CodeRabbit review on cloudposse#2905. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): skip SARIF upload on merge_group to unblock the merge queue The merge queue's ephemeral gh-readonly-queue/main/pr-<n>-<sha> ref is deleted as soon as the queue cycles (observed every ~7-13 minutes), which routinely races the multi-minute CodeQL analysis and golangci-lint SARIF uploads and fails them with "ref not found" / "CodeQL job status was configuration error" -- even though nothing is actually wrong with the code. This was blocking every PR in the merge queue, not just this one (confirmed the same failure on cloudposse#2908 at the same time). Both jobs still run and still report pass/fail on merge_group events, so the merge queue's required checks get a result -- they just skip the upload, which can't succeed against a ref that's already gone by the time it completes. pull_request/push events are unaffected: the upload still runs there, where the ref is stable, so it stays fully required and enforced on PRs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): use codeql-action's upload string enum, not a boolean analyze's `upload` input only accepts always/failure-only/never (see github/codeql-action's analyze/action.yml); a boolean expression stringifies to "true"/"false", which isn't a valid value for it. Use the new case() expression function to emit the right string. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): remediate 5 of 7 open Dependabot alerts - github.com/go-git/go-git/v5 5.19.1 -> 5.19.2 (GHSA-hc8v-wwc9-vgxm high, GHSA-qgq7-7hm3-q39j medium) - nanoid pnpm override 3.3.15 -> 3.3.17 (GHSA-28wg-ghj8-5hjv, GHSA-2v37-7h3g-55p8, both high) - dompurify pnpm override 3.4.12 -> 3.4.13 (GHSA-55q2-fjhq-7xh7, medium) image-size (GHSA-w3rx-r6r6-pgpr, GHSA-5p2g-fcmc-qvqq, both high) has no patched version published yet -- not fixable until upstream ships one. All within existing major versions, so none blocked by dependabot.yml's major-bump ignore policy. NOTICE regenerated for the go-git bump. Verified via targeted go test across every package importing go-git (39/39 pass), atmos lint --changed (0 issues), and npm run build. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(kubernetes,git): consolidate near-duplicate tests into table-driven form - executor_test.go: merge TestRunOperationApplyPropagatesValidateSectionError and its validate-op mirror into one TestRunOperationPropagatesValidateSectionError table over OperationApply/OperationValidate. - cli_test.go: merge the three exact-call-sequence reconcile tests (TestCloneReconcilesExistingWorkdir, TestCloneReconcileUpdatesRemoteURLOnChange, TestCloneReconcileSkipsRemoteSyncWhenURIEmpty) into one TestCloneReconcileRemoteSync table. Left the two error-propagation reconcile tests (RemoteGetURLFailurePropagates/RemoteSetURLFailurePropagates) as-is: their assertion shape (error identity + "no fetch happened") differs enough from the exact-ordered-sequence checks that folding them into the same table would need nullable fields without real clarity benefit. Per CodeRabbit nitpicks on cloudposse#2905. No behavior change; same assertions, same coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): isolate merge_group CI-detection test from the real GITHUB_EVENT_PATH TestSetDescribeAffectedFlagValueInCliArgs_BaseResolution's "CI auto-detect when enabled and no explicit base" subtest sets GITHUB_EVENT_NAME=merge_group and GITHUB_BASE_REF=main to exercise resolveMergeGroupBase's env-fallback path, but never cleared GITHUB_EVENT_PATH. Every other CI-env test in this package explicitly stubs it; this one didn't. That's harmless everywhere except inside an actual merge_group-triggered CI job: there, the real ambient $GITHUB_EVENT_PATH points at a genuine merge_group event payload, so resolveMergeGroupBase successfully reads event.merge_group.base_sha instead of hitting the fallback the test means to exercise, leaving describe.Ref empty and failing the assertion. This went unnoticed until this branch's PR actually made it into the merge queue and Acceptance Tests ran the suite in that exact context, failing identically on linux/macos/windows plus cascading to the k3s demo-helmfile matrix-result gates. Reproduced locally by setting a real GITHUB_EVENT_PATH with merge_group payload as the ambient env before running go test: fails without this fix, passes with it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): make PR Semver Labels report on merge_group too Its job was gated to `if: github.event_name == 'pull_request'`, so it never produced a check run for the merge queue's synthetic commit. Since it's a required status check, the merge queue waited out its full check-response timeout (75 min) hoping for a check that would never appear, then dequeued the PR -- even when every other required check (Tests, CodeQL, Codeowners, symlinks) had already succeeded comfortably within the window (confirmed: this run's other checks finished at 64m38s, but the PR wasn't dequeued until 79 min). The label-check step itself stays pull_request-only (merge_group has no github.event.pull_request for it to check labels against, and the label was already verified before the PR entered the queue); only the job's own if: was widened so it reports a pass for merge_group. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): point CodeQL/SARIF uploads at a stable ref on merge_group The earlier fix (skip the upload entirely on merge_group) traded one problem for a worse one: it dodged the "ref not found" upload failure, but the native "CodeQL" required status check is only ever created when a SARIF upload actually happens for that commit -- so skipping the upload left that required check permanently missing on merge_group commits. The merge queue then waits out its full check-response timeout for a check that will never report, exactly like the PR Semver Labels gap, except invisibly this time (every check-run showed success; the queue was just stuck waiting on one that never existed). Confirmed directly: diffed required_status_checks.contexts against the actual check-runs reported for a recent merge_group commit where every workflow run had already succeeded -- "CodeQL" was the only one absent. Fix: explicitly set the codeql-action ref/sha inputs to the merge group's target branch ref (event.merge_group.base_ref, e.g. refs/heads/main -- a ref that persists) and the actual queued merge commit (event.merge_group.head_sha), instead of letting the action auto-detect from GITHUB_REF/GITHUB_SHA (the ephemeral, soon-deleted gh-readonly-queue/... ref). This lets the upload succeed for real instead of needing to skip it. pull_request/push are unaffected -- ref/sha are only overridden for merge_group. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
Erik Osterman (Cloud Posse) (osterman)
added a commit
to jorrite/atmos
that referenced
this pull request
Aug 14, 2026
…udposse#2888) (cloudposse#2892) * test: add regression tests proving cloudposse#2888 deferred-merge data loss Strengthens the existing "deep merges with yaml functions" assertion (it only checked the key existed, never the merged value) and adds a matching !labels/!tags case. Both fail today: !template, !labels, and !tags all silently lose data when a concrete override collides with an unresolved deferred function, because every ApplyDeferredMerges call site in stack_processor_merge.go passes processor=nil. Also updates the deferred-yaml-functions-evaluation-in-merge PRD to correct its stale "implemented and tested" status and document the completion plan (staged as plumbing-only PR 1 + behavior-change PR 2) for wiring real post-merge resolution, tracked by cloudposse#2888. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: add fix-log record for cloudposse#2888; fix pre-existing editorconfig violations Adds the fix-log record for the cloudposse#2888 regression tests and completion plan. Also fixes ~230 pre-existing editorconfig violations in the PRD (tab-indented Go snippets, 3-space list indentation, misaligned fenced-block content) that were surfaced once the file entered a diff for the first time since it was written — unrelated to cloudposse#2888 itself, but blocking any commit that touches the file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore: trigger CI re-run Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(merge): resolve deferred YAML functions and deep-merge with concrete overrides (cloudposse#2888) Wires up the previously-unconnected "resolve, then deep-merge" half of the deferred-merge design: every production call site of ApplyDeferredMerges was passing processor=nil, so !template/!terraform.output/!terraform.state/!store/ !exec/!env silently lost data whenever a concrete value at another config layer collided with them, and !labels/!tags weren't even deferred at all (the literal cloudposse#2888 report). Adds a real Stage 3 resolution pass (per-invocation, auth- and template-context-aware) that resolves deferred functions and deep-merges the result against any concrete override at the same path — including the mirror-precedence direction (a concrete value at a *lower*-precedence layer than the function), which the original design didn't handle and which a new regression test caught during implementation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: raise PR cloudposse#2892 patch coverage from 70.64% to 86.20% Codecov flagged patch coverage below the 85% gate on the deferred-merge fix. Adds targeted error-injection and regression tests for the previously-uncovered branches in stack_processor_merge.go, stack_processor_process_stacks.go, deferred_contexts.go, generate_adapter_funcs.go, and completions.go — no production code changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): exclude flaky reproducible-builds.org link from markdown link check CI's Check Markdown Links job failed on docs/prd/archive-step.md:175 with "Connection refused" against reproducible-builds.org/docs/source-date-epoch/. The link returns 200 outside CI (verified via curl) — same CI-runner-specific connection-refusal pattern as the ~30 other domains already excluded in lychee.toml. * fix: address CodeRabbit review feedback on PR cloudposse#2892 Fixes two real correctness bugs and updates stale/incomplete test and documentation coverage flagged by CodeRabbit on this PR: - stack_processor_process_stacks.go: add cfg.HelmComponentType to builtInTypes so components.helm is no longer reprocessed (and its merged data clobbered) by the custom-component-type passthrough loop, which would otherwise make Stage 3 resolve deferred YAML functions against mismatched component data. - terraform_generate_backends.go / terraform_generate_varfiles.go: both batch generators now retain FindStacksMap's deferred-merge contexts and call resolveDeferredYamlFunctions after ProcessCustomYamlTags, closing the same cloudposse#2888 data-loss gap in these two call sites that the main describe/plan path already fixed. - yaml_processor.go: the deferred-string template fast path now resolves the configured left template delimiter instead of hardcoding "{{", so custom delimiters (e.g. "[[ ]]") aren't silently skipped. - cmd/emulator/completions_test.go: assert the full expected sorted stack list instead of just Contains("local"). - tests/yaml_functions_integration_test.go: initialize the CLI config in TestYAMLFunctionsDeferredMergeCacheCorrectness so it doesn't depend on state primed by an earlier test in the same file. - docs/prd and docs/fixes: correct stale file/symbol references and pre-fix status claims now that the cloudposse#2888 fix has shipped in this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: close remaining patch-coverage gap in fillMissingLayerValues The last uncovered branch from the cloudposse#2892 patch-coverage fix (the other 7 flagged files were already addressed): the defensive len(values)==0 guard, unreachable via the public AddDeferred API, so seeded directly via the unexported field per this file's existing pattern. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): remediate 7 Dependabot alerts in website pnpm overrides Bumps transitive js-yaml (>=3.15.1, >=4.3.1) and mermaid (>=11.16.1) via pnpm.overrides to their patched versions - quadratic-CPU YAML parsing and mermaid XSS/ReDoS advisories. All fixes are minor/patch bumps within the same major version, so none are blocked by dependabot.yml's major-version ignore policy. Fixes GHSA-5p4m-2wfm-xmqj (js-yaml, alerts cloudposse#268/cloudposse#269), GHSA-rhh3-jpg6-66xh, GHSA-c4c3-pg64-4m4v, GHSA-6x64-9x62-f2gx, GHSA-3rrr-jr9j-h3q3, GHSA-2v8p-3f2j-5mp7 (mermaid, alerts cloudposse#263-267). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: close remaining PR cloudposse#2892 patch-coverage gaps in stack_processor_merge.go and stack_processor_process_stacks.go stack_processor_merge.go (now fully covered): each nil-processor ApplyDeferredMerges write-back error branch (auth, providers, required_providers, hooks, generate, test, plus processAuthConfig's own second pass) forced via a deferred value at a nested path whose parent segment is then overridden by a higher-precedence scalar, so SetValueAtPath can no longer navigate to it (ErrCannotNavigatePath). Also closes the remote-state-backend error path via a non-map value at the backend-type key. stack_processor_process_stacks.go: closes the processComponentsInParallel error-propagation branch for all 5 non-terraform component types (same technique as above, routed through the public ProcessStackConfig entry point), the custom-component metadata.inherits type-validation branch, and the per-component "value is not a map" branch in buildComponentWork. The remaining gap in this file (~37 lines) is the mechanical `if err != nil { return nil, nil, err }` sweep after plain m.Merge calls between two map[string]any layers: traced deepMergeNative's only real error source (ErrMergeNilDst) and confirmed it's unreachable through the public Merge entry point, which always filters nil/empty inputs first — the native merge engine's override-always-wins design makes these lines defensive, not reachable, code. Same conclusion applies to yaml_processor.go's remaining 3 lines/3 partials (verified via 11 probe inputs against its deliberately robust YAML-quoting logic) and the equivalent 1-line gaps in terraform_generate_backends.go/terraform_generate_varfiles.go (the error would need to survive an earlier eager-resolution pass but fail on the later deferred-resolution pass for the same value). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: address CodeRabbit review feedback on PR cloudposse#2892 (round 2) - docs/prd/deferred-yaml-functions-evaluation-in-merge.md: the historical-design-pseudocode note pointed readers to "Implementation Status" for the as-shipped design, but that section records the Version 2.0 pre-fix state (processor = nil, never wired up). Point to the top Status and Completion Plan sections instead, and label Implementation Status as historical. - lychee.toml: anchor the reproducible-builds.org exclusion to the documented /docs/source-date-epoch/ URL (optional trailing slash) so it doesn't also swallow future paths under that prefix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(skills): field-test respects plan mode and always ties fixes to fix-log Add a Plan Mode section so a field-test invoked under plan mode does read-only research/hypothesis phases, writes a plan, and requests approval via ExitPlanMode before building fixtures or executing. Also tighten the existing fix-log guidance so any plan to fix findings — not just its implementation — closes with the fix-log skill. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: order deferred YAML function paths ancestor-before-descendant ApplyDeferredMerges ranged over dctx.GetDeferredValues(), a plain Go map, so a parent path (e.g. vars.combo) and a child path (e.g. vars.combo.nested) occupied by different deferred functions in different merge layers could resolve in either order. Each path's resolution ends in an unconditional SetValueAtPath call, so when the ancestor was processed after the descendant, its wholesale replace of the shared parent map silently discarded the descendant's already- resolved value — live reproduction showed ~40% of runs corrupting output with no error. Fixed by sorting path keys by ascending path-segment length before processing, so descendant leaf writes always happen last. Found via a field-test pass on PR cloudposse#2892 (cloudposse#2888); adds a 200-iteration regression test plus the field-test fixtures that confirmed 7 other scenarios already behaved correctly. See docs/fixes/2026-08-07-deferred-merge- nested-function-collision.md for full validation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: split oversized merge_deferred_test.go into focused files merge_deferred_test.go had grown to 1598 lines, well past this repo's 600-line file-size convention. Split by the function/feature each test group exercises: YAML-function detection/walking, map/slice/path primitives, merge strategies, MergeWithDeferred (kept in the original filename), ApplyDeferredMerges (including the parent/child-collision regression test), and the process/fill helpers. All 16 test functions moved verbatim; no behavior changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: resolve return-signature mismatch from merging origin/main Merging origin/main (which landed cloudposse#2874, a Kubernetes validate/GitOps fix) into this branch combined cleanly at the text level but broke compilation: this branch had already changed mergeComponentConfigurations to return (map[string]any, ComponentDeferredContexts, error), but cloudposse#2874 added a new finalComponentValidate error path and 7 test call sites still using the old 2-value (map[string]any, error) signature — a silent semantic merge conflict CI's PR-preview build caught (all 5 failing jobs shared this one root cause). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: add fix-log record for the CI build failure fix Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(website): dedupe nanoid pnpm override entry, breaks lockfile parsing Merging origin/main introduced a second, identical "nanoid@^3.3.16" key into website/package.json's pnpm.overrides (both branches added it independently at different points, so git's textual merge combined them without a conflict). The duplicate JSON key propagated into pnpm-lock.yaml as a duplicate YAML mapping key, which pnpm rejects outright (ERR_PNPM_BROKEN_LOCKFILE), blocking `pnpm install` and the website build. Removed the duplicate override entry and regenerated the lockfile via `pnpm install --lockfile-only` rather than hand- editing it. Verified `pnpm run build` succeeds. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(toolchain): retry the GitHub releases fetch behind `toolchain info` A single transient network/TLS hiccup on api.github.com silently degraded `atmos toolchain info` to "no available versions" instead of retrying, since getAvailableVersions already treats a fetch failure as non-fatal by design. This surfaced as a flaky CI golden-snapshot mismatch on TestCLICommands/atmos_toolchain_info_shows_atmos-inline_registry. makeGitHubRequest now retries transient failures (429/5xx/transport errors) with bounded exponential backoff via pkg/retry, following the same pattern already used in pkg/oci/pull.go, while still failing fast on deterministic client errors (404, 403, ...). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: address CodeRabbit review feedback on PR cloudposse#2892 Wraps request/response errors in makeGitHubRequest with static errors from errors/errors.go instead of dynamic fmt.Errorf roots. Makes isRetryableGitHubStatus header-aware: distinguishes a rate-limited 403 (Retry-After / X-RateLimit-Remaining: 0) from a terminal one, and honors Retry-After / X-RateLimit-Reset when the wait fits the existing retry budget, failing fast rather than blocking the CLI for GitHub's full mandated cooldown on this best-effort auxiliary fetch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(toolchain): retry the GitHub releases fetch behind `toolchain info` (round 2) Round 2 of CodeRabbit review feedback on PR cloudposse#2892, plus a matching gap found while investigating the attached CI failure logs: - pkg/toolchain/set.go: isRetryableGitHubStatus only decided *whether* to retry a rate-limited response; retry.Do still applied its own fixed ~300ms backoff regardless of what Retry-After specified, so a Retry-After: 1 response could be retried too soon and hit the same rate limit again. makeGitHubRequest is now a self-contained loop that sleeps the exact Retry-After/X-RateLimit-Reset duration when present and within budget, falling back to fixed exponential backoff only when no header supplies a wait. Also fixed a missing trailing period on a doc comment. - pkg/toolchain/registry/aqua: GetLatestVersion and GetAvailableVersionsContext had no retry at all for transient network failures, unlike the sibling getBytes helper in the same file, causing a separate CI golden-snapshot flake (atmos_toolchain_info_yaml_output resolving "latest" instead of a concrete version). Wired them through the same already-established, already-tested retry.TransientRetryConfig()/IsTransientNetworkError pattern via a new getBytesWithLinkHeader helper. Both come with regression tests: timing-based assertions for the Retry-After fix (which fail against the prior buggy behavior), and a mock transport simulating a connection reset for the aqua-registry fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): stop running TestTerraformRegistryCache twice on macOS/Windows It already runs once in its own dedicated step; the subsequent "Acceptance tests" step never excluded it (unlike Linux's coverage step), so it ran a second time concurrently with dozens of other tests that make real TLS calls. On macOS this races the System keychain trust-store mutations the test performs, destabilizing cert verification for the rest of the job and causing spurious "certificate signed by unknown authority" failures on unrelated tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(toolchain): retry rate-limit and server-error statuses in Aqua registry fetches CodeRabbit review on PR cloudposse#2892 flagged that aqua.go's getBytes/ getBytesWithLinkHeader retry predicate only accepted IsTransientNetworkError, so a 429 or 5xx response was wrapped as a plain error the predicate doesn't recognize and returned after a single attempt — exactly the failure class the retry was added to fix. Extracts the GitHub rate-limit classification already written for set.go into pkg/toolchain/registry (IsRetryableGitHubStatus, GitHubSignalsRateLimit, GitHubRetryAfter, HTTPStatusError) so aqua.go reuses it instead of duplicating it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: address CodeRabbit review feedback on PR cloudposse#2892 - Document the exported HTTPStatusError.Error and HTTPStatusError.Unwrap methods per Go documentation conventions. - Widen the X-RateLimit-Reset fallback test's interval from 3 seconds to 1 minute: the Unix-second truncation could zero out the wait if scheduling delayed the test past the 3-second boundary, flaking assert.Positive. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(merge): resolve deferred YAML functions once, not twice, in Stage 3. Add fix doc * fix(merge): resolve deferred YAML functions once, not twice, in Stage 3. Add fix doc * fix(merge): resolve remaining deferred-context gaps flagged in PR cloudposse#2892 review CodeRabbit's re-review of the deferred-YAML-function merge fix found two real gaps left over from the original PR: - describe_stacks.go has its own component processor that never went through processStacks (utils.go), so it never ran Stage 3 (resolveDeferredYamlFunctions). `atmos describe stacks` (bulk) still had the original cloudposse#2888 data-loss bug this PR claims to fix, even though `atmos describe component` (single) was already correct. Threads per-component deferred contexts from FindStacksMap into processComponentEntry and resolves them there. - terraform_generate_backends.go/terraform_generate_varfiles.go built ConfigAndStacksInfo.ComponentSection from a hand-picked subset of sections for Go-template rendering and Stage 3, silently omitting auth (and, for backends, required_providers/generate). A template referencing an omitted section rendered empty. Both now snapshot the complete merged section via a new cloneComponentSectionWithOverrides helper. Also corrects FindStacksMapForGenerate's doc comment, which inaccurately claimed "varfile/backend generation" as its rationale — that flow calls FindStacksMap directly; this wrapper's only caller is the bulk `generate:`-section preview, which never resolves YAML functions at all. Each fix ships with a regression test verified to fail without the fix and pass with it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: address remaining CodeRabbit review feedback on PR cloudposse#2892 - cmd/emulator/completions_test.go: exampleProjectPath now fails loudly (require.NoError) instead of t.Skipf when the checked-in emulator-aws example is missing, so a moved/deleted fixture can't masquerade as a passing test. - internal/exec/deferred_contexts_test.go: build the fixture path with filepath.Join instead of a slash-literal string. - pkg/merge/deferred_test.go: Clone isolation tests now verify both mutation directions (clone->original was already covered; added original->clone for both DeferredValue fields and Path elements). - pkg/merge/merge_deferred_apply_test.go: fixed a mock comment that claimed each invocation yields a distinct value — it returns a constant; the `calls` counter, not the value, is what detects a spurious re-invoke. - pkg/merge/merge_deferred_walk_test.go: added !labels/!labels.keys/ !labels.values/!tags cases to the allowlist table (previously untested directly, only via integration tests). - tests/yaml_functions_integration_test.go: assert exact append order (assert.Equal) instead of assert.ElementsMatch for !tags precedence, and add real assertions (not just "loads without error") for 7 previously unexercised deferred-merge fixtures: nested-in-list, 3-layer type flip, scalar-overrides-map, default (replace) list_merge_strategy, nested parent/child function collision, an untracked (non-allowlisted) function still being clobbered by design, and a deferred function inside the backend section. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: address remaining CodeRabbit review feedback, revert unsound double-execution fix attempt - internal/exec/describe_stacks_component_processor.go: wrap the mapstructure.Decode error in the Stage 3 settings-decode step with component/stack manifest context (%w), matching the existing error pattern in this file. Extract the Stage 3 block from processComponentEntry into a named resolveDeferredForComponent helper for readability (takes the settings map directly rather than the full componentSections struct, avoiding a gocritic hugeParam finding). - internal/exec/describe_stacks_test.go: use checked type assertions (require.True) for the intermediate map lookups instead of unchecked ones that would panic on failure. - tests/yaml_functions_integration_test.go: fix a comment referencing pkg/merge's internal postMergeFunctions variable and a hardcoded count instead of the canonical constants in pkg/utils/yaml_utils.go; explain why the expected map in one assertion is built via json.Unmarshal (float64 from !template's JSON decoding); rename a subtest that implied a deferred-merge collision it doesn't actually exercise. Also attempted and reverted a generalization of the Stage 3 double-execution fix (docs/fixes/2026-08-13-deferred-merge-double-execution.md) to cover collision paths where the higher-precedence layer is itself a function. The approach — a speculative pre-check via MergeDeferredValues on still-unresolved values — is unsound: a raw function string is always scalar-typed before execution, so the check cannot predict whether resolving it would produce a map needing to merge with a concrete sibling layer. Caught by tests/yaml_functions_integration_test.go's "deep merges with yaml function at higher precedence" case, which regressed silently past pkg/merge's own unit tests (they only exercised a scalar base, not the map-base case that actually broke). Full pkg/merge/merge_yaml_functions.go and merge_deferred_apply_test.go changes reverted to the last-known-good state; the collision-path double-execution remains open, documented in the fix log's Follow-ups with the failed approach recorded to save a future attempt from repeating it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com> Co-authored-by: aknysh <andriy.knysh@gmail.com>
This branch was successfully deployed
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
2 participants
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.
what
kubernetes.gitops.provision.targets.<name>(kind: git) now supports asplittri-state:split: falsewritespathas a single merged multi-document YAML file instead of always treatingpathas a directory of auto-named files; unset infers the mode from whetherpath's last segment looks like a manifest filename (.yaml/.yml/.json).metadata.nameon Kustomize's ownKustomization/Componentobjects (matched againstsigs.k8s.io/kustomize/api/types's own kind/version constants), since Kustomize's own schema and field-enforcement never require one.validate: falsecomponent-level flag opts a component out of both the apply/deploy structural auto-gate and the standaloneatmos kubernetes validatecommand.splitdocumented onkubernetes-deploy.mdx, and the Kustomize exemption /validate: falsedocumented onkubernetes-validate.mdx.why
pathwas always treated as a directory, so configuringpath: ".../kustomization.yaml"created a directory by that name containing an auto-generated file inside it, instead of the exact file Kustomize's remote-include mechanism requires.metadata.nameunconditionally, forcing users to add a meaningless name to KustomizeComponent/Kustomizationobjects just to satisfy Atmos, even though Kustomize's own tooling never requires one.!terraform.state) and committing it to a deployment repo as a properkustomization.yamlfor Argo CD/Flux to consume.references