feat: merge new extra resources into existing context#93
feat: merge new extra resources into existing context#93tenstad wants to merge 6 commits intocrossplane-contrib:mainfrom
Conversation
Signed-off-by: Amund Tenstad <github@amund.io>
Signed-off-by: Amund Tenstad <github@amund.io>
6b3bebd to
2c2336a
Compare
phisco
left a comment
There was a problem hiding this comment.
This would be a breaking change, what do you think about having it as a policy, spec.context.Policy: Replace (default) or Merge for this behavior?
Signed-off-by: Amund Tenstad <github@amund.io>
Signed-off-by: Amund Tenstad <github@amund.io>
Good idea! Can then also potentially have a If so, should we also make sure it's possible to add an option to merge all resource results, instead of returning a list? And is |
|
Would any of these options make sense here? Could allow for appending new results onto existing keys when there are duplicates? (not in this PR, just trying to plan ahead) |
I think I would put this outside of However, if the result of each element in # initial context
context:
apiextensions.crossplane.io/environment:
config:
# ...
data:
labels:
example.crossplane.io/env-type: prod
clusters:
- apiVersion: example.crossplane.io/v1
kind: Cluster
metadata:
name: first
# ...
# run extra-resources function
context:
key: apiextensions.crossplane.io/environment
policy: MergeObjectsAppendArrays # merge into 'config', append to 'clusters'
# policy: Replace # replace the entire key with the new results (current behaviour)
extraResources:
- into: config
kind: EnvironmentConfig
mode: Merge # return single object
mergePolicy: MergeObjects # could add policy field if wanting to configure how to handle arrays
# ...
- into: clusters
kind: Cluster
mode: Array # return list of resources
# ...
# resulting context
context:
apiextensions.crossplane.io/environment:
config:
# ...
data:
labels:
example.crossplane.io/env-type: prod
example.crossplane.io/env-source: github.com/org/team # merged in
clusters:
- apiVersion: example.crossplane.io/v1
kind: Cluster
metadata:
name: first
# ...
- apiVersion: example.crossplane.io/v1 # appended
kind: Cluster
metadata:
name: second
# ...Though, if the initital context looked like this, it wouldn't work. # initial context
context:
apiextensions.crossplane.io/environment:
config: # cannot merge object into array, would then replace or fail?
- labels:
example.crossplane.io/env-type: prod
clusters:
first: # cannot merge array into object, would then replace or fail?
apiVersion: example.crossplane.io/v1
kind: Cluster
# ... |
Signed-off-by: Amund Tenstad <github@amund.io>
… CRD Signed-off-by: Amund Tenstad <github@amund.io>
| // Replace replaces the existing context key with the new extra resources. | ||
| // Merge merges the extra resources into the context key's existing value. | ||
| // +kubebuilder:default=Replace | ||
| // +kubebuilder:validation:Enum=Replace;Merge |
There was a problem hiding this comment.
| // +kubebuilder:validation:Enum=Replace;Merge | |
| // +kubebuilder:validation:Enum=Replace;MergeObjects |
So that it matches patch and trasform?
https://github.com/crossplane-contrib/function-patch-and-transform/blob/b12238e354e01df2db426f6347b7850029279773/input/v1beta1/resources_patches.go#L41C70-L42
And potentially add MergeObjectsAppendArrays in a future PR.
Description of your changes
Loads the potentially existing context and inserts the new (
into) keys in it. Instead of replacing the whole context.Allows for running the function multiple times, without loosing the results of previous runs.
Was not sure if I should add to an existing test or create a new one. Let me know if I should create a new one instead.
Fixes #92
I have: