Repository navigation
fix: remove promise status block from generated promises - #275
Merged
Merged
Conversation
PromiseStatus's Workflows/WorkflowsFailed/WorkflowsSucceeded fields lack omitempty, so any command that marshals a full Promise struct (init, build, add container, update api/dependencies/destination-selector) always wrote a flat status: block, even for a freshly scaffolded Promise. That block no longer matches the current Kratix CRD and causes strict-decoding errors on kubectl apply. Fixes #274 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
No functional change — keeps the !splitFile branch first to match the original code's polarity instead of gratuitously flipping it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a Ginkgo unit test suite for promiseutils.MarshalPromiseWithoutStatus covering a zero-value status, a populated status, and that the rest of the Promise round-trips unchanged. Also extends the existing matchPromise integration test helper (used by init, build, and update api/dependencies/destination-selector specs) to assert the raw promise.yaml never has a status key, and adds the same check to the add-container and update-dependencies flows that don't go through matchPromise. All added assertions run against local temp dirs with no network calls, so they're deterministic and fast. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
abangser
commented
Sep 15, 2026
| } | ||
|
|
||
| apiContents := &runtime.RawExtension{Raw: jsonBytes} | ||
| var data interface{} = apiContents |
Member
Author
There was a problem hiding this comment.
This is the only bit I am concerned about 👀
ChunyiLyu
approved these changes
Sep 16, 2026
ChunyiLyu
left a comment
ChunyiLyu
left a comment
Member
There was a problem hiding this comment.
Looks good! Manually tested and saw no 'status: {}'
| } | ||
|
|
||
| apiContents := &runtime.RawExtension{Raw: jsonBytes} | ||
| var data interface{} = apiContents |
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
3 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.
Status is server-managed and shouldn't exist in a manifest the CLI hands you to
kubectl apply. While not a hard failure to include it, the stale block can (and did) drift from the currently-installed Kratix CRD expects, sokubectl applyfailed outright with a strict-decoding error in the demos repo.This fix strips the
statuskey generically at the point of marshaling making it agnostic to futurePromiseStatuschanges rather than pinning to a specific kratix version or special-casing individual status fields which just reintroduces the same problem the next timePromiseStatus's shape changes.Test coverage was previously all indirect (golden-file comparisons on
init's output); nothing exercisedadd container,build promise, orupdate dependencies/api/destination-selector, so this also closes that gap with a couple of fast, local, no-network assertions.Fixes #274