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

feat: add the ability to override merged output from XR fields #49

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

fernandezcuesta
Copy link
Contributor

Description of your changes

Add the ability to override the merged context with fields coming from the XR.
For example this is useful in scenarios where we load fields from an
EnvironmentConfig such as region and we want to transparently override this
to all context users with a value from the XR (e.g. under spec.parameters.region).

Fixes #48

I have:

@fernandezcuesta fernandezcuesta force-pushed the main branch 3 times, most recently from 26e59fe to a50f3cc Compare November 4, 2024 11:26
Signed-off-by: Jesús Fernández <7312236+fernandezcuesta@users.noreply.github.com>
@phisco
Copy link
Collaborator

phisco commented Nov 5, 2024

What if the source field is missing? should we error out or just ignore it? should it be configurable? and what if users want more powerful patching capabilities, e.g. combinations or transforms? I'd be fine supporting a subset of what spec.environment.patches was and is now implemented in function-patch-and-transform

Signed-off-by: Jesús Fernández <7312236+fernandezcuesta@users.noreply.github.com>
@fernandezcuesta
Copy link
Contributor Author

Hi, thanks for having a look at it.
I added another test to validate your first question, when the source file is missing it ignores it. I feel it shouldn't fail in that scenario since that can refer to a status field which may exist or not.
Happy to scout what P&T does with patches, will give it a go since you're right, I was assuming very simple patches and that's (unfortunately) rarely the case.

@fernandezcuesta
Copy link
Contributor Author

fernandezcuesta commented Nov 6, 2024

Hi @phisco I made an attempt here, but this requires exporting most of function-patch-and-transform in order to keep this DRY. For the sake of testing this e2e, I forked the latter there and everything looks to be working fine.
Please let me know if this is what you refer to.

From my point of view, only XR->Env patches apply here, thus only FromCompositeFieldPath and CombineFromComposite patches are allowed in the input.

Related: crossplane-contrib/function-patch-and-transform#153

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.

Support output overrides
2 participants