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

Pass watched Custom Resource to values mapper #93

Closed
wants to merge 2 commits into from
Closed

Pass watched Custom Resource to values mapper #93

wants to merge 2 commits into from

Conversation

SimonBaeumer
Copy link
Member

To map values from Custom Resources it is necessary to have access to the current reconciled CR inside the values mapper.

@joelanford
Copy link
Member

Can you walk me through your use case? The idea here is that the input into the mapper is the spec of the object. Do you have mapping logic that looks at fields other than .spec?

@SimonBaeumer
Copy link
Member Author

Thanks for your fast answer @joelanford!

Can you walk me through your use case? The idea here is that the input into the mapper is the spec of the object. Do you have mapping logic that looks at fields other than .spec?

Yes, we need to be able to read cluster state to read certain values - i.e. if a PVC already exists and this requires us to know the deployed namespace. This may exceed a mapping use-case as it is more detecting config based on cluster state.
For these things it is necessary to have access to the GroupVersionKind and metadata of the CR.

Or do there another way to manipulate values before applying helm upgrade/install/uninstall?

@joelanford
Copy link
Member

I saw the Translator implementation in your fork, which (I think?) addresses this. If so, I am in favor of replacing the Mapper with the Translator (or renaming the Translator to Mapper).

As was mentioned in the PR comments, having both are duplicative since anyone implementing a custom Translator can perform any other necessary mapping that the mapper would have performed.

One notable semantic change would be that all of the translation and mapping would happen before overrides are applied. This would make setting the overrides less ergonomic since mapped values may otherwise be an internal implementation detail of the operator.

@jmrodri
Copy link
Member

jmrodri commented Nov 30, 2021

Closing in favor of #114

@jmrodri jmrodri closed this Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants