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.

Sign up for GitHub

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

Jump to bottom

do not need to delete pg when update networkpolicy #1959

Merged
merged 1 commit into from
Nov 2, 2022
Merged

do not need to delete pg when update networkpolicy #1959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 54 additions & 37 deletions pkg/controller/network_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,6 @@ func (c *Controller) handleUpdateNp(key string) error {
egressAllowAsNamePrefix := strings.Replace(fmt.Sprintf("%s.%s.egress.allow", np.Name, np.Namespace), "-", ".", -1)
egressExceptAsNamePrefix := strings.Replace(fmt.Sprintf("%s.%s.egress.except", np.Name, np.Namespace), "-", ".", -1)

// delete existing pg to update acl
if err = c.ovnLegacyClient.DeletePortGroup(pgName); err != nil {
klog.Errorf("failed to delete port group %s before networkpolicy update process, %v", pgName, err)
}

if err = c.ovnLegacyClient.CreateNpPortGroup(pgName, np.Namespace, np.Name); err != nil {
klog.Errorf("failed to create port group for np %s, %v", key, err)
return err
Expand Down Expand Up @@ -240,12 +235,6 @@ func (c *Controller) handleUpdateNp(key string) error {
}
}

// before update or add ingress info,we should first delete acl and address_set
if err = c.ovnLegacyClient.DeleteACL(pgName, "to-lport"); err != nil {
klog.Errorf("failed to delete np %s ingress acls, %v", key, err)
return err
}

ingressAsNames, err := c.ovnLegacyClient.ListNpAddressSet(np.Namespace, np.Name, "ingress")
if err != nil {
klog.Errorf("failed to list ingress address_set, %v", err)
Expand All @@ -258,6 +247,15 @@ func (c *Controller) handleUpdateNp(key string) error {
}
}

var ingressAclCmd []string
exist, err := c.ovnLegacyClient.PortGroupExists(pgName)
if err != nil {
klog.Errorf("failed to query np %s port group, %v", key, err)
return err
}
if exist {
ingressAclCmd = []string{"--type=port-group", "acl-del", pgName, "to-lport"}
}
if hasIngressRule(np) {
for _, cidrBlock := range strings.Split(subnet.Spec.CIDRBlock, ",") {
protocol := util.CheckProtocol(cidrBlock)
Expand Down Expand Up @@ -309,15 +307,9 @@ func (c *Controller) handleUpdateNp(key string) error {
}

if len(allows) != 0 || len(excepts) != 0 {
if err = c.ovnLegacyClient.CreateIngressACL(pgName, ingressAllowAsName, ingressExceptAsName, svcAsName, protocol, npr.Ports, logEnable); err != nil {
klog.Errorf("failed to create ingress acls for np %s, %v", key, err)
return err
}
ingressAclCmd = c.ovnLegacyClient.CombineIngressACLCmd(pgName, ingressAllowAsName, ingressExceptAsName, svcAsName, protocol, npr.Ports, logEnable, ingressAclCmd, idx)
} else {
if err = c.ovnLegacyClient.CreateIngressACL(pgName, ingressAllowAsName, ingressExceptAsName, svcAsName, protocol, []netv1.NetworkPolicyPort{}, logEnable); err != nil {
klog.Errorf("failed to create default deny all ingress acls for np %s, %v", key, err)
return err
}
ingressAclCmd = c.ovnLegacyClient.CombineIngressACLCmd(pgName, ingressAllowAsName, ingressExceptAsName, svcAsName, protocol, []netv1.NetworkPolicyPort{}, logEnable, ingressAclCmd, idx)
}
}
if len(np.Spec.Ingress) == 0 {
Expand All @@ -333,10 +325,13 @@ func (c *Controller) handleUpdateNp(key string) error {
return err
}
ingressPorts := []netv1.NetworkPolicyPort{}
if err = c.ovnLegacyClient.CreateIngressACL(pgName, ingressAllowAsName, ingressExceptAsName, svcAsName, protocol, ingressPorts, logEnable); err != nil {
klog.Errorf("failed to create ingress acls for np %s, %v", key, err)
return err
}
ingressAclCmd = c.ovnLegacyClient.CombineIngressACLCmd(pgName, ingressAllowAsName, ingressExceptAsName, svcAsName, protocol, ingressPorts, logEnable, ingressAclCmd, 0)
}

klog.Infof("create ingress acl cmd is: %v", ingressAclCmd)
if err = c.ovnLegacyClient.CreateACL(ingressAclCmd); err != nil {
klog.Errorf("failed to create ingress acls for np %s, %v", key, err)
return err
}

if err = c.ovnLegacyClient.SetAclLog(pgName, logEnable, true); err != nil {
Expand Down Expand Up @@ -387,12 +382,6 @@ func (c *Controller) handleUpdateNp(key string) error {
}
}

// before update or add egress info, we should first delete acl and address_set
if err = c.ovnLegacyClient.DeleteACL(pgName, "from-lport"); err != nil {
klog.Errorf("failed to delete np %s egress acls, %v", key, err)
return err
}

egressAsNames, err := c.ovnLegacyClient.ListNpAddressSet(np.Namespace, np.Name, "egress")
if err != nil {
klog.Errorf("failed to list egress address_set, %v", err)
Expand All @@ -404,6 +393,16 @@ func (c *Controller) handleUpdateNp(key string) error {
return err
}
}

var egressAclCmd []string
exist, err = c.ovnLegacyClient.PortGroupExists(pgName)
if err != nil {
klog.Errorf("failed to query np %s port group, %v", key, err)
return err
}
if exist {
egressAclCmd = []string{"--type=port-group", "acl-del", pgName, "from-lport"}
}
if hasEgressRule(np) {
for _, cidrBlock := range strings.Split(subnet.Spec.CIDRBlock, ",") {
protocol := util.CheckProtocol(cidrBlock)
Expand Down Expand Up @@ -455,10 +454,7 @@ func (c *Controller) handleUpdateNp(key string) error {
}

if len(allows) != 0 || len(excepts) != 0 {
if err = c.ovnLegacyClient.CreateEgressACL(pgName, egressAllowAsName, egressExceptAsName, protocol, npr.Ports, svcAsName, logEnable); err != nil {
klog.Errorf("failed to create egress acls for np %s, %v", key, err)
return err
}
egressAclCmd = c.ovnLegacyClient.CombineEgressACLCmd(pgName, egressAllowAsName, egressExceptAsName, protocol, npr.Ports, svcAsName, logEnable, egressAclCmd, idx)
}
}
if len(np.Spec.Egress) == 0 {
Expand All @@ -474,11 +470,15 @@ func (c *Controller) handleUpdateNp(key string) error {
return err
}
egressPorts := []netv1.NetworkPolicyPort{}
if err = c.ovnLegacyClient.CreateEgressACL(pgName, egressAllowAsName, egressExceptAsName, protocol, egressPorts, svcAsName, logEnable); err != nil {
klog.Errorf("failed to create egress acls for np %s, %v", key, err)
return err
}
egressAclCmd = c.ovnLegacyClient.CombineEgressACLCmd(pgName, egressAllowAsName, egressExceptAsName, protocol, egressPorts, svcAsName, logEnable, egressAclCmd, 0)
}

klog.Infof("create egress acl cmd is: %v", egressAclCmd)
if err = c.ovnLegacyClient.CreateACL(egressAclCmd); err != nil {
klog.Errorf("failed to create egress acls for np %s, %v", key, err)
return err
}

if err = c.ovnLegacyClient.SetAclLog(pgName, logEnable, false); err != nil {
// just log and do not return err here
klog.Errorf("failed to set egress acl log for np %s, %v", key, err)
Expand Down Expand Up @@ -509,6 +509,23 @@ func (c *Controller) handleUpdateNp(key string) error {
}
}
}
} else {
if err = c.ovnLegacyClient.DeleteACL(pgName, "from-lport"); err != nil {
klog.Errorf("failed to delete np %s egress acls, %v", key, err)
return err
}

asNames, err := c.ovnLegacyClient.ListNpAddressSet(np.Namespace, np.Name, "egress")
if err != nil {
klog.Errorf("failed to list egress address_set, %v", err)
return err
}
for _, asName := range asNames {
if err = c.ovnLegacyClient.DeleteAddressSet(asName); err != nil {
klog.Errorf("failed to delete np %s address set, %v", key, err)
return err
}
}
}

if err = c.ovnLegacyClient.CreateGatewayACL(pgName, subnet.Spec.Gateway, subnet.Spec.CIDRBlock); err != nil {
Expand Down
45 changes: 26 additions & 19 deletions pkg/ovs/ovn-nbctl-legacy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1611,7 +1611,7 @@ func (c LegacyClient) CreateNpAddressSet(asName, npNamespace, npName, direction
return err
}

func (c LegacyClient) CreateIngressACL(pgName, asIngressName, asExceptName, svcAsName, protocol string, npp []netv1.NetworkPolicyPort, logEnable bool) error {
func (c LegacyClient) CombineIngressACLCmd(pgName, asIngressName, asExceptName, svcAsName, protocol string, npp []netv1.NetworkPolicyPort, logEnable bool, aclCmds []string, index int) []string {
var allowArgs, ovnArgs []string

ipSuffix := "ip4"
Expand All @@ -1620,63 +1620,70 @@ func (c LegacyClient) CreateIngressACL(pgName, asIngressName, asExceptName, svcA
}

if logEnable {
ovnArgs = []string{MayExist, "--type=port-group", "--log", fmt.Sprintf("--severity=%s", "warning"), "acl-add", pgName, "to-lport", util.IngressDefaultDrop, fmt.Sprintf("outport==@%s && ip", pgName), "drop"}
ovnArgs = []string{"--", fmt.Sprintf("--id=@%s.drop.%d", pgName, index), "create", "acl", "action=drop", "direction=to-lport", "log=true", "severity=warning", fmt.Sprintf("priority=%s", util.IngressDefaultDrop), fmt.Sprintf("match=\"%s\"", fmt.Sprintf("outport==@%s && ip", pgName)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.drop.%d", pgName, index)}
} else {
ovnArgs = []string{MayExist, "--type=port-group", "acl-add", pgName, "to-lport", util.IngressDefaultDrop, fmt.Sprintf("outport==@%s && ip", pgName), "drop"}
ovnArgs = []string{"--", fmt.Sprintf("--id=@%s.drop.%d", pgName, index), "create", "acl", "action=drop", "direction=to-lport", "log=false", fmt.Sprintf("priority=%s", util.IngressDefaultDrop), fmt.Sprintf("match=\"%s\"", fmt.Sprintf("outport==@%s && ip", pgName)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.drop.%d", pgName, index)}
}

if len(npp) == 0 {
allowArgs = []string{"--", MayExist, "--type=port-group", "acl-add", pgName, "to-lport", util.IngressAllowPriority, fmt.Sprintf("%s.src == $%s && %s.src != $%s && outport==@%s && ip", ipSuffix, asIngressName, ipSuffix, asExceptName, pgName), "allow-related"}
allowArgs = []string{"--", fmt.Sprintf("--id=@%s.noport.%d", pgName, index), "create", "acl", "action=allow-related", "direction=to-lport", fmt.Sprintf("priority=%s", util.IngressAllowPriority), fmt.Sprintf("match=\"%s\"", fmt.Sprintf("%s.src == $%s && %s.src != $%s && outport==@%s && ip", ipSuffix, asIngressName, ipSuffix, asExceptName, pgName)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.noport.%d", pgName, index)}
ovnArgs = append(ovnArgs, allowArgs...)
} else {
for _, port := range npp {
for pidx, port := range npp {
if port.Port != nil {
if port.EndPort != nil {
allowArgs = []string{"--", MayExist, "--type=port-group", "acl-add", pgName, "to-lport", util.IngressAllowPriority, fmt.Sprintf("%s.src == $%s && %s.src != $%s && %d <= %s.dst <= %d && outport==@%s && ip", ipSuffix, asIngressName, ipSuffix, asExceptName, port.Port.IntVal, strings.ToLower(string(*port.Protocol)), *port.EndPort, pgName), "allow-related"}
allowArgs = []string{"--", fmt.Sprintf("--id=@%s.%d.port.%d", pgName, index, pidx), "create", "acl", "action=allow-related", "direction=to-lport", fmt.Sprintf("priority=%s", util.IngressAllowPriority), fmt.Sprintf("match=\"%s\"", fmt.Sprintf("%s.src == $%s && %s.src != $%s && %d <= %s.dst <= %d && outport==@%s && ip", ipSuffix, asIngressName, ipSuffix, asExceptName, port.Port.IntVal, strings.ToLower(string(*port.Protocol)), *port.EndPort, pgName)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.%d.port.%d", pgName, index, pidx)}
} else {
allowArgs = []string{"--", MayExist, "--type=port-group", "acl-add", pgName, "to-lport", util.IngressAllowPriority, fmt.Sprintf("%s.src == $%s && %s.src != $%s && %s.dst == %d && outport==@%s && ip", ipSuffix, asIngressName, ipSuffix, asExceptName, strings.ToLower(string(*port.Protocol)), port.Port.IntVal, pgName), "allow-related"}
allowArgs = []string{"--", fmt.Sprintf("--id=@%s.%d.port.%d", pgName, index, pidx), "create", "acl", "action=allow-related", "direction=to-lport", fmt.Sprintf("priority=%s", util.IngressAllowPriority), fmt.Sprintf("match=\"%s\"", fmt.Sprintf("%s.src == $%s && %s.src != $%s && %s.dst == %d && outport==@%s && ip", ipSuffix, asIngressName, ipSuffix, asExceptName, strings.ToLower(string(*port.Protocol)), port.Port.IntVal, pgName)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.%d.port.%d", pgName, index, pidx)}
}
} else {
allowArgs = []string{"--", MayExist, "--type=port-group", "acl-add", pgName, "to-lport", util.IngressAllowPriority, fmt.Sprintf("%s.src == $%s && %s.src != $%s && %s && outport==@%s && ip", ipSuffix, asIngressName, ipSuffix, asExceptName, strings.ToLower(string(*port.Protocol)), pgName), "allow-related"}
allowArgs = []string{"--", fmt.Sprintf("--id=@%s.%d.port.%d", pgName, index, pidx), "create", "acl", "action=allow-related", "direction=to-lport", fmt.Sprintf("priority=%s", util.IngressAllowPriority), fmt.Sprintf("match=\"%s\"", fmt.Sprintf("%s.src == $%s && %s.src != $%s && %s && outport==@%s && ip", ipSuffix, asIngressName, ipSuffix, asExceptName, strings.ToLower(string(*port.Protocol)), pgName)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.%d.port.%d", pgName, index, pidx)}
}
ovnArgs = append(ovnArgs, allowArgs...)
}
}
_, err := c.ovnNbCommand(ovnArgs...)
aclCmds = append(aclCmds, ovnArgs...)
return aclCmds
}

func (c LegacyClient) CreateACL(aclCmds []string) error {
_, err := c.ovnNbCommand(aclCmds...)
return err
}

func (c LegacyClient) CreateEgressACL(pgName, asEgressName, asExceptName, protocol string, npp []netv1.NetworkPolicyPort, portSvcName string, logEnable bool) error {
func (c LegacyClient) CombineEgressACLCmd(pgName, asEgressName, asExceptName, protocol string, npp []netv1.NetworkPolicyPort, portSvcName string, logEnable bool, aclCmds []string, index int) []string {
var allowArgs, ovnArgs []string

ipSuffix := "ip4"
if protocol == kubeovnv1.ProtocolIPv6 {
ipSuffix = "ip6"
}

if logEnable {
ovnArgs = []string{"--", MayExist, "--type=port-group", "--log", fmt.Sprintf("--severity=%s", "warning"), "acl-add", pgName, "from-lport", util.EgressDefaultDrop, fmt.Sprintf("inport==@%s && ip", pgName), "drop"}
ovnArgs = []string{"--", fmt.Sprintf("--id=@%s.drop.%d", pgName, index), "create", "acl", "action=drop", "direction=from-lport", "log=true", "severity=warning", fmt.Sprintf("priority=%s", util.EgressDefaultDrop), fmt.Sprintf("match=\"%s\"", fmt.Sprintf("inport==@%s && ip", pgName)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.drop.%d", pgName, index)}
} else {
ovnArgs = []string{"--", MayExist, "--type=port-group", "acl-add", pgName, "from-lport", util.EgressDefaultDrop, fmt.Sprintf("inport==@%s && ip", pgName), "drop"}
ovnArgs = []string{"--", fmt.Sprintf("--id=@%s.drop.%d", pgName, 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)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.drop.%d", pgName, index)}
}

if len(npp) == 0 {
allowArgs = []string{"--", MayExist, "--type=port-group", "acl-add", pgName, "from-lport", util.EgressAllowPriority, fmt.Sprintf("%s.dst == $%s && %s.dst != $%s && inport==@%s && ip", ipSuffix, asEgressName, ipSuffix, asExceptName, pgName), "allow-related"}
allowArgs = []string{"--", fmt.Sprintf("--id=@%s.noport.%d", pgName, 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)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.noport.%d", pgName, index)}
ovnArgs = append(ovnArgs, allowArgs...)
} else {
for _, port := range npp {
for pidx, port := range npp {
if port.Port != nil {
if port.EndPort != nil {
allowArgs = []string{"--", MayExist, "--type=port-group", "acl-add", pgName, "from-lport", util.EgressAllowPriority, fmt.Sprintf("%s.dst == $%s && %s.dst != $%s && %d <= %s.dst <= %d && inport==@%s && ip", ipSuffix, asEgressName, ipSuffix, asExceptName, port.Port.IntVal, strings.ToLower(string(*port.Protocol)), *port.EndPort, pgName), "allow-related"}
allowArgs = []string{"--", fmt.Sprintf("--id=@%s.%d.port.%d", pgName, index, pidx), "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 && %d <= %s.dst <= %d && inport==@%s && ip", ipSuffix, asEgressName, ipSuffix, asExceptName, port.Port.IntVal, strings.ToLower(string(*port.Protocol)), *port.EndPort, pgName)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.%d.port.%d", pgName, index, pidx)}
} else {
allowArgs = []string{"--", MayExist, "--type=port-group", "acl-add", pgName, "from-lport", util.EgressAllowPriority, fmt.Sprintf("%s.dst == $%s && %s.dst != $%s && %s.dst == %d && inport==@%s && ip", ipSuffix, asEgressName, ipSuffix, asExceptName, strings.ToLower(string(*port.Protocol)), port.Port.IntVal, pgName), "allow-related"}
allowArgs = []string{"--", fmt.Sprintf("--id=@%s.%d.port.%d", pgName, index, pidx), "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 && %s.dst == %d && inport==@%s && ip", ipSuffix, asEgressName, ipSuffix, asExceptName, strings.ToLower(string(*port.Protocol)), port.Port.IntVal, pgName)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.%d.port.%d", pgName, index, pidx)}
}
} else {
allowArgs = []string{"--", MayExist, "--type=port-group", "acl-add", pgName, "from-lport", util.EgressAllowPriority, fmt.Sprintf("%s.dst == $%s && %s.dst != $%s && %s && inport==@%s && ip", ipSuffix, asEgressName, ipSuffix, asExceptName, strings.ToLower(string(*port.Protocol)), pgName), "allow-related"}
allowArgs = []string{"--", fmt.Sprintf("--id=@%s.%d.port.%d", pgName, index, pidx), "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 && %s && inport==@%s && ip", ipSuffix, asEgressName, ipSuffix, asExceptName, strings.ToLower(string(*port.Protocol)), pgName)), "--", "add", "port-group", pgName, "acls", fmt.Sprintf("@%s.%d.port.%d", pgName, index, pidx)}
}
ovnArgs = append(ovnArgs, allowArgs...)
}
}
_, err := c.ovnNbCommand(ovnArgs...)
return err
aclCmds = append(aclCmds, ovnArgs...)
return aclCmds
}

func (c LegacyClient) DeleteACL(pgName, direction string) (err error) {
Expand Down