-
Notifications
You must be signed in to change notification settings - Fork 0
Specify initial rule set in firewall spec #81
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
base: main
Are you sure you want to change the base?
Conversation
a4e5cb2 to
fd56c1b
Compare
Gerrit91
left a comment
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.
Looks really good, just one remark.
| // Protocol constraints the protocol this rule applies to. | ||
| Protocol NetworkProtocol `json:"protocol"` | ||
| // To source address cidrs this rule applies to. | ||
| To []string `json:"to"` |
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.
It does not have to be 100% complete (this is done by metal-api anyway) but maybe some very basic validation would be nice to prevent misconfiguration. The validations can be found api/v2/validation/firewall.go.
For me it would be sufficient to add:
- Valid protocol
- Valid CIDRs
- Valid port range
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 checked the metal-api and added some basic checks. Is that what you were thinking about? Could be slightly refactored to remove duplication.
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, that was what I was thinking about. Thanks for adding these validations. :)
Please apply your refactorings and I'll give it another review.
Description
Allows specifying an initial rule set for firewall creation.
Code extracted from: #64