Skip to content
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

Add Support for Admin Network Policy #3489

Closed
wants to merge 16 commits into from
Closed

Conversation

tssurya
Copy link
Member

@tssurya tssurya commented Mar 15, 2023

EDIT: It was decided in the team meeting to change all our controllers into being like the service controller where every object change is re-queued back to the main queue and processed in a single place. ANP will also follow this pattern. I am re-working this as a new PR. Hence this PR is stale and will be superseded by #3659

Not ready for review, development ongoing.
https://docs.google.com/presentation/d/1Y-PrygN5vfDet0K3l89o83kJ3ryial3Dq2ZE3BDVlzM/edit#slide=id.p captures the implementation details in OVN-Kubernetes

Tasks

  • Vendor in network-policy-api
  • Add a feature enable flag
  • Add Controller for ANP
  • Implement add/delete/update ANP
  • Implement add/delete/update BANP
  • Implement selectors for peers
  • Implement namespace label updates for ANP
  • Implement pod label updates for ANP
  • Implement namespace deletion for ANP
  • Implement pod deletion and take care of completed pods logic for ANP
  • Implement namespace label updates for BANP
  • Implement pod label updates for BANP
  • Implement namespace deletion for BANP
  • Implement pod deletion and take care of completed pods logic for BANP
  • Implement sameLabels/notSameLabels for peers - This will be done only in next iteration (betav1 support)
  • Implement Named Ports for Conformance - depends on time, not a big deal to get in in this iteration
  • Ensure external_ids are set correctly for ACLs, address-sets, portgroups
  • Implement PASS action
  • Implement Status field in ANP/BANP
  • Write repair function
  • Fix all TODO's in code

Tests

Improvements

  • Perf improvement - use existing address-sets for namespaces without creating new ones

- What this PR does and why is it needed
This PR adds support for admin network policy in ovn-kubernetes. The API lives at https://github.com/kubernetes-sigs/network-policy-api and is managed by sig-network-policy-api working group. It is currently in v1alpva1 version. Hence until it graduates to beta, we probably will have only a tech preview version of this, not a full GA-ed version.

- Special notes for reviewers

- How to verify it

- Description for the changelog

Closes #3158

@tssurya tssurya changed the title Add Support for Admin Network Policy [WIP] Add Support for Admin Network Policy Mar 15, 2023
@tssurya tssurya marked this pull request as draft March 15, 2023 21:12
@tssurya tssurya changed the title [WIP] Add Support for Admin Network Policy Add Support for Admin Network Policy Mar 15, 2023
@tssurya tssurya force-pushed the ANP-V5 branch 3 times, most recently from 535ed60 to 0e6dd8a Compare March 28, 2023 10:34
@tssurya
Copy link
Member Author

tssurya commented Mar 28, 2023

