-
Notifications
You must be signed in to change notification settings - Fork 7
fix: validates CONTROL_PLANE_ENDPOINT_IP and NUTANIX_ENDPOINT are distinct #1002
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
Conversation
7deb42c
to
06b385d
Compare
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.
Looking good, just a few comments
b02e13e
to
21293e9
Compare
21293e9
to
db904f3
Compare
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.
Thanks!
func (s NutanixPrismCentralEndpointSpec) ParseIP() (netip.Addr, error) { | ||
pcHostname, _, err := s.ParseURL() | ||
if err != nil { | ||
return netip.Addr{}, err |
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.
nit: returning an empty struct here instead of nil
kinda smells
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.
yeah, looks weird at first.. I just followed net/netip
pkg semantic, maybe Addr
struct doesn't hold much data so they return empty struct instead of nil everywhere.
What problem does this PR solve?:
webhook errors out if NUTANIX_ENDPOINT IP is same as PC IP. It only implements dumb check which compares PC IP with control-plane IP.
Which issue(s) this PR fixes:
Fixes #
https://jira.nutanix.com/browse/NCN-102626
How Has This Been Tested?:
Special notes for your reviewer: