Skip to content

[WIP] Refactor: Simplify ACL rule to use positive matching #2765

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[WIP] Refactor: Simplify ACL rule to use positive matching #2765

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
3 changes: 3 additions & 0 deletions go-controller/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ require (
gopkg.in/gcfg.v1 v1.2.3
gopkg.in/k8snetworkplumbingwg/multus-cni.v4 v4.0.2
gopkg.in/natefinch/lumberjack.v2 v2.2.1
inet.af/netaddr v0.0.0-20230525184311-b8eac61e914a
k8s.io/api v0.33.3
k8s.io/apimachinery v0.33.3
k8s.io/client-go v0.33.3
Expand Down Expand Up @@ -131,6 +132,8 @@ require (
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/otel v1.33.0 // indirect
go.opentelemetry.io/otel/trace v1.33.0 // indirect
go4.org/intern v0.0.0-20211027215823-ae77deb06f29 // indirect
go4.org/unsafe/assume-no-moving-gc v0.0.0-20230525183740-e7c30c78aeb2 // indirect
golang.org/x/crypto v0.36.0 // indirect
golang.org/x/oauth2 v0.27.0 // indirect
golang.org/x/term v0.30.0 // indirect
Expand Down
10 changes: 10 additions & 0 deletions go-controller/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZ
github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE=
github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
github.com/dvyukov/go-fuzz v0.0.0-20210103155950-6a8e9d1f2415/go.mod h1:11Gm+ccJnvAhCNLlf5+cS9KjtbaD5I5zaZpFMsTHWTw=
github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc=
github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
Expand Down Expand Up @@ -879,6 +880,11 @@ go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=
go.uber.org/zap v1.17.0/go.mod h1:MXVU+bhUf/A7Xi2HNOnopQOrmycQ5Ih87HtOu4q5SSo=
go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8=
go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E=
go4.org/intern v0.0.0-20211027215823-ae77deb06f29 h1:UXLjNohABv4S58tHmeuIZDO6e3mHpW2Dx33gaNt03LE=
go4.org/intern v0.0.0-20211027215823-ae77deb06f29/go.mod h1:cS2ma+47FKrLPdXFpr7CuxiTW3eyJbWew4qx0qtQWDA=
go4.org/unsafe/assume-no-moving-gc v0.0.0-20211027215541-db492cf91b37/go.mod h1:FftLjUGFEDu5k8lt0ddY+HcrH/qU/0qk+H8j9/nTl3E=
go4.org/unsafe/assume-no-moving-gc v0.0.0-20230525183740-e7c30c78aeb2 h1:WJhcL4p+YeDxmZWg141nRm7XC8IDmhz7lk5GpadO1Sg=
go4.org/unsafe/assume-no-moving-gc v0.0.0-20230525183740-e7c30c78aeb2/go.mod h1:FftLjUGFEDu5k8lt0ddY+HcrH/qU/0qk+H8j9/nTl3E=
golang.org/x/crypto v0.0.0-20171113213409-9f005a07e0d3/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20180214000028-650f4a345ab4/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
Expand Down Expand Up @@ -1063,6 +1069,7 @@ golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20201201145000-ef89a241ccb3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201202213521-69691e467435/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210324051608-47abb6519492/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand Down Expand Up @@ -1148,6 +1155,7 @@ golang.org/x/tools v0.0.0-20200505023115-26f46d2f7ef8/go.mod h1:EkVYQZoAsY45+roY
golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.6-0.20210820212750-d4cc65f0b2ff/go.mod h1:YD9qOF0M9xpSpdWTBbzEl5e/RnCefISl8E5Noe10jFM=
Expand Down Expand Up @@ -1306,6 +1314,8 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
inet.af/netaddr v0.0.0-20230525184311-b8eac61e914a h1:1XCVEdxrvL6c0TGOhecLuB7U9zYNdxZEjvOqJreKZiM=
inet.af/netaddr v0.0.0-20230525184311-b8eac61e914a/go.mod h1:e83i32mAQOW1LAqEIweALsuK2Uw4mhQadA5r7b0Wobo=
k8s.io/api v0.20.1/go.mod h1:KqwcCVogGxQY3nBlRpwt+wpAMF/KjaCc7RpywacvqUo=
k8s.io/api v0.20.4/go.mod h1:++lNL1AJMkDymriNniQsWRkMDzRaX2Y/POTUi8yvqYQ=
k8s.io/api v0.20.6/go.mod h1:X9e8Qag6JV/bL5G6bU8sdVRltWKmdHsFUGS3eVndqE8=
Expand Down
114 changes: 95 additions & 19 deletions go-controller/pkg/ovn/gress_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import (
"strings"
"sync"

"inet.af/netaddr"
corev1 "k8s.io/api/core/v1"
knet "k8s.io/api/networking/v1"
"k8s.io/klog/v2"
utilnet "k8s.io/utils/net"

libovsdbops "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/libovsdb/ops"
Expand Down Expand Up @@ -167,35 +169,105 @@ func (gp *gressPolicy) allIPsMatch() string {
}
}

func (gp *gressPolicy) getMatchFromIPBlock(lportMatch, l4Match string) []string {
var direction string
func (gp *gressPolicy) getMatchFromIPBlock(lportMatch, l4Match string) ([]string, error) {
direction := "dst"
if gp.policyType == knet.PolicyTypeIngress {
direction = "src"
} else {
direction = "dst"
}
var matchStrings []string
var matchStr, ipVersion string

var (
ipv4Builder netaddr.IPSetBuilder
ipv6Builder netaddr.IPSetBuilder
)

// Precompute which blocks are v4/v6 and build sets in a single pass
for _, ipBlock := range gp.ipBlocks {
if utilnet.IsIPv6CIDRString(ipBlock.CIDR) {
ipVersion = "ip6"
cidrPrefix := netaddr.MustParseIPPrefix(ipBlock.CIDR)
var exceptSet *netaddr.IPSet
var err error

if len(ipBlock.Except) > 0 {
var exceptBuilder netaddr.IPSetBuilder
for _, except := range ipBlock.Except {
exceptBuilder.AddPrefix(netaddr.MustParseIPPrefix(except))
}
exceptSet, err = exceptBuilder.IPSet()
if err != nil {
return nil, fmt.Errorf("failed to build IPSet from except %v: %v", ipBlock.Except, err)
}
}

var blockBuilder netaddr.IPSetBuilder
blockBuilder.AddPrefix(cidrPrefix)
cidrSet, err := blockBuilder.IPSet()
if err != nil {
return nil, fmt.Errorf("failed to build IPSet from CIDR %s: %v", ipBlock.CIDR, err)
}

var finalSet *netaddr.IPSet
if exceptSet != nil {
var diffBuilder netaddr.IPSetBuilder
diffBuilder.AddSet(cidrSet)
diffBuilder.RemoveSet(exceptSet)
finalSet, err = diffBuilder.IPSet()
if err != nil {
return nil, fmt.Errorf("failed to build IPSet from diff %s: %v", ipBlock.CIDR, err)
}
} else {
ipVersion = "ip4"
finalSet = cidrSet
}
if len(ipBlock.Except) == 0 {
matchStr = fmt.Sprintf("%s.%s == %s", ipVersion, direction, ipBlock.CIDR)

if utilnet.IsIPv6CIDRString(ipBlock.CIDR) {
ipv6Builder.AddSet(finalSet)
} else {
matchStr = fmt.Sprintf("%s.%s == %s && %s.%s != {%s}", ipVersion, direction, ipBlock.CIDR,
ipVersion, direction, strings.Join(ipBlock.Except, ", "))
ipv4Builder.AddSet(finalSet)
}
}

var matchStrings []string

// Helper to build match string for a given IP version
buildMatch := func(ipVer, direction string, prefixes []netaddr.IPPrefix) string {
if len(prefixes) == 0 {
return ""
}
parts := make([]string, len(prefixes))
for i, p := range prefixes {
parts[i] = p.String()
}
match := fmt.Sprintf("%s.%s == {%s}", ipVer, direction, strings.Join(parts, ", "))
if l4Match == libovsdbutil.UnspecifiedL4Match {
matchStr = fmt.Sprintf("%s && %s", matchStr, lportMatch)
} else {
matchStr = fmt.Sprintf("%s && %s && %s", matchStr, l4Match, lportMatch)
return fmt.Sprintf("%s && %s", match, lportMatch)
}
matchStrings = append(matchStrings, matchStr)
return fmt.Sprintf("%s && %s && %s", match, l4Match, lportMatch)
}
return matchStrings

// Only build match strings if there are prefixes
if ipv4Set, err := ipv4Builder.IPSet(); err != nil {
return nil, fmt.Errorf("failed to build IPSet from final IPv4 IPBlock: %v", err)
} else {
v4Prefixes := ipv4Set.Prefixes()
if len(v4Prefixes) > 0 {
match := buildMatch("ip4", direction, v4Prefixes)
if match != "" {
matchStrings = append(matchStrings, match)
}
}
}

if ipv6Set, err := ipv6Builder.IPSet(); err != nil {
return nil, fmt.Errorf("failed to build IPSet from final IPv6 IPBlock: %v", err)
} else {
v6Prefixes := ipv6Set.Prefixes()
if len(v6Prefixes) > 0 {
match := buildMatch("ip6", direction, v6Prefixes)
if match != "" {
matchStrings = append(matchStrings, match)
}
}
}

return matchStrings, nil
}

// addNamespaceAddressSet adds a namespace address set to the gress policy.
Expand Down Expand Up @@ -285,7 +357,11 @@ func (gp *gressPolicy) buildLocalPodACLs(portGroupName string, aclLogging *libov
for protocol, l4Match := range libovsdbutil.GetL4MatchesFromNetworkPolicyPorts(gp.portPolicies) {
if len(gp.ipBlocks) > 0 {
// Add ACL allow rule for IPBlock CIDR
ipBlockMatches := gp.getMatchFromIPBlock(lportMatch, l4Match)
ipBlockMatches, err := gp.getMatchFromIPBlock(lportMatch, l4Match)
if err != nil {
klog.Errorf("failed to get match from IPBlock: %v", err)
continue
}
for ipBlockIdx, ipBlockMatch := range ipBlockMatches {
aclIDs := gp.getNetpolACLDbIDs(ipBlockIdx, protocol)
acl := libovsdbutil.BuildACLWithDefaultTier(aclIDs, types.DefaultAllowPriority, ipBlockMatch, action,
Expand Down
29 changes: 29 additions & 0 deletions go-controller/vendor/go4.org/intern/LICENSE
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions go-controller/vendor/go4.org/intern/README.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading