Repository: zalando-incubator/kube-ingress-aws-controller
Pull request #411 (2021-03-25 14:49:14)

Add the ability to use an HTTPS target
This adds a boolean flag called `target-https` which will set the
protocol used on a target group to HTTPS.

**Motivation**

AWS VPC networking does prevent external targets from sniffing the wire,
but if the target is a proxy listening on the hostNetwork, any other pod
deployed to that node that is also using the hostNetwork can read the traffic
unencrypted. In cases where end-to-end encryption is expected or
desirable, this allows for that option.

jhuntwork: Just as a note: I believe this will always be a non-validated cert on the target. Self-signed certs work, and appear to be acceptable. AWS documentation supports that, here: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/load-balancer-target-groups.html#target-group-routing-configuration

> Because the load balancer is in a virtual private cloud (VPC), traffic between the load balancer and the targets is authenticated at the packet level, so it is not at risk of man-in-the-middle attacks or spoofing even if the certificates on the targets are not valid. 

And further insight into AWS thinking about why they feel this is reasonable is here: https://kevin.burke.dev/kevin/aws-alb-validation-tls-reply/
jhuntwork: Will do!
jhuntwork: Still a small drop in coverage, mostly due to the new function in `adapter.go`. There don't seem to be any existing similar tests for the Adapter struct. Should I add new ones?
mikkeloscar: :+1: 

Patch:
diff --git a/README.md b/README.md
index e8d090c..0195157 100644
--- a/README.md
+++ b/README.md
@@ -565,6 +565,9 @@ listening on that port.
 If you want to change the default ports, you can control it using the
 `-target-port` and `-health-check-port` flags.
 
+If you want to use an HTTPS enabled target port, use the `-target-https` flag.
+This will only affect ALBs, NLBs ignore this flag.
+
 ## HTTP to HTTPS Redirection
 
 By default, the controller will expose both HTTP and HTTPS ports on the load balancer, and forward both listeners to the target port. Setting the flag `-redirect-http-to-https` will instead configure the HTTP listener to emit a 301 redirect for any request received, with the destination location being the same URL but with the HTTPS scheme vs. HTTP. The specifics are described in the [relevant aws documentation](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-elasticloadbalancingv2-listener-redirectconfig.html).
