-
Notifications
You must be signed in to change notification settings - Fork 0
reconcile allowedaddresses in nic #83
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
Changes from all commits
8e55bdc
67136bd
2105a7e
1d64de7
186eef8
00165b0
70ee4c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ package validation | |
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "net" | ||
| "regexp" | ||
|
|
||
| api "github.com/stackitcloud/machine-controller-manager-provider-stackit/pkg/provider/apis" | ||
|
|
@@ -165,6 +166,15 @@ func ValidateProviderSpecNSecret(spec *api.ProviderSpec, secrets *corev1.Secret) | |
| } | ||
| } | ||
|
|
||
| // Validate AllowedAddresses | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move into
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above |
||
| if len(spec.AllowedAddresses) > 0 { | ||
| for _, cidr := range spec.AllowedAddresses { | ||
| if _, _, err := net.ParseCIDR(cidr); err != nil { | ||
| errors = append(errors, fmt.Errorf("providerSpec.allowedAddresses has an invalid CIDR: %s", cidr)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Validate AffinityGroup | ||
| if spec.AffinityGroup != "" { | ||
| if !isValidUUID(spec.AffinityGroup) { | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Might be worth to separate functions into other files (e.g.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. solve in another story |
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.
Should be part of NetworkingSpec
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 would not put that in NetworkingSpec since other MCM providers are also passing almost all config in the top level.
The NetworkingSpec has only 2 configuration options which are mutually exclusive. And even Networking is optional because if it is not set, the VM is put into the "default" network. So the Networking field has its own logic separate from the allowedAddresses.