Scope in object references may be unnecessary

[robscott]

As a follow up from @howardjohn's v1alpha2 feedback in #790:

ParentRef.Scope is redundant. I know sig-apimachinery suggested it, but I disagree. Its inconsistent with Kubernetes - I don't run kubectl get clusterrole --scope Cluster, I run kubectl get clusterrole and the api-server knows it is cluster scoped. Why make users specify this boilerplate? It also gives users another way to create invalid config (ie Scope=Cluster Kind=Gateway); a great API makes it hard to create invalid configs.

[robscott]

This was also raised by @danwinship in #780 (comment).

If Namespace is optional then Scope seems redundant; if Namespace is nil, then it's cluster scope, otherwise it's namespace scope

[hbagdi]

I think this is mostly about clarity. Nil namespace could potentially imply the same namespace.

Because there is a major distinction between namespace-level and cluster-level resources, the additional clarity at the cost of verbosity could be helpful to the end user, who may not always remember the scope of each resource (unless the resource names have the required context (like ClusterRole).

I think this is one of those areas where historical design decision should not be used to justify the choice we make in the present.

[howardjohn]

the additional clarity at the cost of verbosity could be helpful to the end user, who may not always remember the scope of each resource (unless the resource names have the required context (like ClusterRole).

I disagree - if the user does not remember the scope of the type, requiring it is the opposite of what we want - that means that there is a blocker to the user using the API.

There is already substantial prior art here with kubectl and the api-server. Users are NOT expected to need to know if the resource is cluster/scoped today - they can get resources and even create resources with or without a namespace, not worrying about "scope".

By adding the Scope we field we make the user do something the computer already can do. Aside from being annyoing and adding additional cognitize complexity, it also introduces more errors. See https://khalilstemmler.com/articles/typescript-domain-driven-design/make-illegal-states-unrepresentable for some more discussion on this.

I cannot see why as user would want to set this field. Its a bit equivalent to requiring a name and a md5-hash-of-name. If the API requires a hash, it should compute it itself - not ask the user to set it as well

[hbagdi]

John, I see your point. This is asking the user to do more than necessary.
But I think this leads to a safer API as the user has to be more aware when they are crossing the scope boundary, which can have security implications.
Maybe this is something already covered by ReferencePolicy so we could omit it.

I'm still ruminating on what is the right balance here. Would love to hear more thoughts.

[youngnick]

Given that this discussion is about parentRef.scope, I think that it's relevant to consider what we anticipate being used as parents. Right now, all the options are namespaced things, and I can't really see that you'd ever want to attach a namespaced Route to a cluster-scoped object - that seems like a bad idea to me.

That said, if we do want to allow it, I think having the Kind work the same way as kubectl make sense, as @howardjohn said originally. If the namespace is unspecified, it means a cluster-scoped reference if the Kind is cluster-scoped, and the same namespace if the Kind is namespaced. That's actually a pretty safe default because a cross-namespace reference to a non-cluster-scoped object will require the equivalent of a ReferencePolicy on the parent object anyway.

I just don't see that having the Scope specified really adds that much. I think that specifying the Kind is enough.

[robscott]

I tried to look back through the GEP, and I can't find a clear motivation for a cluster-scoped parent ref. I think I'd originally considered that as a way to target potential cluster-scoped mesh resources. Although that could still be relevant, I'm not sure we have a clear use for it today. The in-progress mesh implementations I'm aware of right now are actually planning on a namespace-scoped resource at this level, so I think we may not actually need to support cluster-scoped parent resources, at least not yet.

If anyone has a use case for cluster-scoped route parents, I think that could be a compelling reason to leave the cluster-scoped ref option in place. Otherwise, the simplest way to resolve this may just be to remove scope and require namespace in ParentRef.

[robscott]

@lavalamp If I'm remembering correctly, the rationale for scope in object reference fields was to avoid any form of ambiguity as far as which resource was being referred to. Now that we've clarified that all implementations of this API must have a predefined kind->resource mapping, it appears that there is no room for ambiguity here. Would it be acceptable to remove the scope field from these references?

[lavalamp]

If the scope is known, then yes, remove the field (and add a schema annotation when/if those become a thing).

[robscott]

Thanks @lavalamp! What would a "schema annotation" look like here?

[lavalamp]

The idea is that we'd put an annotation in the schema to state all the items of an object reference that happen to be constant, so tools can also follow the references. It looks like we haven't published this idea yet, though.