Skip to content

Commit

Permalink
fix legacy network policy err (#2313)
Browse files Browse the repository at this point in the history
* 1. create np's acl rules before the pod completely starts
    2. fix not allowing gateway packet pass
    3. sctp and udp use the same lb

* remove mask
  • Loading branch information
changluyi committed Feb 15, 2023
1 parent 9c51bd9 commit 156d597
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 26 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build-x86-image.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,7 @@ jobs:
- build-centos-compile
- k8s-conformance-e2e
- k8s-netpol-e2e
# - k8s-netpol-legacy-e2e
- k8s-netpol-legacy-e2e
- cyclonus-netpol-e2e
- kube-ovn-conformance-e2e
- kube-ovn-ic-conformance-e2e
Expand Down
33 changes: 21 additions & 12 deletions pkg/controller/network_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ func (c *Controller) handleUpdateNp(key string) error {
}
}

if err = c.ovnLegacyClient.CreateGatewayACL(pgName, subnet.Spec.Gateway, subnet.Spec.CIDRBlock); err != nil {
if err = c.ovnLegacyClient.CreateGatewayACL("", pgName, subnet.Spec.Gateway, subnet.Spec.CIDRBlock); err != nil {
klog.Errorf("failed to create gateway acl, %v", err)
return err
}
Expand Down Expand Up @@ -594,7 +594,7 @@ func (c *Controller) fetchSelectedPorts(namespace string, selector *metav1.Label

ports := make([]string, 0, len(pods))
for _, pod := range pods {
if !isPodAlive(pod) || pod.Spec.HostNetwork {
if pod.Spec.HostNetwork {
continue
}
podName := c.getNameByPod(pod)
Expand Down Expand Up @@ -724,19 +724,28 @@ func (c *Controller) fetchPolicySelectedAddresses(namespace, protocol string, np
}

for _, pod := range pods {
for _, podIP := range pod.Status.PodIPs {
if podIP.IP != "" && util.CheckProtocol(podIP.IP) == protocol {
selectedAddresses = append(selectedAddresses, podIP.IP)
if len(svcs) == 0 {
continue
podNets, err := c.getPodKubeovnNets(pod)
if err != nil {
klog.Errorf("failed to get pod nets %v", err)
return nil, nil, err
}
for _, podNet := range podNets {
podIPAnnotation := pod.Annotations[fmt.Sprintf(util.IpAddressAnnotationTemplate, podNet.ProviderName)]
podIPs := strings.Split(podIPAnnotation, ",")
for _, podIP := range podIPs {
if podIP != "" && util.CheckProtocol(podIP) == protocol {
selectedAddresses = append(selectedAddresses, podIP)
}
}
if len(svcs) == 0 {
continue
}

svcIPs, err := svcMatchPods(svcs, pod, protocol)
if err != nil {
return nil, nil, err
}
selectedAddresses = append(selectedAddresses, svcIPs...)
svcIPs, err := svcMatchPods(svcs, pod, protocol)
if err != nil {
return nil, nil, err
}
selectedAddresses = append(selectedAddresses, svcIPs...)
}
}
}
Expand Down
13 changes: 7 additions & 6 deletions pkg/controller/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,6 @@ func (c *Controller) enqueueUpdatePod(oldObj, newObj interface{}) {
c.updateNpQueue.Add(np)
}
}

if oldPod.Status.PodIP != newPod.Status.PodIP {
for _, np := range c.podMatchNetworkPolicies(newPod) {
c.updateNpQueue.Add(np)
}
}
}

if newPod.Spec.HostNetwork {
Expand Down Expand Up @@ -344,6 +338,13 @@ func (c *Controller) enqueueUpdatePod(oldObj, newObj interface{}) {

if newPod.Annotations[fmt.Sprintf(util.AllocatedAnnotationTemplate, podNet.ProviderName)] == "true" && newPod.Spec.NodeName != "" {
if newPod.Annotations[fmt.Sprintf(util.RoutedAnnotationTemplate, podNet.ProviderName)] != "true" {
if c.config.EnableNP {
for _, np := range c.podMatchNetworkPolicies(newPod) {
klog.V(3).Infof("enqueue update pod %s' network policy", key)
c.updateNpQueue.Add(np)
}
}

klog.V(3).Infof("enqueue update pod %s", key)
c.updatePodQueue.Add(key)
break
Expand Down
5 changes: 5 additions & 0 deletions pkg/controller/subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,11 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error {
c.patchSubnetStatus(subnet, "ResetLogicalSwitchAclSuccess", "")
}

if err := c.ovnLegacyClient.CreateGatewayACL(subnet.Name, "", subnet.Spec.Gateway, subnet.Spec.CIDRBlock); err != nil {
klog.Errorf("create gateway acl %s failed, %v", subnet.Name, err)
return err
}

if err := c.ovnLegacyClient.UpdateSubnetACL(subnet.Name, subnet.Spec.Acls); err != nil {
c.patchSubnetStatus(subnet, "SetLogicalSwitchAclsFailed", err.Error())
return err
Expand Down
24 changes: 17 additions & 7 deletions pkg/ovs/ovn-nbctl-legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -1871,10 +1871,6 @@ func (c LegacyClient) CombineEgressACLCmd(pgName, asEgressName, asExceptName, pr
ovnArgs = []string{"--", fmt.Sprintf("--id=@%s.drop.%d", id, index), "create", "acl", "action=drop", "direction=from-lport", "log=false", fmt.Sprintf("priority=%s", util.EgressDefaultDrop), fmt.Sprintf("match=\"%s\"", fmt.Sprintf("inport==@%s && ip", pgName)), "options={apply-after-lb=\"true\"}", "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.drop.%d", id, index)}
}

if ipSuffix == "ip6" {
ovnArgs = append(ovnArgs, []string{"--", fmt.Sprintf("--id=@%s.ip6nd.%d", id, index), "create", "acl", "action=allow-related", "direction=from-lport", fmt.Sprintf("priority=%s", util.EgressAllowPriority), "match=\"nd || nd_ra || nd_rs\"", "options={apply-after-lb=\"true\"}", "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.ip6nd.%d", id, index)}...)
}

if len(npp) == 0 {
allowArgs = []string{"--", fmt.Sprintf("--id=@%s.noport.%d", id, index), "create", "acl", "action=allow-related", "direction=from-lport", fmt.Sprintf("priority=%s", util.EgressAllowPriority), fmt.Sprintf("match=\"%s\"", fmt.Sprintf("%s.dst == $%s && %s.dst != $%s && inport==@%s && ip", ipSuffix, asEgressName, ipSuffix, asExceptName, pgName)), "options={apply-after-lb=\"true\"}", "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.noport.%d", id, index)}
ovnArgs = append(ovnArgs, allowArgs...)
Expand Down Expand Up @@ -1926,7 +1922,7 @@ func (c LegacyClient) DeleteACL(pgName, direction string) (err error) {
return
}

func (c LegacyClient) CreateGatewayACL(pgName, gateway, cidr string) error {
func (c LegacyClient) CreateGatewayACL(ls, pgName, gateway, cidr string) error {
for _, cidrBlock := range strings.Split(cidr, ",") {
for _, gw := range strings.Split(gateway, ",") {
if util.CheckProtocol(cidrBlock) != util.CheckProtocol(gw) {
Expand All @@ -1937,8 +1933,22 @@ func (c LegacyClient) CreateGatewayACL(pgName, gateway, cidr string) error {
if protocol == kubeovnv1.ProtocolIPv6 {
ipSuffix = "ip6"
}
ingressArgs := []string{MayExist, "--type=port-group", "acl-add", pgName, "to-lport", util.IngressAllowPriority, fmt.Sprintf("%s.src == %s", ipSuffix, gw), "allow-related"}
egressArgs := []string{"--", MayExist, "--type=port-group", "--apply-after-lb", "acl-add", pgName, "from-lport", util.EgressAllowPriority, fmt.Sprintf("%s.dst == %s", ipSuffix, gw), "allow-related"}

var ingressArgs, egressArgs []string
if pgName != "" {
ingressArgs = []string{"--", MayExist, "--type=port-group", "acl-add", pgName, "to-lport", util.IngressAllowPriority, fmt.Sprintf("%s.src == %s", ipSuffix, gw), "allow-related"}
egressArgs = []string{"--", MayExist, "--type=port-group", "--apply-after-lb", "acl-add", pgName, "from-lport", util.EgressAllowPriority, fmt.Sprintf("%s.dst == %s", ipSuffix, gw), "allow-related"}
if ipSuffix == "ip6" {
egressArgs = append(egressArgs, []string{"--", "--type=port-group", MayExist, "--apply-after-lb", "acl-add", pgName, "from-lport", util.EgressAllowPriority, `nd || nd_ra || nd_rs`, "allow-related"}...)
}
} else if ls != "" {
ingressArgs = []string{"--", MayExist, "acl-add", ls, "to-lport", util.IngressAllowPriority, fmt.Sprintf(`%s.src == %s`, ipSuffix, gw), "allow-related"}
egressArgs = []string{"--", MayExist, "--apply-after-lb", "acl-add", ls, "from-lport", util.EgressAllowPriority, fmt.Sprintf(`%s.dst == %s`, ipSuffix, gw), "allow-related"}
if ipSuffix == "ip6" {
egressArgs = append(egressArgs, []string{"--", MayExist, "--apply-after-lb", "acl-add", ls, "from-lport", util.EgressAllowPriority, `nd || nd_ra || nd_rs`, "allow-related"}...)
}
}

ovnArgs := append(ingressArgs, egressArgs...)
if _, err := c.ovnNbCommand(ovnArgs...); err != nil {
return err
Expand Down

0 comments on commit 156d597

Please sign in to comment.