diff --git a/.changelog/3437.txt b/.changelog/3437.txt new file mode 100644 index 0000000000..3e70f0d4c0 --- /dev/null +++ b/.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. +``` \ No newline at end of file diff --git a/cli/cmd/proxy/list/command.go b/cli/cmd/proxy/list/command.go index 49881417a3..e0211ebe37 100644 --- a/cli/cmd/proxy/list/command.go +++ b/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 { diff --git a/cli/cmd/proxy/list/command_test.go b/cli/cmd/proxy/list/command_test.go index 9e7104886d..5493ab88a9 100644 --- a/cli/cmd/proxy/list/command_test.go +++ b/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", }, diff --git a/control-plane/api-gateway/common/labels.go b/control-plane/api-gateway/common/labels.go index cba13a603e..06f7857c30 100644 --- a/control-plane/api-gateway/common/labels.go +++ b/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()), diff --git a/control-plane/api-gateway/gatekeeper/deployment.go b/control-plane/api-gateway/gatekeeper/deployment.go index f3e9545e57..6bc1402eed 100644 --- a/control-plane/api-gateway/gatekeeper/deployment.go +++ b/control-plane/api-gateway/gatekeeper/deployment.go @@ -6,18 +6,19 @@ package gatekeeper import ( "context" - "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common" - "github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1" - "k8s.io/apimachinery/pkg/types" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + + "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 ( @@ -110,7 +111,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{ diff --git a/control-plane/api-gateway/gatekeeper/gatekeeper_test.go b/control-plane/api-gateway/gatekeeper/gatekeeper_test.go index 8d61e6948c..72bf6aeabc 100644 --- a/control-plane/api-gateway/gatekeeper/gatekeeper_test.go +++ b/control-plane/api-gateway/gatekeeper/gatekeeper_test.go @@ -25,6 +25,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 ( @@ -34,6 +35,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, @@ -1007,6 +1009,8 @@ func validateResourcesExist(t *testing.T, client client.Client, resources resour 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 a consul-dataplane container dropping ALL capabilities, adding // back the NET_BIND_SERVICE capability, and establishing a read-only root filesystem @@ -1189,7 +1193,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{ diff --git a/control-plane/connect-inject/constants/annotations_and_labels.go b/control-plane/connect-inject/constants/annotations_and_labels.go index 4cff554e83..5a3567fd6f 100644 --- a/control-plane/connect-inject/constants/annotations_and_labels.go +++ b/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 diff --git a/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go b/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go index 4ac62cd291..b0b8bea054 100644 --- a/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go +++ b/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" envoyPrometheusBindAddr = "envoy_prometheus_bind_addr" envoyTelemetryCollectorBindSocketDir = "envoy_telemetry_collector_bind_socket_dir" @@ -801,9 +802,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 { @@ -1444,10 +1447,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,