Skip to content

feat: merge new extra resources into existing context#93

Open
tenstad wants to merge 6 commits intocrossplane-contrib:mainfrom
tenstad:merge-context-keys
Open

feat: merge new extra resources into existing context#93
tenstad wants to merge 6 commits intocrossplane-contrib:mainfrom
tenstad:merge-context-keys

Conversation

@tenstad
Copy link
Contributor

@tenstad tenstad commented Feb 21, 2026

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:

Signed-off-by: Amund Tenstad <github@amund.io>
Signed-off-by: Amund Tenstad <github@amund.io>
Copy link
Collaborator

@phisco phisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@tenstad
Copy link
Contributor Author

tenstad commented Feb 21, 2026

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?

Good idea! Can then also potentially have a DeepMerge if supporting merging like function-environment-configs does.

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 policy specifig enough? mergePolicy is also an option. Maybe take some inspiration from *xpv1.MergeOptions?

@tenstad
Copy link
Contributor Author

tenstad commented Feb 21, 2026

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)

https://github.com/crossplane-contrib/function-patch-and-transform/blob/b12238e354e01df2db426f6347b7850029279773/input/v1beta1/resources_patches.go#L38-L44

@tenstad tenstad mentioned this pull request Feb 21, 2026
2 tasks
@tenstad
Copy link
Contributor Author

tenstad commented Feb 21, 2026

If so, should we also make sure it's possible to add an option to merge all resource results, instead of returning a list?

I think I would put this outside of context, as it relates more to how the resources are gathered than how the data is returned into the context. Meaning we might not need to concider it too much in this PR.

However, if the result of each element in extraResources can be either an object or an array, the context.policy would need to handle that. I think the following could make sense (not in this PR, but trying to thinking ahead).

# 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// +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.

@tenstad tenstad changed the title fix: merge new extra resources into existing context feat: merge new extra resources into existing context Feb 22, 2026
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.

function overrides the whole context (key) instead of inserting new (into) keys

2 participants