diff --git a/aws/adapter.go b/aws/adapter.go
index 3c26f32..4f7925e 100644
--- a/aws/adapter.go
+++ b/aws/adapter.go
@@ -43,6 +43,7 @@ type Adapter struct {
 	healthCheckInterval         time.Duration
 	healthCheckTimeout          time.Duration
 	targetPort                  uint
+	targetHTTPS                 bool
 	creationTimeout             time.Duration
 	idleConnectionTimeout       time.Duration
 	deregistrationDelayTimeout  time.Duration
@@ -249,6 +250,12 @@ func (a *Adapter) WithTargetPort(port uint) *Adapter {
 	return a
 }
 
+// WithTargetHTTPS returns the receiver adapter after specifying the target port will use HTTPS
+func (a *Adapter) WithTargetHTTPS(https bool) *Adapter {
+	a.targetHTTPS = https
+	return a
+}
+
 // WithHealthCheckInterval returns the receiver adapter after changing the health check interval that will be used by
 // the resources created by the adapter
 func (a *Adapter) WithHealthCheckInterval(interval time.Duration) *Adapter {
@@ -638,6 +645,7 @@ func (a *Adapter) UpdateStack(stackName string, certificateARNs map[string]time.
 			timeout:  a.healthCheckTimeout,
 		},
 		targetPort:                        a.targetPort,
+		targetHTTPS:                       a.targetHTTPS,
 		timeoutInMinutes:                  uint(a.creationTimeout.Minutes()),
 		stackTerminationProtection:        a.stackTerminationProtection,
 		idleConnectionTimeoutSeconds:      uint(a.idleConnectionTimeout.Seconds()),
diff --git a/aws/cf.go b/aws/cf.go
index 0d2cdfd..f943939 100644
--- a/aws/cf.go
+++ b/aws/cf.go
@@ -130,6 +130,7 @@ type stackSpec struct {
 	vpcID                             string
 	healthCheck                       *healthCheck
 	targetPort                        uint
+	targetHTTPS                       bool
 	timeoutInMinutes                  uint
 	customTemplate                    string
 	stackTerminationProtection        bool
diff --git a/aws/cf_template.go b/aws/cf_template.go
index 3bf8b3c..2372c9f 100644
--- a/aws/cf_template.go
+++ b/aws/cf_template.go
@@ -24,6 +24,9 @@ const (
 	//
 	// [0]: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-listenerrule.html#cfn-elasticloadbalancingv2-listenerrule-priority
 	internalTrafficDenyRulePriority int64 = 1
+
+	httpProtocol  = "HTTP"
+	httpsProtocol = "HTTPS"
 )
 
 func hashARNs(certARNs []string) []byte {
@@ -112,11 +115,13 @@ func generateTemplate(spec *stackSpec) (string, error) {
 		}
 	}
 
-	protocol := "HTTP"
-	tlsProtocol := "HTTPS"
+	protocol := httpProtocol
+	tlsProtocol := httpsProtocol
 	if spec.loadbalancerType == LoadBalancerTypeNetwork {
 		protocol = "TCP"
 		tlsProtocol = "TLS"
+	} else if spec.targetHTTPS {
+		protocol = httpsProtocol
 	}
 
 	if spec.loadbalancerType == LoadBalancerTypeApplication && spec.httpRedirectToHTTPS {
@@ -125,7 +130,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
 				{
 					Type: cloudformation.String("redirect"),
 					RedirectConfig: &cloudformation.ElasticLoadBalancingV2ListenerRedirectConfig{
-						Protocol:   cloudformation.String("HTTPS"),
+						Protocol:   cloudformation.String(httpsProtocol),
 						Port:       cloudformation.String("443"),
 						Host:       cloudformation.String("#{host}"),
 						Path:       cloudformation.String("/#{path}"),
@@ -136,7 +141,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
 			},
 			LoadBalancerArn: cloudformation.Ref("LB").String(),
 			Port:            cloudformation.Integer(80),
-			Protocol:        cloudformation.String("HTTP"),
+			Protocol:        cloudformation.String(httpProtocol),
 		})
 	} else if spec.loadbalancerType == LoadBalancerTypeApplication || spec.nlbHTTPEnabled {
 		listenerName := "HTTPListener"
@@ -325,7 +330,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
 		HealthCheckIntervalSeconds: cloudformation.Ref(parameterTargetGroupHealthCheckIntervalParameter).Integer(),
 		HealthCheckPath:            cloudformation.Ref(parameterTargetGroupHealthCheckPathParameter).String(),
 		HealthCheckPort:            cloudformation.Ref(parameterTargetGroupHealthCheckPortParameter).String(),
-		HealthCheckProtocol:        cloudformation.String("HTTP"),
+		HealthCheckProtocol:        cloudformation.String(protocol),
 		Port:                       cloudformation.Ref(parameterTargetTargetPortParameter).Integer(),
 		Protocol:                   cloudformation.String(protocol),
 		VPCID:                      cloudformation.Ref(parameterTargetGroupVPCIDParameter).String(),
diff --git a/controller.go b/controller.go
index f2418d9..aaa8d90 100644
--- a/controller.go
+++ b/controller.go
@@ -43,6 +43,7 @@ var (
 	healthCheckInterval           time.Duration
 	healthCheckTimeout            time.Duration
 	targetPort                    uint
+	targetHTTPS                   bool
 	metricsAddress                string
 	disableSNISupport             bool
 	disableInstrumentedHttpClient bool
@@ -110,6 +111,8 @@ func loadSettings() error {
 		Default(strconv.FormatUint(aws.DefaultHealthCheckPort, 10)).UintVar(&healthCheckPort)
 	kingpin.Flag("target-port", "sets the target port for the created target groups").
 		Default(strconv.FormatUint(aws.DefaultTargetPort, 10)).UintVar(&targetPort)
+	kingpin.Flag("target-https", "sets the target protocol to https").
+		Default("false").BoolVar(&targetHTTPS)
 	kingpin.Flag("health-check-interval", "sets the health check interval for the created target groups. The flag accepts a value acceptable to time.ParseDuration").
 		Default(aws.DefaultHealthCheckInterval.String()).DurationVar(&healthCheckInterval)
 	kingpin.Flag("health-check-timeout", "sets the health check timeout for the created target groups. The flag accepts a value acceptable to time.ParseDuration").
@@ -252,6 +255,7 @@ func main() {
 		WithHealthCheckInterval(healthCheckInterval).
 		WithHealthCheckTimeout(healthCheckTimeout).
 		WithTargetPort(targetPort).
+		WithTargetHTTPS(targetHTTPS).
 		WithCreationTimeout(creationTimeout).
 		WithStackTerminationProtection(stackTerminationProtection).
 		WithIdleConnectionTimeout(idleConnectionTimeout).


Test patch:
diff --git a/aws/adapter_test.go b/aws/adapter_test.go
index 2630eeb..db2c47e 100644
--- a/aws/adapter_test.go
+++ b/aws/adapter_test.go
@@ -910,3 +910,20 @@ func TestNonTargetedASGs(t *testing.T) {
 		})
 	}
 }
+
+func TestWithTargetPort(t *testing.T) {
+	t.Run("WithTargetPort sets the targetPort property", func(t *testing.T) {
+		a := Adapter{}
+		port := uint(9977)
+		b := a.WithTargetPort(port)
+		require.Equal(t, port, b.targetPort)
+	})
+}
+
+func TestWithTargetHTTPS(t *testing.T) {
+	t.Run("WithTargetHTTPS sets the targetHTTPS property", func(t *testing.T) {
+		a := Adapter{}
+		b := a.WithTargetHTTPS(true)
+		require.Equal(t, true, b.targetHTTPS)
+	})
+}
diff --git a/aws/cf_template_test.go b/aws/cf_template_test.go
index 16860e9..7f40d5f 100644
--- a/aws/cf_template_test.go
+++ b/aws/cf_template_test.go
@@ -355,6 +355,50 @@ func TestGenerateTemplate(t *testing.T) {
 				require.NotContains(t, template.Resources, "HTTPRuleBlockInternalTraffic")
 			},
 		},
+		{
+			name: "Uses HTTPS targets when asked to",
+			spec: &stackSpec{
+				loadbalancerType: LoadBalancerTypeApplication,
+				certificateARNs:  map[string]time.Time{"domain.company.com": time.Now()},
+				targetHTTPS:      true,
+			},
+			validate: func(t *testing.T, template *cloudformation.Template) {
+				tg, ok := template.Resources["TG"].Properties.(*cloudformation.ElasticLoadBalancingV2TargetGroup)
+				require.True(t, ok, "couldn't convert resource to ElasticLoadBalancingV2TargetGroup")
+				require.Equal(t, cloudformation.String("HTTPS"), tg.Protocol)
+			},
+		},
+		{
+			name: "NLBs should always use the TCP protocol for listeners and TGs",
+			spec: &stackSpec{
+				loadbalancerType: LoadBalancerTypeNetwork,
+				certificateARNs:  map[string]time.Time{"domain.company.com": time.Now()},
+				nlbHTTPEnabled:   true,
+			},
+			validate: func(t *testing.T, template *cloudformation.Template) {
+				tg, ok := template.Resources["TG"].Properties.(*cloudformation.ElasticLoadBalancingV2TargetGroup)
+				require.True(t, ok, "couldn't convert resource to ElasticLoadBalancingV2TargetGroup")
+				listener := template.Resources["HTTPListener"].Properties.(*cloudformation.ElasticLoadBalancingV2Listener)
+				require.Equal(t, cloudformation.String("TCP"), listener.Protocol)
+				require.Equal(t, cloudformation.String("TCP"), tg.Protocol)
+			},
+		},
+		{
+			name: "NLBs should always use the TCP protocol for listeners and TGs - targetHTTPS should not impact it",
+			spec: &stackSpec{
+				loadbalancerType: LoadBalancerTypeNetwork,
+				certificateARNs:  map[string]time.Time{"domain.company.com": time.Now()},
+				nlbHTTPEnabled:   true,
+				targetHTTPS:      true,
+			},
+			validate: func(t *testing.T, template *cloudformation.Template) {
+				tg, ok := template.Resources["TG"].Properties.(*cloudformation.ElasticLoadBalancingV2TargetGroup)
+				require.True(t, ok, "couldn't convert resource to ElasticLoadBalancingV2TargetGroup")
+				listener := template.Resources["HTTPListener"].Properties.(*cloudformation.ElasticLoadBalancingV2Listener)
+				require.Equal(t, cloudformation.String("TCP"), listener.Protocol)
+				require.Equal(t, cloudformation.String("TCP"), tg.Protocol)
+			},
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			generated, err := generateTemplate(test.spec)