[NET-8091] Use file-system-certificate in Consul instead of inline-ce…

…rtificate (#3767)

* Use file-system-certificate in Consul instead of inline-certificate

* Actually update correctly from merges

* Adds changelog

* Updates go.mod in acceptance tests with latest consul api, updates the acceptance gateway lifecycle test

* Small updates

* Update comment

---------

Co-authored-by: Melisa Griffin <melisa.griffin@hashicorp.com>

.changelog/3767.txt

@@ -0,0 +1,3 @@
+```release-note:feature
+gateways: api-gateway now uses the Consul file-system-certificate by default for TLS
+```

acceptance/go.mod

@@ -14,7 +14,7 @@ require (
 	github.com/google/uuid v1.3.0
 	github.com/gruntwork-io/terratest v0.46.7
 	github.com/hashicorp/consul-k8s/control-plane v0.0.0-20240226161840-f3842c41cb2b
-	github.com/hashicorp/consul/api v1.28.2
+	github.com/hashicorp/consul/api v1.10.1-0.20240422130714-057ad7e95280
 	github.com/hashicorp/consul/proto-public v0.6.0
 	github.com/hashicorp/consul/sdk v0.16.0
 	github.com/hashicorp/go-multierror v1.1.1

acceptance/go.sum

@@ -290,8 +290,8 @@ github.com/hashicorp/consul-k8s/control-plane v0.0.0-20240226161840-f3842c41cb2b
 github.com/hashicorp/consul-k8s/control-plane v0.0.0-20240226161840-f3842c41cb2b/go.mod h1:TVaSJM7vYM/mtKGpVc/Lch53lrqLI9XAXJgy/gY8v4A=
 github.com/hashicorp/consul-server-connection-manager v0.1.6 h1:ktj8Fi+dRXn9hhM+FXsfEJayhzzgTqfH08Ne5M6Fmug=
 github.com/hashicorp/consul-server-connection-manager v0.1.6/go.mod h1:HngMIv57MT+pqCVeRQMa1eTB5dqnyMm8uxjyv+Hn8cs=
-github.com/hashicorp/consul/api v1.28.2 h1:mXfkRHrpHN4YY3RqL09nXU1eHKLNiuAN4kHvDQ16k/8=
-github.com/hashicorp/consul/api v1.28.2/go.mod h1:KyzqzgMEya+IZPcD65YFoOVAgPpbfERu4I/tzG6/ueE=
+github.com/hashicorp/consul/api v1.10.1-0.20240422130714-057ad7e95280 h1:LDbQAc0Zx9vBChuwHxzegb+6Dyx9udRjpng0MyAfnSc=
+github.com/hashicorp/consul/api v1.10.1-0.20240422130714-057ad7e95280/go.mod h1:cb1Sh6DxIGZ+5XBwpWmJF6ImxxV0vP8S5NMFRXsNN+8=
 github.com/hashicorp/consul/proto-public v0.6.0 h1:9qrBujmoTB5gQQ84kQO+YWvhjgYoYBNrOoHdo4cpHHM=
 github.com/hashicorp/consul/proto-public v0.6.0/go.mod h1:JF6983XNCzvw4wDNOLEwLqOq2IPw7iyT+pkswHSz08U=
 github.com/hashicorp/consul/sdk v0.16.0 h1:SE9m0W6DEfgIVCJX7xU+iv/hUl4m/nxqMTnCdMxDpJ8=

acceptance/tests/api-gateway/api_gateway_external_servers_test.go

@@ -76,8 +76,19 @@ func TestAPIGateway_ExternalServers(t *testing.T) {
 	require.NoError(t, err)
 	logger.Log(t, "set consul config entry")
 
+	// Create certificate secret, we do this separately since
+	// applying the secret will make an invalid certificate that breaks other tests
+	logger.Log(t, "creating certificate secret")
+	out, err := k8s.RunKubectlAndGetOutputE(t, ctx.KubectlOptions(t), "apply", "-f", "../fixtures/bases/api-gateway/certificate.yaml")
+	require.NoError(t, err, out)
+	helpers.Cleanup(t, cfg.NoCleanupOnFailure, cfg.NoCleanup, func() {
+		// Ignore errors here because if the test ran as expected
+		// the custom resources will have been deleted.
+		k8s.RunKubectlAndGetOutputE(t, ctx.KubectlOptions(t), "delete", "-f", "../fixtures/bases/api-gateway/certificate.yaml")
+	})
+
 	logger.Log(t, "creating api-gateway resources")
-	out, err := k8s.RunKubectlAndGetOutputE(t, ctx.KubectlOptions(t), "apply", "-k", "../fixtures/bases/api-gateway")
+	out, err = k8s.RunKubectlAndGetOutputE(t, ctx.KubectlOptions(t), "apply", "-k", "../fixtures/bases/api-gateway")
 	require.NoError(t, err, out)
 	helpers.Cleanup(t, cfg.NoCleanupOnFailure, cfg.NoCleanup, func() {
 		// Ignore errors here because if the test ran as expected

acceptance/tests/api-gateway/api_gateway_lifecycle_test.go

@@ -279,7 +279,7 @@ func TestAPIGateway_Lifecycle(t *testing.T) {
 
 	// make sure our certificate exists
 	logger.Log(t, "checking that the certificate is synchronized to Consul")
-	checkConsulExists(t, consulClient, api.InlineCertificate, certificateName)
+	checkConsulExists(t, consulClient, api.FileSystemCertificate, certificateName)
 
 	// delete the certificate in Kubernetes
 	logger.Log(t, "deleting the certificate in Kubernetes")
@@ -288,7 +288,7 @@ func TestAPIGateway_Lifecycle(t *testing.T) {
 
 	// make sure the certificate no longer exists in Consul
 	logger.Log(t, "checking that the certificate is deleted from Consul")
-	checkConsulNotExists(t, consulClient, api.InlineCertificate, certificateName)
+	checkConsulNotExists(t, consulClient, api.FileSystemCertificate, certificateName)
 }
 
 func checkConsulNotExists(t *testing.T, client *api.Client, kind, name string, namespace ...string) {

acceptance/tests/api-gateway/api_gateway_tenancy_test.go

@@ -168,7 +168,7 @@ func TestAPIGateway_Tenancy(t *testing.T) {
 			})
 
 			// we only sync validly referenced certificates over, so check to make sure it is not created.
-			checkConsulNotExists(t, consulClient, api.InlineCertificate, "certificate", namespaceForConsul(c.namespaceMirroring, certificateNamespace))
+			checkConsulNotExists(t, consulClient, api.FileSystemCertificate, "certificate", namespaceForConsul(c.namespaceMirroring, certificateNamespace))
 
 			// now create reference grants
 			createReferenceGrant(t, k8sClient, "gateway-certificate", gatewayNamespace, certificateNamespace)
@@ -237,11 +237,11 @@ func TestAPIGateway_Tenancy(t *testing.T) {
 
 			// and check to make sure that the certificate exists
 			retryCheck(t, 30, func(r *retry.R) {
-				entry, _, err := consulClient.ConfigEntries().Get(api.InlineCertificate, "certificate", &api.QueryOptions{
+				entry, _, err := consulClient.ConfigEntries().Get(api.FileSystemCertificate, "certificate", &api.QueryOptions{
 					Namespace: namespaceForConsul(c.namespaceMirroring, certificateNamespace),
 				})
 				require.NoError(r, err)
-				certificate := entry.(*api.InlineCertificateConfigEntry)
+				certificate := entry.(*api.FileSystemCertificateConfigEntry)
 
 				require.EqualValues(r, "certificate", certificate.Meta["k8s-name"])
 				require.EqualValues(r, certificateNamespace, certificate.Meta["k8s-namespace"])

acceptance/tests/partitions/partitions_gateway_test.go

@@ -219,8 +219,19 @@ func TestPartitions_Gateway(t *testing.T) {
 	logger.Log(t, "creating static-client pod in secondary partition cluster")
 	k8s.DeployKustomize(t, secondaryPartitionClusterStaticClientOpts, cfg.NoCleanupOnFailure, cfg.NoCleanup, cfg.DebugDirectory, "../fixtures/bases/static-client")
 
+	// Create certificate secret, we do this separately since
+	// applying the secret will make an invalid certificate that breaks other tests
+	logger.Log(t, "creating certificate secret")
+	out, err := k8s.RunKubectlAndGetOutputE(t, secondaryPartitionClusterStaticServerOpts, "apply", "-f", "../fixtures/bases/api-gateway/certificate.yaml")
+	require.NoError(t, err, out)
+	helpers.Cleanup(t, cfg.NoCleanupOnFailure, cfg.NoCleanup, func() {
+		// Ignore errors here because if the test ran as expected
+		// the custom resources will have been deleted.
+		k8s.RunKubectlAndGetOutputE(t, secondaryPartitionClusterStaticServerOpts, "delete", "-f", "../fixtures/bases/api-gateway/certificate.yaml")
+	})
+
 	logger.Log(t, "creating api-gateway resources")
-	out, err := k8s.RunKubectlAndGetOutputE(t, secondaryPartitionClusterStaticServerOpts, "apply", "-k", "../fixtures/bases/api-gateway")
+	out, err = k8s.RunKubectlAndGetOutputE(t, secondaryPartitionClusterStaticServerOpts, "apply", "-k", "../fixtures/bases/api-gateway")
 	require.NoError(t, err, out)
 	helpers.Cleanup(t, cfg.NoCleanupOnFailure, cfg.NoCleanup, func() {
 		// Ignore errors here because if the test ran as expected

control-plane/api-gateway/binding/binder.go

@@ -172,10 +172,7 @@ func (b *Binder) Snapshot() *Snapshot {
 		for secret := range gatewaySecrets.Iter() {
 			// ignore the error if the certificate cannot be processed and just don't add it into the final
 			// sync set
-			if err := b.config.Resources.TranslateInlineCertificate(secret.(types.NamespacedName)); err != nil {
-				b.config.Logger.Error(err, "error parsing referenced secret, ignoring")
-				continue
-			}
+			b.config.Resources.TranslateFileSystemCertificate(secret.(types.NamespacedName))
 		}
 	}
 

control-plane/api-gateway/binding/binder_test.go

@@ -55,19 +55,19 @@ var (
 )
 
 type resourceMapResources struct {
-	grants                   []gwv1beta1.ReferenceGrant
-	secrets                  []corev1.Secret
-	gateways                 []gwv1beta1.Gateway
-	httpRoutes               []gwv1beta1.HTTPRoute
-	tcpRoutes                []gwv1alpha2.TCPRoute
-	meshServices             []v1alpha1.MeshService
-	services                 []types.NamespacedName
-	jwtProviders             []*v1alpha1.JWTProvider
-	gatewayPolicies          []*v1alpha1.GatewayPolicy
-	externalAuthFilters      []*v1alpha1.RouteAuthFilter
-	consulInlineCertificates []api.InlineCertificateConfigEntry
-	consulHTTPRoutes         []api.HTTPRouteConfigEntry
-	consulTCPRoutes          []api.TCPRouteConfigEntry
+	grants                       []gwv1beta1.ReferenceGrant
+	secrets                      []corev1.Secret
+	gateways                     []gwv1beta1.Gateway
+	httpRoutes                   []gwv1beta1.HTTPRoute
+	tcpRoutes                    []gwv1alpha2.TCPRoute
+	meshServices                 []v1alpha1.MeshService
+	services                     []types.NamespacedName
+	jwtProviders                 []*v1alpha1.JWTProvider
+	gatewayPolicies              []*v1alpha1.GatewayPolicy
+	externalAuthFilters          []*v1alpha1.RouteAuthFilter
+	consulFileSystemCertificates []api.FileSystemCertificateConfigEntry
+	consulHTTPRoutes             []api.HTTPRouteConfigEntry
+	consulTCPRoutes              []api.TCPRouteConfigEntry
 }
 
 func newTestResourceMap(t *testing.T, resources resourceMapResources) *common.ResourceMap {
@@ -282,7 +282,7 @@ func TestBinder_Lifecycle(t *testing.T) {
 						Protocol: "http",
 						TLS: api.APIGatewayTLSConfiguration{
 							Certificates: []api.ResourceReference{{
-								Kind: api.InlineCertificate,
+								Kind: api.FileSystemCertificate,
 								Name: "secret-one",
 							}},
 						},
@@ -644,7 +644,7 @@ func TestBinder_Lifecycle(t *testing.T) {
 						},
 					},
 				},
-				consulInlineCertificates: []api.InlineCertificateConfigEntry{
+				consulFileSystemCertificates: []api.FileSystemCertificateConfigEntry{
 					*certificateOne,
 					*certificateTwo,
 				},
@@ -771,7 +771,7 @@ func TestBinder_Lifecycle(t *testing.T) {
 			expectedConsulDeletions: []api.ResourceReference{
 				{Kind: api.HTTPRoute, Name: "http-route-one"},
 				{Kind: api.TCPRoute, Name: "tcp-route-one"},
-				{Kind: api.InlineCertificate, Name: "secret-two"},
+				{Kind: api.FileSystemCertificate, Name: "secret-two"},
 				{Kind: api.APIGateway, Name: "gateway-deleted"},
 			},
 		},
@@ -3133,7 +3133,7 @@ func controlledBinder(config BinderConfig) BinderConfig {
 	return config
 }
 
-func generateTestCertificate(t *testing.T, namespace, name string) (*api.InlineCertificateConfigEntry, corev1.Secret) {
+func generateTestCertificate(t *testing.T, namespace, name string) (*api.FileSystemCertificateConfigEntry, corev1.Secret) {
 	privateKey, err := rsa.GenerateKey(rand.Reader, common.MinKeyLength)
 	require.NoError(t, err)
 
@@ -3180,8 +3180,7 @@ func generateTestCertificate(t *testing.T, namespace, name string) (*api.InlineC
 		},
 	}
 
-	certificate, err := (common.ResourceTranslator{}).ToInlineCertificate(secret)
-	require.NoError(t, err)
+	certificate := (common.ResourceTranslator{}).ToFileSystemCertificate(secret)
 
 	return certificate, secret
 }

control-plane/api-gateway/cache/consul.go

@@ -59,7 +59,7 @@ const (
 	apiTimeout        = 5 * time.Minute
 )
 
-var Kinds = []string{api.APIGateway, api.HTTPRoute, api.TCPRoute, api.InlineCertificate, api.JWTProvider}
+var Kinds = []string{api.APIGateway, api.HTTPRoute, api.TCPRoute, api.FileSystemCertificate, api.JWTProvider}
 
 type Config struct {
 	ConsulClientConfig      *consul.Config

control-plane/api-gateway/cache/consul_test.go

@@ -1539,9 +1539,9 @@ func Test_Run(t *testing.T) {
 	tcpRoute := setupTCPRoute()
 	tcpRoutes := []*api.TCPRouteConfigEntry{tcpRoute}
 
-	// setup inline certs
-	inlineCert := setupInlineCertificate()
-	certs := []*api.InlineCertificateConfigEntry{inlineCert}
+	// setup file-system certs
+	fileSystemCert := setupFileSystemCertificate()
+	certs := []*api.FileSystemCertificateConfigEntry{fileSystemCert}
 
 	// setup jwt providers
 	jwtProvider := setupJWTProvider()
@@ -1573,7 +1573,7 @@ func Test_Run(t *testing.T) {
 				return
 			}
 			fmt.Fprintln(w, string(val))
-		case "/v1/config/inline-certificate":
+		case "/v1/config/file-system-certificate":
 			val, err := json.Marshal(certs)
 			if err != nil {
 				w.WriteHeader(500)
@@ -1627,7 +1627,7 @@ func Test_Run(t *testing.T) {
 	}
 
 	expectedCache := loadedReferenceMaps([]api.ConfigEntry{
-		gw, tcpRoute, httpRouteOne, httpRouteTwo, inlineCert, jwtProvider,
+		gw, tcpRoute, httpRouteOne, httpRouteTwo, fileSystemCert, jwtProvider,
 	})
 
 	ctx, cancelFn := context.WithCancel(context.Background())
@@ -1677,11 +1677,11 @@ func Test_Run(t *testing.T) {
 	})
 
 	certNsn := types.NamespacedName{
-		Name:      inlineCert.Name,
-		Namespace: inlineCert.Namespace,
+		Name:      fileSystemCert.Name,
+		Namespace: fileSystemCert.Namespace,
 	}
 
-	certSubscriber := c.Subscribe(ctx, api.InlineCertificate, func(cfe api.ConfigEntry) []types.NamespacedName {
+	certSubscriber := c.Subscribe(ctx, api.FileSystemCertificate, func(cfe api.ConfigEntry) []types.NamespacedName {
 		return []types.NamespacedName{
 			{Name: cfe.GetName(), Namespace: cfe.GetNamespace()},
 		}
@@ -1968,10 +1968,10 @@ func setupTCPRoute() *api.TCPRouteConfigEntry {
 	}
 }
 
-func setupInlineCertificate() *api.InlineCertificateConfigEntry {
-	return &api.InlineCertificateConfigEntry{
-		Kind:        api.InlineCertificate,
-		Name:        "inline-cert",
+func setupFileSystemCertificate() *api.FileSystemCertificateConfigEntry {
+	return &api.FileSystemCertificateConfigEntry{
+		Kind:        api.FileSystemCertificate,
+		Name:        "file-system-cert",
 		Certificate: "cert",
 		PrivateKey:  "super secret",
 		Meta: map[string]string{

control-plane/api-gateway/common/diff.go

@@ -70,8 +70,8 @@ func EntriesEqual(a, b api.ConfigEntry) bool {
 		if bCast, ok := b.(*api.TCPRouteConfigEntry); ok {
 			return tcpRoutesEqual(aCast, bCast)
 		}
-	case *api.InlineCertificateConfigEntry:
-		if bCast, ok := b.(*api.InlineCertificateConfigEntry); ok {
+	case *api.FileSystemCertificateConfigEntry:
+		if bCast, ok := b.(*api.FileSystemCertificateConfigEntry); ok {
 			return certificatesEqual(aCast, bCast)
 		}
 	}
@@ -323,7 +323,7 @@ func (e entryComparator) tcpRouteServicesEqual(a, b api.TCPService) bool {
 		orDefault(a.Partition, e.partitionA) == orDefault(b.Partition, e.partitionB)
 }
 
-func certificatesEqual(a, b *api.InlineCertificateConfigEntry) bool {
+func certificatesEqual(a, b *api.FileSystemCertificateConfigEntry) bool {
 	if a == nil || b == nil {
 		return false
 	}
@@ -336,7 +336,7 @@ func certificatesEqual(a, b *api.InlineCertificateConfigEntry) bool {
 	}).certificatesEqual(*a, *b)
 }
 
-func (e entryComparator) certificatesEqual(a, b api.InlineCertificateConfigEntry) bool {
+func (e entryComparator) certificatesEqual(a, b api.FileSystemCertificateConfigEntry) bool {
 	return a.Kind == b.Kind &&
 		a.Name == b.Name &&
 		e.namespaceA == e.namespaceB &&

control-plane/api-gateway/common/resources.go

@@ -191,7 +191,7 @@ func (s *ResourceMap) Certificate(key types.NamespacedName) *corev1.Secret {
 	if !s.certificates.Contains(key) {
 		return nil
 	}
-	consulKey := NormalizeMeta(s.toConsulReference(api.InlineCertificate, key))
+	consulKey := NormalizeMeta(s.toConsulReference(api.FileSystemCertificate, key))
 	if secret, ok := s.certificateGateways[consulKey]; ok {
 		return secret.secret
 	}
@@ -201,7 +201,7 @@ func (s *ResourceMap) Certificate(key types.NamespacedName) *corev1.Secret {
 func (s *ResourceMap) ReferenceCountCertificate(secret corev1.Secret) {
 	key := client.ObjectKeyFromObject(&secret)
 	s.certificates.Add(key)
-	consulKey := NormalizeMeta(s.toConsulReference(api.InlineCertificate, key))
+	consulKey := NormalizeMeta(s.toConsulReference(api.FileSystemCertificate, key))
 	if _, ok := s.certificateGateways[consulKey]; !ok {
 		s.certificateGateways[consulKey] = &certificate{
 			secret:   &secret,
@@ -231,7 +231,7 @@ func (s *ResourceMap) ReferenceCountGateway(gateway gwv1beta1.Gateway) {
 
 				set.certificates.Add(certificateKey)
 
-				consulCertificateKey := s.toConsulReference(api.InlineCertificate, certificateKey)
+				consulCertificateKey := s.toConsulReference(api.FileSystemCertificate, certificateKey)
 				certificate, ok := s.certificateGateways[NormalizeMeta(consulCertificateKey)]
 				if ok {
 					certificate.gateways.Add(key)
@@ -270,7 +270,7 @@ func (s *ResourceMap) ResourcesToGC(key types.NamespacedName) []api.ResourceRefe
 				// the route altogether
 				toGC = append(toGC, id)
 			}
-		case api.InlineCertificate:
+		case api.FileSystemCertificate:
 			if s.processedCertificates.Contains(id) {
 				continue
 			}
@@ -323,7 +323,7 @@ func (s *ResourceMap) ReferenceCountConsulTCPRoute(route api.TCPRouteConfigEntry
 	s.consulTCPRoutes[NormalizeMeta(key)] = set
 }
 
-func (s *ResourceMap) ReferenceCountConsulCertificate(cert api.InlineCertificateConfigEntry) {
+func (s *ResourceMap) ReferenceCountConsulCertificate(cert api.FileSystemCertificateConfigEntry) {
 	key := s.objectReference(&cert)
 
 	var referenced *certificate
@@ -644,36 +644,29 @@ func (s *ResourceMap) CanGCTCPRouteOnUnbind(id api.ResourceReference) bool {
 	return true
 }
 
-func (s *ResourceMap) TranslateInlineCertificate(key types.NamespacedName) error {
-	consulKey := s.toConsulReference(api.InlineCertificate, key)
+func (s *ResourceMap) TranslateFileSystemCertificate(key types.NamespacedName) {
+	consulKey := s.toConsulReference(api.FileSystemCertificate, key)
 
 	certificate, ok := s.certificateGateways[NormalizeMeta(consulKey)]
 	if !ok {
-		return nil
+		return
 	}
 
 	if certificate.secret == nil {
-		return nil
-	}
-
-	consulCertificate, err := s.translator.ToInlineCertificate(*certificate.secret)
-	if err != nil {
-		return err
+		return
 	}
 
-	// add to the processed set so we don't GC it.
+	// add to the processed set so that we don't GC it.
 	s.processedCertificates.Add(consulKey)
 	s.consulMutations = append(s.consulMutations, &ConsulUpdateOperation{
-		Entry: consulCertificate,
+		Entry: s.translator.ToFileSystemCertificate(*certificate.secret),
 		// just swallow the error and log it since we can't propagate status back on a certificate.
-		OnUpdate: func(error) {
+		OnUpdate: func(err error) {
 			if err != nil {
 				s.logger.Error(err, "error syncing certificate to Consul")
 			}
 		},
 	})
-
-	return nil
 }
 
 func (s *ResourceMap) Mutations() []*ConsulUpdateOperation {