-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[kpt deployer] Fix non-kustomize manifests not rendered issue #5627
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5627 +/- ##
==========================================
- Coverage 71.42% 70.53% -0.90%
==========================================
Files 398 410 +12
Lines 14654 15643 +989
==========================================
+ Hits 10467 11034 +567
- Misses 3409 3796 +387
- Partials 778 813 +35
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good overall, I would just like to have a bit of documentation in code to explain what's happening here
@@ -312,7 +312,28 @@ func (k *Deployer) renderManifests(ctx context.Context, _ io.Writer, builds []bu | |||
if err != nil { | |||
return nil, fmt.Errorf("kustomize build: %w", err) | |||
} | |||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a comment here to just explain why this workaround is needed because I don't think it's super clear what's happening here without context from the github issue/PR
root cause: version changes. kpt-sourced manfiest cannot pipeline directly into the kpt function pipeline. Fix: The sourced manifest is of type "ResourceList" and the raw manifests are stored in the ResourceList.Items.
Thanks for the changes, CI flaked on this but I restarted the job. Once it's green I'll go ahead and merge |
Thank you @MarlonGamez! |
Fixes: #5580
Description
root cause: version changes. kpt-sourced manfiest cannot pipeline directly into the kpt function pipeline.
Fix: The sourced manifest is of type "ResourceList" and the raw manifests are stored in the ResourceList.Items.