crossplane-contrib / function-environment-configs

A function building the environment from EnvironmentConfigs
Apache License 2.0
15 stars 4 forks source link

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

Open fernandezcuesta opened 2 weeks ago

fernandezcuesta commented 2 weeks ago

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:

phisco commented 2 weeks ago

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

fernandezcuesta commented 2 weeks ago

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 commented 1 week ago

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: https://github.com/crossplane-contrib/function-patch-and-transform/pull/153