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

Enable Namespace as an optional field in TypedReference #541

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

diviner524
Copy link

Description of your changes

Added support for namespace scoped objects in TypedReference, so we can introduce general namespace scoped resource in the resources list of composition. This change is backward compatible with existing implementation and does not require any updates from existing providers.

This also requires some changes in the main Crossplane repo after the runtime is updated. I have created a corresponding draft PR to demonstrate the idea:

crossplane/crossplane#4606

Fixes #251

I have:

  • Read and followed Crossplane's [contribution process].
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Some manually testing with namespace scoped objects like ConfigMap.

For example, a Composition like below will help to update a namespace scoped ConfigMap with a redisHost URL from the status of certain CompositeResources. This will then enable workloads (like Deployments/StatefulSets) under the same namespace to consume the data from ConfigMap.

apiVersion: apiextensions.crossplane.io/v1
kind: Composition
...
spec:
...
  resources:
  - name: configmap
    base:
      apiVersion: v1
      kind: ConfigMap
      metadata:
        namespace: "some-namespace"
    patches:
    - type: FromCompositeFieldPath
      fromFieldPath: status.redisHost
      toFieldPath: data.redisHost

Signed-off-by: Shuxian Cai <shuxiancai@google.com>
@diviner524 diviner524 requested review from a team as code owners September 11, 2023 23:01
@negz negz self-assigned this Sep 11, 2023
@negz
Copy link
Member

negz commented Sep 11, 2023

I'm not sure this is something we want to support.

Today, Crossplane is designed to allow you to compose Crossplane managed resources (MRs). This is an intentional constraint. The consistent API shape and behaviors of these MRs makes Crossplane easier to reason about, both for end-users and Crossplane maintainers.

I appreciate that this is just a small change. What I am concerned about is the "slippery slope" - that folks will begin to use Crossplane as a composition layer for arbitrary Kubernetes resources and we as maintainers will end up on the hook for trying to make that a good experience.

@jbw976
Copy link
Member

jbw976 commented Sep 12, 2023

Here is some historical related discussion and thinking around this topic that is helpful for context and understanding more nuances of the impact of this type of change: crossplane/crossplane#1730

edit: link to the correct issue 🤪

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 Namespaced Managed Resources
3 participants