Repository: zalando-incubator/kube-ingress-aws-controller
Pull request #318 (2020-03-24 13:46:02)

Add per ingress WAF support
Add the ability to set a 'WAF Id' in ingress configuration as an annotation #238

WAF ACL id is set in annotations like below:
```zalando.org/aws-load-balancer-waf-id: "foobar"```

This would create a new loadbalancer and associate WAF if there is no loadbalancer with the same waf attached.


Patch:
diff --git a/aws/adapter.go b/aws/adapter.go
index c4bf6bf..b523a67 100644
--- a/aws/adapter.go
+++ b/aws/adapter.go
@@ -98,8 +98,10 @@ const (
 	DefaultAlbS3LogsBucket = ""
 	// DefaultAlbS3LogsPrefix is a blank string, and optionally set if desired
 	DefaultAlbS3LogsPrefix = ""
-	DefaultWafWebAclId     = ""
-	DefaultCustomFilter    = ""
+	// DefaultWafWebAclId is a blank string, set if desired.
+	DefaultWafWebAclId = ""
+
+	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
@@ -477,7 +479,7 @@ 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, sslPolicy, ipAddressType string, cwAlarms CloudWatchAlarmList, loadBalancerType string, http2 bool) (string, error) {
+func (a *Adapter) CreateStack(certificateARNs []string, scheme, securityGroup, owner, sslPolicy, ipAddressType, wafWebACLId string, cwAlarms CloudWatchAlarmList, loadBalancerType string, http2 bool) (string, error) {
 	certARNs := make(map[string]time.Time, len(certificateARNs))
 	for _, arn := range certificateARNs {
 		certARNs[arn] = time.Time{}
@@ -487,6 +489,10 @@ func (a *Adapter) CreateStack(certificateARNs []string, scheme, securityGroup, o
 		sslPolicy = a.sslPolicy
 	}
 
+	if wafWebACLId == "" {
+		wafWebACLId = a.wafWebAclId
+	}
+
 	if _, ok := SSLPolicies[sslPolicy]; !ok {
 		return "", fmt.Errorf("invalid SSLPolicy '%s' defined", sslPolicy)
 	}
@@ -515,7 +521,7 @@ func (a *Adapter) CreateStack(certificateARNs []string, scheme, securityGroup, o
 		loadbalancerType:             loadBalancerType,
 		albLogsS3Bucket:              a.albLogsS3Bucket,
 		albLogsS3Prefix:              a.albLogsS3Prefix,
-		wafWebAclId:                  a.wafWebAclId,
+		wafWebAclId:                  wafWebACLId,
 		cwAlarms:                     cwAlarms,
 		httpRedirectToHTTPS:          a.httpRedirectToHTTPS,
 		nlbCrossZone:                 a.nlbCrossZone,
@@ -526,11 +532,15 @@ 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, sslPolicy, ipAddressType string, cwAlarms CloudWatchAlarmList, loadBalancerType string, http2 bool) (string, error) {
+func (a *Adapter) UpdateStack(stackName string, certificateARNs map[string]time.Time, scheme, sslPolicy, ipAddressType, wafWebACLId string, cwAlarms CloudWatchAlarmList, loadBalancerType string, http2 bool) (string, error) {
 	if _, ok := SSLPolicies[sslPolicy]; !ok {
 		return "", fmt.Errorf("invalid SSLPolicy '%s' defined", sslPolicy)
 	}
 
+	if wafWebACLId == "" {
+		wafWebACLId = a.wafWebAclId
+	}
+
 	spec := &stackSpec{
 		name:            stackName,
 		scheme:          scheme,
@@ -554,7 +564,7 @@ func (a *Adapter) UpdateStack(stackName string, certificateARNs map[string]time.
 		loadbalancerType:             loadBalancerType,
 		albLogsS3Bucket:              a.albLogsS3Bucket,
 		albLogsS3Prefix:              a.albLogsS3Prefix,
-		wafWebAclId:                  a.wafWebAclId,
+		wafWebAclId:                  wafWebACLId,
 		cwAlarms:                     cwAlarms,
 		httpRedirectToHTTPS:          a.httpRedirectToHTTPS,
 		nlbCrossZone:                 a.nlbCrossZone,
diff --git a/aws/cf.go b/aws/cf.go
index f75b63c..393a293 100644
--- a/aws/cf.go
+++ b/aws/cf.go
@@ -31,6 +31,7 @@ type Stack struct {
 	OwnerIngress      string
 	CWAlarmConfigHash string
 	TargetGroupARN    string
+	WafWebACLId       string
 	CertificateARNs   map[string]time.Time
 	tags              map[string]string
 }
@@ -113,6 +114,7 @@ const (
 	parameterListenerSslPolicyParameter              = "ListenerSslPolicyParameter"
 	parameterIpAddressTypeParameter                  = "IpAddressType"
 	parameterLoadBalancerTypeParameter               = "Type"
+	parameterLoadBalancerWafWebACLIdParameter        = "LoadBalancerWafWebACLIdParameter"
 	parameterHTTP2Parameter                          = "HTTP2"
 )
 
@@ -170,6 +172,7 @@ func createStack(svc cloudformationiface.CloudFormationAPI, spec *stackSpec) (st
 			cfParam(parameterIpAddressTypeParameter, spec.ipAddressType),
 			cfParam(parameterLoadBalancerTypeParameter, spec.loadbalancerType),
 			cfParam(parameterHTTP2Parameter, fmt.Sprintf("%t", spec.http2)),
+			cfParam(parameterLoadBalancerWafWebACLIdParameter, spec.wafWebAclId),
 		},
 		Tags: []*cloudformation.Tag{
 			cfTag(kubernetesCreatorTag, spec.controllerID),
@@ -226,6 +229,7 @@ func updateStack(svc cloudformationiface.CloudFormationAPI, spec *stackSpec) (st
 			cfParam(parameterIpAddressTypeParameter, spec.ipAddressType),
 			cfParam(parameterLoadBalancerTypeParameter, spec.loadbalancerType),
 			cfParam(parameterHTTP2Parameter, fmt.Sprintf("%t", spec.http2)),
+			cfParam(parameterLoadBalancerWafWebACLIdParameter, spec.wafWebAclId),
 		},
 		Tags: []*cloudformation.Tag{
 			cfTag(kubernetesCreatorTag, spec.controllerID),
@@ -384,6 +388,7 @@ func mapToManagedStack(stack *cloudformation.Stack) *Stack {
 		OwnerIngress:      ownerIngress,
 		status:            aws.StringValue(stack.StackStatus),
 		CWAlarmConfigHash: tags[cwAlarmConfigHashTag],
+		WafWebACLId:       parameters[parameterLoadBalancerWafWebACLIdParameter],
 	}
 }
 
diff --git a/kubernetes/adapter.go b/kubernetes/adapter.go
index d7a4b05..4eef262 100644
--- a/kubernetes/adapter.go
+++ b/kubernetes/adapter.go
@@ -89,6 +89,7 @@ type Ingress struct {
 	SSLPolicy        string
 	IPAddressType    string
 	LoadBalancerType string
+	WafWebACLId      string
 	Hostnames        []string
 	resourceType     ingressType
 }
@@ -240,6 +241,7 @@ func (a *Adapter) parseAnnotations(annotations map[string]string) *Ingress {
 		SSLPolicy:        sslPolicy,
 		IPAddressType:    ipAddressType,
 		LoadBalancerType: loadBalancerType,
+		WafWebACLId:      getAnnotationsString(annotations, ingressWafWebACLIdAnnotation, ""),
 		HTTP2:            http2,
 	}
 }
@@ -266,6 +268,7 @@ func newMetadataForKube(i *Ingress) kubeItemMetadata {
 			ingressSecurityGroupAnnotation:    i.SecurityGroup,
 			ingressSSLPolicyAnnotation:        i.SSLPolicy,
 			ingressALBIPAddressType:           i.IPAddressType,
+			ingressWafWebACLIdAnnotation:      i.WafWebACLId,
 			ingressLoadBalancerTypeAnnotation: loadBalancerTypesAWSToIngress[i.LoadBalancerType],
 		},
 	}
diff --git a/kubernetes/ingress.go b/kubernetes/ingress.go
index 1376da6..d51f8df 100644
--- a/kubernetes/ingress.go
+++ b/kubernetes/ingress.go
@@ -70,6 +70,7 @@ const (
 	ingressSSLPolicyAnnotation        = "zalando.org/aws-load-balancer-ssl-policy"
 	ingressLoadBalancerTypeAnnotation = "zalando.org/aws-load-balancer-type"
 	ingressHTTP2Annotation            = "zalando.org/aws-load-balancer-http2"
+	ingressWafWebACLIdAnnotation      = "zalando.org/aws-waf-web-acl-id"
 	ingressClassAnnotation            = "kubernetes.io/ingress.class"
 )
 
diff --git a/worker.go b/worker.go
index 33af4f0..edb7271 100644
--- a/worker.go
+++ b/worker.go
@@ -31,6 +31,7 @@ type loadBalancer struct {
 	securityGroup    string
 	sslPolicy        string
 	ipAddressType    string
+	wafWebACLId      string
 	certTTL          time.Duration
 	cwAlarms         aws.CloudWatchAlarmList
 	loadBalancerType string
@@ -91,7 +92,7 @@ func (l *loadBalancer) AddIngress(certificateARNs []string, ingress *kubernetes.
 		l.securityGroup != ingress.SecurityGroup ||
 		l.sslPolicy != ingress.SSLPolicy ||
 		l.loadBalancerType != ingress.LoadBalancerType ||
-		l.http2 != ingress.HTTP2 {
+		l.http2 != ingress.HTTP2 || l.wafWebACLId != ingress.WafWebACLId {
 		return false
 	}
 
@@ -324,6 +325,7 @@ func getAllLoadBalancers(certTTL time.Duration, stacks []*aws.Stack) []*loadBala
 			loadBalancerType: stack.LoadBalancerType,
 			http2:            stack.HTTP2,
 			certTTL:          certTTL,
+			wafWebACLId:      stack.WafWebACLId,
 		}
 		// initialize ingresses map with existing certificates from the
 		// stack.
@@ -402,6 +404,7 @@ func matchIngressesToLoadBalancers(loadBalancers []*loadBalancer, certs Certific
 					ipAddressType:    ingress.IPAddressType,
 					loadBalancerType: ingress.LoadBalancerType,
 					http2:            ingress.HTTP2,
+					wafWebACLId:      ingress.WafWebACLId,
 				},
 			)
 		}
@@ -440,7 +443,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(), lb.sslPolicy, lb.ipAddressType, lb.cwAlarms, lb.loadBalancerType, lb.http2)
+	stackId, err := awsAdapter.CreateStack(certificates, lb.scheme, lb.securityGroup, lb.Owner(), lb.sslPolicy, lb.ipAddressType, lb.wafWebACLId, lb.cwAlarms, lb.loadBalancerType, lb.http2)
 	if err != nil {
 		if isAlreadyExistsError(err) {
 			lb.stack, err = awsAdapter.GetStack(stackId)
@@ -459,7 +462,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, lb.sslPolicy, lb.ipAddressType, lb.cwAlarms, lb.loadBalancerType, lb.http2)
+	stackId, err := awsAdapter.UpdateStack(lb.stack.Name, certificates, lb.scheme, lb.sslPolicy, lb.ipAddressType, lb.wafWebACLId, lb.cwAlarms, lb.loadBalancerType, lb.http2)
 	if isNoUpdatesToBePerformedError(err) {
 		log.Debugf("stack(%q) is already up to date", certificates)
 	} else if err != nil {


Test patch:
diff --git a/aws/cf_test.go b/aws/cf_test.go
index 4e49540..2ad22e9 100644
--- a/aws/cf_test.go
+++ b/aws/cf_test.go
@@ -63,6 +63,60 @@ func TestCreatingStack(t *testing.T) {
 	}
 }
 
+func TestUpdatingStack(t *testing.T) {
+	for _, ti := range []struct {
+		name         string
+		givenSpec    stackSpec
+		givenOutputs cfMockOutputs
+		want         string
+		wantErr      bool
+	}{
+		{
+			"successful-call",
+			stackSpec{
+				name:            "foo",
+				securityGroupID: "bar",
+				vpcID:           "baz",
+				certificateARNs: map[string]time.Time{
+					"arn-default": time.Time{},
+					"arn-second":  time.Time{},
+				},
+			},
+			cfMockOutputs{updateStack: R(mockUSOutput("fake-stack-id"), nil)},
+			"fake-stack-id",
+			false,
+		},
+		{
+			"successful-call",
+			stackSpec{name: "foo", securityGroupID: "bar", vpcID: "baz"},
+			cfMockOutputs{updateStack: R(mockUSOutput("fake-stack-id"), nil)},
+			"fake-stack-id",
+			false,
+		},
+		{
+			"fail-call",
+			stackSpec{name: "foo", securityGroupID: "bar", vpcID: "baz"},
+			cfMockOutputs{updateStack: R(nil, errDummy)},
+			"fake-stack-id",
+			true,
+		},
+	} {
+		t.Run(ti.name, func(t *testing.T) {
+			c := &mockCloudFormationClient{outputs: ti.givenOutputs}
+			got, err := updateStack(c, &ti.givenSpec)
+			if ti.wantErr {
+				if !ti.wantErr {
+					t.Error("unexpected error", err)
+				}
+			} else {
+				if ti.want != got {
+					t.Errorf("unexpected result. wanted %+v, got %+v", ti.want, got)
+				}
+			}
+		})
+	}
+}
+
 func TestDeleteStack(t *testing.T) {
 	for _, ti := range []struct {
 		msg          string
diff --git a/aws/cfmock_test.go b/aws/cfmock_test.go
index 3c0f8e4..57487ff 100644
--- a/aws/cfmock_test.go
+++ b/aws/cfmock_test.go
@@ -10,6 +10,7 @@ type cfMockOutputs struct {
 	describeStackPages          *apiResponse
 	describeStacks              *apiResponse
 	createStack                 *apiResponse
+	updateStack                 *apiResponse
 	deleteStack                 *apiResponse
 	updateTerminationProtection *apiResponse
 }
@@ -57,6 +58,19 @@ func mockCSOutput(stackId string) *cloudformation.CreateStackOutput {
 	}
 }
 
+func (m *mockCloudFormationClient) UpdateStack(params *cloudformation.UpdateStackInput) (*cloudformation.UpdateStackOutput, error) {
+	if out, ok := m.outputs.updateStack.response.(*cloudformation.UpdateStackOutput); ok {
+		return out, m.outputs.updateStack.err
+	}
+	return nil, m.outputs.updateStack.err
+}
+
+func mockUSOutput(stackId string) *cloudformation.UpdateStackOutput {
+	return &cloudformation.UpdateStackOutput{
+		StackId: aws.String(stackId),
+	}
+}
+
 func (m *mockCloudFormationClient) DeleteStack(params *cloudformation.DeleteStackInput) (*cloudformation.DeleteStackOutput, error) {
 	if out, ok := m.outputs.deleteStack.response.(*cloudformation.DeleteStackOutput); ok {
 		return out, m.outputs.deleteStack.err
diff --git a/kubernetes/adapter_test.go b/kubernetes/adapter_test.go
index 72e8d88..af1cae4 100644
--- a/kubernetes/adapter_test.go
+++ b/kubernetes/adapter_test.go
@@ -24,6 +24,7 @@ var (
 	testIPAddressTypeDefault        = aws.IPAddressTypeIPV4
 	testLoadBalancerTypeIngress     = loadBalancerTypeALB
 	testLoadBalancerTypeAWS         = aws.LoadBalancerTypeApplication
+	testWafWebACLId                 = "zbr-1234"
 )
 
 func TestMappingRoundtrip(tt *testing.T) {
@@ -48,6 +49,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 				IPAddressType:    testIPAddressTypeDefault,
 				LoadBalancerType: testLoadBalancerTypeAWS,
 				resourceType:     ingressTypeIngress,
+				WafWebACLId:      testWafWebACLId,
 			},
 			kubeIngress: &ingress{
 				Metadata: kubeItemMetadata{
@@ -62,6 +64,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 						ingressSSLPolicyAnnotation:        testSSLPolicy,
 						ingressALBIPAddressType:           testIPAddressTypeDefault,
 						ingressLoadBalancerTypeAnnotation: testLoadBalancerTypeIngress,
+						ingressWafWebACLIdAnnotation:      testWafWebACLId,
 					},
 				},
 				Spec: ingressSpec{
@@ -97,6 +100,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 				IPAddressType:    testIPAddressTypeDefault,
 				LoadBalancerType: testLoadBalancerTypeAWS,
 				resourceType:     ingressTypeIngress,
+				WafWebACLId:      testWafWebACLId,
 			},
 			kubeIngress: &ingress{
 				Metadata: kubeItemMetadata{
@@ -111,6 +115,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 						ingressSSLPolicyAnnotation:        testSSLPolicy,
 						ingressALBIPAddressType:           testIPAddressTypeDefault,
 						ingressLoadBalancerTypeAnnotation: testLoadBalancerTypeIngress,
+						ingressWafWebACLIdAnnotation:      testWafWebACLId,
 					},
 				},
 				Spec: ingressSpec{
@@ -146,6 +151,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 				IPAddressType:    testIPAddressTypeDefault,
 				LoadBalancerType: testLoadBalancerTypeAWS,
 				resourceType:     ingressTypeIngress,
+				WafWebACLId:      testWafWebACLId,
 			},
 			kubeIngress: &ingress{
 				Metadata: kubeItemMetadata{
@@ -160,6 +166,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 						ingressSSLPolicyAnnotation:        testSSLPolicy,
 						ingressALBIPAddressType:           testIPAddressTypeDefault,
 						ingressLoadBalancerTypeAnnotation: testLoadBalancerTypeIngress,
+						ingressWafWebACLIdAnnotation:      testWafWebACLId,
 					},
 				},
 				Status: ingressStatus{
@@ -188,6 +195,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 				IPAddressType:    testIPAddressTypeDualStack,
 				LoadBalancerType: testLoadBalancerTypeAWS,
 				resourceType:     ingressTypeIngress,
+				WafWebACLId:      testWafWebACLId,
 			},
 			kubeIngress: &ingress{
 				Metadata: kubeItemMetadata{
@@ -202,6 +210,7 @@ func TestMappingRoundtrip(tt *testing.T) {
 						ingressSSLPolicyAnnotation:        testSSLPolicy,
 						ingressALBIPAddressType:           testIPAddressTypeDualStack,
 						ingressLoadBalancerTypeAnnotation: testLoadBalancerTypeIngress,
+						ingressWafWebACLIdAnnotation:      testWafWebACLId,
 					},
 				},
 				Status: ingressStatus{
diff --git a/worker_test.go b/worker_test.go
index 5afc6f6..a2ee12d 100644
--- a/worker_test.go
+++ b/worker_test.go
@@ -118,6 +118,26 @@ func TestAddIngress(tt *testing.T) {
 			maxCerts: 1,
 			added:    false,
 		},
+		{
+			name: "wafacl id not matching",
+			loadBalancer: &loadBalancer{
+				wafWebACLId: "WAFZXX",
+			},
+			ingress: &kubernetes.Ingress{
+				WafWebACLId: "WAFZXC",
+			},
+			added: false,
+		},
+		{
+			name: "wafacl id not matching",
+			loadBalancer: &loadBalancer{
+				wafWebACLId: aws.DefaultWafWebAclId,
+			},
+			ingress: &kubernetes.Ingress{
+				WafWebACLId: "WAFZXC",
+			},
+			added: false,
+		},
 	} {
 		tt.Run(test.name, func(t *testing.T) {
 			assert.Equal(t, test.loadBalancer.AddIngress(test.certificateARNs, test.ingress, test.maxCerts), test.added)