Repository: zalando-incubator/kube-ingress-aws-controller
Pull request #649 (2023-10-18 12:46:00)

Make Resource conversion tests also check params and tags
This changes should fix the current behavior of golden file tests that do not really check important parameters and only check the correctness of the template, which is not really testing the stack generation algorithm of the ingress controller.

https://github.com/zalando-incubator/kube-ingress-aws-controller/issues/626


Patch:
diff --git a/aws/fake/cf.go b/aws/fake/cf.go
index 3b367f7..8c269de 100644
--- a/aws/fake/cf.go
+++ b/aws/fake/cf.go
@@ -17,12 +17,22 @@ type CFOutputs struct {
 
 type CFClient struct {
 	cloudformationiface.CloudFormationAPI
-	lastGeneratedTemplate string
-	Outputs               CFOutputs
+	lastStackTemplate string
+	lastStackParams   []*cloudformation.Parameter
+	lastStackTags     []*cloudformation.Tag
+	Outputs           CFOutputs
 }
 
-func (m *CFClient) GetLastGeneratedTemplate() string {
-	return m.lastGeneratedTemplate
+func (m *CFClient) GetLastStackTemplate() string {
+	return m.lastStackTemplate
+}
+
+func (m *CFClient) GetLastStackParams() []*cloudformation.Parameter {
+	return m.lastStackParams
+}
+
+func (m *CFClient) GetLastStackTags() []*cloudformation.Tag {
+	return m.lastStackTags
 }
 
 func (m *CFClient) DescribeStacksPages(in *cloudformation.DescribeStacksInput, fn func(*cloudformation.DescribeStacksOutput, bool) bool) (err error) {
@@ -52,7 +62,10 @@ func (m *CFClient) DescribeStacks(in *cloudformation.DescribeStacksInput) (*clou
 }
 
 func (m *CFClient) CreateStack(params *cloudformation.CreateStackInput) (*cloudformation.CreateStackOutput, error) {
-	m.lastGeneratedTemplate = *params.TemplateBody
+	m.lastStackTags = params.Tags
+	m.lastStackParams = params.Parameters
+	m.lastStackTemplate = *params.TemplateBody
+
 	out, ok := m.Outputs.CreateStack.response.(*cloudformation.CreateStackOutput)
 	if !ok {
 		return nil, m.Outputs.CreateStack.err
@@ -67,7 +80,10 @@ func MockCSOutput(stackId string) *cloudformation.CreateStackOutput {
 }
 
 func (m *CFClient) UpdateStack(params *cloudformation.UpdateStackInput) (*cloudformation.UpdateStackOutput, error) {
-	m.lastGeneratedTemplate = *params.TemplateBody
+	m.lastStackTags = params.Tags
+	m.lastStackParams = params.Parameters
+	m.lastStackTemplate = *params.TemplateBody
+
 	out, ok := m.Outputs.UpdateStack.response.(*cloudformation.UpdateStackOutput)
 	if !ok {
 		return nil, m.Outputs.UpdateStack.err
diff --git a/go.mod b/go.mod
index 6ffd278..457ab93 100644
--- a/go.mod
+++ b/go.mod
@@ -69,12 +69,12 @@ require (
 	github.com/tidwall/pretty v1.2.1 // indirect
 	go.uber.org/atomic v1.10.0 // indirect
 	go4.org/netipx v0.0.0-20230125063823-8449b0a6169f // indirect
-	golang.org/x/crypto v0.14.0 // indirect
-	golang.org/x/net v0.17.0 // indirect
+	golang.org/x/crypto v0.12.0 // indirect
+	golang.org/x/net v0.14.0 // indirect
 	golang.org/x/oauth2 v0.11.0 // indirect
-	golang.org/x/sys v0.13.0 // indirect
-	golang.org/x/term v0.13.0 // indirect
-	golang.org/x/text v0.13.0 // indirect
+	golang.org/x/sys v0.11.0 // indirect
+	golang.org/x/term v0.11.0 // indirect
+	golang.org/x/text v0.12.0 // indirect
 	golang.org/x/time v0.3.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/protobuf v1.31.0 // indirect
diff --git a/go.sum b/go.sum
index 94c9e4a..2ad083a 100644
--- a/go.sum
+++ b/go.sum
@@ -299,8 +299,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
-golang.org/x/crypto v0.14.0 h1:wBqGXzWJW6m1XrIKlAH0Hs1JJ7+9KBwnIO8v66Q9cHc=
-golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4=
+golang.org/x/crypto v0.12.0 h1:tFM/ta59kqch6LlvYnPa0yx5a83cL2nHflFhYKvv9Yk=
+golang.org/x/crypto v0.12.0/go.mod h1:NF0Gs7EO5K4qLn+Ylc+fih8BSTeIjAP05siRnAh98yw=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea h1:vLCWI/yYrdEHyN2JzIzPO3aaQJHQdp89IZBA/+azVC4=
 golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w=
@@ -327,8 +327,8 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v
 golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
 golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
 golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco=
-golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM=
-golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE=
+golang.org/x/net v0.14.0 h1:BONx9s002vGdD9umnlX1Po8vOZmrgH34qlHcD1MfK14=
+golang.org/x/net v0.14.0/go.mod h1:PpSgVXXLK0OxS0F31C1/tv6XNguvCrnXIDrFMspZIUI=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.11.0 h1:vPL4xzxBM4niKCW6g9whtaWVXTJf1U5e4aZxxFx/gbU=
 golang.org/x/oauth2 v0.11.0/go.mod h1:LdF7O/8bLR/qWK9DrpXmbHLTouvRHK0SgJl0GmDBchk=
@@ -355,20 +355,20 @@ golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE=
-golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM=
+golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
-golang.org/x/term v0.13.0 h1:bb+I9cTfFazGW51MZqBVmZy7+JEJMouUHTUSKVQLBek=
-golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U=
+golang.org/x/term v0.11.0 h1:F9tnn/DA/Im8nCwm+fX+1/eBwi4qFjRT++MhtVC4ZX0=
+golang.org/x/term v0.11.0/go.mod h1:zC9APTIj3jG3FdV/Ons+XE1riIZXG4aZ4GTHiPZJPIU=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
 golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
-golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k=
-golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
+golang.org/x/text v0.12.0 h1:k+n5B8goJNdU7hSvEtMUz3d1Q6D/XW4COJSJR6fN0mc=
+golang.org/x/text v0.12.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
 golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4=
 golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=


Test patch:
diff --git a/testdata/simple_alb/params.json b/testdata/simple_alb/params.json
new file mode 100644
index 0000000..c1ecf89
--- /dev/null
+++ b/testdata/simple_alb/params.json
@@ -0,0 +1,54 @@
+[
+  {
+    "parameterKey": "LoadBalancerSchemeParameter",
+    "parameterValue": "internet-facing"
+  },
+  {
+    "parameterKey": "LoadBalancerSecurityGroupParameter",
+    "parameterValue": "42"
+  },
+  {
+    "parameterKey": "LoadBalancerSubnetsParameter",
+    "parameterValue": "foo1"
+  },
+  {
+    "parameterKey": "TargetGroupVPCIDParameter",
+    "parameterValue": "1"
+  },
+  {
+    "parameterKey": "TargetGroupTargetPortParameter",
+    "parameterValue": "0"
+  },
+  {
+    "parameterKey": "ListenerSslPolicyParameter",
+    "parameterValue": "ELBSecurityPolicy-2016-08"
+  },
+  {
+    "parameterKey": "IpAddressType",
+    "parameterValue": "ipv4"
+  },
+  {
+    "parameterKey": "Type",
+    "parameterValue": "application"
+  },
+  {
+    "parameterKey": "HTTP2",
+    "parameterValue": "true"
+  },
+  {
+    "parameterKey": "TargetGroupHealthCheckPathParameter",
+    "parameterValue": ""
+  },
+  {
+    "parameterKey": "TargetGroupHealthCheckPortParameter",
+    "parameterValue": "0"
+  },
+  {
+    "parameterKey": "TargetGroupHealthCheckIntervalParameter",
+    "parameterValue": "0"
+  },
+  {
+    "parameterKey": "TargetGroupHealthCheckTimeoutParameter",
+    "parameterValue": "0"
+  }
+]
diff --git a/testdata/simple_alb/tags.json b/testdata/simple_alb/tags.json
new file mode 100644
index 0000000..d64eed4
--- /dev/null
+++ b/testdata/simple_alb/tags.json
@@ -0,0 +1,12 @@
+[
+  {
+    "key": "kubernetes:application",
+    "value": ""
+  },{
+    "key": "kubernetes.io/cluster/aws:123:eu-central-1:kube-1",
+    "value": "owned"
+  },{
+    "key": "ingress:certificate-arn/DUMMY",
+    "value": "0001-01-01T00:00:00Z"
+  }
+]
diff --git a/testdata/simple_alb/expected.cf b/testdata/simple_alb/template.cf
similarity index 100%
rename from testdata/simple_alb/expected.cf
rename to testdata/simple_alb/template.cf
diff --git a/testdata/simple_nlb/params.json b/testdata/simple_nlb/params.json
new file mode 100644
index 0000000..adec04c
--- /dev/null
+++ b/testdata/simple_nlb/params.json
@@ -0,0 +1,54 @@
+[
+  {
+    "parameterKey": "LoadBalancerSchemeParameter",
+    "parameterValue": "internet-facing"
+  },
+  {
+    "parameterKey": "LoadBalancerSecurityGroupParameter",
+    "parameterValue": "42"
+  },
+  {
+    "parameterKey": "LoadBalancerSubnetsParameter",
+    "parameterValue": "foo1"
+  },
+  {
+    "parameterKey": "TargetGroupVPCIDParameter",
+    "parameterValue": "1"
+  },
+  {
+    "parameterKey": "TargetGroupTargetPortParameter",
+    "parameterValue": "0"
+  },
+  {
+    "parameterKey": "ListenerSslPolicyParameter",
+    "parameterValue": "ELBSecurityPolicy-2016-08"
+  },
+  {
+    "parameterKey": "IpAddressType",
+    "parameterValue": "ipv4"
+  },
+  {
+    "parameterKey": "Type",
+    "parameterValue": "network"
+  },
+  {
+    "parameterKey": "HTTP2",
+    "parameterValue": "true"
+  },
+  {
+    "parameterKey": "TargetGroupHealthCheckPathParameter",
+    "parameterValue": ""
+  },
+  {
+    "parameterKey": "TargetGroupHealthCheckPortParameter",
+    "parameterValue": "0"
+  },
+  {
+    "parameterKey": "TargetGroupHealthCheckIntervalParameter",
+    "parameterValue": "0"
+  },
+  {
+    "parameterKey": "TargetGroupHealthCheckTimeoutParameter",
+    "parameterValue": "0"
+  }
+]
diff --git a/testdata/simple_nlb/tags.json b/testdata/simple_nlb/tags.json
new file mode 100644
index 0000000..d64eed4
--- /dev/null
+++ b/testdata/simple_nlb/tags.json
@@ -0,0 +1,12 @@
+[
+  {
+    "key": "kubernetes:application",
+    "value": ""
+  },{
+    "key": "kubernetes.io/cluster/aws:123:eu-central-1:kube-1",
+    "value": "owned"
+  },{
+    "key": "ingress:certificate-arn/DUMMY",
+    "value": "0001-01-01T00:00:00Z"
+  }
+]
diff --git a/testdata/simple_nlb/expected.cf b/testdata/simple_nlb/template.cf
similarity index 100%
rename from testdata/simple_nlb/expected.cf
rename to testdata/simple_nlb/template.cf
diff --git a/worker_test.go b/worker_test.go
index dccda0f..d4cf4ef 100644
--- a/worker_test.go
+++ b/worker_test.go
@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"crypto/x509"
+	"encoding/json"
 	"net/http/httptest"
 	"os"
 	"reflect"
@@ -11,6 +12,7 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go/service/autoscaling"
+	cf "github.com/aws/aws-sdk-go/service/cloudformation"
 	cloudformation "github.com/mweagle/go-cloudformation"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
@@ -148,11 +150,28 @@ func TestResourceConversion(tt *testing.T) {
 		},
 	} {
 		tt.Run(scenario.name, func(t *testing.T) {
-			b, err := os.ReadFile("./testdata/" + scenario.name + "/expected.cf")
+			readFile := func(fileName string) []byte {
+				b, err := os.ReadFile("./testdata/" + scenario.name + "/" + fileName)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				return b
+			}
+
+			template := string(readFile("template.cf"))
+
+			var tags []*cf.Tag
+			err := json.Unmarshal(readFile("tags.json"), &tags)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var params []*cf.Parameter
+			err = json.Unmarshal(readFile("params.json"), &params)
 			if err != nil {
 				t.Fatal(err)
 			}
-			expected := string(b)
 
 			clientEC2 := &fake.EC2Client{Outputs: scenario.responsesEC2}
 			clientASG := &fake.ASGClient{Outputs: scenario.responsesASG}
@@ -210,10 +229,29 @@ func TestResourceConversion(tt *testing.T) {
 				t.Error(problems.Errors())
 			}
 
+			// When a stack is created using cloud formation API the stack information is sent in a split way.
+			// There is a template with the description of the stack, but this template references parameters
+			// and tags that are not defined in this template. These parameters and tags are sent as different
+			// fields in the request.
+			// That is why when we validate the content of the template we also need to check the parameters
+			// and tags and this is why this check is split in three parts, we check the template,
+			// the parameters and tags generated by the ingress controller and not only the template.
 			assert.Equal(
 				t,
-				expected,
-				clientCF.GetLastGeneratedTemplate(),
+				template,
+				clientCF.GetLastStackTemplate(),
+			)
+
+			assert.ElementsMatch(
+				t,
+				tags,
+				clientCF.GetLastStackTags(),
+			)
+
+			assert.ElementsMatch(
+				t,
+				params,
+				clientCF.GetLastStackParams(),
 			)
 		})
 	}