Repository: zalando-incubator/kube-ingress-aws-controller
Pull request #289 (2019-11-06 10:30:10)

Make HTTP support opt-in for Network Load Balancers
This adds a flag `--nlb-http-enabled` which when set
to true it will create an HTTP listener (port 80) for Network Load
Balancers. This value is disabled by default for security reasons since
NLB can't support HTTP -> HTTPS redirection.

This also improves the README by documenting the different features
available for the two different load balancer types supported.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/26798315/badge)](https://coveralls.io/builds/26798315)

Coverage decreased (-0.2%) to 49.089% when pulling **a912a0238674410bdb979144169df981f5d34e5d on opt-in-http-nlb** into **a204393f48b558aa0842f861a996c2c0194a9343 on master**.

szuecs: :+1: 
aermakov-zalando: Shouldn't this be configurable per ingress?
mikkeloscar: > Shouldn't this be configurable per ingress?

Yes, we can do this but I would think of this as a bigger separate task considering ALB also doesn't support this per ingress at the moment.

I don't see a use case for this in Zalando so it's not high priority for us IMO. WDYT?
aermakov-zalando: @mikkeloscar Some clients could be relying on the http->https redirect, I guess. But then it's better to break them since it's insecure, so this is probably fine.
mikkeloscar: > @mikkeloscar Some clients could be relying on the http->https redirect, I guess. But then it's better to break them since it's insecure, so this is probably fine.

Note this change won't deploy NLBs by default. You have to opt-in to get an NLB. and http -> https redirect doesn't work for NLB anyway, so they would just go to the port 80 and not get redirected if they were depending on this and the ingress changed to NLB.
mikkeloscar: :+1: 

Patch:
diff --git a/README.md b/README.md
index 0c23e55..5e07a58 100644
--- a/README.md
+++ b/README.md
@@ -20,7 +20,7 @@ This information is used to manage AWS resources for each ingress objects of the
 - Automatic discovery of SSL certificates
 - Automatic forwarding of requests to all Worker Nodes, even with auto scaling
 - Automatic cleanup of unnecessary managed resources
-- Support for both [Application Load Balancers](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/introduction.html) and [Network Load Balancers](https://docs.aws.amazon.com/elasticloadbalancing/latest/network/introduction.html).
+- Support for both [Application Load Balancers][alb] and [Network Load Balancers][nlb].
 - Support for internet-facing and internal load balancers
 - Support for multiple Auto Scaling Groups
 - Support for instances that are not part of Auto Scaling Group
@@ -92,7 +92,7 @@ running has the clusterID tag `kubernetes.io/cluster/<cluster-id>=owned` set
 
 ## Ingress annotations
 
-Overview of annotations which can be set.
+Overview of configuration which can be set via Ingress annotations.
 
 ### Annotations
 |Name                       | Value |Default
@@ -108,6 +108,26 @@ Overview of annotations which can be set.
 
 The defaults can also be configured globally via a flag on the controller.
 
+## Load Balancers types
+
+The controller supports both [Application Load Balancers][alb] and [Network
+Load Balancers][nlb]. Below is an overview of which features can be used with
+the individual Load Balancer types.
+
+|Feature                 | Application Load Balancer | Network Load Balancer |
+|------------------------|------|------|
+| HTTPS                  | :heavy_check_mark: | :heavy_check_mark:  |
+| HTTP                   | :heavy_check_mark: | :heavy_check_mark: `--nlb-http-enabled` |
+| HTTP -> HTTPS redirect | :heavy_check_mark: `--redirect-http-to-https` | :heavy_multiplication_x: |
+| [Cross Zone Load Balancing][cross_zone] | :heavy_check_mark: (only option) | :heavy_check_mark: `--nlb-cross-zone` |
+| [Dualstack support][dualstack] | :heavy_check_mark: `--ip-addr-type=dualstack` | :heavy_multiplication_x: |
+| [Idle Timeout][idle_timeout] | :heavy_check_mark: `--idle-connection-timeout` | :heavy_multiplication_x: |
+| Custom Security Group | :heavy_check_mark: | :heavy_multiplication_x: |
+
+[cross_zone]: https://docs.aws.amazon.com/elasticloadbalancing/latest/network/network-load-balancers.html#availability-zones
+[dualstack]: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/application-load-balancers.html#ip-address-type
+[idle_timeout]: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/application-load-balancers.html#load-balancer-attributes
+
 ## AWS Tags
 
 SecurityGroup auto detection needs the following AWS Tags on the
@@ -463,3 +483,9 @@ The MIT License (MIT) Copyright © [2017] Zalando SE, https://tech.zalando.com
 Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the “Software”), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
 The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
 THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+
+
+
+[alb]: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/introduction.html
+[nlb]: https://docs.aws.amazon.com/elasticloadbalancing/latest/network/introduction.html
diff --git a/aws/adapter.go b/aws/adapter.go
index eea0b3c..1ce6c12 100644
--- a/aws/adapter.go
+++ b/aws/adapter.go
@@ -58,6 +58,7 @@ type Adapter struct {
 	wafWebAclId                string
 	httpRedirectToHTTPS        bool
 	nlbCrossZone               bool
+	nlbHTTPEnabled             bool
 	customFilter               string
 }
 
@@ -100,7 +101,8 @@ const (
 	DefaultCustomFilter    = ""
 	// DefaultNLBCrossZone specifies the default configuration for cross
 	// zone load balancing: https://docs.aws.amazon.com/elasticloadbalancing/latest/network/network-load-balancers.html#load-balancer-attributes
-	DefaultNLBCrossZone = false
+	DefaultNLBCrossZone   = false
+	DefaultNLBHTTPEnabled = false
 
 	nameTag                     = "Name"
 	LoadBalancerTypeApplication = "application"
@@ -189,6 +191,7 @@ func NewAdapter(newControllerID string) (adapter *Adapter, err error) {
 		albLogsS3Prefix:     DefaultAlbS3LogsPrefix,
 		wafWebAclId:         DefaultWafWebAclId,
 		nlbCrossZone:        DefaultNLBCrossZone,
+		nlbHTTPEnabled:      DefaultNLBHTTPEnabled,
 		customFilter:        DefaultCustomFilter,
 	}
 
@@ -315,6 +318,13 @@ func (a *Adapter) WithNLBCrossZone(nlbCrossZone bool) *Adapter {
 	return a
 }
 
+// WithNLBHTTPEnabled returns the receiver adapter after setting the
+// nlbHTTPEnabled config.
+func (a *Adapter) WithNLBHTTPEnabled(nlbHTTPEnabled bool) *Adapter {
+	a.nlbHTTPEnabled = nlbHTTPEnabled
+	return a
+}
+
 // WithCustomFilter returns the receiver adapter after setting a custom filter expression
 func (a *Adapter) WithCustomFilter(customFilter string) *Adapter {
 	a.customFilter = customFilter
@@ -511,6 +521,7 @@ func (a *Adapter) CreateStack(certificateARNs []string, scheme, securityGroup, o
 		cwAlarms:                     cwAlarms,
 		httpRedirectToHTTPS:          a.httpRedirectToHTTPS,
 		nlbCrossZone:                 a.nlbCrossZone,
+		nlbHTTPEnabled:               a.nlbHTTPEnabled,
 	}
 
 	return createStack(a.cloudformation, spec)
@@ -547,6 +558,8 @@ func (a *Adapter) UpdateStack(stackName string, certificateARNs map[string]time.
 		wafWebAclId:                  a.wafWebAclId,
 		cwAlarms:                     cwAlarms,
 		httpRedirectToHTTPS:          a.httpRedirectToHTTPS,
+		nlbCrossZone:                 a.nlbCrossZone,
+		nlbHTTPEnabled:               a.nlbHTTPEnabled,
 	}
 
 	return updateStack(a.cloudformation, spec)
diff --git a/aws/cf.go b/aws/cf.go
index acf18bf..4182765 100644
--- a/aws/cf.go
+++ b/aws/cf.go
@@ -139,6 +139,7 @@ type stackSpec struct {
 	cwAlarms                     CloudWatchAlarmList
 	httpRedirectToHTTPS          bool
 	nlbCrossZone                 bool
+	nlbHTTPEnabled               bool
 }
 
 type healthCheck struct {
diff --git a/aws/cf_template.go b/aws/cf_template.go
index e931bd9..28f883e 100644
--- a/aws/cf_template.go
+++ b/aws/cf_template.go
@@ -105,7 +105,7 @@ func generateTemplate(spec *stackSpec) (string, error) {
 			Port:            cloudformation.Integer(80),
 			Protocol:        cloudformation.String("HTTP"),
 		})
-	} else {
+	} else if spec.loadbalancerType == LoadBalancerTypeApplication || spec.nlbHTTPEnabled {
 		template.AddResource("HTTPListener", &cloudformation.ElasticLoadBalancingV2Listener{
 			DefaultActions: &cloudformation.ElasticLoadBalancingV2ListenerActionList{
 				{
diff --git a/controller.go b/controller.go
index e2d586c..885b000 100644
--- a/controller.go
+++ b/controller.go
@@ -64,6 +64,7 @@ var (
 	cwAlarmConfigMapLocation   *kubernetes.ResourceLocation
 	loadBalancerType           string
 	nlbCrossZone               bool
+	nlbHTTPEnabled             bool
 )
 
 func loadSettings() error {
@@ -120,6 +121,8 @@ func loadSettings() error {
 		Default(aws.LoadBalancerTypeApplication).EnumVar(&loadBalancerType, aws.LoadBalancerTypeApplication, aws.LoadBalancerTypeNetwork)
 	kingpin.Flag("nlb-cross-zone", "Specify whether Network Load Balancers should balance cross availablity zones. This setting only apply to 'network' Load Balancers.").
 		Default("false").BoolVar(&nlbCrossZone)
+	kingpin.Flag("nlb-http-enabled", "Enable HTTP (port 80) for Network Load Balancers. By default this is disabled as NLB can't provide HTTP -> HTTPS redirect.").
+		Default("false").BoolVar(&nlbHTTPEnabled)
 	kingpin.Parse()
 
 	blacklistCertArnMap = make(map[string]bool)
@@ -228,6 +231,7 @@ func main() {
 		WithWafWebAclId(wafWebAclId).
 		WithHTTPRedirectToHTTPS(httpRedirectToHTTPS).
 		WithNLBCrossZone(nlbCrossZone).
+		WithNLBHTTPEnabled(nlbHTTPEnabled).
 		WithCustomFilter(customFilter)
 
 	certificatesProvider, err := certs.NewCachingProvider(


Test patch:
diff --git a/aws/cf_template_test.go b/aws/cf_template_test.go
index 8e4b78a..878a6dc 100644
--- a/aws/cf_template_test.go
+++ b/aws/cf_template_test.go
@@ -117,6 +117,7 @@ func TestGenerateTemplate(t *testing.T) {
 			spec: &stackSpec{
 				loadbalancerType:    LoadBalancerTypeNetwork,
 				httpRedirectToHTTPS: true,
+				nlbHTTPEnabled:      true,
 			},
 			validate: func(t *testing.T, template *cloudformation.Template) {
 				require.NotNil(t, template.Resources["HTTPListener"])
@@ -140,6 +141,16 @@ func TestGenerateTemplate(t *testing.T) {
 				require.Equal(t, attributes[0].Value.Literal, "true")
 			},
 		},
+		{
+			name: "nlb HTTP listener should not be enabled when nlbHTTPEnabled is set to false",
+			spec: &stackSpec{
+				loadbalancerType: LoadBalancerTypeNetwork,
+				nlbHTTPEnabled:   false,
+			},
+			validate: func(t *testing.T, template *cloudformation.Template) {
+				require.NotContains(t, template.Resources, "HTTPListener")
+			},
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			generated, err := generateTemplate(test.spec)