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
Normalize service and router names for ingress. #5623
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -335,10 +335,7 @@ func (p *Provider) loadConfigurationFromIngresses(ctx context.Context, client Cl | |||
rules = append(rules, "PathPrefix(`"+p.Path+"`)") | |||
} | |||
|
|||
routerKey := strings.Replace(rule.Host, ".", "-", -1) + strings.Replace(p.Path, "/", "-", 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the second strings.Replace was doing the replacement only on the first occurrence found, so is it safe to replace it with a call to provider.Normalize? is that what we want for sure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it's the expected behavior.
cab53d5
to
9e741e8
Compare
What does this PR do?
Normalize service and router names for ingress.
Motivation
remove
/
from namesFixes #5611
More
Added/updated testsAdded/updated documentation