Skip to content

Commit 1d1f391

Browse files
julianwiedmannjulianwiedmann
committed
egressgw: replace open-coded CIDR format in CEGP
Replace the ugly CIDR regex with the wrapper types introduced by cilium#46047. Suggested-by: Hadrien Patte <hadrien.patte@datadoghq.com> Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
1 parent aaa350e commit 1d1f391

8 files changed

Lines changed: 35 additions & 45 deletions

File tree

‎cilium-cli/connectivity/check/deployment.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/cilium/cilium/cilium-cli/k8s"
3030
"github.com/cilium/cilium/cilium-cli/utils/features"
3131
"github.com/cilium/cilium/pkg/annotation"
32+
iputil "github.com/cilium/cilium/pkg/ip"
3233
k8sconst "github.com/cilium/cilium/pkg/k8s/apis/cilium.io"
3334
ciliumv2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"
3435
slimcorev1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/api/core/v1"
@@ -656,8 +657,8 @@ func newConnDisruptCEGP(ns, gwNode string) *ciliumv2.CiliumEgressGatewayPolicy {
656657
},
657658
},
658659
},
659-
DestinationCIDRs: []ciliumv2.CIDR{"0.0.0.0/0"},
660-
ExcludedCIDRs: []ciliumv2.CIDR{},
660+
DestinationCIDRs: []iputil.Prefix{iputil.PrefixFrom(netip.MustParsePrefix("0.0.0.0/0"))},
661+
ExcludedCIDRs: []iputil.Prefix{},
661662
EgressGateway: &ciliumv2.EgressGateway{
662663
NodeSelector: &slimmetav1.LabelSelector{
663664
MatchLabels: map[string]slimmetav1.MatchLabelsValue{

‎cilium-cli/connectivity/check/test.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"fmt"
1313
"io"
1414
"net"
15+
"net/netip"
1516
"slices"
1617
"time"
1718

@@ -32,6 +33,7 @@ import (
3233
"github.com/cilium/cilium/cilium-cli/k8s"
3334
"github.com/cilium/cilium/cilium-cli/sysdump"
3435
"github.com/cilium/cilium/cilium-cli/utils/features"
36+
iputil "github.com/cilium/cilium/pkg/ip"
3537
k8sConst "github.com/cilium/cilium/pkg/k8s/apis/cilium.io"
3638
ciliumv2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"
3739
"github.com/cilium/cilium/pkg/lock"
@@ -663,25 +665,25 @@ func (t *Test) WithCiliumEgressGatewayPolicy(params CiliumEgressGatewayPolicyPar
663665

664666
// If IPv6 egress policies are enabled, add the necessary destination CIDR
665667
if ipv6Enabled {
666-
pl.Spec.DestinationCIDRs = append(pl.Spec.DestinationCIDRs, "::/0")
668+
pl.Spec.DestinationCIDRs = append(pl.Spec.DestinationCIDRs, iputil.PrefixFrom(netip.MustParsePrefix("::/0")))
667669
}
668670

669671
// Set the excluded CIDRs
670-
pl.Spec.ExcludedCIDRs = []ciliumv2.CIDR{}
672+
pl.Spec.ExcludedCIDRs = []iputil.Prefix{}
671673

672674
switch params.ExcludedCIDRsConf {
673675
case ExternalNodeExcludedCIDRs:
674676
for _, nodeWithoutCiliumIP := range t.Context().params.NodesWithoutCiliumIPs {
675677
if parsedIP := net.ParseIP(nodeWithoutCiliumIP.IP); parsedIP.To4() == nil {
676678
// If it is an IPv6 address, add the necessary excluded CIDR
677679
if ipv6Enabled {
678-
cidr := ciliumv2.CIDR(fmt.Sprintf("%s/128", nodeWithoutCiliumIP.IP))
680+
cidr := iputil.PrefixFrom(netip.MustParsePrefix(fmt.Sprintf("%s/128", nodeWithoutCiliumIP.IP)))
679681
pl.Spec.ExcludedCIDRs = append(pl.Spec.ExcludedCIDRs, cidr)
680682
}
681683
continue
682684
}
683685

684-
cidr := ciliumv2.CIDR(fmt.Sprintf("%s/32", nodeWithoutCiliumIP.IP))
686+
cidr := iputil.PrefixFrom(netip.PrefixFrom(netip.MustParseAddr(nodeWithoutCiliumIP.IP), 32))
685687
pl.Spec.ExcludedCIDRs = append(pl.Spec.ExcludedCIDRs, cidr)
686688
}
687689
}

‎pkg/egressgateway/helpers_test.go‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"k8s.io/apimachinery/pkg/runtime"
1515
"k8s.io/apimachinery/pkg/types"
1616

17+
iputil "github.com/cilium/cilium/pkg/ip"
1718
cilium_api_v2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"
1819
v2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"
1920
"github.com/cilium/cilium/pkg/k8s/resource"
@@ -97,16 +98,22 @@ type policyParams struct {
9798
}
9899

99100
func newCEGP(params *policyParams) (*v2.CiliumEgressGatewayPolicy, *PolicyConfig) {
101+
// Create destination CIDRs list
102+
var destinationCIDRs []iputil.Prefix
100103
parsedDestinationCIDRs := make([]netip.Prefix, 0, len(params.destinationCIDRs))
101104
for _, destCIDR := range params.destinationCIDRs {
102105
parsedDestinationCIDR, _ := netip.ParsePrefix(destCIDR)
103106
parsedDestinationCIDRs = append(parsedDestinationCIDRs, parsedDestinationCIDR)
107+
destinationCIDRs = append(destinationCIDRs, iputil.PrefixFrom(parsedDestinationCIDR))
104108
}
105109

110+
// Create excluded CIDRs list
111+
excludedCIDRs := []iputil.Prefix{}
106112
parsedExcludedCIDRs := make([]netip.Prefix, 0, len(params.excludedCIDRs))
107113
for _, excludedCIDR := range params.excludedCIDRs {
108114
parsedExcludedCIDR, _ := netip.ParsePrefix(excludedCIDR)
109115
parsedExcludedCIDRs = append(parsedExcludedCIDRs, parsedExcludedCIDR)
116+
excludedCIDRs = append(excludedCIDRs, iputil.PrefixFrom(parsedExcludedCIDR))
110117
}
111118

112119
policy := &PolicyConfig{
@@ -157,18 +164,6 @@ func newCEGP(params *policyParams) (*v2.CiliumEgressGatewayPolicy, *PolicyConfig
157164
}
158165
}
159166

160-
// Create destination CIDRs list
161-
var destinationCIDRs []v2.CIDR
162-
for _, destCIDR := range params.destinationCIDRs {
163-
destinationCIDRs = append(destinationCIDRs, v2.CIDR(destCIDR))
164-
}
165-
166-
// Create excluded CIDRs list
167-
excludedCIDRs := []v2.CIDR{}
168-
for _, excludedCIDR := range params.excludedCIDRs {
169-
excludedCIDRs = append(excludedCIDRs, v2.CIDR(excludedCIDR))
170-
}
171-
172167
cegp := &v2.CiliumEgressGatewayPolicy{
173168
ObjectMeta: metav1.ObjectMeta{
174169
Name: params.name,

‎pkg/egressgateway/policy.go‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -472,25 +472,17 @@ func ParseCEGP(cegp *v2.CiliumEgressGatewayPolicy) (*PolicyConfig, error) {
472472
policyGwConfigs = append(policyGwConfigs, *policyGwc)
473473
}
474474

475-
for _, cidrString := range destinationCIDRs {
476-
cidr, err := netip.ParsePrefix(string(cidrString))
477-
if err != nil {
478-
return nil, fmt.Errorf("failed to parse destination CIDR %s: %w", cidrString, err)
479-
}
480-
dstCidrList = append(dstCidrList, cidr)
475+
for _, cidr := range destinationCIDRs {
476+
dstCidrList = append(dstCidrList, cidr.Prefix)
481477
if cidr.Addr().Is6() {
482478
v6Needed = true
483479
} else {
484480
v4Needed = true
485481
}
486482
}
487483

488-
for _, cidrString := range cegp.Spec.ExcludedCIDRs {
489-
cidr, err := netip.ParsePrefix(string(cidrString))
490-
if err != nil {
491-
return nil, fmt.Errorf("failed to parse excluded CIDR %s: %w", cidr, err)
492-
}
493-
excludedCIDRs = append(excludedCIDRs, cidr)
484+
for _, cidr := range cegp.Spec.ExcludedCIDRs {
485+
excludedCIDRs = append(excludedCIDRs, cidr.Prefix)
494486
}
495487

496488
for _, egressRule := range cegp.Spec.Selectors {

‎pkg/k8s/apis/cilium.io/client/crds/v2/ciliumegressgatewaypolicies.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ spec:
5151
DestinationCIDRs is a list of destination CIDRs for destination IP addresses.
5252
If a destination IP matches any one CIDR, it will be selected.
5353
items:
54-
pattern: ^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\/([0-9]|[1-2][0-9]|3[0-2])$|^s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:)))(%.+)?s*(\/(12[0-8]|1[0-1][0-9]|[1-9][0-9]|[0-9]))$
54+
format: cidr
5555
type: string
5656
type: array
5757
egressGateway:
@@ -281,7 +281,7 @@ spec:
281281
Should be a subset of destinationCIDRs otherwise it will not have any
282282
effect.
283283
items:
284-
pattern: ^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\/([0-9]|[1-2][0-9]|3[0-2])$|^s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:)))(%.+)?s*(\/(12[0-8]|1[0-1][0-9]|[1-9][0-9]|[0-9]))$
284+
format: cidr
285285
type: string
286286
type: array
287287
selectors:

‎pkg/k8s/apis/cilium.io/v2/cegp_types.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package v2
66
import (
77
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
88

9+
iputil "github.com/cilium/cilium/pkg/ip"
910
slimv1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/apis/meta/v1"
1011
)
1112

@@ -42,11 +43,6 @@ type CiliumEgressGatewayPolicyList struct {
4243
Items []CiliumEgressGatewayPolicy `json:"items"`
4344
}
4445

45-
// +kubebuilder:validation:Pattern=`^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\/([0-9]|[1-2][0-9]|3[0-2])$|^s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:)))(%.+)?s*(\/(12[0-8]|1[0-1][0-9]|[1-9][0-9]|[0-9]))$`
46-
// + The regex for the IPv6 CIDR range (second part of the OR) was taken from
47-
// + https://blog.markhatton.co.uk/2011/03/15/regular-expressions-for-ip-addresses-cidr-ranges-and-hostnames/`
48-
type CIDR string
49-
5046
type CiliumEgressGatewayPolicySpec struct {
5147
// Egress represents a list of rules by which egress traffic is
5248
// filtered from the source pods.
@@ -58,15 +54,15 @@ type CiliumEgressGatewayPolicySpec struct {
5854
// If a destination IP matches any one CIDR, it will be selected.
5955
//
6056
// +kubebuilder:validation:Required
61-
DestinationCIDRs []CIDR `json:"destinationCIDRs"`
57+
DestinationCIDRs []iputil.Prefix `json:"destinationCIDRs"`
6258

6359
// ExcludedCIDRs is a list of destination CIDRs that will be excluded
6460
// from the egress gateway redirection and SNAT logic.
6561
// Should be a subset of destinationCIDRs otherwise it will not have any
6662
// effect.
6763
//
6864
// +kubebuilder:validation:Optional
69-
ExcludedCIDRs []CIDR `json:"excludedCIDRs,omitempty"`
65+
ExcludedCIDRs []iputil.Prefix `json:"excludedCIDRs,omitempty"`
7066

7167
// EgressGateway is the gateway node responsible for SNATing traffic.
7268
// In case multiple nodes are a match for the given set of labels, the first node

‎pkg/k8s/apis/cilium.io/v2/zz_generated.deepcopy.go‎

Lines changed: 8 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/k8s/apis/cilium.io/v2/zz_generated.deepequal.go‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)