Expose ingress configuration options for missing backends (#3342)

kc9ddi · web-flow · commit 36c6c4fa088e · 2023-10-16T12:00:51.000-07:00
* Expose configuration for missing backends

This PR adds command line flags to specify whether the the controller should tolerate missing backend services and actions. If the flags are not specified, the default value will be to tolerate missing backend actions and services (which is the current behavior.)

The code changes here are simply to thread these values through to `enhanced_backend_builder.go`, and rely on them rather than using the `defaultTolerateNonExistentBackendService` and `defaultTolerateNonExistentBackendAction` constant fields. The logic here is already unit tested, so no new tests have been added.

* add new settings to values.yaml
diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -1,4 +1,3 @@
----
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
 metadata:
diff --git a/controllers/ingress/group_controller.go b/controllers/ingress/group_controller.go
@@ -51,7 +51,7 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
 
 	annotationParser := annotations.NewSuffixAnnotationParser(annotations.AnnotationPrefixIngress)
 	authConfigBuilder := ingress.NewDefaultAuthConfigBuilder(annotationParser)
-	enhancedBackendBuilder := ingress.NewDefaultEnhancedBackendBuilder(k8sClient, annotationParser, authConfigBuilder)
+	enhancedBackendBuilder := ingress.NewDefaultEnhancedBackendBuilder(k8sClient, annotationParser, authConfigBuilder, controllerConfig.IngressConfig.TolerateNonExistentBackendService, controllerConfig.IngressConfig.TolerateNonExistentBackendAction)
 	referenceIndexer := ingress.NewDefaultReferenceIndexer(enhancedBackendBuilder, authConfigBuilder, logger)
 	trackingProvider := tracking.NewDefaultProvider(ingressTagPrefix, controllerConfig.ClusterName)
 	elbv2TaggingManager := elbv2deploy.NewDefaultTaggingManager(cloud.ELBV2(), cloud.VpcID(), controllerConfig.FeatureGates, cloud.RGT(), logger)
diff --git a/docs/deploy/configurations.md b/docs/deploy/configurations.md
@@ -69,16 +69,16 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne
 |aws-api-endpoints                      | AWS API Endpoints Config        |                 | AWS API endpoints mapping, format: serviceID1=URL1,serviceID2=URL2 |
 |aws-api-throttle                       | AWS Throttle Config             | [default value](#default-throttle-config ) | throttle settings for AWS APIs, format: serviceID1:operationRegex1=rate:burst,serviceID2:operationRegex2=rate:burst |
 |aws-max-retries                        | int                             | 10              | Maximum retries for AWS APIs |
-|aws-region                             | string                          | [instance metadata](#instance-metadata)    | AWS Region for the kubernetes cluster |
-|aws-vpc-id                             | string                          | [instance metadata](#instance-metadata)    | AWS VPC ID for the Kubernetes cluster |
+|aws-region                             | string                          | [instance metadata](#instance-metadata)   | AWS Region for the kubernetes cluster |
+|aws-vpc-id                             | string                          | [instance metadata](#instance-metadata)   | AWS VPC ID for the Kubernetes cluster |
 |backend-security-group                 | string                          |                 | Backend security group id to use for the ingress rules on the worker node SG|
 |cluster-name                           | string                          |                 | Kubernetes cluster name|
 |default-ssl-policy                     | string                          | ELBSecurityPolicy-2016-08 | Default SSL Policy that will be applied to all Ingresses or Services that do not have the SSL Policy annotation |
 |default-tags                           | stringMap                       |                 | AWS Tags that will be applied to all AWS resources managed by this controller. Specified Tags takes highest priority |
 |default-target-type                    | string                          | instance        | Default target type for Ingresses and Services - ip, instance |
 |[disable-ingress-class-annotation](#disable-ingress-class-annotation)       | boolean                         | false           | Disable new usage of the `kubernetes.io/ingress.class` annotation |
 |[disable-ingress-group-name-annotation](#disable-ingress-group-name-annotation)  | boolean                         | false           | Disallow new use of the `alb.ingress.kubernetes.io/group.name` annotation |
-|disable-restricted-sg-rules            | boolean                         | false            | Disable the usage of restricted security group rules |
+|disable-restricted-sg-rules            | boolean                         | false           | Disable the usage of restricted security group rules |
 |enable-backend-security-group          | boolean                         | true            | Enable sharing of security groups for backend traffic |
 |enable-endpoint-slices                 | boolean                         | false           | Use EndpointSlices instead of Endpoints for pod endpoint and TargetGroupBinding resolution for load balancers with IP targets. |
 |enable-leader-election                 | boolean                         | true            | Enable leader election for the load balancer controller manager. Enabling this will ensure there is only one active controller manager |
@@ -98,9 +98,11 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne
 |log-level                              | string                          | info            | Set the controller log level - info, debug |
 |metrics-bind-addr                      | string                          | :8080           | The address the metric endpoint binds to |
 |service-max-concurrent-reconciles      | int                             | 3               | Maximum number of concurrently running reconcile loops for service |
-|[sync-period](#sync-period)                            | duration                        | 10h0m0s          | Period at which the controller forces the repopulation of its local object stores|
+|[sync-period](#sync-period)                            | duration                        | 10h0m0s         | Period at which the controller forces the repopulation of its local object stores|
 |targetgroupbinding-max-concurrent-reconciles | int                       | 3               | Maximum number of concurrently running reconcile loops for targetGroupBinding |
 |targetgroupbinding-max-exponential-backoff-delay | duration              | 16m40s          | Maximum duration of exponential backoff for targetGroupBinding reconcile failures |
+|tolerate-non-existent-backend-service  | boolean                         | true            | Whether to allow rules which refer to backend services that do not exist |
+|tolerate-non-existent-backend-action  | boolean                         | true            | Whether to allow rules which refer to backend actions that do not exist |
 |watch-namespace                        | string                          |                 | Namespace the controller watches for updates to Kubernetes objects, If empty, all namespaces are watched. |
 |webhook-bind-port                      | int                             | 9443            | The TCP port the Webhook server binds to |
 |webhook-cert-dir                       | string                          | /tmp/k8s-webhook-server/serving-certs | The directory that contains the server key and certificate |
diff --git a/helm/aws-load-balancer-controller/templates/deployment.yaml b/helm/aws-load-balancer-controller/templates/deployment.yaml
@@ -122,6 +122,12 @@ spec:
         {{- if kindIs "bool" .Values.disableIngressGroupNameAnnotation }}
         - --disable-ingress-group-name-annotation={{ .Values.disableIngressGroupNameAnnotation }}
         {{- end }}
+        {{- if kindIs "bool" .Values.tolerateNonExistentBackendService }}
+        - --tolerate-non-existent-backend-service={{ .Values.tolerateNonExistentBackendService }}
+        {{- end }}
+        {{- if kindIs "bool" .Values.tolerateNonExistentBackendAction }}
+        - --tolerate-non-existent-backend-action={{ .Values.tolerateNonExistentBackendAction }}
+        {{- end }}
         {{- if .Values.defaultSSLPolicy }}
         - --default-ssl-policy={{ .Values.defaultSSLPolicy }}
         {{- end }}
diff --git a/helm/aws-load-balancer-controller/test.yaml b/helm/aws-load-balancer-controller/test.yaml
@@ -209,6 +209,12 @@ disableIngressClassAnnotation:
 # disableIngressGroupNameAnnotation disables the usage of alb.ingress.kubernetes.io/group.name annotation, false by default
 disableIngressGroupNameAnnotation:
 
+# tolerateNonExistentBackendService permits rules which specify backend services that don't exist, true by default
+tolerateNonExistentBackendService:
+
+# tolerateNonExistentBackendAction permits rules which specify backend actions that don't exist, true by default
+tolerateNonExistentBackendAction:
+
 # defaultSSLPolicy specifies the default SSL policy to use for TLS/HTTPS listeners
 defaultSSLPolicy:
 
diff --git a/helm/aws-load-balancer-controller/values.yaml b/helm/aws-load-balancer-controller/values.yaml
@@ -227,6 +227,12 @@ disableIngressClassAnnotation:
 # disableIngressGroupNameAnnotation disables the usage of alb.ingress.kubernetes.io/group.name annotation, false by default
 disableIngressGroupNameAnnotation:
 
+# tolerateNonExistentBackendService permits rules which specify backend services that don't exist, true by default
+tolerateNonExistentBackendService:
+
+# tolerateNonExistentBackendAction permits rules which specify backend actions that don't exist, true by default
+tolerateNonExistentBackendAction:
+
 # defaultSSLPolicy specifies the default SSL policy to use for TLS/HTTPS listeners
 defaultSSLPolicy:
 
diff --git a/pkg/config/ingress_config.go b/pkg/config/ingress_config.go
@@ -7,10 +7,14 @@ const (
 	flagDisableIngressClassAnnotation        = "disable-ingress-class-annotation"
 	flagDisableIngressGroupNameAnnotation    = "disable-ingress-group-name-annotation"
 	flagIngressMaxConcurrentReconciles       = "ingress-max-concurrent-reconciles"
+	flagTolerateNonExistentBackendService    = "tolerate-non-existent-backend-service"
+	flagTolerateNonExistentBackendAction     = "tolerate-non-existent-backend-action"
 	defaultIngressClass                      = "alb"
 	defaultDisableIngressClassAnnotation     = false
 	defaultDisableIngressGroupNameAnnotation = false
 	defaultMaxIngressConcurrentReconciles    = 3
+	defaultTolerateNonExistentBackendService = true
+	defaultTolerateNonExistentBackendAction  = true
 )
 
 // IngressConfig contains the configurations for the Ingress controller
@@ -30,6 +34,14 @@ type IngressConfig struct {
 
 	// Max concurrent reconcile loops for Ingress objects
 	MaxConcurrentReconciles int
+
+	// TolerateNonExistentBackendService specifies whether to allow rules that reference a backend service that does not
+	// exist. In this case, requests to that rule will result in a 503 error.
+	TolerateNonExistentBackendService bool
+
+	// TolerateNonExistentBackendAction specifies whether to allow rules that reference a backend action that does not
+	// exist. In this case, requests to that rule will result in a 503 error.
+	TolerateNonExistentBackendAction bool
 }
 
 // BindFlags binds the command line flags to the fields in the config object
@@ -42,4 +54,8 @@ func (cfg *IngressConfig) BindFlags(fs *pflag.FlagSet) {
 		"Disable new usage of alb.ingress.kubernetes.io/group.name annotation")
 	fs.IntVar(&cfg.MaxConcurrentReconciles, flagIngressMaxConcurrentReconciles, defaultMaxIngressConcurrentReconciles,
 		"Maximum number of concurrently running reconcile loops for ingress")
+	fs.BoolVar(&cfg.TolerateNonExistentBackendService, flagTolerateNonExistentBackendService, defaultTolerateNonExistentBackendService,
+		"Tolerate rules that specify a non-existent backend service")
+	fs.BoolVar(&cfg.TolerateNonExistentBackendAction, flagTolerateNonExistentBackendAction, defaultTolerateNonExistentBackendAction,
+		"Tolerate rules that specify a non-existent backend action")
 }
diff --git a/pkg/ingress/enhanced_backend_builder.go b/pkg/ingress/enhanced_backend_builder.go
@@ -24,10 +24,6 @@ const (
 	nonExistentBackendServiceMessageBody = "Backend service does not exist"
 	// the message body of fixed 503 response used when referencing a non-existent annotation Action as backend.
 	nonExistentBackendActionMessageBody = "Backend action does not exist"
-	// by default, we tolerate a missing backend service, and use a fixed 503 response instead.
-	defaultTolerateNonExistentBackendService = true
-	// by default, we tolerate a missing backend action, and use a fixed 503 response instead.
-	defaultTolerateNonExistentBackendAction = true
 )
 
 // EnhancedBackend is an enhanced version of Ingress backend.
@@ -80,14 +76,13 @@ type EnhancedBackendBuilder interface {
 }
 
 // NewDefaultEnhancedBackendBuilder constructs new defaultEnhancedBackendBuilder.
-func NewDefaultEnhancedBackendBuilder(k8sClient client.Client, annotationParser annotations.Parser, authConfigBuilder AuthConfigBuilder) *defaultEnhancedBackendBuilder {
+func NewDefaultEnhancedBackendBuilder(k8sClient client.Client, annotationParser annotations.Parser, authConfigBuilder AuthConfigBuilder, tolerateNonExistentBackendService bool, tolerateNonExistentBackendAction bool) *defaultEnhancedBackendBuilder {
 	return &defaultEnhancedBackendBuilder{
-		k8sClient:         k8sClient,
-		annotationParser:  annotationParser,
-		authConfigBuilder: authConfigBuilder,
-
-		tolerateNonExistentBackendService: defaultTolerateNonExistentBackendAction,
-		tolerateNonExistentBackendAction:  defaultTolerateNonExistentBackendService,
+		k8sClient:                         k8sClient,
+		annotationParser:                  annotationParser,
+		authConfigBuilder:                 authConfigBuilder,
+		tolerateNonExistentBackendService: tolerateNonExistentBackendService,
+		tolerateNonExistentBackendAction:  tolerateNonExistentBackendAction,
 	}
 }
 
diff --git a/pkg/ingress/model_builder_test.go b/pkg/ingress/model_builder_test.go
@@ -2915,7 +2915,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) {
 			certDiscovery := NewMockCertDiscovery(ctrl)
 			annotationParser := annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io")
 			authConfigBuilder := NewDefaultAuthConfigBuilder(annotationParser)
-			enhancedBackendBuilder := NewDefaultEnhancedBackendBuilder(k8sClient, annotationParser, authConfigBuilder)
+			enhancedBackendBuilder := NewDefaultEnhancedBackendBuilder(k8sClient, annotationParser, authConfigBuilder, true, true)
 			ruleOptimizer := NewDefaultRuleOptimizer(logr.New(&log.NullLogSink{}))
 			trackingProvider := tracking.NewDefaultProvider("ingress.k8s.aws", clusterName)
 			stackMarshaller := deploy.NewDefaultStackMarshaller()
diff --git a/pkg/ingress/reference_indexer_test.go b/pkg/ingress/reference_indexer_test.go
@@ -314,7 +314,7 @@ func Test_defaultReferenceIndexer_BuildServiceRefIndexes(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			annotationParser := annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io")
 			authConfigBuilder := NewDefaultAuthConfigBuilder(annotationParser)
-			enhancedBackendBuilder := NewDefaultEnhancedBackendBuilder(nil, annotationParser, nil)
+			enhancedBackendBuilder := NewDefaultEnhancedBackendBuilder(nil, annotationParser, nil, true, true)
 			i := &defaultReferenceIndexer{
 				enhancedBackendBuilder: enhancedBackendBuilder,
 				authConfigBuilder:      authConfigBuilder,
@@ -365,7 +365,7 @@ func Test_defaultReferenceIndexer_BuildSecretRefIndexes(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			annotationParser := annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io")
 			authConfigBuilder := NewDefaultAuthConfigBuilder(annotationParser)
-			enhancedBackendBuilder := NewDefaultEnhancedBackendBuilder(nil, annotationParser, nil)
+			enhancedBackendBuilder := NewDefaultEnhancedBackendBuilder(nil, annotationParser, nil, true, true)
 			i := &defaultReferenceIndexer{
 				enhancedBackendBuilder: enhancedBackendBuilder,
 				authConfigBuilder:      authConfigBuilder,

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`----`
`2`	`1`	`apiVersion: admissionregistration.k8s.io/v1`
`3`	`2`	`kind: MutatingWebhookConfiguration`
`4`	`3`	`metadata:`