-
Notifications
You must be signed in to change notification settings - Fork 144
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
Compliance multi-tenant changes #3111
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.
Looking good! A few relatively minor comments. Main one is re: how network policy is structured.
5417b37
to
2f2fe10
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.
A few more small things. Looks like you have a couple of unused TenantNamespaces fields left over from earlier commits as well.
Removing "merge-when-ready" label due to new commits |
Removing "merge-when-ready" label due to new commits |
|
||
// Watch for changes to primary resource Compliance | ||
err = c.Watch(&source.Kind{Type: &operatorv1.Compliance{}}, &handler.EnqueueRequestForObject{}) | ||
err = complianceController.Watch(&source.Kind{Type: &operatorv1.Compliance{}}, &handler.EnqueueRequestForObject{}) |
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 think this needs to be eventHandler
@@ -431,25 +431,25 @@ func guardianAllowTigeraPolicy(cfg *GuardianConfiguration) (*v3.NetworkPolicy, e | |||
{ | |||
Action: v3.Allow, | |||
Protocol: &networkpolicy.TCPProtocol, | |||
Source: ComplianceBenchmarkerSourceEntityRule, | |||
Source: networkpolicy.CreateSourceEntityRule(ComplianceNamespace, ComplianceBenchmarkerName), |
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 think for all of these single-tenant rules, you just want to use the DefaultHelper
- https://github.com/Josh-Tigera/operator/blob/691931ed32299b33efcbd98c8c69d8b49f1ba760/pkg/render/common/networkpolicy/networkpolicy.go#L205
This way we still follow the same code path for single / multi-tenant code.
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.
Two minor comments but otherwise LGTM
Description
For PR author
- [ ] If changing pkg/apis/, runmake gen-files
- [ ] If changing versions, runmake gen-versions
For PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bug
if this is a bugfix.kind/enhancement
if this is a a new feature.enterprise
if this PR applies to Calico Enterprise only.