-
Notifications
You must be signed in to change notification settings - Fork 22
Cluster Network Policy #199
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/assign @danwinship @npinaeva @bowei I just vibe coded this in 30 mins to unblock the progress on the feature and conformance, so I sstill need to review the code too, but with the new architecture is pretty easy also to add unit tests |
@npinaeva this does not work as it is because the controller only matches on the pods that matches a network policy, you can force that by setting admin network policy the boolean to true on the controller config |
fixed in last commit and image. uploaded to |
661b9e5
to
3f2d10e
Compare
pushed latest and most sure definitive version for the project structure, right now you checkout the PR and build the image with
no more flags for the same binary, now we have one binary per feature and as soon as they graduate they belong to the main binary, this is less risky and easier to iterate |
tested latest version with kubernetes-sigs/network-policy-api#307 locally and it seems to work! |
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.
this code looks surprisingly good :D
cmd/npa-v1alpha2/main.go
Outdated
@@ -0,0 +1,217 @@ | |||
package main |
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.
shouldn't this file be under cmd/kube-network-policies/npa-v1alpha2
?
} | ||
} | ||
for _, domain := range to.DomainNames { | ||
if c.domainResolver != nil && c.domainResolver.ContainsIP(string(domain), dstIP) { |
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 domainResolver is only nil for unit tests, but should we log something anyway in this case?
tested again, still works! |
/hold wait for merging the conformance tests so we also add the ci jobs for the new apis |
Network Policy API v1alpha2
/hold cancel I let nadia decide when she wants to merge and how :) |
let's get this in, then update the CI when the new conformance is merged |
Implementation of the new v1alpha2 api for ClusterNetworkPolicy