2023-03-28T10:53:07.1750963Z I0328 10:53:07.164221   20835 gateway_shared_intf.go:562] Adding service service1 in namespace namespace1
2023-03-28T10:53:07.1751378Z I0328 10:53:07.164266   20835 gateway_shared_intf.go:583] Rules already programmed for service1 in namespace namespace1
2023-03-28T10:53:07.1751639Z 
2023-03-28T10:53:07.1751838Z �[91m�[1m• Failure [0.119 seconds]�[0m
2023-03-28T10:53:07.1752065Z Node Operations
2023-03-28T10:53:07.1752538Z �[90m/home/runner/work/ovn-kubernetes/ovn-kubernetes/go-controller/pkg/node/gateway_localnet_linux_test.go:219�[0m
2023-03-28T10:53:07.1752852Z   on add
2023-03-28T10:53:07.1756119Z   �[90m/home/runner/work/ovn-kubernetes/ovn-kubernetes/go-controller/pkg/node/gateway_localnet_linux_test.go:377�[0m
2023-03-28T10:53:07.1756904Z     �[91m�[1minits iptables rules and openflows with LoadBalancer where AllocateLoadBalancerNodePorts=False, ETP=local, LGW mode [It]�[0m
2023-03-28T10:53:07.1757645Z     �[90m/home/runner/work/ovn-kubernetes/ovn-kubernetes/go-controller/pkg/node/gateway_localnet_linux_test.go:837�[0m
2023-03-28T10:53:07.1757890Z 
2023-03-28T10:53:07.1758100Z     �[91mExpected success, but got an error:
2023-03-28T10:53:07.1758475Z         <*errors.errorString | 0xc000e14230>: {
2023-03-28T10:53:07.1760212Z             s: "expected rule \"-p TCP -d 5.5.5.5 --dport 80 -j DNAT --to-destination 10.244.0.3:8080 -m statistic --mode random --probability 0.5000000000\" at pos 0 in chain nat/OVN-KUBE-ETP, got \"-p TCP -d 5.5.5.5 --dport 80 -j DNAT --to-destination 10.244.0.4:8080 -m statistic --mode random --probability 0.5000000000\"",
2023-03-28T10:53:07.1760752Z         }
2023-03-28T10:53:07.1762142Z         expected rule "-p TCP -d 5.5.5.5 --dport 80 -j DNAT --to-destination 10.244.0.3:8080 -m statistic --mode random --probability 0.5000000000" at pos 0 in chain nat/OVN-KUBE-ETP, got "-p TCP -d 5.5.5.5 --dport 80 -j DNAT --to-destination 10.244.0.4:8080 -m statistic --mode random --probability 0.5000000000"�[0m
2023-03-28T10:53:07.1762563Z 
2023-03-28T10:53:07.1763018Z     /home/runner/work/ovn-kubernetes/ovn-kubernetes/go-controller/pkg/node/gateway_localnet_linux_test.go:957
2023-03-28T10:53:07.1763417Z �[90m------------------------------�[0m
2023-03-28T10:53:07.1763846Z �[0mNode Operations�[0m �[90mon add�[0m 
2023-03-28T10:53:07.1764300Z   �[1minits iptables rules and openflows with LoadBalancer where ETP=cluster, LGW mode�[0m

the pod's positions are changing or what?! ugh.

@tssurya
Copy link
Member Author

tssurya commented Mar 28, 2023

/retest-failed

c.anpQueue.Forget(anpKey)
return true
}
klog.Infof("SURYA")
Copy link
Collaborator

@astoycos astoycos May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit :)

if err != nil && !apierrors.IsNotFound(err) {
return err
}
klog.Infof("SURYA %v", anp)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit comment

tssurya added 13 commits June 3, 2023 10:22
Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
We have a new feature called Hierarchical
ACLs that is introduced in OVN to enable
support for tiered ACLs. This commit
ensures all existing ACLs for all features
are migrated towards tier2. By default all
new ACLs must be added to tier2.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This commit vendors in a new sigs.k8s.io repo
called network-policy-api that brings in the
changes needed for admin network policy.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This PR adds the pieces required to install
ANP&BANP CRDs on kind cluster.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This commit adds the preparation bits needed to be
used by the ANP controller in the following commit.
It also provides sufficient permissions to the
systemaccount to list/watch/patch the new CRDs
accordingly.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This commit adds a level driven controller using
workqueues and shared informers inspired off the
documentation and recommendations done by sig-api-machinery.
This follows suit to existing level driven controllers like
services, egressQoS, egressSVC.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This commit adds an elaborate test that tries to
ensure all plausible perms/combs work for ANP
and its interaction with pods/namespaces.

Steps:
"1. creating a pod that will act as subject of admin network policy; check if lsp is created"
"2. update the resource version and labels of the pod that matches the subject selector; check if lsp is added to port-group"
"3. update the ANP by adding one ingress rule; check if acl is created and added on the port-group"
"4. creating a pod that will act as peer of admin network policy; check if lsp is created"
"5. update the resource version and labels of the pod that matches the peer selector; check if IP is added to address-set"
"6. update the ANP by adding two more ingress rules with allow and pass actions; check if acls are created and added on the port-group"
"7. update the ANP by adding three egress rules with deny, allow and pass actions; check if acls are created and added on the port-group"
"8. update the labels of the pod that matches the peer selector; check if IPs are updated in address-sets"
"9. update the labels of the namespace that matches the peer selector; check if IPs are updated in address-sets"
"10. update the subject of admin network policy so that subject pod stops matching; check if port group is updated"
"11. update the resource version and labels of the pod that matches the new subject selector; check if port group is updated"
"12. update the labels of the namespace that matches the subject selector to stop matching; check if port group is updated"
"13. update the labels of the namespace that matches the subject selector to start matching; check if port group is updated"
"14. update the ANP by changing its priority and deleting 1 ingress rule & 1 egress rule; check if all objects are re-created correctly"
"15. delete pod matching peer selector; check if IPs are updated in address-sets"
"16. update the subject pod to go into completed state; check if port group is updated"
"17. delete the subject and peer selected namespaces; check if port group and address-set's are updated"
"18. update the ANP by deleting all rules; check if all objects are re-created correctly"
"19. delete the ANP; check if all objects are re-created correctly"

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
This commit adds an elaborate test that tries to
ensure all plausible perms/combs work for BANP
and its interaction with pods/namespaces.

Steps:
"1. creating a pod that will act as subject of baseline admin network policy; check if lsp is created"
"2. update the resource version and labels of the pod that matches the subject selector; check if lsp is added to port-group"
"3. update the BANP by adding one ingress rule; check if acl is created and added on the port-group"
"4. creating a pod that will act as peer of baseline admin network policy; check if lsp is created"
"5. update the resource version and labels of the pod that matches the peer selector; check if IP is added to address-set"
"6. update the BANP by adding two more ingress rules with allow and pass actions; check if acls are created and added on the port-group"
"7. update the BANP by adding three egress rules with deny, allow and pass actions; check if acls are created and added on the port-group"
"8. update the labels of the pod that matches the peer selector; check if IPs are updated in address-sets"
"9. update the labels of the namespace that matches the peer selector; check if IPs are updated in address-sets"
"10. update the subject of baseline admin network policy so that subject pod stops matching; check if port group is updated"
"11. update the resource version and labels of the pod that matches the new subject selector; check if port group is updated"
"12. update the labels of the namespace that matches the subject selector to stop matching; check if port group is updated"
"13. update the labels of the namespace that matches the subject selector to start matching; check if port group is updated"
"14. update the BANP by changing its priority and deleting 1 ingress rule & 1 egress rule; check if all objects are re-created correctly"
"15. delete pod matching peer selector; check if IPs are updated in address-sets"
"16. update the subject pod to go into completed state; check if port group is updated"
"17. delete the subject and peer selected namespaces; check if port group and address-set's are updated"
"18. update the BANP by deleting all rules; check if all objects are re-created correctly"
"19. delete the BANP; check if all objects are re-created correctly"

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
The actual tests are defined upstream,
but this commit ensures we can
consume them from upstream and run it
middlestream.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
The ANP API supports a status.Condition which
can be updated according to plugin's implementation.
Let's start a basic status update logic that we
can build more profusely upon on the future so that
users can leverage what went wrong from the status
itself

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
The BANP API supports a status.Condition which
can be updated according to plugin's implementation.
Let's start a basic status update logic that we
can build more profusely upon in the future so that
users can leverage status to understand what went wrong.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Admin Cluster Network Policy
2 participants