Skip to content

Commit

Permalink
allow space separated load balancer source ranges
Browse files Browse the repository at this point in the history
  • Loading branch information
@jwtty
jwtty authored and k8s-infra-cherrypick-robot committed Apr 9, 2024
1 parent c8fd09b commit 46e14d3
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 106 deletions.
75 changes: 31 additions & 44 deletions pkg/provider/loadbalancer/configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,76 +46,63 @@ func AllowedServiceTags(svc *v1.Service) ([]string, error) {
return nil, nil
}

return strings.Split(strings.TrimSpace(value), Sep), nil
tags := strings.Split(strings.TrimSpace(value), Sep)
for i := range tags {
tags[i] = strings.TrimSpace(tags[i])
}
return tags, nil
}

// AllowedIPRanges returns the allowed IP ranges configured by user through AKS custom annotation.
// AllowedIPRanges returns the allowed IP ranges configured by user through AKS custom annotations:
// service.beta.kubernetes.io/azure-allowed-ip-ranges and service.beta.kubernetes.io/load-balancer-source-ranges
func AllowedIPRanges(svc *v1.Service) ([]netip.Prefix, []string, error) {
const (
Sep = ","
Key = consts.ServiceAnnotationAllowedIPRanges
)
var (
errs []error
validRanges []netip.Prefix
invalidRanges []string
)

value, found := svc.Annotations[Key]
if !found {
return nil, nil, nil
}
for _, key := range []string{consts.ServiceAnnotationAllowedIPRanges, v1.AnnotationLoadBalancerSourceRangesKey} {
value, found := svc.Annotations[key]
if !found {
continue
}

for _, p := range strings.Split(strings.TrimSpace(value), Sep) {
prefix, err := iputil.ParsePrefix(p)
if err != nil {
errs = append(errs, err)
invalidRanges = append(invalidRanges, p)
} else {
validRanges = append(validRanges, prefix)
var errsByKey []error
for _, p := range strings.Split(strings.TrimSpace(value), Sep) {
p = strings.TrimSpace(p)
prefix, err := iputil.ParsePrefix(p)
if err != nil {
errsByKey = append(errsByKey, err)
invalidRanges = append(invalidRanges, p)
} else {
validRanges = append(validRanges, prefix)
}
}
if len(errsByKey) > 0 {
errs = append(errs, NewErrAnnotationValue(key, value, errors.Join(errsByKey...)))
}
}

if len(errs) > 0 {
return validRanges, invalidRanges, NewErrAnnotationValue(Key, value, errors.Join(errs...))
return validRanges, invalidRanges, errors.Join(errs...)
}
return validRanges, invalidRanges, nil
}

// SourceRanges returns the allowed IP ranges configured by user through `spec.LoadBalancerSourceRanges` and standard annotation.
// If `spec.LoadBalancerSourceRanges` is not set, it will try to parse the annotation.
// SourceRanges returns the allowed IP ranges configured by user through `spec.LoadBalancerSourceRanges`.
func SourceRanges(svc *v1.Service) ([]netip.Prefix, []string, error) {
var (
errs []error
validRanges []netip.Prefix
invalidRanges []string
)
// Read from spec
if len(svc.Spec.LoadBalancerSourceRanges) > 0 {
for _, p := range svc.Spec.LoadBalancerSourceRanges {
prefix, err := iputil.ParsePrefix(p)
if err != nil {
errs = append(errs, err)
invalidRanges = append(invalidRanges, p)
} else {
validRanges = append(validRanges, prefix)
}
}
if len(errs) > 0 {
return validRanges, invalidRanges, fmt.Errorf("invalid service.Spec.LoadBalancerSourceRanges [%v]: %w", svc.Spec.LoadBalancerSourceRanges, errors.Join(errs...))
}
return validRanges, invalidRanges, nil
}

// Read from annotation
const (
Sep = ","
Key = v1.AnnotationLoadBalancerSourceRangesKey
)
value, found := svc.Annotations[Key]
if !found {
return nil, nil, nil
}
for _, p := range strings.Split(strings.TrimSpace(value), Sep) {
for _, p := range svc.Spec.LoadBalancerSourceRanges {
p = strings.TrimSpace(p)
prefix, err := iputil.ParsePrefix(p)
if err != nil {
errs = append(errs, err)
Expand All @@ -125,7 +112,7 @@ func SourceRanges(svc *v1.Service) ([]netip.Prefix, []string, error) {
}
}
if len(errs) > 0 {
return validRanges, invalidRanges, NewErrAnnotationValue(Key, value, errors.Join(errs...))
return validRanges, invalidRanges, fmt.Errorf("invalid service.Spec.LoadBalancerSourceRanges [%v]: %w", svc.Spec.LoadBalancerSourceRanges, errors.Join(errs...))
}
return validRanges, invalidRanges, nil
}
Expand Down
102 changes: 40 additions & 62 deletions pkg/provider/loadbalancer/configuration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestAllowedServiceTags(t *testing.T) {
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
consts.ServiceAnnotationAllowedServiceTags: "Microsoft.ContainerInstance/containerGroups,foo,bar",
consts.ServiceAnnotationAllowedServiceTags: " Microsoft.ContainerInstance/containerGroups, foo, bar ",
},
},
})
Expand All @@ -125,7 +125,7 @@ func TestAllowedIPRanges(t *testing.T) {
assert.Empty(t, actual)
assert.Empty(t, invalid)
})
t.Run("with 1 IPv4 range", func(t *testing.T) {
t.Run("with 1 IPv4 range in allowed ip ranges", func(t *testing.T) {
actual, invalid, err := AllowedIPRanges(&v1.Service{
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeLoadBalancer,
Expand All @@ -140,7 +140,22 @@ func TestAllowedIPRanges(t *testing.T) {
assert.Equal(t, []netip.Prefix{netip.MustParsePrefix("10.10.0.0/24")}, actual)
assert.Empty(t, invalid)
})
t.Run("with 1 IPv6 range", func(t *testing.T) {
t.Run("with 1 IPv4 range in load balancer source ranges", func(t *testing.T) {
actual, invalid, err := AllowedIPRanges(&v1.Service{
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeLoadBalancer,
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1.AnnotationLoadBalancerSourceRangesKey: "10.10.0.0/24",
},
},
})
assert.NoError(t, err)
assert.Equal(t, []netip.Prefix{netip.MustParsePrefix("10.10.0.0/24")}, actual)
assert.Empty(t, invalid)
})
t.Run("with 1 IPv6 range in allowed ip ranges", func(t *testing.T) {
actual, invalid, err := AllowedIPRanges(&v1.Service{
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeLoadBalancer,
Expand All @@ -155,21 +170,39 @@ func TestAllowedIPRanges(t *testing.T) {
assert.Equal(t, []netip.Prefix{netip.MustParsePrefix("2001:db8::/32")}, actual)
assert.Empty(t, invalid)
})
t.Run("with 1 IPv6 range in load balancer source ranges", func(t *testing.T) {
actual, invalid, err := AllowedIPRanges(&v1.Service{
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeLoadBalancer,
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1.AnnotationLoadBalancerSourceRangesKey: "2001:db8::/32",
},
},
})
assert.NoError(t, err)
assert.Equal(t, []netip.Prefix{netip.MustParsePrefix("2001:db8::/32")}, actual)
assert.Empty(t, invalid)
})
t.Run("with multiple IP ranges", func(t *testing.T) {
actual, invalid, err := AllowedIPRanges(&v1.Service{
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeLoadBalancer,
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
consts.ServiceAnnotationAllowedIPRanges: "10.10.0.0/24,2001:db8::/32",
consts.ServiceAnnotationAllowedIPRanges: " 10.10.0.0/24, 2001:db8::/32 ",
v1.AnnotationLoadBalancerSourceRangesKey: " 10.20.0.0/24, 2002:db8::/32 ",
},
},
})
assert.NoError(t, err)
assert.Equal(t, []netip.Prefix{
netip.MustParsePrefix("10.10.0.0/24"),
netip.MustParsePrefix("2001:db8::/32"),
netip.MustParsePrefix("10.20.0.0/24"),
netip.MustParsePrefix("2002:db8::/32"),
}, actual)
assert.Empty(t, invalid)
})
Expand All @@ -180,16 +213,16 @@ func TestAllowedIPRanges(t *testing.T) {
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
consts.ServiceAnnotationAllowedIPRanges: "foobar,10.0.0.1/24",
consts.ServiceAnnotationAllowedIPRanges: "foobar,10.0.0.1/24",
v1.AnnotationLoadBalancerSourceRangesKey: "barfoo,2002:db8::1/32",
},
},
})
assert.Error(t, err)

var e *ErrAnnotationValue
assert.ErrorAs(t, err, &e)
assert.Equal(t, e.AnnotationKey, consts.ServiceAnnotationAllowedIPRanges)
assert.Equal(t, []string{"foobar", "10.0.0.1/24"}, invalid)
assert.Equal(t, []string{"foobar", "10.0.0.1/24", "barfoo", "2002:db8::1/32"}, invalid)
})
}

