Labels specified in the Gatway Infrastructure (i.e., .spec.infrastructure.labels) might be invalid

[snorwin]

This enhancement request is related to GEP: In Cluster Gateway Deployments (#1762)

What would you like to be added:
Along with AnnotationValue (which was introduced in #886), a new type alias called LabelValue should be introduced. Label values follow a well-defined format that can be validated using a pattern:

Valid label value:

• must be 63 characters or less (can be empty),
• unless empty, must begin and end with an alphanumeric character ([a-z0-9A-Z]),
• could contain dashes (-), underscores (_), dots (.), and alphanumerics between.

Reference: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/

Why this is needed:
If users can specify invalid label values in the Gateway resource, it may cause issues in the controllers if the values are not validated. The validation in the API schema eliminates the need for each controller to implement its own validation.

[gauravkghildiyal]

Thanks for the details and including relevant links. I have two thoughts regarding this:

1. There's a possibility that the choice of not having any strict validation on the AnnotationValue type (which is used both within .spec.infrastructure.labels and .spec.infrastructure.annotations) may have been deliberate to cater to the needs of implementations that have some other form of "infrastructure" i.e. non Kubernetes resources:
   This would imply that validating what is and is-not valid as a label value rests on the implementation to communicate.

2. Disregarding the previous point, enforcing such a constraint now may be considered a breaking change (since we are tightening the validation).

We can wait and see what other's might feel regarding this, but my take here is to keep things as they are.

[snorwin]

@gauravkghildiyal thanks for looking into the issue. I fully understand your perspective. I had assumed that the infrastructure section was more tightly integrated with Kubernetes, as it is part of the in-cluster deployment GEP.

[robscott]

Thanks @gauravkghildiyal and @snorwin! I think that's a reasonable explanation for why this might have happened, but especially as we're considering taking this API to GA, we likely want to make sure we have the validation as precise as we can.

@keithmattix or @howardjohn would it be problematic to tighten this validation to match k8s labels? We could theoretically loosen this in the future if needed.

[howardjohn]

From Istio POV we are fine with matching labels. I do see the valid point that it could be used for non-k8s, but I am not personally using it for that purpose at this time.

[youngnick]

I agree that this should match the validation for Kubernetes labels, since most if not all implementations will be using those.

[snorwin]

@youngnick is the discussion still ongoing or should I prepare a PR?

[youngnick]

I think a PR is a good next step, we'll make sure to mention it in the community meeting once it's opened, and leave it for folks to check out for a bit before merging.

This is a breaking change, so it's important we get this in as part of the 1.2 release.