Use RFC 1123 instead of RFC 1035 for names validation

[norbjd]

Hello 👋

When trying to create a Knative Service with a name starting with a digit:

kn service create 1valid --image gcr.io/knative-samples/helloworld-go --port 8080

We are facing errors because of the validation webhook:

Error: admission webhook "validation.webhook.serving.knative.dev" denied the request: validation failed: not a DNS 1035 label: [a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')]: metadata.name

What I've found while debugging is that it seems "OK" to create a ksvc with a name starting with a digit, but the problem is in the revision name validation (hence the metadata.name at the end of the error) because a DNS record is generated from that revision. But, since the Revision name has to start with the Service name, we transitively cannot give the ksvc a name starting with a digit.

This is a little deceptive for users, as there is nothing in the documentation (as far as I can tell) stating that this restriction applies. However, I've found that such a strict restriction (added #3019 and #3045) might have not much sense as of today. I've read the RFC 1035, and it seems the restriction is introduced to follow the ARPANET host names rules 😅:

The labels must follow the rules for ARPANET host names. They must start with a letter, end with a letter or digit, and have as interior characters only letters, digits, and hyphen.

This restriction is blocking if someone tries to use something like an UUID as their ksvc names (like me 😁), as only UUIDs starting with a letter will work.

That's why I'm proposing to use the RFC 1123 validation, already available in the validation package used under the hood (https://github.com/kubernetes/apimachinery/blob/v0.33.3/pkg/api/validation/generic.go#L45) instead of RFC 1035, to be able to create Knative Services with a name starting with a digit. The RFC 1123 is indeed "more lax", and I don't think allowing names starting with a digit is a problem (feel free to correct me if I'm wrong).

To be honest, I'm not sure how the change I'm adding here plays with the ValidateObjectMetadata defined in https://github.com/knative/pkg/blob/release-1.19/apis/metadata_validation.go#L39, and called here: https://github.com/knative/serving/blob/knative-v1.19.0/pkg/apis/serving/metadata_validation.go#L39.

I'm assuming that, to be consistent, this validation in https://github.com/knative/pkg/ should also be changed. If my change is OK for you, I'm happy to do also the changes there.

Thanks!

Proposed Changes

• validate revision and configuration names using RFC 1123 instead of RFC 1035, to be able to create ksvc with a name starting with a digit

Release Note

Validate Revision and Configuration names using RFC 1123 instead of RFC 1035

[knative-prow]

Welcome @norbjd! It looks like this is your first PR to knative/serving 🎉

[knative-prow]

Hi @norbjd. Thanks for your PR.

I'm waiting for a knative 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.

Details

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-sigs/prow repository.

[knative-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: norbjd
Once this PR has been reviewed and has the lgtm label, please assign dprotaso for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• pkg/apis/OWNERS

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

[dprotaso]

/ok-to-test

[dprotaso]

I don't think this change will work. We create a corresponding Kubernetes Service for the Knative Service - this allows for intra-cluster routing to said service using DNS. For this to work the Service Name must follow RFC1035 (as per Kubernetes).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.27%. Comparing base (098102d) to head (62f519e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16007      +/-   ##
==========================================
+ Coverage   80.22%   80.27%   +0.04%     
==========================================
  Files         214      214              
  Lines       16904    16904              
==========================================
+ Hits        13562    13569       +7     
+ Misses       2979     2974       -5     
+ Partials      363      361       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dprotaso]

I don't think this change will work. We create a corresponding Kubernetes Service for the Knative Service - this allows for intra-cluster routing to said service using DNS. For this to work the Service Name must follow RFC1035 (as per Kubernetes).

Going to close this out for this reason.