Add InboundCIDRs field to IngressClassParams (kubernetes-sigs#3089)

apis/elbv2/v1beta1/ingressclassparams_types.go

@@ -99,6 +99,10 @@ type IngressClassParamsSpec struct {
 	// +optional
 	Scheme *LoadBalancerScheme `json:"scheme,omitempty"`
 
+	// InboundCIDRs specifies the CIDRs that are allowed to access the Ingresses that belong to IngressClass with this IngressClassParams.
+	// +optional
+	InboundCIDRs []string `json:"inboundCIDRs,omitempty"`
+
 	// SSLPolicy specifies the SSL Policy for all Ingresses that belong to IngressClass with this IngressClassParams.
 	// +optional
 	SSLPolicy string `json:"sslPolicy,omitEmpty"`

config/crd/bases/elbv2.k8s.aws_ingressclassparams.yaml

@@ -61,6 +61,12 @@ spec:
                 required:
                 - name
                 type: object
+              inboundCIDRs:
+                description: InboundCIDRs specifies the CIDRs that are allowed to
+                  access the Ingresses that belong to IngressClass with this IngressClassParams.
+                items:
+                  type: string
+                type: array
               ipAddressType:
                 description: IPAddressType defines the ip address type for all Ingresses
                   that belong to IngressClass with this IngressClassParams.

docs/guide/ingress/ingress_class.md

@@ -135,6 +135,11 @@ Cluster administrators can use the `scheme` field to restrict the scheme for all
 1. If `scheme` specified, all Ingresses with this IngressClass will have the specified scheme.
 2. If `scheme` un-specified, Ingresses with this IngressClass can continue to use `alb.ingress.kubernetes.io/scheme annotation` to specify scheme.
 
+#### spec.inboundCIDRs
+
+Cluster administrators can use the optional `inboundCIDRs` field to specify the CIDRs that are allowed to access the load balancers that belong to this IngressClass.
+If the field is specified, LBC will ignore the `alb.ingress.kubernetes.io/inbound-cidrs` annotation.
+
 #### spec.sslPolicy
 
 Cluster administrators can use the optional `sslPolicy` field to specify the SSL policy for the load balancers that belong to this IngressClass.

helm/aws-load-balancer-controller/crds/crds.yaml

@@ -60,6 +60,12 @@ spec:
                 required:
                 - name
                 type: object
+              inboundCIDRs:
+                description: InboundCIDRs specifies the CIDRs that are allowed to
+                  access the Ingresses that belong to IngressClass with this IngressClassParams.
+                items:
+                  type: string
+                type: array
               ipAddressType:
                 description: IPAddressType defines the ip address type for all Ingresses
                   that belong to IngressClass with this IngressClassParams.

pkg/ingress/model_build_listener.go

@@ -107,7 +107,7 @@ type listenPortConfig struct {
 func (t *defaultModelBuildTask) computeIngressListenPortConfigByPort(ctx context.Context, ing *ClassifiedIngress) (map[int64]listenPortConfig, error) {
 	explicitTLSCertARNs := t.computeIngressExplicitTLSCertARNs(ctx, ing.Ing)
 	explicitSSLPolicy := t.computeIngressExplicitSSLPolicy(ctx, ing)
-	inboundCIDRv4s, inboundCIDRV6s, err := t.computeIngressExplicitInboundCIDRs(ctx, ing.Ing)
+	inboundCIDRv4s, inboundCIDRV6s, err := t.computeIngressExplicitInboundCIDRs(ctx, ing)
 	if err != nil {
 		return nil, err
 	}
@@ -209,15 +209,24 @@ func (t *defaultModelBuildTask) computeIngressListenPorts(_ context.Context, ing
 	return portAndProtocols, nil
 }
 
-func (t *defaultModelBuildTask) computeIngressExplicitInboundCIDRs(_ context.Context, ing *networking.Ingress) ([]string, []string, error) {
+func (t *defaultModelBuildTask) computeIngressExplicitInboundCIDRs(_ context.Context, ing *ClassifiedIngress) ([]string, []string, error) {
 	var rawInboundCIDRs []string
-	_ = t.annotationParser.ParseStringSliceAnnotation(annotations.IngressSuffixInboundCIDRs, &rawInboundCIDRs, ing.Annotations)
+	fromIngressClassParams := false
+	if ing.IngClassConfig.IngClassParams != nil && len(ing.IngClassConfig.IngClassParams.Spec.InboundCIDRs) != 0 {
+		rawInboundCIDRs = ing.IngClassConfig.IngClassParams.Spec.InboundCIDRs
+		fromIngressClassParams = true
+	} else {
+		_ = t.annotationParser.ParseStringSliceAnnotation(annotations.IngressSuffixInboundCIDRs, &rawInboundCIDRs, ing.Ing.Annotations)
+	}
 
 	var inboundCIDRv4s, inboundCIDRv6s []string
 	for _, cidr := range rawInboundCIDRs {
 		_, _, err := net.ParseCIDR(cidr)
 		if err != nil {
-			return nil, nil, errors.Wrapf(err, "invalid %v settings on Ingress: %v", annotations.IngressSuffixInboundCIDRs, k8s.NamespacedName(ing))
+			if fromIngressClassParams {
+				return nil, nil, fmt.Errorf("invalid CIDR in IngressClassParams InboundCIDR %s: %w", cidr, err)
+			}
+			return nil, nil, fmt.Errorf("invalid %v settings on Ingress: %v: %w", annotations.IngressSuffixInboundCIDRs, k8s.NamespacedName(ing.Ing), err)
 		}
 		if strings.Contains(cidr, ":") {
 			inboundCIDRv6s = append(inboundCIDRv6s, cidr)

pkg/ingress/model_builder_test.go

@@ -1474,6 +1474,132 @@ func Test_defaultModelBuilder_Build(t *testing.T) {
 			"ns-1/ing-1-svc-3:https": null
 		}
 	}
+}`,
+		},
+		{
+			name: "Ingress - inboundCIDRs in IngressClassParams",
+			env: env{
+				svcs: []*corev1.Service{ns_1_svc_1, ns_1_svc_2, ns_1_svc_3},
+			},
+			fields: fields{
+				resolveViaDiscoveryCalls: []resolveViaDiscoveryCall{resolveViaDiscoveryCallForInternalLB},
+				listLoadBalancersCalls:   []listLoadBalancersCall{listLoadBalancerCallForEmptyLB},
+				enableBackendSG:          true,
+			},
+			args: args{
+				ingGroup: Group{
+					ID: GroupID{Namespace: "ns-1", Name: "ing-1"},
+					Members: []ClassifiedIngress{
+						{
+							IngClassConfig: ClassConfiguration{
+								IngClassParams: &v1beta1.IngressClassParams{
+									Spec: v1beta1.IngressClassParamsSpec{
+										InboundCIDRs: []string{
+											"10.0.0.0/8",
+											"172.16.0.0/12",
+										},
+									},
+								},
+							},
+							Ing: &networking.Ingress{ObjectMeta: metav1.ObjectMeta{
+								Namespace: "ns-1",
+								Name:      "ing-1",
+								Annotations: map[string]string{
+									"alb.ingress.kubernetes.io/inbound-cidrs": "20.0.0.0/8",
+								},
+							},
+								Spec: networking.IngressSpec{
+									Rules: []networking.IngressRule{
+										{
+											Host: "app-1.example.com",
+											IngressRuleValue: networking.IngressRuleValue{
+												HTTP: &networking.HTTPIngressRuleValue{
+													Paths: []networking.HTTPIngressPath{
+														{
+															Path: "/svc-1",
+															Backend: networking.IngressBackend{
+																Service: &networking.IngressServiceBackend{
+																	Name: ns_1_svc_1.Name,
+																	Port: networking.ServiceBackendPort{
+																		Name: "http",
+																	},
+																},
+															},
+														},
+														{
+															Path: "/svc-2",
+															Backend: networking.IngressBackend{
+																Service: &networking.IngressServiceBackend{
+																	Name: ns_1_svc_2.Name,
+																	Port: networking.ServiceBackendPort{
+																		Name: "http",
+																	},
+																},
+															},
+														},
+													},
+												},
+											},
+										},
+										{
+											Host: "app-2.example.com",
+											IngressRuleValue: networking.IngressRuleValue{
+												HTTP: &networking.HTTPIngressRuleValue{
+													Paths: []networking.HTTPIngressPath{
+														{
+															Path: "/svc-3",
+															Backend: networking.IngressBackend{
+																Service: &networking.IngressServiceBackend{
+																	Name: ns_1_svc_3.Name,
+																	Port: networking.ServiceBackendPort{
+																		Name: "https",
+																	},
+																},
+															},
+														},
+													},
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantStackPatch: `
+{
+	"resources": {
+		"AWS::EC2::SecurityGroup": {
+			"ManagedLBSecurityGroup": {
+				"spec": {
+					"ingress": [
+						{
+							"fromPort": 80,
+							"ipProtocol": "tcp",
+							"ipRanges": [
+								{
+									"cidrIP": "10.0.0.0/8"
+								}
+							],
+							"toPort": 80
+						},
+						{
+							"fromPort": 80,
+							"ipProtocol": "tcp",
+							"ipRanges": [
+								{
+									"cidrIP": "172.16.0.0/12"
+								}
+							],
+							"toPort": 80
+						}
+					]
+				}
+			}
+		}
+	}
 }`,
 		},
 		{

webhooks/elbv2/ingressclassparams_validator.go

@@ -2,6 +2,9 @@ package elbv2
 
 import (
 	"context"
+	"fmt"
+	"net"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -30,6 +33,7 @@ func (v *ingressClassParamsValidator) Prototype(_ admission.Request) (runtime.Ob
 func (v *ingressClassParamsValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error {
 	icp := obj.(*elbv2api.IngressClassParams)
 	allErrs := field.ErrorList{}
+	allErrs = append(allErrs, v.checkInboundCIDRs(icp)...)
 	allErrs = append(allErrs, v.checkSubnetSelectors(icp)...)
 
 	return allErrs.ToAggregate()
@@ -38,6 +42,7 @@ func (v *ingressClassParamsValidator) ValidateCreate(ctx context.Context, obj ru
 func (v *ingressClassParamsValidator) ValidateUpdate(ctx context.Context, obj runtime.Object, oldObj runtime.Object) error {
 	icp := obj.(*elbv2api.IngressClassParams)
 	allErrs := field.ErrorList{}
+	allErrs = append(allErrs, v.checkInboundCIDRs(icp)...)
 	allErrs = append(allErrs, v.checkSubnetSelectors(icp)...)
 
 	return allErrs.ToAggregate()
@@ -47,6 +52,43 @@ func (v *ingressClassParamsValidator) ValidateDelete(ctx context.Context, obj ru
 	return nil
 }
 
+// checkInboundCIDRs will check for valid inboundCIDRs.
+func (v *ingressClassParamsValidator) checkInboundCIDRs(icp *elbv2api.IngressClassParams) (allErrs field.ErrorList) {
+	for idx, cidr := range icp.Spec.InboundCIDRs {
+		fieldPath := field.NewPath("spec", "inboundCIDRs").Index(idx)
+		allErrs = append(allErrs, validateCIDR(cidr, fieldPath)...)
+	}
+
+	return allErrs
+}
+
+// validateCIDR will check for a valid CIDR.
+func validateCIDR(cidr string, fieldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	ip, ipNet, err := net.ParseCIDR(cidr)
+	if err != nil {
+		detail := "Could not be parsed as a CIDR"
+		if !strings.Contains(cidr, "/") {
+			ip := net.ParseIP(cidr)
+			if ip != nil {
+				if ip.To4() != nil && !strings.Contains(cidr, ":") {
+					detail += fmt.Sprintf(" (did you mean \"%s/32\")", cidr)
+				} else {
+					detail += fmt.Sprintf(" (did you mean \"%s/64\")", cidr)
+				}
+			}
+		}
+		allErrs = append(allErrs, field.Invalid(fieldPath, cidr, detail))
+	} else if !ip.Equal(ipNet.IP) {
+		maskSize, _ := ipNet.Mask.Size()
+		detail := fmt.Sprintf("Network contains bits outside prefix (did you mean \"%s/%d\")", ipNet.IP, maskSize)
+		allErrs = append(allErrs, field.Invalid(fieldPath, cidr, detail))
+	}
+
+	return allErrs
+}
+
 // checkSubnetSelectors will check for valid SubnetSelectors
 func (v *ingressClassParamsValidator) checkSubnetSelectors(icp *elbv2api.IngressClassParams) (allErrs field.ErrorList) {
 	if icp.Spec.Subnets != nil {

webhooks/elbv2/ingressclassparams_validator_test.go

@@ -18,6 +18,72 @@ func Test_ingressClassParamsValidator_ValidateCreate(t *testing.T) {
 			name: "empty",
 			obj:  &elbv2api.IngressClassParams{},
 		},
+		{
+			name: "inboundCIDRs is valid CIDR list",
+			obj: &elbv2api.IngressClassParams{
+				Spec: elbv2api.IngressClassParamsSpec{
+					InboundCIDRs: []string{
+						"10.0.0.0/8",
+						"2001:DB8::/32",
+					},
+				},
+			},
+		},
+		{
+			name: "inboundCIDRs IPv4 no length",
+			obj: &elbv2api.IngressClassParams{
+				Spec: elbv2api.IngressClassParamsSpec{
+					InboundCIDRs: []string{
+						"192.168.0.1",
+					},
+				},
+			},
+			wantErr: "spec.inboundCIDRs[0]: Invalid value: \"192.168.0.1\": Could not be parsed as a CIDR (did you mean \"192.168.0.1/32\")",
+		},
+		{
+			name: "inboundCIDRs IPv6 no length",
+			obj: &elbv2api.IngressClassParams{
+				Spec: elbv2api.IngressClassParamsSpec{
+					InboundCIDRs: []string{
+						"2001:DB8::",
+					},
+				},
+			},
+			wantErr: "spec.inboundCIDRs[0]: Invalid value: \"2001:DB8::\": Could not be parsed as a CIDR (did you mean \"2001:DB8::/64\")",
+		},
+		{
+			name: "inboundCIDRs bits outside prefix",
+			obj: &elbv2api.IngressClassParams{
+				Spec: elbv2api.IngressClassParamsSpec{
+					InboundCIDRs: []string{
+						"10.128.0.0/8",
+					},
+				},
+			},
+			wantErr: "spec.inboundCIDRs[0]: Invalid value: \"10.128.0.0/8\": Network contains bits outside prefix (did you mean \"10.0.0.0/8\")",
+		},
+		{
+			name: "inboundCIDRs empty string",
+			obj: &elbv2api.IngressClassParams{
+				Spec: elbv2api.IngressClassParamsSpec{
+					InboundCIDRs: []string{
+						"",
+					},
+				},
+			},
+			wantErr: "spec.inboundCIDRs[0]: Invalid value: \"\": Could not be parsed as a CIDR",
+		},
+		{
+			name: "inboundCIDRs domain",
+			obj: &elbv2api.IngressClassParams{
+				Spec: elbv2api.IngressClassParamsSpec{
+					InboundCIDRs: []string{
+						"invalid.example.com",
+					},
+				},
+			},
+			wantErr: "spec.inboundCIDRs[0]: Invalid value: \"invalid.example.com\": Could not be parsed as a CIDR",
+		},
 		{
 			name: "subnet is valid ID list",
 			obj: &elbv2api.IngressClassParams{