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

fix(k8s/build/sanitize): implement similar line length and lowercase logic as containers for build #266

Merged
merged 1 commit into from Sep 16, 2022

Conversation

ecrupper
Copy link
Contributor

@ecrupper ecrupper commented Sep 15, 2022

Closes go-vela/community#679

https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names

Users run into trouble if their org or repo have uppercase letters, or the combination of the two results in a string longer than 63 characters. Fix was already in place for container names, but not pods.

@ecrupper ecrupper requested a review from a team as a code owner September 15, 2022 14:46
@ecrupper ecrupper self-assigned this Sep 15, 2022
@codecov
Copy link

codecov bot commented Sep 15, 2022

Codecov Report

Merging #266 (6ac8647) into master (ef95941) will decrease coverage by 0.15%.
The diff coverage is 23.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #266      +/-   ##
==========================================
- Coverage   97.08%   96.93%   -0.16%     
==========================================
  Files          57       57              
  Lines        6288     6301      +13     
==========================================
+ Hits         6105     6108       +3     
- Misses        134      143       +9     
- Partials       49       50       +1     
Impacted Files Coverage Δ
pipeline/build.go 81.53% <23.07%> (-14.62%) ⬇️

Copy link
Collaborator

@wass3r wass3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool with it, but we might consider adding a utility function for sanitizing name according to the spec or maybe there is one already?

@ecrupper ecrupper merged commit 26719d9 into master Sep 16, 2022
@ecrupper ecrupper deleted the fix-lowercase-pod branch September 16, 2022 14:46
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.

When creating a pipeline pod the name is not lower-cased
4 participants