chore: remove random names from knuu #581
Conversation
WalkthroughThe changes in the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
pkg/traefik/traefik.go (2)
295-295: LGTM: Consistent ingress route naming patternGood use of
k8s.SanitizeNamewith a clear suffix. Consider standardizing the suffix format across the codebase (e.g., either use "ing-route" or "ingress-route" consistently).
Line range hint
1-365: Excellent implementation of deterministic naming strategyThe changes successfully replace random name generation with a consistent and predictable naming scheme. This improves test reliability while maintaining uniqueness through:
- Static constants for fixed names
- Namespace-prefixed names for cluster-scoped resources
- Sanitized, purpose-indicating suffixes for dynamic resources
Consider documenting the naming conventions in a central location to ensure consistency as the codebase grows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
pkg/traefik/traefik.go(5 hunks)
🔇 Additional comments (4)
pkg/traefik/traefik.go (4)
28-28: LGTM: Good replacement of random name with static constant
The introduction of a static service account name constant aligns well with the PR objective of removing random names, improving test predictability.
204-204: LGTM: Clear and predictable middleware naming
Good use of k8s.SanitizeName with a descriptive suffix that indicates the middleware's purpose.
Line range hint 327-331: LGTM: Appropriate error handling
The error handling properly wraps and propagates any creation failures.
58-61: LGTM: Good use of sanitized naming for cluster role
The combination of namespace and static role name with sanitization ensures both uniqueness and consistency. This is a good replacement for random naming.
Let's verify the cluster role naming pattern across the codebase:
✅ Verification successful
Let me verify the cluster role naming pattern by checking the actual implementation and test cases.
Let me check the actual implementation of SanitizeName to verify its behavior.
LGTM: Verified consistent and safe cluster role naming pattern
The implementation is verified to be robust and consistent across the codebase:
SanitizeNameproperly handles DNS-1123 compliance by converting to lowercase, replacing invalid chars with hyphens, and enforcing length limits- The namespace-based naming pattern is used exclusively for cluster roles, with no instances of random naming
- Test cases confirm proper sanitization behavior for various edge cases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other potential random name generations in cluster role creation
rg -g '*.go' 'CreateClusterRole|NewRandomK8.*Role'
Length of output: 1132
Script:
#!/bin/bash
# Check the test cases for cluster role naming patterns
rg -g '*_test.go' -B 2 -A 5 'TestCreateClusterRole.*\{|TestCreateClusterRoleBinding.*\{'
# Check for any other SanitizeName usages
rg -g '*.go' 'SanitizeName'
Length of output: 1813
Script:
#!/bin/bash
# Check the SanitizeName implementation and its test cases
cat pkg/k8s/utils.go | grep -A 15 "func SanitizeName"
cat pkg/k8s/utils_test.go | grep -A 30 "func TestSanitizeName"
Length of output: 1345
Closes #573
Note: we cannot delete completely random names from knuu as sometimes we need them. For example when reading a file from a docker image we need to create a random name or when kaniko wants to build an image, random names are used for jobs, pods, etc... which does not hurt the long running tests that uses static names.
Summary by CodeRabbit
New Features
Bug Fixes