fixed the mTLS bug (#3717)

* fixed the mTLS bug #3715 * addressed comments * addressed comments * added tests * updated tests * updated tests * updated fix * updated annotation * fixed the mTLS bug #3715 addressed comments addressed comments added tests updated tests updated tests updated fix updated annotation fixed broken tests
kubernetes-sigs · May 31, 2024 · e5d625f · e5d625f
1 parent 756fc5d
commit e5d625f
Show file tree

Hide file tree

Showing 6 changed files with 174 additions and 80 deletions.
diff --git a/docs/guide/ingress/annotations.md b/docs/guide/ingress/annotations.md
@@ -14,51 +14,51 @@ You can add annotations to kubernetes Ingress and Service objects to customize t
         - Merge: such annotation can be specified on all Ingresses within IngressGroup, and will be merged together.
 
 ## Annotations
-| Name                                                                                                  | Type                        |Default|Location|MergeBehavior|
-|-------------------------------------------------------------------------------------------------------|-----------------------------|-------|--------|------|
-| [alb.ingress.kubernetes.io/load-balancer-name](#load-balancer-name)                                   | string                      |N/A|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/group.name](#group.name)                                                   | string                      |N/A|Ingress|N/A|
-| [alb.ingress.kubernetes.io/group.order](#group.order)                                                 | integer                     |0|Ingress|N/A|
-| [alb.ingress.kubernetes.io/tags](#tags)                                                               | stringMap                   |N/A|Ingress,Service|Merge|
-| [alb.ingress.kubernetes.io/ip-address-type](#ip-address-type)                                         | ipv4 \| dualstack \|  dualstack-without-public-ipv4           |ipv4|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/scheme](#scheme)                                                           | internal \| internet-facing |internal|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/subnets](#subnets)                                                         | stringList                  |N/A|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/security-groups](#security-groups)                                         | stringList                  |N/A|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/manage-backend-security-group-rules](#manage-backend-security-group-rules) | boolean                     |N/A|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/customer-owned-ipv4-pool](#customer-owned-ipv4-pool)                       | string                      |N/A|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/load-balancer-attributes](#load-balancer-attributes)                       | stringMap                   |N/A|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/wafv2-acl-arn](#wafv2-acl-arn)                                             | string                      |N/A|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/waf-acl-id](#waf-acl-id)                                                   | string                      |N/A|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/shield-advanced-protection](#shield-advanced-protection)                   | boolean                     |N/A|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/listen-ports](#listen-ports)                                               | json                        |'[{"HTTP": 80}]' \| '[{"HTTPS": 443}]'|Ingress|Merge|
-| [alb.ingress.kubernetes.io/ssl-redirect](#ssl-redirect)                                               | integer                     |N/A|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/inbound-cidrs](#inbound-cidrs)                                             | stringList                  |0.0.0.0/0, ::/0|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/security-group-prefix-lists](#security-group-prefix-lists)                                               | stringList                        |pl-00000000, pl-1111111|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/certificate-arn](#certificate-arn)                                         | stringList                  |N/A|Ingress|Merge|
-| [alb.ingress.kubernetes.io/ssl-policy](#ssl-policy)                                                   | string                      |ELBSecurityPolicy-2016-08|Ingress|Exclusive|
-| [alb.ingress.kubernetes.io/target-type](#target-type)                                                 | instance \| ip              |instance|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/backend-protocol](#backend-protocol)                                       | HTTP \| HTTPS               |HTTP|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/backend-protocol-version](#backend-protocol-version)                       | string                      | HTTP1 |Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/target-group-attributes](#target-group-attributes)                         | stringMap                   |N/A|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/healthcheck-port](#healthcheck-port)                                       | integer \| traffic-port     |traffic-port|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/healthcheck-protocol](#healthcheck-protocol)                               | HTTP \| HTTPS               |HTTP|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/healthcheck-path](#healthcheck-path)                                       | string                      |/ \| /AWS.ALB/healthcheck |Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/healthcheck-interval-seconds](#healthcheck-interval-seconds)               | integer                     |'15'|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/healthcheck-timeout-seconds](#healthcheck-timeout-seconds)                 | integer                     |'5'|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/healthy-threshold-count](#healthy-threshold-count)                         | integer                     |'2'|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/unhealthy-threshold-count](#unhealthy-threshold-count)                     | integer                     |'2'|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/success-codes](#success-codes)                                             | string                      |'200' \| '12' |Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/auth-type](#auth-type)                                                     | none\|oidc\|cognito         |none|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/auth-idp-cognito](#auth-idp-cognito)                                       | json                        |N/A|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/auth-idp-oidc](#auth-idp-oidc)                                             | json                        |N/A|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/auth-on-unauthenticated-request](#auth-on-unauthenticated-request)         | authenticate\|allow\|deny   |authenticate|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/auth-scope](#auth-scope)                                                   | string                      |openid|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/auth-session-cookie](#auth-session-cookie)                                 | string                      |AWSELBAuthSessionCookie|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/auth-session-timeout](#auth-session-timeout)                               | integer                     |'604800'|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/actions.${action-name}](#actions)                                          | json                        |N/A|Ingress|N/A|
-| [alb.ingress.kubernetes.io/conditions.${conditions-name}](#conditions)                                | json                        |N/A|Ingress|N/A|
-| [alb.ingress.kubernetes.io/target-node-labels](#target-node-labels)                                   | stringMap                   |N/A|Ingress,Service|N/A|
-| [alb.ingress.kubernetes.io/mutual-authentication](#mutual-authentication)                             | json                        |'[{"port": 443, "mode": "off"}]'|Ingress|Exclusive|
+| Name                                                                                                  | Type                        |Default| Location        | MergeBehavior |
+|-------------------------------------------------------------------------------------------------------|-----------------------------|------|-----------------|-----------|
+| [alb.ingress.kubernetes.io/load-balancer-name](#load-balancer-name)                                   | string                      |N/A| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/group.name](#group.name)                                                   | string                      |N/A| Ingress         | N/A       |
+| [alb.ingress.kubernetes.io/group.order](#group.order)                                                 | integer                     |0| Ingress         | N/A       |
+| [alb.ingress.kubernetes.io/tags](#tags)                                                               | stringMap                   |N/A| Ingress,Service | Merge     |
+| [alb.ingress.kubernetes.io/ip-address-type](#ip-address-type)                                         | ipv4 \| dualstack \|  dualstack-without-public-ipv4           |ipv4| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/scheme](#scheme)                                                           | internal \| internet-facing |internal| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/subnets](#subnets)                                                         | stringList                  |N/A| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/security-groups](#security-groups)                                         | stringList                  |N/A| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/manage-backend-security-group-rules](#manage-backend-security-group-rules) | boolean                     |N/A| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/customer-owned-ipv4-pool](#customer-owned-ipv4-pool)                       | string                      |N/A| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/load-balancer-attributes](#load-balancer-attributes)                       | stringMap                   |N/A| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/wafv2-acl-arn](#wafv2-acl-arn)                                             | string                      |N/A| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/waf-acl-id](#waf-acl-id)                                                   | string                      |N/A| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/shield-advanced-protection](#shield-advanced-protection)                   | boolean                     |N/A| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/listen-ports](#listen-ports)                                               | json                        |'[{"HTTP": 80}]' \| '[{"HTTPS": 443}]'| Ingress         | Merge     |
+| [alb.ingress.kubernetes.io/ssl-redirect](#ssl-redirect)                                               | integer                     |N/A| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/inbound-cidrs](#inbound-cidrs)                                             | stringList                  |0.0.0.0/0, ::/0| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/security-group-prefix-lists](#security-group-prefix-lists)                                               | stringList                        |pl-00000000, pl-1111111| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/certificate-arn](#certificate-arn)                                         | stringList                  |N/A| Ingress         | Merge     |
+| [alb.ingress.kubernetes.io/ssl-policy](#ssl-policy)                                                   | string                      |ELBSecurityPolicy-2016-08| Ingress         | Exclusive |
+| [alb.ingress.kubernetes.io/target-type](#target-type)                                                 | instance \| ip              |instance| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/backend-protocol](#backend-protocol)                                       | HTTP \| HTTPS               |HTTP| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/backend-protocol-version](#backend-protocol-version)                       | string                      | HTTP1 | Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/target-group-attributes](#target-group-attributes)                         | stringMap                   |N/A| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/healthcheck-port](#healthcheck-port)                                       | integer \| traffic-port     |traffic-port| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/healthcheck-protocol](#healthcheck-protocol)                               | HTTP \| HTTPS               |HTTP| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/healthcheck-path](#healthcheck-path)                                       | string                      |/ \| /AWS.ALB/healthcheck | Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/healthcheck-interval-seconds](#healthcheck-interval-seconds)               | integer                     |'15'| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/healthcheck-timeout-seconds](#healthcheck-timeout-seconds)                 | integer                     |'5'| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/healthy-threshold-count](#healthy-threshold-count)                         | integer                     |'2'| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/unhealthy-threshold-count](#unhealthy-threshold-count)                     | integer                     |'2'| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/success-codes](#success-codes)                                             | string                      |'200' \| '12' | Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/auth-type](#auth-type)                                                     | none\|oidc\|cognito         |none| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/auth-idp-cognito](#auth-idp-cognito)                                       | json                        |N/A| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/auth-idp-oidc](#auth-idp-oidc)                                             | json                        |N/A| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/auth-on-unauthenticated-request](#auth-on-unauthenticated-request)         | authenticate\|allow\|deny   |authenticate| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/auth-scope](#auth-scope)                                                   | string                      |openid| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/auth-session-cookie](#auth-session-cookie)                                 | string                      |AWSELBAuthSessionCookie| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/auth-session-timeout](#auth-session-timeout)                               | integer                     |'604800'| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/actions.${action-name}](#actions)                                          | json                        |N/A| Ingress         | N/A       |
+| [alb.ingress.kubernetes.io/conditions.${conditions-name}](#conditions)                                | json                        |N/A| Ingress         | N/A       |
+| [alb.ingress.kubernetes.io/target-node-labels](#target-node-labels)                                   | stringMap                   |N/A| Ingress,Service | N/A       |
+| [alb.ingress.kubernetes.io/mutual-authentication](#mutual-authentication)                             | json                        |N/A| Ingress         |Exclusive|
 
 ## IngressGroup
 IngressGroup feature enables you to group multiple Ingress resources together.
@@ -790,16 +790,19 @@ TLS support can be controlled with the following annotations:
 
 - <a name="mutual-authentication">`alb.ingress.kubernetes.io/mutual-authentication`</a>  specifies the mutual authentication configuration that should be assigned to the Application Load Balancer secure listener ports. See [Mutual authentication with TLS](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/mutual-authentication.html) in the AWS documentation for more details.
 
-    !!!note "Configuration Options"
-        - `port: listen port ` 
-            - Must be a HTTPS port specified by [listen-ports](#listen-ports).
-        - `mode: "off" (default) | "passthrough" | "verify"`
-            - `verify` mode requires an existing trust store resource.
-            -  See [Create a trust store](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/mutual-authentication.html#create-trust-store) in the AWS documentation for more details.
-        - `trustStore: ARN (arn:aws:elasticloadbalancing:trustStoreArn) | Name (my-trust-store)`
-            - Both ARN and Name of trustStore are supported values.
-            - `trustStore` is required when mode is `verify`.
-        - `ignoreClientCertificateExpiry : true | false (default)`
+    !!!note 
+        - This annotation is not applicable for Outposts, Local Zones or Wavelength zones.
+        - "Configuration Options"
+            - `port: listen port ` 
+               - Must be a HTTPS port specified by [listen-ports](#listen-ports).
+            - `mode: "off" (default) | "passthrough" | "verify"`
+               - `verify` mode requires an existing trust store resource.
+               -  See [Create a trust store](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/mutual-authentication.html#create-trust-store) in the AWS documentation for more details.
+            - `trustStore: ARN (arn:aws:elasticloadbalancing:trustStoreArn) | Name (my-trust-store)`
+               - Both ARN and Name of trustStore are supported values.
+               - `trustStore` is required when mode is `verify`.
+            - `ignoreClientCertificateExpiry : true | false (default)`
+        - Once the Mutual Authentication is set, to turn it off, you will have to explicitly pass in this annotation with `mode : "off"`.
 
     !!!example
         - [listen-ports](#listen-ports) specifies four HTTPS ports: `80, 443, 8080, 8443`

diff --git a/pkg/deploy/elbv2/listener_manager.go b/pkg/deploy/elbv2/listener_manager.go
@@ -267,7 +267,7 @@ func isSDKListenerSettingsDrifted(lsSpec elbv2model.ListenerSpec, sdkLS Listener
 	if len(lsSpec.ALPNPolicy) != 0 && !cmp.Equal(lsSpec.ALPNPolicy, awssdk.StringValueSlice(sdkLS.Listener.AlpnPolicy), cmpopts.EquateEmpty()) {
 		return true
 	}
-	if !reflect.DeepEqual(desiredDefaultMutualAuthentication, sdkLS.Listener.MutualAuthentication) {
+	if desiredDefaultMutualAuthentication != nil && !reflect.DeepEqual(desiredDefaultMutualAuthentication, sdkLS.Listener.MutualAuthentication) {
 		return true
 	}
 

diff --git a/pkg/ingress/model_build_listener.go b/pkg/ingress/model_build_listener.go
@@ -273,14 +273,10 @@ type MutualAuthenticationConfig struct {
 
 func (t *defaultModelBuildTask) computeIngressMutualAuthentication(ctx context.Context, ing *ClassifiedIngress) (map[int64]*elbv2model.MutualAuthenticationAttributes, error) {
 	var rawMtlsConfigString string
-
-	// If both Ingress annotation is missing mutual-authentication config, return default mutualAuthentication mode
 	if exists := t.annotationParser.ParseStringAnnotation(annotations.IngressSuffixMutualAuthentication, &rawMtlsConfigString, ing.Ing.Annotations); !exists {
-		return map[int64]*elbv2model.MutualAuthenticationAttributes{443: {
-			Mode: string(elbv2model.MutualAuthenticationOffMode),
-		}}, nil
-
+		return nil, nil
 	}
+
 	var ingressAnnotationEntries []MutualAuthenticationConfig
 
 	if err := json.Unmarshal([]byte(rawMtlsConfigString), &ingressAnnotationEntries); err != nil {