Repository: zalando-incubator/kube-ingress-aws-controller
Pull request #259 (2019-06-24 18:11:23)

feature: sslpolicy can be set by ingress
Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>

szuecs: I guess it won't split/merge stacks, so please review this carefully 
coveralls: 
[![Coverage Status](https://coveralls.io/builds/24195537/badge)](https://coveralls.io/builds/24195537)

Coverage decreased (-0.2%) to 46.641% when pulling **be3d12953c49fb45242d66b0040760f2c231725a on feature/sslpolicy-per-ingress** into **cf5701492fddcdff1ce0fa75adbeec91f60d86e3 on master**.

szuecs: :+1: 
szuecs: :+1: 
mikkeloscar: :+1: 
szuecs: :+1: 
mikkeloscar: :+1: 

Patch:
diff --git a/README.md b/README.md
index 812e91c..a486247 100644
--- a/README.md
+++ b/README.md
@@ -23,6 +23,7 @@ This information is used to manage AWS resources for each ingress objects of the
 - 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
+- Support for SSLPolicy, set default and per ingress
 - Can be used in clusters created by [Kops](https://github.com/kubernetes/kops), see our [deployment guide for Kops](deploy/kops.md)
 - [Support Multiple TLS Certificates per ALB (SNI)](https://aws.amazon.com/blogs/aws/new-application-load-balancer-sni/).
 
@@ -30,7 +31,7 @@ This information is used to manage AWS resources for each ingress objects of the
 
 ### <v0.8.0 to >=v0.8.0
 
-Version `v0.8.0` added certificate verification check to automatically ignore 
+Version `v0.8.0` added certificate verification check to automatically ignore
 self-signed and certificates from internal CAs. The IAM role used by the controller
 now needs the `acm:GetCertificate` permission. `acm:DescribeCertificate` permission
 is no longer needed and can be removed from the role.
@@ -77,6 +78,22 @@ Additionally you must ensure that the instance where the ingress-controller is
 running has the clusterID tag `kubernetes.io/cluster/<cluster-id>=owned` set
 (was `ClusterID=<cluster-id>` before v0.4.0).
 
+## AWS Tags
+
+SecurityGroup auto detection needs the following AWS Tags on the
+SecurityGroup:
+- `kubernetes.io/cluster/<cluster-id>=owned`
+- `kubernetes:application=<controller-id>`, controller-id defaults to
+`kube-ingress-aws-controller` and can be set by flag `-controller-id=<my-ctrl-id>`.
+
+AutoScalingGroup auto detection needs the same AWS tags  on the
+AutoScalingGroup as defined for the SecurityGroup.
+
+In case you want to attach/detach single EC2 instances to the ALB
+TargetGroup, you have to have the same `<cluster-id>` set as on the
+running kube-ingress-aws-controller. Normally this would be
+`kubernetes.io/cluster/<cluster-id>=owned`.
+
 ## Development Status
 
 This controller is used in production since Q1 2017. It aims to be out-of-the-box useful for anyone
@@ -185,6 +202,21 @@ spec:
 The Application Load Balancer created by the controller will have both an HTTP listener and an HTTPS listener. The
 latter will use the automatically selected certificates.
 
+By default the ingress-controller will aggregate all ingresses under as few
+Application Load Balancers as possible (unless running with
+`-disable-sni-support`). If you like to provision an Application Load Balancer
+that is unique for an ingress you can use the annotation
+`zalando.org/aws-load-balancer-shared: "false"`.
+
+The new Application Load Balancers have a custom tag marking them as *managed* load balancers to differentiate them
+from other load balancers. The tag looks like this:
+
+    `kubernetes:application` = `kube-ingress-aws-controller`
+
+They also share the `kubernetes.io/cluster/<cluster-id>` tag with other resources from the cluster where it belongs.
+
+#### Create a Load Balancer with a pinned certificate
+
 As a second option you can specify the [Amazon Resource Name](https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html) (ARN)
 of the desired certificate with an annotation like the one shown here:
 
@@ -205,6 +237,8 @@ spec:
           servicePort: main-port
 ```
 
+#### Create an internal Load Balancer
+
 You can select the [Application Load Balancer Scheme](http://docs.aws.amazon.com/elasticloadbalancing/latest/userguide/how-elastic-load-balancing-works.html#load-balancer-scheme)
 with an annotation like the one shown here:
 
@@ -227,18 +261,66 @@ spec:
 
 You can only select from `internet-facing` (default) and `internal` options.
 
-By default the ingress-controller will aggregate all ingresses under as few
-Application Load Balancers as possible (unless running with
-`-disable-sni-support`). If you like to provision an Application Load Balancer
-that is unique for an ingress you can use the annotation
-`zalando.org/aws-load-balancer-shared: "false"`.
+#### Create Load Balancer with SSL Policy
 
-The new Application Load Balancers have a custom tag marking them as *managed* load balancers to differentiate them
-from other load balancers. The tag looks like this:
+You can select the default
+[SSLPolicy](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/create-https-listener.html#describe-ssl-policies),
+with the flag `-ssl-policy=ELBSecurityPolicy-TLS-1-2-2017-01`. This
+choice can be overriden by the Kubernetes Ingress annotation
+`zalando.org/aws-load-balancer-ssl-policy` to any valid value. Valid
+values will be checked by the controller.
+
+Example:
+
+```yaml
+apiVersion: extensions/v1beta1
+kind: Ingress
+metadata:
+  name: myingress
+  annotations:
+    zalando.org/aws-load-balancer-ssl-policy: ELBSecurityPolicy-FS-2018-06
+spec:
+  rules:
+  - host: test-app.example.org
+    http:
+      paths:
+      - backend:
+          serviceName: test-app-service
+          servicePort: main-port
+```
+
+#### Create Load Balancer with SecurityGroup
+
+The controller will normally automatically detect the SecurityGroup to
+use. Auto detection is done by filtering all SecurityGroups with AWS
+Tags. The `kubernetes.io/cluster/<cluster-id>` tag of the Security
+Group should match clusterID for the controller node with value
+`owned` and `kubernetes:application` tag should match the value
+`kube-ingress-aws-controller`.
+
+If you want to override the detected SecurityGroup, you can set a
+SecurityGroup of your choice with the
+`zalando.org/aws-load-balancer-security-group` annotation like the
+shown here:
+
+```yaml
+apiVersion: extensions/v1beta1
+kind: Ingress
+metadata:
+  name: myingress
+  annotations:
+    zalando.org/aws-load-balancer-security-group: sg-somegroupeid
+spec:
+  rules:
+  - host: test-app.example.org
+    http:
+      paths:
+      - backend:
+          serviceName: test-app-service
+          servicePort: main-port
+```
 
-    `kubernetes:application` = `kube-ingress-aws-controller`
 
-They also share the `kubernetes.io/cluster/<cluster-id>` tag with other resources from the cluster where it belongs.
 
 ### Deleting load balancers
 
diff --git a/aws/adapter.go b/aws/adapter.go
index 84b4087..f9dc9ad 100644
--- a/aws/adapter.go
+++ b/aws/adapter.go
@@ -120,6 +120,16 @@ var (
 	ErrMissingAutoScalingGroupTag = errors.New(`instance is missing the "` + autoScalingGroupNameTag + `" tag`)
 	// ErrNoRunningInstances is used to signal that no instances were found in the running state
 	ErrNoRunningInstances = errors.New("no reservations or instances in the running state")
+
+	validSSLPolicy = map[string]bool{
+		"ELBSecurityPolicy-2016-08":             true,
+		"ELBSecurityPolicy-FS-2018-06":          true,
+		"ELBSecurityPolicy-TLS-1-2-2017-01":     true,
+		"ELBSecurityPolicy-TLS-1-2-Ext-2018-06": true,
+		"ELBSecurityPolicy-TLS-1-1-2017-01":     true,
+		"ELBSecurityPolicy-2015-05":             true,
+		"ELBSecurityPolicy-TLS-1-0-2015-04":     true,
+	}
 )
 
 var configProvider = defaultConfigProvider
@@ -428,12 +438,15 @@ func (a *Adapter) UpdateTargetGroupsAndAutoScalingGroups(stacks []*Stack) {
 // All the required resources (listeners and target group) are created in a
 // transactional fashion.
 // Failure to create the stack causes it to be deleted automatically.
-func (a *Adapter) CreateStack(certificateARNs []string, scheme, securityGroup, owner string) (string, error) {
+func (a *Adapter) CreateStack(certificateARNs []string, scheme, securityGroup, owner, sslPolicy string) (string, error) {
 	certARNs := make(map[string]time.Time, len(certificateARNs))
 	for _, arn := range certificateARNs {
 		certARNs[arn] = time.Time{}
 	}
 
+	if _, ok := validSSLPolicy[sslPolicy]; !ok {
+		sslPolicy = a.sslPolicy
+	}
 	spec := &stackSpec{
 		name:            a.stackName(),
 		scheme:          scheme,
@@ -453,7 +466,7 @@ func (a *Adapter) CreateStack(certificateARNs []string, scheme, securityGroup, o
 		stackTerminationProtection:   a.stackTerminationProtection,
 		idleConnectionTimeoutSeconds: uint(a.idleConnectionTimeout.Seconds()),
 		controllerID:                 a.controllerID,
-		sslPolicy:                    a.sslPolicy,
+		sslPolicy:                    sslPolicy,
 		ipAddressType:                a.ipAddressType,
 		albLogsS3Bucket:              a.albLogsS3Bucket,
 		albLogsS3Prefix:              a.albLogsS3Prefix,
@@ -463,7 +476,11 @@ func (a *Adapter) CreateStack(certificateARNs []string, scheme, securityGroup, o
 	return createStack(a.cloudformation, spec)
 }
 
-func (a *Adapter) UpdateStack(stackName string, certificateARNs map[string]time.Time, scheme string) (string, error) {
+func (a *Adapter) UpdateStack(stackName string, certificateARNs map[string]time.Time, scheme, sslPolicy string) (string, error) {
+	if _, ok := validSSLPolicy[sslPolicy]; !ok {
+		sslPolicy = a.sslPolicy
+	}
+
 	spec := &stackSpec{
 		name:            stackName,
 		scheme:          scheme,
@@ -482,7 +499,7 @@ func (a *Adapter) UpdateStack(stackName string, certificateARNs map[string]time.
 		stackTerminationProtection:   a.stackTerminationProtection,
 		idleConnectionTimeoutSeconds: uint(a.idleConnectionTimeout.Seconds()),
 		controllerID:                 a.controllerID,
-		sslPolicy:                    a.sslPolicy,
+		sslPolicy:                    sslPolicy,
 		ipAddressType:                a.ipAddressType,
 		albLogsS3Bucket:              a.albLogsS3Bucket,
 		albLogsS3Prefix:              a.albLogsS3Prefix,
diff --git a/aws/cf.go b/aws/cf.go
index 0169ab1..cf25174 100644
--- a/aws/cf.go
+++ b/aws/cf.go
@@ -24,6 +24,7 @@ type Stack struct {
 	DNSName         string
 	Scheme          string
 	SecurityGroup   string
+	SSLPolicy       string
 	TargetGroupARN  string
 	CertificateARNs map[string]time.Time
 	OwnerIngress    string
@@ -345,6 +346,7 @@ func mapToManagedStack(stack *cloudformation.Stack) *Stack {
 		TargetGroupARN:  outputs.targetGroupARN(),
 		Scheme:          parameters[parameterLoadBalancerSchemeParameter],
 		SecurityGroup:   parameters[parameterLoadBalancerSecurityGroupParameter],
+		SSLPolicy:       parameters[parameterListenerSslPolicyParameter],
 		CertificateARNs: certificateARNs,
 		tags:            tags,
 		OwnerIngress:    ownerIngress,
diff --git a/controller.go b/controller.go
index 7835472..c71ba14 100644
--- a/controller.go
+++ b/controller.go
@@ -253,7 +253,7 @@ func main() {
 		ingressClassFiltersList = strings.Split(ingressClassFilters, ",")
 	}
 
-	kubeAdapter, err = kubernetes.NewAdapter(kubeConfig, ingressClassFiltersList, awsAdapter.SecurityGroupID())
+	kubeAdapter, err = kubernetes.NewAdapter(kubeConfig, ingressClassFiltersList, awsAdapter.SecurityGroupID(), sslPolicy)
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/kubernetes/adapter.go b/kubernetes/adapter.go
index ac3c9b2..03a1d29 100644
--- a/kubernetes/adapter.go
+++ b/kubernetes/adapter.go
@@ -12,6 +12,7 @@ type Adapter struct {
 	kubeClient                  client
 	ingressFilters              []string
 	ingressDefaultSecurityGroup string
+	ingressDefaultSSLPolicy     string
 }
 
 var (
@@ -44,6 +45,7 @@ type Ingress struct {
 	Hostnames      []string
 	Shared         bool
 	SecurityGroup  string
+	SSLPolicy      string
 }
 
 // String returns a string representation of the Ingress instance containing the namespace and the resource name.
@@ -52,7 +54,7 @@ func (i *Ingress) String() string {
 }
 
 // NewAdapter creates an Adapter for Kubernetes using a given configuration.
-func NewAdapter(config *Config, ingressClassFilters []string, ingressDefaultSecurityGroup string) (*Adapter, error) {
+func NewAdapter(config *Config, ingressClassFilters []string, ingressDefaultSecurityGroup, ingressDefaultSSLPolicy string) (*Adapter, error) {
 	if config == nil || config.BaseURL == "" {
 		return nil, ErrInvalidConfiguration
 	}
@@ -60,7 +62,12 @@ func NewAdapter(config *Config, ingressClassFilters []string, ingressDefaultSecu
 	if err != nil {
 		return nil, err
 	}
-	return &Adapter{kubeClient: c, ingressFilters: ingressClassFilters, ingressDefaultSecurityGroup: ingressDefaultSecurityGroup}, nil
+	return &Adapter{
+		kubeClient:                  c,
+		ingressFilters:              ingressClassFilters,
+		ingressDefaultSecurityGroup: ingressDefaultSecurityGroup,
+		ingressDefaultSSLPolicy:     ingressDefaultSSLPolicy,
+	}, nil
 }
 
 func (a *Adapter) newIngressFromKube(kubeIngress *ingress) *Ingress {
@@ -101,6 +108,7 @@ func (a *Adapter) newIngressFromKube(kubeIngress *ingress) *Ingress {
 		Hostnames:      hostnames,
 		Shared:         shared,
 		SecurityGroup:  kubeIngress.getAnnotationsString(ingressSecurityGroupAnnotation, a.ingressDefaultSecurityGroup),
+		SSLPolicy:      kubeIngress.getAnnotationsString(ingressSSLPolicyAnnotation, a.ingressDefaultSSLPolicy),
 	}
 }
 
@@ -120,6 +128,7 @@ func newIngressForKube(i *Ingress) *ingress {
 				ingressSchemeAnnotation:         i.Scheme,
 				ingressSharedAnnotation:         shared,
 				ingressSecurityGroupAnnotation:  i.SecurityGroup,
+				ingressSSLPolicyAnnotation:      i.SSLPolicy,
 			},
 		},
 		Status: ingressStatus{
diff --git a/kubernetes/ingress.go b/kubernetes/ingress.go
index 5260722..04d7100 100644
--- a/kubernetes/ingress.go
+++ b/kubernetes/ingress.go
@@ -65,6 +65,7 @@ const (
 	ingressSchemeAnnotation         = "zalando.org/aws-load-balancer-scheme"
 	ingressSharedAnnotation         = "zalando.org/aws-load-balancer-shared"
 	ingressSecurityGroupAnnotation  = "zalando.org/aws-load-balancer-security-group"
+	ingressSSLPolicyAnnotation      = "zalando.org/aws-load-balancer-ssl-policy"
 	ingressClassAnnotation          = "kubernetes.io/ingress.class"
 )
 
diff --git a/worker.go b/worker.go
index 1a62738..991a265 100644
--- a/worker.go
+++ b/worker.go
@@ -26,6 +26,7 @@ type loadBalancer struct {
 	stack         *aws.Stack
 	shared        bool
 	securityGroup string
+	sslPolicy     string
 	certTTL       time.Duration
 }
 
@@ -65,7 +66,7 @@ func (l *loadBalancer) inSync() bool {
 // adding can fail in case the load balancer reached its limit of ingress
 // certificates or if the scheme doesn't match.
 func (l *loadBalancer) AddIngress(certificateARNs []string, ingress *kubernetes.Ingress, maxCerts int) bool {
-	if l.scheme != ingress.Scheme || l.securityGroup != ingress.SecurityGroup {
+	if l.scheme != ingress.Scheme || l.securityGroup != ingress.SecurityGroup || l.sslPolicy != ingress.SSLPolicy {
 		return false
 	}
 
@@ -282,6 +283,7 @@ func getAllLoadBalancers(certTTL time.Duration, stacks []*aws.Stack) []*loadBala
 			scheme:        stack.Scheme,
 			shared:        stack.OwnerIngress == "",
 			securityGroup: stack.SecurityGroup,
+			sslPolicy:     stack.SSLPolicy,
 			certTTL:       certTTL,
 		}
 		// initialize ingresses map with existing certificates from the
@@ -338,6 +340,7 @@ func matchIngressesToLoadBalancers(loadBalancers []*loadBalancer, certs Certific
 					scheme:        ingress.Scheme,
 					shared:        ingress.Shared,
 					securityGroup: ingress.SecurityGroup,
+					sslPolicy:     ingress.SSLPolicy,
 				},
 			)
 		}
@@ -362,7 +365,7 @@ func createStack(awsAdapter *aws.Adapter, lb *loadBalancer) {
 
 	log.Infof("creating stack for certificates %q / ingress %q", certificates, lb.ingresses)
 
-	stackId, err := awsAdapter.CreateStack(certificates, lb.scheme, lb.securityGroup, lb.Owner())
+	stackId, err := awsAdapter.CreateStack(certificates, lb.scheme, lb.securityGroup, lb.Owner(), lb.sslPolicy)
 	if err != nil {
 		if isAlreadyExistsError(err) {
 			lb.stack, err = awsAdapter.GetStack(stackId)
@@ -381,7 +384,7 @@ func updateStack(awsAdapter *aws.Adapter, lb *loadBalancer) {
 
 	log.Infof("updating %q stack for %d certificates / %d ingresses", lb.scheme, len(certificates), len(lb.ingresses))
 
-	stackId, err := awsAdapter.UpdateStack(lb.stack.Name, certificates, lb.scheme)
+	stackId, err := awsAdapter.UpdateStack(lb.stack.Name, certificates, lb.scheme, lb.sslPolicy)
 	if isNoUpdatesToBePerformedError(err) {
 		log.Debugf("stack(%q) is already up to date", certificates)
 	} else if err != nil {


Test patch:
diff --git a/kubernetes/adapter_test.go b/kubernetes/adapter_test.go
index 2a43cca..0dcd235 100644
--- a/kubernetes/adapter_test.go
+++ b/kubernetes/adapter_test.go
@@ -12,10 +12,13 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-var testConfig = InsecureConfig("dummy-url")
-var testIngressFilter = []string{"skipper"}
-var testIngressDefaultSecurityGroup = "sg-foobar"
-var testSecurityGroup = "sg-123456"
+var (
+	testConfig                      = InsecureConfig("dummy-url")
+	testIngressFilter               = []string{"skipper"}
+	testIngressDefaultSecurityGroup = "sg-foobar"
+	testSecurityGroup               = "sg-123456"
+	testSSLPolicy                   = "ELBSecurityPolicy-TLS-1-2-2017-01"
+)
 
 func TestMappingRoundtrip(tt *testing.T) {
 	for _, tc := range []struct {
@@ -34,6 +37,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 				Shared:         true,
 				Hostnames:      []string{"domain.example.org"},
 				SecurityGroup:  testSecurityGroup,
+				SSLPolicy:      testSSLPolicy,
 			},
 			kubeIngress: &ingress{
 				Metadata: ingressItemMetadata{
@@ -44,6 +48,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 						ingressSchemeAnnotation:         "internal",
 						ingressSharedAnnotation:         "true",
 						ingressSecurityGroupAnnotation:  testSecurityGroup,
+						ingressSSLPolicyAnnotation:      testSSLPolicy,
 					},
 				},
 				Spec: ingressSpec{
@@ -73,6 +78,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 				CertificateARN: "zbr",
 				Shared:         false,
 				SecurityGroup:  testSecurityGroup,
+				SSLPolicy:      testSSLPolicy,
 			},
 			kubeIngress: &ingress{
 				Metadata: ingressItemMetadata{
@@ -83,6 +89,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 						ingressSchemeAnnotation:         "internal",
 						ingressSharedAnnotation:         "false",
 						ingressSecurityGroupAnnotation:  testSecurityGroup,
+						ingressSSLPolicyAnnotation:      testSSLPolicy,
 					},
 				},
 				Status: ingressStatus{
@@ -97,7 +104,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 		},
 	} {
 		tt.Run(tc.msg, func(t *testing.T) {
-			a, err := NewAdapter(testConfig, testIngressFilter, testIngressDefaultSecurityGroup)
+			a, err := NewAdapter(testConfig, testIngressFilter, testIngressDefaultSecurityGroup, testSSLPolicy)
 			if err != nil {
 				t.Fatalf("cannot create kubernetes adapter: %v", err)
 			}
@@ -153,7 +160,7 @@ func (c *mockClient) patch(res string, payload []byte) (io.ReadCloser, error) {
 }
 
 func TestListIngress(t *testing.T) {
-	a, _ := NewAdapter(testConfig, testIngressFilter, testIngressDefaultSecurityGroup)
+	a, _ := NewAdapter(testConfig, testIngressFilter, testIngressDefaultSecurityGroup, testSSLPolicy)
 	client := &mockClient{}
 	a.kubeClient = client
 	ingresses, err := a.ListIngress()
@@ -171,7 +178,7 @@ func TestListIngress(t *testing.T) {
 }
 
 func TestUpdateIngressLoadBalancer(t *testing.T) {
-	a, _ := NewAdapter(testConfig, testIngressFilter, testSecurityGroup)
+	a, _ := NewAdapter(testConfig, testIngressFilter, testSecurityGroup, testSSLPolicy)
 	client := &mockClient{}
 	a.kubeClient = client
 	ing := &Ingress{
@@ -206,7 +213,7 @@ func TestBrokenConfig(t *testing.T) {
 		{"broken-cert", &Config{BaseURL: "dontcare", TLSClientConfig: TLSClientConfig{CAFile: "testdata/broken.pem"}}},
 	} {
 		t.Run(fmt.Sprintf("%v", test.cfg), func(t *testing.T) {
-			_, err := NewAdapter(test.cfg, testIngressFilter, testSecurityGroup)
+			_, err := NewAdapter(test.cfg, testIngressFilter, testSecurityGroup, testSSLPolicy)
 			if err == nil {
 				t.Error("expected an error")
 			}