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

Avoid corev1.ObjectReference and LocalObjectReference #49

Open
negz opened this issue Oct 13, 2019 · 4 comments
Open

Avoid corev1.ObjectReference and LocalObjectReference #49

negz opened this issue Oct 13, 2019 · 4 comments
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@negz
Copy link
Member

negz commented Oct 13, 2019

What problem are you facing?

Most inter-resource references in Crossplane are either an ObjectReference or a LocalObjectReference. ObjectReference contains many optional fields (e.g. UID, FieldPath, etc) that Crossplane never uses. This could be misleading to users, since some fields (i.e. Name) are always optional, and it's not obvious that the other fields are frequently not required when reading the API documentation or CRD specs. The same is true for LocalObjectReference, which exposes only one field - name - that is (for some reason that is not obvious to me) marked optional.

How could Crossplane help solve your problem?

Crossplane could use purpose-specific references that represent a subset of ObjectReference. This is compliant with the API conventions, which state:

Object references should either be called fooName if referring to an object of kind Foo by just the name (within the current namespace, if a namespaced resource), or should be called fooRef, and should contain a subset of the fields of the ObjectReference type.

@negz negz added the enhancement New feature or request label Oct 13, 2019
@negz negz changed the title Avoid corev1.ObjectReference Avoid corev1.ObjectReference and LocalObjectReference Oct 14, 2019
@negz
Copy link
Member Author

negz commented Nov 22, 2019

A related issue: kubernetes-retired/service-catalog#1411. I strongly recommend clicking through for context - @deads2k (SIG API Machinery chair) explains in more detail how and why ObjectReference is inappropriate.

@muvaf
Copy link
Member

muvaf commented Jun 13, 2020

I agree. A PR that implements all meta methods we have that are built on this object for other reference types we have could be helpful for adoption.

@muvaf
Copy link
Member

muvaf commented Jun 13, 2020

The refactor should include the interfaces where corev1.ObjectReference is accepted as parameter, too, like SetCompositionReference(*corev1.ObjectReference) etc.

@stale
Copy link

stale bot commented Aug 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants