Backport of Net 6289 improve consul api gateway annotations into rele…

…ase/1.2.x (#3627)

Net 6289 improve consul api gateway annotations (#3437)

* Add api-gateway to allow list for gateway-kind annotation

* Update godoc for AnnotationGatewayKind to list correct allowable values

* Exclude api-gateway kind when checking if endpoints controller should act on gateway

* Add gateway-kind and gateway-consul-service-name annotations to API gateway pods

* Add appropriate component label to api-gateway pods

* Update consul-k8s CLI to rely on standard component label for api-gateway

This makes api-gateway behave the same as ingress, mesh and terminating gateways; however, it won't pick up pods created by the legacy consul-api-gateway controller.

* add backwards compatabiilty, additional tests

* add changelog

* Update .changelog/3437.txt



* Update cli/cmd/proxy/list/command.go



* Update cli/cmd/proxy/list/command_test.go



* Update cli/cmd/proxy/list/command_test.go



* Update cli/cmd/proxy/list/command.go



* cleanup

* Update cli/cmd/proxy/list/command.go



---------

Co-authored-by: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com>
Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com>

.changelog/3437.txt

@@ -0,0 +1,3 @@
+```release-note:bug-fix
+api-gateways: API Gateway pods now include `gateway-kind` and `gateway-consul-service-name` annotations consistent with other Consul gateway types.
+```

cli/cmd/proxy/list/command.go

@@ -10,15 +10,16 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/hashicorp/consul-k8s/cli/common"
-	"github.com/hashicorp/consul-k8s/cli/common/flag"
-	"github.com/hashicorp/consul-k8s/cli/common/terminal"
 	"github.com/posener/complete"
 	helmCLI "helm.sh/helm/v3/pkg/cli"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/validation"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
+
+	"github.com/hashicorp/consul-k8s/cli/common"
+	"github.com/hashicorp/consul-k8s/cli/common/flag"
+	"github.com/hashicorp/consul-k8s/cli/common/terminal"
 )
 
 const (
@@ -211,21 +212,51 @@ func (c *ListCommand) fetchPods() ([]v1.Pod, error) {
 
 	// Fetch all pods in the namespace with labels matching the gateway component names.
 	gatewaypods, err := c.kubernetes.CoreV1().Pods(c.namespace()).List(c.Ctx, metav1.ListOptions{
-		LabelSelector: "component in (ingress-gateway, mesh-gateway, terminating-gateway), chart=consul-helm",
+		LabelSelector: "component in (api-gateway, ingress-gateway, mesh-gateway, terminating-gateway), chart=consul-helm",
 	})
 	if err != nil {
 		return nil, err
 	}
 	pods = append(pods, gatewaypods.Items...)
 
-	// Fetch all pods in the namespace with a label indicating they are an API gateway.
+	// Fetch API Gateway pods with deprecated label and append if they aren't already in the list
+	// TODO this block can be deleted if and when we decide we are ok with no longer listing pods of people using previous API Gateway
+	// versions.
 	apigatewaypods, err := c.kubernetes.CoreV1().Pods(c.namespace()).List(c.Ctx, metav1.ListOptions{
 		LabelSelector: "api-gateway.consul.hashicorp.com/managed=true",
 	})
+
+	namespacedName := func(pod v1.Pod) string {
+		return pod.Namespace + pod.Name
+	}
 	if err != nil {
 		return nil, err
 	}
-	pods = append(pods, apigatewaypods.Items...)
+	if len(apigatewaypods.Items) > 0 {
+		//Deduplicated pod list
+		seenPods := map[string]struct{}{}
+		for _, pod := range apigatewaypods.Items {
+			if _, ok := seenPods[namespacedName(pod)]; ok {
+				continue
+			}
+			found := false
+			for _, gatewayPod := range gatewaypods.Items {
+				//note that we already have this pod in the list so we can exit early.
+				seenPods[namespacedName(gatewayPod)] = struct{}{}
+
+				if (namespacedName(gatewayPod)) == namespacedName(pod) {
+					found = true
+					break
+				}
+			}
+			//pod isn't in the list already, we can add it.
+			if !found {
+				pods = append(pods, pod)
+			}
+
+		}
+	}
+	//---
 
 	// Fetch all pods in the namespace with a label indicating they are a service networked by Consul.
 	sidecarpods, err := c.kubernetes.CoreV1().Pods(c.namespace()).List(c.Ctx, metav1.ListOptions{
@@ -268,22 +299,22 @@ func (c *ListCommand) output(pods []v1.Pod) {
 
 		// Get the type for ingress, mesh, and terminating gateways.
 		switch pod.Labels["component"] {
+		case "api-gateway":
+			proxyType = "API Gateway"
 		case "ingress-gateway":
 			proxyType = "Ingress Gateway"
 		case "mesh-gateway":
 			proxyType = "Mesh Gateway"
 		case "terminating-gateway":
 			proxyType = "Terminating Gateway"
-		}
-
-		// Determine if the pod is an API Gateway.
-		if pod.Labels["api-gateway.consul.hashicorp.com/managed"] == "true" {
-			proxyType = "API Gateway"
-		}
-
-		// Fallback to "Sidecar" as a default
-		if proxyType == "" {
+		default:
+			// Fallback to "Sidecar" as a default
 			proxyType = "Sidecar"
+
+			// Determine if deprecated API Gateway pod.
+			if pod.Labels["api-gateway.consul.hashicorp.com/managed"] == "true" {
+				proxyType = "API Gateway"
+			}
 		}
 
 		if c.flagAllNamespaces {

cli/cmd/proxy/list/command_test.go

@@ -274,12 +274,33 @@ func TestListCommandOutput(t *testing.T) {
 				},
 			},
 		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "depricated-api-gateway",
+				Namespace: "consul",
+				Labels: map[string]string{
+					"api-gateway.consul.hashicorp.com/managed": "true",
+				},
+			},
+		},
 		{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "api-gateway",
 				Namespace: "consul",
+				Labels: map[string]string{
+					"component": "api-gateway",
+					"chart":     "consul-helm",
+				},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "both-labels-api-gateway",
+				Namespace: "consul",
 				Labels: map[string]string{
 					"api-gateway.consul.hashicorp.com/managed": "true",
+					"component": "api-gateway",
+					"chart":     "consul-helm",
 				},
 			},
 		},
@@ -321,7 +342,7 @@ func TestListCommandOutput(t *testing.T) {
 
 func TestListCommandOutputInJsonFormat(t *testing.T) {
 	// These regular expressions must be present in the output.
-	expected := ".*Name.*mesh-gateway.*\n.*Namespace.*consul.*\n.*Type.*Mesh Gateway.*\n.*\n.*\n.*Name.*terminating-gateway.*\n.*Namespace.*consul.*\n.*Type.*Terminating Gateway.*\n.*\n.*\n.*Name.*ingress-gateway.*\n.*Namespace.*default.*\n.*Type.*Ingress Gateway.*\n.*\n.*\n.*Name.*api-gateway.*\n.*Namespace.*consul.*\n.*Type.*API Gateway.*\n.*\n.*\n.*Name.*pod1.*\n.*Namespace.*default.*\n.*Type.*Sidecar.*"
+	expected := ".*Name.*api-gateway.*\n.*Namespace.*consul.*\n.*Type.*API Gateway.*\n.*\n.*\n.*Name.*both-labels-api-gateway.*\n.*Namespace.*consul.*\n.*Type.*API Gateway.*\n.*\n.*\n.*Name.*mesh-gateway.*\n.*Namespace.*consul.*\n.*Type.*Mesh Gateway.*\n.*\n.*\n.*Name.*terminating-gateway.*\n.*Namespace.*consul.*\n.*Type.*Terminating Gateway.*\n.*\n.*\n.*Name.*ingress-gateway.*\n.*Namespace.*default.*\n.*Type.*Ingress Gateway.*\n.*\n.*\n.*Name.*deprecated-api-gateway.*\n.*Namespace.*consul.*\n.*Type.*API Gateway.*\n.*\n.*\n.*Name.*pod1.*\n.*Namespace.*default.*\n.*Type.*Sidecar.*"
 	notExpected := "default.*dont-fetch.*Sidecar"
 
 	pods := []v1.Pod{
@@ -359,6 +380,27 @@ func TestListCommandOutputInJsonFormat(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "api-gateway",
 				Namespace: "consul",
+				Labels: map[string]string{
+					"component": "api-gateway",
+					"chart":     "consul-helm",
+				},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "both-labels-api-gateway",
+				Namespace: "consul",
+				Labels: map[string]string{
+					"api-gateway.consul.hashicorp.com/managed": "true",
+					"component": "api-gateway",
+					"chart":     "consul-helm",
+				},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "deprecated-api-gateway",
+				Namespace: "consul",
 				Labels: map[string]string{
 					"api-gateway.consul.hashicorp.com/managed": "true",
 				},

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

@@ -12,6 +12,7 @@ import (
 )
 
 const (
+	componentLabel = "component"
 	nameLabel      = "gateway.consul.hashicorp.com/name"
 	namespaceLabel = "gateway.consul.hashicorp.com/namespace"
 	createdAtLabel = "gateway.consul.hashicorp.com/created"
@@ -21,6 +22,7 @@ const (
 // LabelsForGateway formats the default labels that appear on objects managed by the controllers.
 func LabelsForGateway(gateway *gwv1beta1.Gateway) map[string]string {
 	return map[string]string{
+		componentLabel: "api-gateway",
 		nameLabel:      gateway.Name,
 		namespaceLabel: gateway.Namespace,
 		createdAtLabel: fmt.Sprintf("%d", gateway.CreationTimestamp.Unix()),

control-plane/api-gateway/gatekeeper/deployment.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
 	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
+	"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
 )
 
 const (
@@ -111,7 +112,9 @@ func (g *Gatekeeper) deployment(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayC
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: common.LabelsForGateway(&gateway),
 					Annotations: map[string]string{
-						"consul.hashicorp.com/connect-inject": "false",
+						constants.AnnotationInject:                   "false",
+						constants.AnnotationGatewayConsulServiceName: gateway.Name,
+						constants.AnnotationGatewayKind:              "api-gateway",
 					},
 				},
 				Spec: corev1.PodSpec{

control-plane/api-gateway/gatekeeper/gatekeeper_test.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
 	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
+	"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
 )
 
 var (
@@ -35,6 +36,7 @@ var (
 	name                = "test"
 	namespace           = "default"
 	labels              = map[string]string{
+		"component":                              "api-gateway",
 		"gateway.consul.hashicorp.com/name":      name,
 		"gateway.consul.hashicorp.com/namespace": namespace,
 		createdAtLabelKey:                        createdAtLabelValue,
@@ -1018,6 +1020,8 @@ func validateResourcesExist(t *testing.T, client client.Client, helmConfig commo
 			require.NotNil(t, actual.Spec.Replicas)
 			require.EqualValues(t, *expected.Spec.Replicas, *actual.Spec.Replicas)
 		}
+		require.Equal(t, expected.Spec.Template.ObjectMeta.Annotations, actual.Spec.Template.ObjectMeta.Annotations)
+		require.Equal(t, expected.Spec.Template.ObjectMeta.Labels, actual.Spec.Template.Labels)
 
 		// Ensure there is an init container
 		hasInitContainer := false
@@ -1215,7 +1219,9 @@ func configureDeployment(name, namespace string, labels map[string]string, repli
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: labels,
 					Annotations: map[string]string{
-						"consul.hashicorp.com/connect-inject": "false",
+						constants.AnnotationInject:                   "false",
+						constants.AnnotationGatewayConsulServiceName: name,
+						constants.AnnotationGatewayKind:              "api-gateway",
 					},
 				},
 				Spec: corev1.PodSpec{

control-plane/connect-inject/constants/annotations_and_labels.go

@@ -25,7 +25,8 @@ const (
 
 	// AnnotationGatewayKind is the key of the annotation that indicates pods
 	// that represent Consul Connect Gateways. This should be set to a
-	// value that is either "mesh", "ingress" or "terminating".
+	// value that is either "mesh-gateway", "ingress-gateway", "terminating-gateway",
+	// or "api-gateway".
 	AnnotationGatewayKind = "consul.hashicorp.com/gateway-kind"
 
 	// AnnotationGatewayConsulServiceName is the key of the annotation whose value

control-plane/connect-inject/controllers/endpoints/endpoints_controller.go

@@ -43,6 +43,7 @@ const (
 	meshGateway        = "mesh-gateway"
 	terminatingGateway = "terminating-gateway"
 	ingressGateway     = "ingress-gateway"
+	apiGateway         = "api-gateway"
 
 	kubernetesSuccessReasonMsg           = "Kubernetes health checks passing"
 	envoyPrometheusBindAddr              = "envoy_prometheus_bind_addr"
@@ -786,9 +787,11 @@ func (r *Controller) createGatewayRegistrations(pod corev1.Pod, serviceEndpoints
 				"address": "0.0.0.0",
 			},
 		}
-
+	case apiGateway:
+		// Do nothing. This is only here so that API gateway pods have annotations
+		// consistent with other gateway types but don't return an error below.
 	default:
-		return nil, fmt.Errorf("%s must be one of %s, %s, or %s", constants.AnnotationGatewayKind, meshGateway, terminatingGateway, ingressGateway)
+		return nil, fmt.Errorf("%s must be one of %s, %s, %s, or %s", constants.AnnotationGatewayKind, meshGateway, terminatingGateway, ingressGateway, apiGateway)
 	}
 
 	if r.MetricsConfig.DefaultEnableMetrics && r.MetricsConfig.EnableGatewayMetrics {
@@ -1448,10 +1451,11 @@ func hasBeenInjected(pod corev1.Pod) bool {
 	return false
 }
 
-// isGateway checks the value of the gateway annotation and returns true if the Pod represents a Gateway.
+// isGateway checks the value of the gateway annotation and returns true if the Pod
+// represents a Gateway kind that should be acted upon by the endpoints controller.
 func isGateway(pod corev1.Pod) bool {
 	anno, ok := pod.Annotations[constants.AnnotationGatewayKind]
-	return ok && anno != ""
+	return ok && anno != "" && anno != apiGateway
 }
 
 // isTelemetryCollector checks whether a pod is part of a deployment for a Consul Telemetry Collector. If so,