-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add proxy port to CNP if needed #1531
Conversation
} | ||
proxyPort := proxyURL.Port() | ||
if proxyPort == "" { | ||
proxyPort = "80" |
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.
Well this is nasty but if it works
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.
Do you have a suggestion to avoid that ?
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.
Well, the think is if it is https, it should be 443 and not 80 but no the nasty part was more about the nested proxy things above btw
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.
I probably would have used this https://pkg.go.dev/golang.org/x/net/http/httpproxy#FromEnvironment and only checked the HTTPProxy and HTTPSProxy instead of having 4 nested ifs but it's temporary code so :)
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.
Code refactored, tell me ;-)
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.
Nice :)
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.
I like nasty code :)
Need to be tested on
|
…eus-meta-operator into update-cnp-with-proxy
towards https://github.com/giantswarm/giantswarm/issues/29873
Checklist
I have:
CHANGELOG.md