Backport of [NET-9420] clean up acl role policy for apigw into releas…

…e/1.4.x (#4072)

* backport of commit 3ae87ff

* small refactor, added tests

* refactor remove functionality

* updating acceptance tests

* fix helm value for test

* changelog

* fix set logger call

---------

Co-authored-by: jm96441n <[email protected]>

.changelog/4060.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+api-gateway: fix issue where API Gateway specific acl roles/policy were not being cleaned up on deletion of an api-gateway
+```

acceptance/framework/environment/environment.go

@@ -5,6 +5,8 @@ package environment
 
 import (
 	"fmt"
+
+	"github.com/go-logr/logr"
 	"github.com/gruntwork-io/terratest/modules/k8s"
 	"github.com/hashicorp/consul-k8s/acceptance/framework/config"
 	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
@@ -15,6 +17,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 	gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
 )
@@ -187,6 +190,7 @@ func (k kubernetesContext) ControllerRuntimeClient(t testutil.TestingTB) client.
 
 	client, err := client.New(config, client.Options{Scheme: s})
 	require.NoError(t, err)
+	logf.SetLogger(logr.New(nil))
 
 	k.runtimeClient = client
 

acceptance/go.mod

@@ -50,6 +50,7 @@ require (
 	github.com/go-jose/go-jose/v3 v3.0.3 // indirect
 	github.com/go-logr/logr v1.2.4 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
+	github.com/go-logr/zapr v1.2.4 // indirect
 	github.com/go-openapi/analysis v0.21.4 // indirect
 	github.com/go-openapi/errors v0.20.3 // indirect
 	github.com/go-openapi/jsonpointer v0.19.6 // indirect
@@ -121,6 +122,8 @@ require (
 	go.opentelemetry.io/otel/metric v1.19.0 // indirect
 	go.opentelemetry.io/otel/sdk v1.19.0 // indirect
 	go.opentelemetry.io/otel/trace v1.19.0 // indirect
+	go.uber.org/multierr v1.11.0 // indirect
+	go.uber.org/zap v1.25.0 // indirect
 	golang.org/x/crypto v0.22.0 // indirect
 	golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 // indirect
 	golang.org/x/mod v0.14.0 // indirect

acceptance/go.sum

@@ -20,6 +20,9 @@ github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl
 github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
 github.com/aws/aws-sdk-go v1.44.262 h1:gyXpcJptWoNkK+DiAiaBltlreoWKQXjAIh6FRh60F+I=
 github.com/aws/aws-sdk-go v1.44.262/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
+github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
+github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A=
+github.com/benbjohnson/clock v1.3.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
@@ -454,8 +457,14 @@ go.opentelemetry.io/otel/trace v1.19.0 h1:DFVQmlVbfVeOuBRrwdtaehRrWiL1JoVs9CPIQ1
 go.opentelemetry.io/otel/trace v1.19.0/go.mod h1:mfaSyvGyEJEI0nyV2I4qhNQnbBOUUmYZpYojqMnX2vo=
 go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lIVU/I=
 go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM=
+go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
+go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ=
+go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A=
+go.uber.org/goleak v1.2.1/go.mod h1:qlT2yGI9QafXHhZZLxlSuNsMw3FFLxBr+tBRlmO1xH4=
+go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
 go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
 go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
+go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg=
 go.uber.org/zap v1.25.0 h1:4Hvk6GtkucQ790dqmj7l1eEnRdKm3k3ZUrUMS2d5+5c=
 go.uber.org/zap v1.25.0/go.mod h1:JIAUzQIH94IC4fOJQm7gMmBJP5k7wQfdcnYdPoEXJYk=
 golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
@@ -472,6 +481,7 @@ golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30=
 golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M=
 golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 h1:m64FZMko/V45gv0bNmrNYoDEq8U5YUhetc9cBWKS1TQ=
 golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63/go.mod h1:0v4NqG35kSWCMzLaMeX+IQrlSnVE/bqGSyC2cz/9Le8=
+golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
@@ -574,6 +584,7 @@ golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
 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=
+golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 golang.org/x/tools v0.0.0-20190329151228-23e29df326fe/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 golang.org/x/tools v0.0.0-20190416151739-9c9e1878f421/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
 golang.org/x/tools v0.0.0-20190420181800-aa740d480789/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
@@ -582,6 +593,7 @@ golang.org/x/tools v0.0.0-20190907020128-2ca718005c18/go.mod h1:b+2E5dAYhXwXZwtn
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
+golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.6-0.20210726203631-07bc1bf47fb2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
 golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=

acceptance/tests/api-gateway/api_gateway_lifecycle_test.go

@@ -27,8 +27,10 @@ func TestAPIGateway_Lifecycle(t *testing.T) {
 	ctx := suite.Environment().DefaultContext(t)
 	cfg := suite.Config()
 	helmValues := map[string]string{
-		"global.logLevel":       "trace",
-		"connectInject.enabled": "true",
+		"global.logLevel":              "trace",
+		"connectInject.enabled":        "true",
+		"global.acls.manageSystemACLs": "true",
+		"global.tls.enabled":           "true",
 	}
 
 	releaseName := helpers.RandomName()
@@ -37,7 +39,7 @@ func TestAPIGateway_Lifecycle(t *testing.T) {
 	consulCluster.Create(t)
 
 	k8sClient := ctx.ControllerRuntimeClient(t)
-	consulClient, _ := consulCluster.SetupConsulClient(t, false)
+	consulClient, _ := consulCluster.SetupConsulClient(t, true)
 
 	defaultNamespace := "default"
 
@@ -141,6 +143,13 @@ func TestAPIGateway_Lifecycle(t *testing.T) {
 	logger.Log(t, "creating route two")
 	routeTwo := createRoute(t, k8sClient, routeTwoName, defaultNamespace, controlledGatewayTwoName, targetName)
 
+	// Scenario: Ensure ACL roles/policies are set correctly
+	logger.Log(t, "checking that ACL roles/policies are set correctly for controlled gateway one")
+	checkACLRolesPolicies(t, consulClient, controlledGatewayOneName)
+
+	logger.Log(t, "checking that ACL roles/policies are set correctly for controlled gateway two")
+	checkACLRolesPolicies(t, consulClient, controlledGatewayTwoName)
+
 	// Scenario: Swapping a route to another controlled gateway should clean up the old parent statuses and references on Consul resources
 
 	// check that the route is bound properly and objects are reflected in Consul
@@ -210,6 +219,7 @@ func TestAPIGateway_Lifecycle(t *testing.T) {
 	checkConsulNotExists(t, consulClient, api.HTTPRoute, routeOneName)
 
 	// Scenario: Deleting a gateway should result in routes only referencing it to get cleaned up from Consul and their statuses/finalizers cleared, but routes referencing another controlled gateway should still exist in Consul and only have their statuses cleaned up from referencing the gateway we previously controlled. Any referenced certificates should also get cleaned up.
+	// and acl roles and policies for that gateway should be cleaned up
 
 	// delete gateway two
 	logger.Log(t, "deleting gateway two in Kubernetes")
@@ -224,6 +234,12 @@ func TestAPIGateway_Lifecycle(t *testing.T) {
 	logger.Log(t, "checking that http route one is cleaned up in Kubernetes")
 	checkEmptyRoute(t, k8sClient, routeOneName, defaultNamespace)
 
+	logger.Log(t, "checking that ACL roles/policies are set correctly for controlled gateway one")
+	checkACLRolesPolicies(t, consulClient, controlledGatewayOneName)
+
+	logger.Log(t, "checking that ACL roles/policies are removed for controlled gateway two")
+	checkACLRolesPoliciesDontExist(t, consulClient, controlledGatewayTwoName)
+
 	// Scenario: Changing a gateway class name on a gateway to something we don’t control should have the same affect as deleting it with the addition of cleaning up our finalizer from the gateway.
 
 	// reset route one to point to our first gateway and check that it's bound properly
@@ -442,3 +458,27 @@ func createGatewayClass(t *testing.T, client client.Client, name, controllerName
 	err := client.Create(context.Background(), gatewayClass)
 	require.NoError(t, err)
 }
+
+func checkACLRolesPolicies(t *testing.T, client *api.Client, gatewayName string) {
+	t.Helper()
+	retryCheck(t, 60, func(r *retry.R) {
+		role, _, err := client.ACL().RoleReadByName(fmt.Sprint("managed-gateway-acl-role-", gatewayName), nil)
+		require.NoError(r, err)
+		require.NotNil(r, role)
+		policy, _, err := client.ACL().PolicyReadByName(fmt.Sprint("api-gateway-policy-for-", gatewayName), nil)
+		require.NoError(r, err)
+		require.NotNil(r, policy)
+	})
+}
+
+func checkACLRolesPoliciesDontExist(t *testing.T, client *api.Client, gatewayName string) {
+	t.Helper()
+	retryCheck(t, 60, func(r *retry.R) {
+		role, _, err := client.ACL().RoleReadByName(fmt.Sprint("managed-gateway-acl-role-", gatewayName), nil)
+		require.NoError(r, err)
+		require.Nil(r, role)
+		policy, _, err := client.ACL().PolicyReadByName(fmt.Sprint("api-gateway-policy-for-", gatewayName), nil)
+		require.NoError(r, err)
+		require.Nil(r, policy)
+	})
+}