Expand Down Expand Up @@ -218,42 +251,6 @@ func TestSourceRanges(t *testing.T) {
}, actual)
assert.Empty(t, invalid)
})
t.Run("specified in annotation", func(t *testing.T) {
actual, invalid, err := SourceRanges(&v1.Service{
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeLoadBalancer,
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1.AnnotationLoadBalancerSourceRangesKey: "10.10.0.0/24,2001:db8::/32",
},
},
})
assert.NoError(t, err)
assert.Equal(t, []netip.Prefix{
netip.MustParsePrefix("10.10.0.0/24"),
netip.MustParsePrefix("2001:db8::/32"),
}, actual)
assert.Empty(t, invalid)
})
t.Run("specified in both spec and annotation", func(t *testing.T) {
actual, invalid, err := SourceRanges(&v1.Service{
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeLoadBalancer,
LoadBalancerSourceRanges: []string{"10.10.0.0/24"},
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1.AnnotationLoadBalancerSourceRangesKey: "2001:db8::/32",
},
},
})
assert.NoError(t, err)
assert.Equal(t, []netip.Prefix{
netip.MustParsePrefix("10.10.0.0/24"),
}, actual, "spec should take precedence over annotation")
assert.Empty(t, invalid)
})
t.Run("with invalid IP range in spec", func(t *testing.T) {
_, invalid, err := SourceRanges(&v1.Service{
Spec: v1.ServiceSpec{
Expand All @@ -267,25 +264,6 @@ func TestSourceRanges(t *testing.T) {
assert.Error(t, err)
assert.Equal(t, []string{"foobar", "10.0.0.1/24"}, invalid)
})

t.Run("with invalid IP range in annotation", func(t *testing.T) {
_, invalid, err := SourceRanges(&v1.Service{
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeLoadBalancer,
},
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1.AnnotationLoadBalancerSourceRangesKey: "foobar,10.0.0.1/24",
},
},
})
assert.Error(t, err)

var e *ErrAnnotationValue
assert.ErrorAs(t, err, &e)
assert.Equal(t, e.AnnotationKey, v1.AnnotationLoadBalancerSourceRangesKey)
assert.Equal(t, []string{"foobar", "10.0.0.1/24"}, invalid)
})
}

func TestAdditionalPublicIPs(t *testing.T) {
Expand Down

0 comments on commit 46e14d3

Please sign in to comment.