Skip to content
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

fix: duplicated objects when charts are nested #40

Merged
merged 4 commits into from Jul 29, 2021
Merged

Conversation

Chriscbr
Copy link
Contributor

Fixes #31

I think the expected behavior from users is that in most circumstances, you should be able to directly apply resources generated by cdk8s synth to your k8s cluster (e.g. via kubectl apply -f dist/). However, it's possible for nested charts to cause resources to be listed twice. This PR resolves this by de-duplicating resources so that each one is only rendered once during a given "synth" call.

This PR makes the assumption that nesting charts within each other is implicitly a feature (I don't believe there are any other ways to arrive at charts containing the same resource twice?). It's possible we could instead add some kind of validation to prevent charts from being nested to resolve this issue instead, but this would become problematic if users are defining constructs that include charts.

Questions:

  • Should there be some way to synthesize a specific chart within an app from the CLI?
  • Should there be an option for synthesis (e.g. a CLI flag) to override this and allow duplicate objects?

@Chriscbr Chriscbr requested a review from iliapolo July 21, 2021 01:26
src/app.ts Outdated Show resolved Hide resolved
test/app.test.ts Outdated Show resolved Hide resolved
test/app.test.ts Show resolved Hide resolved
src/app.ts Outdated Show resolved Hide resolved
src/app.ts Outdated Show resolved Hide resolved
@Chriscbr
Copy link
Contributor Author

Per our discussion, I updated all synth methods so that the deduplication behavior is consistent across all of them.

@Chriscbr Chriscbr requested a review from iliapolo July 27, 2021 00:43
@mergify mergify bot merged commit f55ab10 into main Jul 29, 2021
@mergify mergify bot deleted the rybickic/dedup-objects branch July 29, 2021 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicated synthed resources when nesting Charts
2 participants