AIP-122, AIP-123: Validate the singular and plural fields of google.api.resource annotations.

[odsod]

I think we're missing a linter that enforces consistency between the plural/singular fields within google.api.ResourceDescriptor - specifically:

• singular should be in sync with type (the resource type)
• plural should be in sync with the collection identifier in pattern (the resource name)

From AIP-122 (Resource names):

The collection identifier segments in a resource name must be the plural form of the noun used for the resource. (For example, a collection of Publisher resources is called publishers in the resource name.)

From AIP-123 (Resource types):

APIs must define a resource type for each resource in the API, according to the following pattern: {Service Name}/{Type}. The type name must be singular and use PascalCase (UpperCamelCase).

From google/api/resource.proto:

// The plural name used in the resource name and permission names, such as
// 'projects' for the resource name of 'projects/{project}' and the permission
// name of 'cloudresourcemanager.googleapis.com/projects.get'. It is the same
// concept of the plural field in k8s CRD spec
// https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/
//
// Note: The plural form is required even for singleton resources. See
// https://aip.dev/156
string plural = 5;

// The same concept of the singular field in k8s CRD spec
// https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/
// Such as "project" for the resourcemanager.googleapis.com/Project type.
string singular = 6;

From the k8s CRD spec:

// plural name to be used in the URL: /apis///
plural: crontabs

As such, what are your thoughts about a linter rules that enforce:

• The plural field must be URL safe (i.e. no whitespace) and use lowerCamelCase
• The collection identifiers in the resource name pattern values match the plural field
• The Type component of the resource type field ({Service Name}/{Type}) matches the UpperCamelCase version of the singular field

[lukesneeringer]

I talked to Eric Tune, who said the k8s convention is all lower case (meaning they suppress word separators, ick).

Honestly, I wonder if we need these fields at all. I am not aware of anyone that uses them and we have the information in the type and pattern already.

[odsod]

I talked to Eric Tune, who said the k8s convention is all lower case (meaning they suppress word separators, ick).

Good to know! And yeah, adopting that would break convention with a bunch of existing examples...

I am not aware of anyone that uses them and we have the information in the type and pattern already.

We use these fields when generating names of GraphQL connections and other similar plural stuff, so I can see the benefit of having the information accessible in a more structured way than by parsing the resource type and resource name pattern(s).

If the fields are set, do you still think it would make sense to the above-mentioned consistency checks on them?

[odsod]

I went through our code and we also use these fields to infer method names for resources (also for GraphQL generation, to know e.g. whether a resource has List and BatchGet methods).

[lukesneeringer]

If the fields are set, do you still think it would make sense to the above-mentioned consistency checks on them?

Yes, I think it makes sense that if they are set, we ensure they are correct. But I would like to not make people set them if we can help it.

And still not sure what the naming convention should be.

[toumorokoshi]

Yes, I think it makes sense that if they are set, we ensure they are correct. But I would like to not make people set them if we can help it.

I think we do need to set them, so that consumers of protobufs have the ability to clearly understand what the resource singular and plural are intended to be.

There already is linting against plural / singular nouns in rules such as aip0136/http_name_variable. I'm a little wary myself that we would rely on a library for plural / singular (e.g. https://github.com/gertd/go-pluralize) and match against those instead of referencing a field of the service owners choosing as the source for consistency.

[toumorokoshi]

A few more thoughts:

• extracting the plural from the pattern is unreliable: there is an AIP provision to allow eliminating redundant components of a collection name (pattern): https://google.aip.dev/122#collection-identifiers.

If a resource name contains multiple levels of a hierarchy, and a parent collection's name is used as a prefix for the child resource's name, the child collection's name may omit the prefix. For example, given a collection of UserEvent resources that would normally be nested underneath users:

So we have to require the plural at least. At that point, why not also require stable? it can match the type, and it's one extra line.