kubernetes-sigs / cluster-api-provider-nested

Cluster API Provider for Nested Clusters
Apache License 2.0
298 stars 65 forks source link

🐛 namespace names should valid RFC 1123 DNS labels #340

Closed wondywang closed 1 year ago

wondywang commented 1 year ago

What type of PR is this? /kind bug

What this PR does / why we need it: As mentioned here, https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/#namespaces-and-dns, all namespace names must be valid RFC 1123 DNS labels.

wondywang commented 1 year ago

@christopherhein PTAL, thanks.

Fei-Guo commented 1 year ago

Out of curiosity, what is the difference between these two values?

wondywang commented 1 year ago

Out of curiosity, what is the difference between these two values?

// DNS1123SubdomainMaxLength is a subdomain's max length in DNS (RFC 1123)
const DNS1123SubdomainMaxLength int = 253

// DNS1123LabelMaxLength is a label's max length in DNS (RFC 1123)
const DNS1123LabelMaxLength int = 63

And the maximum length supported by namespace is 63 characters.

Fei-Guo commented 1 year ago

Is DNS1123SubdomainMaxLength changed? I clearly remember this should be 64, otherwise the code is wrong.

wondywang commented 1 year ago

I'm not sure about this. But I found the latest definition, it is indeed the case. Because I recently discovered that there was too long namespace that was not synchronized, and then I found the reason here.

https://github.com/kubernetes/apimachinery/blob/master/pkg/util/validation/validation.go#L209

Fei-Guo commented 1 year ago

Did you change your kubernetes version locally?

wondywang commented 1 year ago

Did you change your kubernetes version locally?

no. and I checked the historical version of k8s.io/apimachinery, it should always be that value.

Fei-Guo commented 1 year ago

cool. Thanks for fixing the long-term bug then.

/lgtm /approve

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Fei-Guo, wondywang

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[virtualcluster/OWNERS](https://github.com/kubernetes-sigs/cluster-api-provider-nested/blob/main/virtualcluster/OWNERS)~~ [Fei-Guo] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment