Skip to content

Commit 23c3b5a

Browse files
committed
[ACL tier] Rename BuildACL to BuildACLWithDefaultTier
This helps to avoid confusion about defaulting the ACL tier. Update BuildACL to require the tier as an argument. Signed-off-by: Nadia Pinaeva <[email protected]>
1 parent d14d848 commit 23c3b5a

File tree

9 files changed

+36
-30
lines changed

9 files changed

+36
-30
lines changed

go-controller/pkg/libovsdb/util/acl.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,18 @@ func GetACLName(dbIDs *libovsdbops.DbObjectIDs) string {
8888
return fmt.Sprintf("%.63s", aclName)
8989
}
9090

91+
// BuildACLWithDefaultTier is used for the most ACL-related features with the default ACL tier.
92+
// That includes egress firewall, network policy, multicast.
93+
func BuildACLWithDefaultTier(dbIDs *libovsdbops.DbObjectIDs, priority int, match, action string, logLevels *ACLLoggingLevels,
94+
aclT ACLPipelineType) *nbdb.ACL {
95+
return BuildACL(dbIDs, priority, match, action, logLevels, aclT, types.DefaultACLTier)
96+
}
97+
9198
// BuildACL should be used to build ACL instead of directly calling libovsdbops.BuildACL.
9299
// It can properly set and reset log settings for ACL based on ACLLoggingLevels, and
93100
// set acl.Name and acl.ExternalIDs based on given DbIDs
94101
func BuildACL(dbIDs *libovsdbops.DbObjectIDs, priority int, match, action string, logLevels *ACLLoggingLevels,
95-
aclT ACLPipelineType) *nbdb.ACL {
102+
aclT ACLPipelineType, tier int) *nbdb.ACL {
96103
var options map[string]string
97104
var direction string
98105
switch aclT {
@@ -122,13 +129,13 @@ func BuildACL(dbIDs *libovsdbops.DbObjectIDs, priority int, match, action string
122129
log,
123130
externalIDs,
124131
options,
125-
types.DefaultACLTier,
132+
tier,
126133
)
127134
return ACL
128135
}
129136

130137
func BuildANPACL(dbIDs *libovsdbops.DbObjectIDs, priority int, match, action string, aclT ACLPipelineType, logLevels *ACLLoggingLevels) *nbdb.ACL {
131-
anpACL := BuildACL(dbIDs, priority, match, action, logLevels, aclT)
138+
anpACL := BuildACLWithDefaultTier(dbIDs, priority, match, action, logLevels, aclT)
132139
anpACL.Tier = GetACLTier(dbIDs)
133140
return anpACL
134141
}

go-controller/pkg/ovn/admin_network_policy_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func getANPGressACL(action, anpName, direction string, rulePriority int32,
9494
ruleIndex int32, ports *[]anpapi.AdminNetworkPolicyPort,
9595
namedPorts map[string][]libovsdbutil.NamedNetworkPolicyPort, banp bool) []*nbdb.ACL {
9696
retACLs := []*nbdb.ACL{}
97-
// we are not using BuildACL and instead manually building it on purpose so that the code path for BuildACL is also tested
97+
// we are not using BuildACLWithDefaultTier and instead manually building it on purpose so that the code path for BuildACLWithDefaultTier is also tested
9898
acl := nbdb.ACL{}
9999
acl.Action = action
100100
acl.Severity = nil

go-controller/pkg/ovn/base_network_controller_multicast.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,13 @@ func (bnc *BaseNetworkController) createMulticastAllowPolicy(ns string, nsInfo *
119119
egressMatch := libovsdbutil.GetACLMatch(portGroupName, bnc.getMulticastACLEgrMatch(), aclDir)
120120
dbIDs := getNamespaceMcastACLDbIDs(ns, aclDir, bnc.controllerName)
121121
aclPipeline := libovsdbutil.ACLDirectionToACLPipeline(aclDir)
122-
egressACL := libovsdbutil.BuildACL(dbIDs, types.DefaultMcastAllowPriority, egressMatch, nbdb.ACLActionAllow, nil, aclPipeline)
122+
egressACL := libovsdbutil.BuildACLWithDefaultTier(dbIDs, types.DefaultMcastAllowPriority, egressMatch, nbdb.ACLActionAllow, nil, aclPipeline)
123123

124124
aclDir = libovsdbutil.ACLIngress
125125
ingressMatch := libovsdbutil.GetACLMatch(portGroupName, bnc.getMulticastACLIgrMatch(nsInfo), aclDir)
126126
dbIDs = getNamespaceMcastACLDbIDs(ns, aclDir, bnc.controllerName)
127127
aclPipeline = libovsdbutil.ACLDirectionToACLPipeline(aclDir)
128-
ingressACL := libovsdbutil.BuildACL(dbIDs, types.DefaultMcastAllowPriority, ingressMatch, nbdb.ACLActionAllow, nil, aclPipeline)
128+
ingressACL := libovsdbutil.BuildACLWithDefaultTier(dbIDs, types.DefaultMcastAllowPriority, ingressMatch, nbdb.ACLActionAllow, nil, aclPipeline)
129129

130130
acls := []*nbdb.ACL{egressACL, ingressACL}
131131
ops, err := libovsdbops.CreateOrUpdateACLsOps(bnc.nbClient, nil, bnc.GetSamplingConfig(), acls...)
@@ -186,7 +186,7 @@ func (bnc *BaseNetworkController) createDefaultDenyMulticastPolicy() error {
186186
for _, aclDir := range []libovsdbutil.ACLDirection{libovsdbutil.ACLEgress, libovsdbutil.ACLIngress} {
187187
dbIDs := getDefaultMcastACLDbIDs(mcastDefaultDenyID, aclDir, bnc.controllerName)
188188
aclPipeline := libovsdbutil.ACLDirectionToACLPipeline(aclDir)
189-
acl := libovsdbutil.BuildACL(dbIDs, types.DefaultMcastDenyPriority, match, nbdb.ACLActionDrop, nil, aclPipeline)
189+
acl := libovsdbutil.BuildACLWithDefaultTier(dbIDs, types.DefaultMcastDenyPriority, match, nbdb.ACLActionDrop, nil, aclPipeline)
190190
acls = append(acls, acl)
191191
}
192192
ops, err := libovsdbops.CreateOrUpdateACLsOps(bnc.nbClient, nil, bnc.GetSamplingConfig(), acls...)
@@ -228,7 +228,7 @@ func (bnc *BaseNetworkController) createDefaultAllowMulticastPolicy() error {
228228
match := libovsdbutil.GetACLMatch(rtrPGName, mcastMatch, aclDir)
229229
dbIDs := getDefaultMcastACLDbIDs(mcastAllowInterNodeID, aclDir, bnc.controllerName)
230230
aclPipeline := libovsdbutil.ACLDirectionToACLPipeline(aclDir)
231-
acl := libovsdbutil.BuildACL(dbIDs, types.DefaultMcastAllowPriority, match, nbdb.ACLActionAllow, nil, aclPipeline)
231+
acl := libovsdbutil.BuildACLWithDefaultTier(dbIDs, types.DefaultMcastAllowPriority, match, nbdb.ACLActionAllow, nil, aclPipeline)
232232
acls = append(acls, acl)
233233
}
234234

go-controller/pkg/ovn/base_network_controller_policy.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -246,11 +246,11 @@ func (bnc *BaseNetworkController) addHairpinAllowACL() error {
246246
}
247247

248248
ingressACLIDs := bnc.getNetpolDefaultACLDbIDs(string(knet.PolicyTypeIngress))
249-
ingressACL := libovsdbutil.BuildACL(ingressACLIDs, types.DefaultAllowPriority, match,
249+
ingressACL := libovsdbutil.BuildACLWithDefaultTier(ingressACLIDs, types.DefaultAllowPriority, match,
250250
nbdb.ACLActionAllowRelated, nil, libovsdbutil.LportIngress)
251251

252252
egressACLIDs := bnc.getNetpolDefaultACLDbIDs(string(knet.PolicyTypeEgress))
253-
egressACL := libovsdbutil.BuildACL(egressACLIDs, types.DefaultAllowPriority, match,
253+
egressACL := libovsdbutil.BuildACLWithDefaultTier(egressACLIDs, types.DefaultAllowPriority, match,
254254
nbdb.ACLActionAllowRelated, nil, libovsdbutil.LportEgressAfterLB)
255255

256256
ops, err := libovsdbops.CreateOrUpdateACLsOps(bnc.nbClient, nil, nil, ingressACL, egressACL)
@@ -329,7 +329,7 @@ func (bnc *BaseNetworkController) addAllowACLFromNode(switchName string, mgmtPor
329329
}
330330
match := fmt.Sprintf("%s.src==%s", ipFamily, mgmtPortIP.String())
331331
dbIDs := getAllowFromNodeACLDbIDs(switchName, mgmtPortIP.String(), bnc.controllerName)
332-
nodeACL := libovsdbutil.BuildACL(dbIDs, types.DefaultAllowPriority, match,
332+
nodeACL := libovsdbutil.BuildACLWithDefaultTier(dbIDs, types.DefaultAllowPriority, match,
333333
nbdb.ACLActionAllowRelated, nil, libovsdbutil.LportIngress)
334334

335335
ops, err := libovsdbops.CreateOrUpdateACLsOps(bnc.nbClient, nil, bnc.GetSamplingConfig(), nodeACL)
@@ -382,9 +382,9 @@ func (bnc *BaseNetworkController) buildDenyACLs(namespace, pgName string, aclLog
382382
allowMatch := libovsdbutil.GetACLMatch(pgName, arpAllowPolicyMatch, aclDir)
383383
aclPipeline := libovsdbutil.ACLDirectionToACLPipeline(aclDir)
384384

385-
denyACL = libovsdbutil.BuildACL(bnc.getDefaultDenyPolicyACLIDs(namespace, aclDir, defaultDenyACL),
385+
denyACL = libovsdbutil.BuildACLWithDefaultTier(bnc.getDefaultDenyPolicyACLIDs(namespace, aclDir, defaultDenyACL),
386386
types.DefaultDenyPriority, denyMatch, nbdb.ACLActionDrop, aclLogging, aclPipeline)
387-
allowACL = libovsdbutil.BuildACL(bnc.getDefaultDenyPolicyACLIDs(namespace, aclDir, arpAllowACL),
387+
allowACL = libovsdbutil.BuildACLWithDefaultTier(bnc.getDefaultDenyPolicyACLIDs(namespace, aclDir, arpAllowACL),
388388
types.DefaultAllowPriority, allowMatch, nbdb.ACLActionAllow, nil, aclPipeline)
389389
return
390390
}

go-controller/pkg/ovn/egressfirewall.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ func (oc *DefaultNetworkController) addEgressFirewallRules(ef *egressFirewall, p
467467
func (oc *DefaultNetworkController) createEgressFirewallACLOps(ops []ovsdb.Operation, ruleIdx int, match, action, namespace, pgName string, aclLogging *libovsdbutil.ACLLoggingLevels) ([]ovsdb.Operation, error) {
468468
aclIDs := oc.getEgressFirewallACLDbIDs(namespace, ruleIdx)
469469
priority := types.EgressFirewallStartPriority - ruleIdx
470-
egressFirewallACL := libovsdbutil.BuildACL(
470+
egressFirewallACL := libovsdbutil.BuildACLWithDefaultTier(
471471
aclIDs,
472472
priority,
473473
match,

go-controller/pkg/ovn/gateway_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func generateAdvertisedUDNIsolationExpectedNB(testData []libovsdbtest.TestData,
4343
passMatches = append(passMatches, fmt.Sprintf("(%s.src == %s && %s.dst == %s)", ipPrefix, subnet, ipPrefix, subnet))
4444

4545
}
46-
passACL := libovsdbutil.BuildACL(
46+
passACL := libovsdbutil.BuildACLWithDefaultTier(
4747
GetAdvertisedNetworkSubnetsPassACLdbIDs(DefaultNetworkControllerName, networkName, networkID),
4848
types.AdvertisedNetworkPassPriority,
4949
strings.Join(passMatches, " || "),

go-controller/pkg/ovn/gress_policy.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ func (gp *gressPolicy) buildLocalPodACLs(portGroupName string, aclLogging *libov
281281
ipBlockMatches := gp.getMatchFromIPBlock(lportMatch, l4Match)
282282
for ipBlockIdx, ipBlockMatch := range ipBlockMatches {
283283
aclIDs := gp.getNetpolACLDbIDs(ipBlockIdx, protocol)
284-
acl := libovsdbutil.BuildACL(aclIDs, types.DefaultAllowPriority, ipBlockMatch, action,
284+
acl := libovsdbutil.BuildACLWithDefaultTier(aclIDs, types.DefaultAllowPriority, ipBlockMatch, action,
285285
aclLogging, gp.aclPipeline)
286286
createdACLs = append(createdACLs, acl)
287287
}
@@ -302,7 +302,7 @@ func (gp *gressPolicy) buildLocalPodACLs(portGroupName string, aclLogging *libov
302302
addrSetMatch = fmt.Sprintf("%s && %s && %s", l3Match, l4Match, lportMatch)
303303
}
304304
aclIDs := gp.getNetpolACLDbIDs(emptyIdx, protocol)
305-
acl := libovsdbutil.BuildACL(aclIDs, types.DefaultAllowPriority, addrSetMatch, action,
305+
acl := libovsdbutil.BuildACLWithDefaultTier(aclIDs, types.DefaultAllowPriority, addrSetMatch, action,
306306
aclLogging, gp.aclPipeline)
307307
if l3Match == "" {
308308
// if l3Match is empty, then no address sets are selected for a given gressPolicy.

go-controller/pkg/ovn/udn_isolation.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func (oc *DefaultNetworkController) setupUDNACLs(mgmtPortIPs []net.IP) error {
6363
pgName := libovsdbutil.GetPortGroupName(pgIDs)
6464
egressDenyIDs := oc.getUDNACLDbIDs(DenySecondaryACL, libovsdbutil.ACLEgress)
6565
match := libovsdbutil.GetACLMatch(pgName, "", libovsdbutil.ACLEgress)
66-
egressDenyACL := libovsdbutil.BuildACL(egressDenyIDs, types.PrimaryUDNDenyPriority, match, nbdb.ACLActionDrop, nil, libovsdbutil.LportEgress)
66+
egressDenyACL := libovsdbutil.BuildACLWithDefaultTier(egressDenyIDs, types.PrimaryUDNDenyPriority, match, nbdb.ACLActionDrop, nil, libovsdbutil.LportEgress)
6767

6868
getARPMatch := func(direction libovsdbutil.ACLDirection) string {
6969
match := "("
@@ -89,15 +89,15 @@ func (oc *DefaultNetworkController) setupUDNACLs(mgmtPortIPs []net.IP) error {
8989

9090
egressARPIDs := oc.getUDNACLDbIDs(AllowHostARPACL, libovsdbutil.ACLEgress)
9191
match = libovsdbutil.GetACLMatch(pgName, getARPMatch(libovsdbutil.ACLEgress), libovsdbutil.ACLEgress)
92-
egressARPACL := libovsdbutil.BuildACL(egressARPIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllow, nil, libovsdbutil.LportEgress)
92+
egressARPACL := libovsdbutil.BuildACLWithDefaultTier(egressARPIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllow, nil, libovsdbutil.LportEgress)
9393

9494
ingressDenyIDs := oc.getUDNACLDbIDs(DenySecondaryACL, libovsdbutil.ACLIngress)
9595
match = libovsdbutil.GetACLMatch(pgName, "", libovsdbutil.ACLIngress)
96-
ingressDenyACL := libovsdbutil.BuildACL(ingressDenyIDs, types.PrimaryUDNDenyPriority, match, nbdb.ACLActionDrop, nil, libovsdbutil.LportIngress)
96+
ingressDenyACL := libovsdbutil.BuildACLWithDefaultTier(ingressDenyIDs, types.PrimaryUDNDenyPriority, match, nbdb.ACLActionDrop, nil, libovsdbutil.LportIngress)
9797

9898
ingressARPIDs := oc.getUDNACLDbIDs(AllowHostARPACL, libovsdbutil.ACLIngress)
9999
match = libovsdbutil.GetACLMatch(pgName, getARPMatch(libovsdbutil.ACLIngress), libovsdbutil.ACLIngress)
100-
ingressARPACL := libovsdbutil.BuildACL(ingressARPIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllow, nil, libovsdbutil.LportIngress)
100+
ingressARPACL := libovsdbutil.BuildACLWithDefaultTier(ingressARPIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllow, nil, libovsdbutil.LportIngress)
101101

102102
ingressAllowIDs := oc.getUDNACLDbIDs(AllowHostSecondaryACL, libovsdbutil.ACLIngress)
103103
match = "("
@@ -114,7 +114,7 @@ func (oc *DefaultNetworkController) setupUDNACLs(mgmtPortIPs []net.IP) error {
114114
}
115115
match += ")"
116116
match = libovsdbutil.GetACLMatch(pgName, match, libovsdbutil.ACLIngress)
117-
ingressAllowACL := libovsdbutil.BuildACL(ingressAllowIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllowRelated, nil, libovsdbutil.LportIngress)
117+
ingressAllowACL := libovsdbutil.BuildACLWithDefaultTier(ingressAllowIDs, types.PrimaryUDNAllowPriority, match, nbdb.ACLActionAllowRelated, nil, libovsdbutil.LportIngress)
118118

119119
ops, err := libovsdbops.CreateOrUpdateACLsOps(oc.nbClient, nil, oc.GetSamplingConfig(), egressDenyACL, egressARPACL, ingressARPACL, ingressDenyACL, ingressAllowACL)
120120
if err != nil {
@@ -199,11 +199,11 @@ func (oc *DefaultNetworkController) setUDNPodOpenPortsOps(podNamespacedName stri
199199
ingressMatch, egressMatch, parseErr := getPortsMatches(podAnnotations, lspName)
200200
// don't return on parseErr, as we need to cleanup potentially present ACLs from the previous config
201201
ingressIDs := oc.getUDNOpenPortDbIDs(podNamespacedName, libovsdbutil.ACLIngress)
202-
ingressACL := libovsdbutil.BuildACL(ingressIDs, types.PrimaryUDNAllowPriority,
202+
ingressACL := libovsdbutil.BuildACLWithDefaultTier(ingressIDs, types.PrimaryUDNAllowPriority,
203203
ingressMatch, nbdb.ACLActionAllowRelated, nil, libovsdbutil.LportIngress)
204204

205205
egressIDs := oc.getUDNOpenPortDbIDs(podNamespacedName, libovsdbutil.ACLEgress)
206-
egressACL := libovsdbutil.BuildACL(egressIDs, types.PrimaryUDNAllowPriority,
206+
egressACL := libovsdbutil.BuildACLWithDefaultTier(egressIDs, types.PrimaryUDNAllowPriority,
207207
egressMatch, nbdb.ACLActionAllow, nil, libovsdbutil.LportEgress)
208208

209209
var err error
@@ -282,7 +282,7 @@ func BuildAdvertisedNetworkSubnetsDropACL(advertisedNetworkSubnetsAddressSet add
282282
dropMatches = append(dropMatches, fmt.Sprintf("(ip6.src == $%s && ip6.dst == $%s)", v6AddrSet, v6AddrSet))
283283
}
284284

285-
dropACL := libovsdbutil.BuildACL(
285+
dropACL := libovsdbutil.BuildACLWithDefaultTier(
286286
GetAdvertisedNetworkSubnetsDropACLdbIDs(),
287287
types.AdvertisedNetworkDenyPriority,
288288
strings.Join(dropMatches, " || "),
@@ -325,7 +325,7 @@ func (bnc *BaseNetworkController) addAdvertisedNetworkIsolation(nodeName string)
325325
ops = append(ops, addrOps...)
326326

327327
if len(passMatches) > 0 {
328-
passACL := libovsdbutil.BuildACL(
328+
passACL := libovsdbutil.BuildACLWithDefaultTier(
329329
GetAdvertisedNetworkSubnetsPassACLdbIDs(bnc.controllerName, bnc.GetNetworkName(), bnc.GetNetworkID()),
330330
types.AdvertisedNetworkPassPriority,
331331
strings.Join(passMatches, " || "),

go-controller/pkg/types/const.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ const (
7474
TransitSwitchToRouterPrefix = "tstor-"
7575
RouterToTransitSwitchPrefix = "rtots-"
7676

77-
// ACL Default Tier Priorities
77+
// DefaultACLTier Priorities
7878

7979
// Default routed multicast allow acl rule priority
8080
DefaultRoutedMcastAllowPriority = 1013
@@ -91,16 +91,15 @@ const (
9191
// Deny priority for isolated advertised networks
9292
AdvertisedNetworkDenyPriority = 1050
9393

94-
// ACL PlaceHolderACL Tier Priorities
94+
// PrimaryACLTier Priorities
95+
9596
PrimaryUDNAllowPriority = 1001
9697
// Default deny acl rule priority
9798
PrimaryUDNDenyPriority = 1000
9899

99100
// ACL Tiers
100101
// Tier 0 is called Primary as it is evaluated before any other feature-related Tiers.
101102
// Currently used for User Defined Network Feature.
102-
// NOTE: When we upgrade from an OVN version without tiers to the new version with
103-
// tiers, all values in the new ACL.Tier column will be set to 0.
104103
PrimaryACLTier = 0
105104
// Default Tier for all ACLs
106105
DefaultACLTier = 2

0 commit comments

Comments
 (0)