API conventions: Discourage use of ObjectReference

[Miciah]

Document that use of ObjectReference, LocalObjectReference, and TypedLocalObjectReference is discouraged in favor of defining use-appropriate object reference types.

• contributors/devel/sig-architecture/api-conventions.md (References to related objects): Document how object reference types should be defined and used. Discourage use of ObjectReference, LocalObjectReference, and TypedLocalObjectReference.
  (Object references):
  (Naming conventions): Update references to "ObjectType".

---

@deads2k, this is a follow-up to kubernetes/kubernetes#87459. Is the added text accurate and reasonable?

[k8s-ci-robot]

Welcome @Miciah!

It looks like this is your first PR to kubernetes/community 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/community has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @Miciah. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Miciah
To complete the pull request process, please assign dims
You can assign the PR to them by writing /assign @dims in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• contributors/devel/sig-architecture/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[jdumars]

/ok-to-test

[liggitt]

ObjectReference has lots of rarely-used, poorly defined fields, so I agree it is not great to recommend. Are LocalObjectReference and TypedLocalObjectReference not recommended? LocalObjectReference seems reasonable to use

[robscott]

@liggitt We're having trouble understanding the current recommendations here. The guidance added to ObjectReference by @deads2k suggests the following:

Instead of using this type, create a locally provided and used type that is well-focused on your reference.

At the same time, it sounds like you're suggesting that TypedLocalObjectReference might still be worth using here?

This is in the context of a new set of Service APIs that use lots of object references. So far we've relied on the guidance from that comment on ObjectReference and created custom object reference types for all the things. Would it be preferable to reuse the core type?

We're also having trouble understanding the suggestion added in that same PR to use Resource instead of Kind in object references. That seems like it would be especially confusing for users that are used to Kind in object refs.

Right now we're in a confusing state where the godocs seem to disagree with the API conventions and it's hard to know which to follow.

[liggitt]

So far we've relied on the guidance from that comment on ObjectReference and created custom object reference types for all the things. Would it be preferable to reuse the core type?

A scoped type with exactly the fields you need is a good idea. LocalObjectReference is so simplistic as to make reuse almost more trouble than it's worth. TypedLocalObjectReference is more constrained than ObjectReference, but reusing the shared struct means you can't put any context-specific documentation/descriptions on the generated API doc.

We're also having trouble understanding the suggestion added in that same PR to use Resource instead of Kind in object references. That seems like it would be especially confusing for users that are used to Kind in object refs.

Typically, references are used to look up other objects via API calls. To make those API calls, you always need to resolve to a resource (which shows up in the API request URL path). You can use discovery to resolve a Kind to a resource, but since multiple resources could accept the same Kind, you would have to have a convention of taking the first resource associated with the Kind. Using the resource directly in the reference is less ambiguous.

[hiddeco]

You can use discovery to resolve a Kind to a resource, but since multiple resources could accept the same Kind, you would have to have a convention of taking the first resource associated with the Kind. Using the resource directly in the reference is less ambiguous.

Isn't this solved by the introduction of the Group? The convention I am used to as a controller developer is that a Resource translates to <kind>/<name>, but that does not seem to apply here as there also is a Name field in the example structure. Given this, I think convention like the following would be more intuitive to users and developers:

// FooReference holds a reference to an API object that can be used as a foo.
type FooReference struct {
   // Group is the group of the referent. The empty string represents
   // the core API group.
   Group string
   // Kind is the kind of of the referent, and must exist in the referenced Group.
   Kind string
   // Namespace is the namespace of the referent.
   Namespace string
   // Name is the name of the referent.
   Name string
}

This would also match what is currently defined in CustomResourceDefinition resources:

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: secrets.secret.example.com
spec:
  group: secret.example.com
  names:
    kind: Secret
    listKind: SecretList
    plural: secrets
    singular: secret

which would translate to:

&FooReference{
	Group:     "secret.example.com",
	Kind:      "Secret",
	Namespace: "default",
	Name:      "foo",
}

while not causing conflicts with the core Secret resource kind.

[dims]

@Miciah looks like there's some feedback from @liggitt can you please incorporate them?

/uncc

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

[k8s-ci-robot]

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.