Skip to content

Bug 2070703: change networkpolicy ACLs to use "apply-after-lb" for egress #1020

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

Closed
JacobTanenbaum wants to merge 5 commits into from
Closed

Bug 2070703: change networkpolicy ACLs to use "apply-after-lb" for egress #1020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2ff4503
Revert "DPTP reconciliation from downstream"
JacobTanenbaum Apr 1, 2022
40296c7
change networkpolicy ACLs to use "apply-after-lb" for egress
JacobTanenbaum Mar 9, 2022
8ecde12
bump ovn version to 21.12.0-5
JacobTanenbaum Mar 29, 2022
a137272
update autogenerated files for bumped ovn version
JacobTanenbaum Mar 29, 2022
f84d086
bump downstream builds ovn version to ovn-2021-21.12.0-32.el8fdp
JacobTanenbaum Mar 14, 2022
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
2 changes: 1 addition & 1 deletion dist/images/Dockerfile.fedora
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ USER root

ENV PYTHONDONTWRITEBYTECODE yes

ARG ovnver=ovn-21.12.0-4.fc35
ARG ovnver=ovn-21.12.0-5.fc35
# Automatically populated when using docker buildx
ARG TARGETPLATFORM
ARG BUILDPLATFORM
Expand Down
6 changes: 2 additions & 4 deletions go-controller/pkg/libovsdbops/acl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func ensureACLUUID(acl *nbdb.ACL) {
}
}

func BuildACL(name string, direction nbdb.ACLDirection, priority int, match string, action nbdb.ACLAction, meter string, severity nbdb.ACLSeverity, log bool, externalIds map[string]string) *nbdb.ACL {
func BuildACL(name string, direction nbdb.ACLDirection, priority int, match string, action nbdb.ACLAction, meter string, severity nbdb.ACLSeverity, log bool, externalIds map[string]string, options map[string]string) *nbdb.ACL {
name = fmt.Sprintf("%.63s", name)

var realName *string
Expand All @@ -149,9 +149,7 @@ func BuildACL(name string, direction nbdb.ACLDirection, priority int, match stri
Log: log,
Meter: realMeter,
ExternalIDs: externalIds,
}
if direction == nbdb.ACLDirectionFromLport {
acl.Options = map[string]string{"apply-after-lb": "true"}
Options: options,
}

return acl
Expand Down
16 changes: 16 additions & 0 deletions go-controller/pkg/ovn/egressfirewall_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for local gateway mode",
"",
false,
map[string]string{"egressFirewall": "none"},
nil,
)
purgeACL.UUID = libovsdbops.BuildNamedUUID()

Expand All @@ -101,6 +102,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for local gateway mode",
"",
false,
map[string]string{"egressFirewall": "default"},
nil,
)
keepACL.UUID = libovsdbops.BuildNamedUUID()

Expand All @@ -115,6 +117,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for local gateway mode",
"",
false,
map[string]string{"egressFirewall": "default"},
nil,
)
otherACL.UUID = libovsdbops.BuildNamedUUID()

Expand Down Expand Up @@ -248,6 +251,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for local gateway mode",
"",
false,
map[string]string{"egressFirewall": "namespace1"},
nil,
)
ipv4ACL.UUID = libovsdbops.BuildNamedUUID()

Expand Down Expand Up @@ -336,6 +340,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for local gateway mode",
"",
false,
map[string]string{"egressFirewall": "namespace1"},
nil,
)
ipv6ACL.UUID = libovsdbops.BuildNamedUUID()

Expand Down Expand Up @@ -432,6 +437,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for local gateway mode",
"",
false,
map[string]string{"egressFirewall": "namespace1"},
nil,
)

udpACL.UUID = libovsdbops.BuildNamedUUID()
Expand Down Expand Up @@ -529,6 +535,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for local gateway mode",
"",
false,
map[string]string{"egressFirewall": "namespace1"},
nil,
)
ipv4ACL.UUID = libovsdbops.BuildNamedUUID()

Expand Down Expand Up @@ -633,6 +640,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for local gateway mode",
"",
false,
map[string]string{"egressFirewall": "namespace1"},
nil,
)
ipv4ACL.UUID = libovsdbops.BuildNamedUUID()

Expand Down Expand Up @@ -712,6 +720,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for shared gateway mode",
"",
false,
map[string]string{"egressFirewall": "none"},
nil,
)
purgeACL.UUID = libovsdbops.BuildNamedUUID()

Expand All @@ -725,6 +734,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for shared gateway mode",
"",
false,
map[string]string{"egressFirewall": "default"},
nil,
)
keepACL.UUID = libovsdbops.BuildNamedUUID()

Expand All @@ -739,6 +749,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for shared gateway mode",
"",
false,
map[string]string{"egressFirewall": "default"},
nil,
)
otherACL.UUID = libovsdbops.BuildNamedUUID()

Expand Down Expand Up @@ -867,6 +878,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for shared gateway mode",
"",
false,
map[string]string{"egressFirewall": "namespace1"},
nil,
)
ipv4ACL.UUID = libovsdbops.BuildNamedUUID()

Expand Down Expand Up @@ -950,6 +962,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for shared gateway mode",
"",
false,
map[string]string{"egressFirewall": "namespace1"},
nil,
)
ipv6ACL.UUID = libovsdbops.BuildNamedUUID()

Expand Down Expand Up @@ -1043,6 +1056,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for shared gateway mode",
"",
false,
map[string]string{"egressFirewall": "namespace1"},
nil,
)

udpACL.UUID = libovsdbops.BuildNamedUUID()
Expand Down Expand Up @@ -1124,6 +1138,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for shared gateway mode",
"",
false,
map[string]string{"egressFirewall": "namespace1"},
nil,
)
ipv4ACL.UUID = libovsdbops.BuildNamedUUID()

Expand Down Expand Up @@ -1212,6 +1227,7 @@ var _ = ginkgo.Describe("OVN EgressFirewall Operations for shared gateway mode",
"",
false,
map[string]string{"egressFirewall": "namespace1"},
nil,
)
ipv4ACL.UUID = libovsdbops.BuildNamedUUID()

Expand Down
4 changes: 3 additions & 1 deletion go-controller/pkg/ovn/gress_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,12 @@ func (gp *gressPolicy) buildLocalPodACLs(portGroupName, aclLogging string) []*nb
// buildACLAllow builds an allow-related ACL for a given given match
func (gp *gressPolicy) buildACLAllow(match, l4Match string, ipBlockCIDR int, aclLogging string) *nbdb.ACL {
var direction string
var options map[string]string
if gp.policyType == knet.PolicyTypeIngress {
direction = nbdb.ACLDirectionToLport
} else {
direction = nbdb.ACLDirectionFromLport
options = map[string]string{"apply-after-lb": "true"}
}
priority := types.DefaultAllowPriority
action := nbdb.ACLActionAllowRelated
Expand Down Expand Up @@ -394,7 +396,7 @@ func (gp *gressPolicy) buildACLAllow(match, l4Match string, ipBlockCIDR int, acl
policyTypeNum: policyTypeIndex,
}

acl := libovsdbops.BuildACL(aclName, direction, priority, match, action, types.OvnACLLoggingMeter, getACLLoggingSeverity(aclLogging), aclLogging != "", externalIds)
acl := libovsdbops.BuildACL(aclName, direction, priority, match, action, types.OvnACLLoggingMeter, getACLLoggingSeverity(aclLogging), aclLogging != "", externalIds, options)
return acl
}

Expand Down
14 changes: 11 additions & 3 deletions go-controller/pkg/ovn/policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,9 @@ func (oc *Controller) syncNetworkPoliciesRetriable(networkPolicies []interface{}
if err != nil {
return fmt.Errorf("cannot update old Egress NetworkPolicy ACLs: %v", err)
}

}

return nil
}

Expand All @@ -200,7 +202,7 @@ func addAllowACLFromNode(nodeName string, mgmtPortIP net.IP, nbClient libovsdbcl
}
match := fmt.Sprintf("%s.src==%s", ipFamily, mgmtPortIP.String())

nodeACL := libovsdbops.BuildACL("", types.DirectionToLPort, types.DefaultAllowPriority, match, "allow-related", "", "", false, nil)
nodeACL := libovsdbops.BuildACL("", types.DirectionToLPort, types.DefaultAllowPriority, match, "allow-related", "", "", false, nil, nil)

if err := libovsdbops.AddACLToNodeSwitch(nbClient, nodeName, nodeACL); err != nil {
return fmt.Errorf("failed to add acl to allow node sourced traffic err: %v", err)
Expand Down Expand Up @@ -237,6 +239,7 @@ func namespacePortGroupACLName(namespace, portGroup, name string) string {
}

func buildACL(namespace, portGroup, name, direction string, priority int, match, action, aclLogging string, policyType knet.PolicyType) *nbdb.ACL {
var options map[string]string
aclName := namespacePortGroupACLName(namespace, portGroup, name)
log := aclLogging != ""
severity := getACLLoggingSeverity(aclLogging)
Expand All @@ -247,8 +250,13 @@ func buildACL(namespace, portGroup, name, direction string, priority int, match,
defaultDenyPolicyTypeACLExtIdKey: string(policyType),
}
}
if policyType == knet.PolicyTypeEgress {
options = map[string]string{
"apply-after-lb": "true",
}
}

return libovsdbops.BuildACL(aclName, direction, priority, match, action, meter, severity, log, externalIds)
return libovsdbops.BuildACL(aclName, direction, priority, match, action, meter, severity, log, externalIds, options)
}

func defaultDenyPortGroup(namespace, gressSuffix string) string {
Expand All @@ -257,7 +265,7 @@ func defaultDenyPortGroup(namespace, gressSuffix string) string {

func buildDenyACLs(namespace, policy, pg, aclLogging string, policyType knet.PolicyType) (denyACL, allowACL *nbdb.ACL) {
denyMatch := getACLMatch(pg, "", policyType)
allowMatch := getACLMatch(pg, "arp", policyType)
allowMatch := getACLMatch(pg, "(arp || nd)", policyType)
if policyType == knet.PolicyTypeIngress {
denyACL = buildACL(namespace, pg, policy, nbdb.ACLDirectionToLport, types.DefaultDenyPriority, denyMatch, nbdb.ACLActionDrop, aclLogging, policyType)
allowACL = buildACL(namespace, pg, "ARPallowPolicy", nbdb.ACLDirectionToLport, types.DefaultAllowPriority, allowMatch, nbdb.ACLActionAllow, "", policyType)
Expand Down
Loading