Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TRON-2333: Fix pod names ending in .--xxxx #222

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jfongatyelp
Copy link
Contributor

@jfongatyelp jfongatyelp commented Jan 25, 2025

There was an edge case where sanitized pod names ending up with .<6 characters> would then get truncated to something like .--<4 random chars> due to our truncation logic in get_sanitised_kubernetes_name.

However .--aaaa is not valid in a pod name as each section between .s must start with an alphanumeric character.

We saw errors such as the following when this case was hit:

{"reason":"FieldValueInvalid","message":"Invalid value: \"some-long-pod-name.--9b3a\": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and
must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')","field":"metadata.name"}

Solution

Before we try to add our truncation suffix, we simply strip any trailing . in our truncated name. This yields a pod name 1 character less than expected, but ensures pod name formats will continue to match previous expectations.

I also did not strip any further characters as . was the most important one that would cause validation to fail.

Verification

with the new test in place but using the old get_sanitised_kubernetes_name, the test fails as expected, with the non-RFC1123-compliant result:

E       AssertionError: assert 'hello--world.--cf28' == 'hello--world--cf28'
E
E         - hello--world--cf28
E         + hello--world.--cf28
E         ?             +

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant