From 318a4e7f07e0cc1690a8496afda60ee4b06efe5d Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Mon, 13 Jun 2022 17:58:10 -0400 Subject: [PATCH] Address review comments to Peering Acceptor and Dialer. --- .../templates/connect-inject-clusterrole.yaml | 18 +- .../templates/crd-exportedservices.yaml | 6 +- .../api/v1alpha1/exportedservices_types.go | 21 +- .../v1alpha1/exportedservices_types_test.go | 92 +- ...consul.hashicorp.com_exportedservices.yaml | 6 +- control-plane/connect-inject/annotations.go | 2 +- .../connect-inject/connect_webhook.go | 657 ------ .../connect_webhook_ent_test.go | 692 ------ .../connect-inject/connect_webhook_test.go | 1851 ----------------- .../connect-inject/consul_sidecar.go | 4 +- .../connect-inject/consul_sidecar_test.go | 28 +- control-plane/connect-inject/container_env.go | 2 +- .../connect-inject/container_env_test.go | 4 +- .../connect-inject/container_init.go | 6 +- .../connect-inject/container_init_test.go | 72 +- .../connect-inject/container_volume.go | 2 +- .../connect-inject/endpoints_controller.go | 124 +- .../endpoints_controller_test.go | 35 +- control-plane/connect-inject/envoy_sidecar.go | 8 +- .../connect-inject/envoy_sidecar_test.go | 54 +- .../connect-inject/metrics_configuration.go | 12 +- .../metrics_configuration_test.go | 6 +- .../peering_acceptor_controller.go | 38 +- .../peering_acceptor_controller_test.go | 166 +- .../peering_dialer_controller.go | 11 +- .../peering_dialer_controller_test.go | 145 +- control-plane/go.mod | 4 +- control-plane/go.sum | 4 +- control-plane/main.go | 3 +- .../subcommand/inject-connect/command.go | 2 +- 30 files changed, 456 insertions(+), 3619 deletions(-) delete mode 100644 control-plane/connect-inject/connect_webhook.go delete mode 100644 control-plane/connect-inject/connect_webhook_ent_test.go delete mode 100644 control-plane/connect-inject/connect_webhook_test.go diff --git a/charts/consul/templates/connect-inject-clusterrole.yaml b/charts/consul/templates/connect-inject-clusterrole.yaml index 00e2e23a50..1165782c4b 100644 --- a/charts/consul/templates/connect-inject-clusterrole.yaml +++ b/charts/consul/templates/connect-inject-clusterrole.yaml @@ -24,14 +24,6 @@ rules: - "get" - "list" - "watch" -- apiGroups: [ "" ] - resources: ["secrets"] - verbs: - - "get" - - "list" - - "watch" - - "create" - - "delete" - apiGroups: - coordination.k8s.io resources: @@ -52,6 +44,15 @@ rules: - watch - patch {{- end }} +{{- if .Values.global.peering.enabled }} +- apiGroups: [ "" ] + resources: ["secrets"] + verbs: + - "get" + - "list" + - "watch" + - "create" + - "delete" - apiGroups: ["consul.hashicorp.com"] resources: ["peeringacceptors"] verbs: @@ -88,6 +89,7 @@ rules: - get - patch - update +{{- end }} {{- if .Values.global.enablePodSecurityPolicies }} - apiGroups: [ "policy" ] resources: [ "podsecuritypolicies" ] diff --git a/charts/consul/templates/crd-exportedservices.yaml b/charts/consul/templates/crd-exportedservices.yaml index 6ed5406cba..27d03dbb06 100644 --- a/charts/consul/templates/crd-exportedservices.yaml +++ b/charts/consul/templates/crd-exportedservices.yaml @@ -75,9 +75,9 @@ spec: description: Partition is the admin partition to export the service to. type: string - peerName: - description: '[Experimental] PeerName is the name of the - peer to export the service to.' + peer: + description: '[Experimental] Peer is the name of the peer + to export the service to.' type: string type: object type: array diff --git a/control-plane/api/v1alpha1/exportedservices_types.go b/control-plane/api/v1alpha1/exportedservices_types.go index 477574d945..3087379c62 100644 --- a/control-plane/api/v1alpha1/exportedservices_types.go +++ b/control-plane/api/v1alpha1/exportedservices_types.go @@ -68,8 +68,8 @@ type ExportedService struct { type ServiceConsumer struct { // Partition is the admin partition to export the service to. Partition string `json:"partition,omitempty"` - // [Experimental] PeerName is the name of the peer to export the service to. - PeerName string `json:"peerName,omitempty"` + // [Experimental] Peer is the name of the peer to export the service to. + Peer string `json:"peer,omitempty"` } func (in *ExportedServices) GetObjectMeta() metav1.ObjectMeta { @@ -165,11 +165,10 @@ func (in *ExportedServices) ToConsul(datacenter string) api.ConfigEntry { func (in *ExportedService) toConsul() capi.ExportedService { var consumers []capi.ServiceConsumer for _, consumer := range in.Consumers { - if consumer.PeerName != "" { - consumers = append(consumers, capi.ServiceConsumer{PeerName: consumer.PeerName}) - } else { - consumers = append(consumers, capi.ServiceConsumer{Partition: consumer.Partition}) - } + consumers = append(consumers, capi.ServiceConsumer{ + Partition: consumer.Partition, + PeerName: consumer.Peer, + }) } return capi.ExportedService{ Name: in.Name, @@ -228,11 +227,11 @@ func (in *ExportedService) validate(path *field.Path, consulMeta common.ConsulMe } func (in *ServiceConsumer) validate(path *field.Path, consulMeta common.ConsulMeta) *field.Error { - if in.Partition != "" && in.PeerName != "" { - return field.Invalid(path, *in, "both partition and peerName cannot be specified.") + if in.Partition != "" && in.Peer != "" { + return field.Invalid(path, *in, "both partition and peer cannot be specified.") } - if in.Partition == "" && in.PeerName == "" { - return field.Invalid(path, *in, "either partition or peerName must be specified.") + if in.Partition == "" && in.Peer == "" { + return field.Invalid(path, *in, "either partition or peer must be specified.") } if !consulMeta.PartitionsEnabled && in.Partition != "" { return field.Invalid(path.Child("partitions"), in.Partition, "Consul Admin Partitions need to be enabled to specify partition.") diff --git a/control-plane/api/v1alpha1/exportedservices_types_test.go b/control-plane/api/v1alpha1/exportedservices_types_test.go index c7c1df6b62..0810f8edaf 100644 --- a/control-plane/api/v1alpha1/exportedservices_types_test.go +++ b/control-plane/api/v1alpha1/exportedservices_types_test.go @@ -54,7 +54,7 @@ func TestExportedServices_MatchesConsul(t *testing.T) { Partition: "third", }, { - PeerName: "second-peer", + Peer: "second-peer", }, }, }, @@ -69,7 +69,7 @@ func TestExportedServices_MatchesConsul(t *testing.T) { Partition: "fifth", }, { - PeerName: "third-peer", + Peer: "third-peer", }, }, }, @@ -178,7 +178,7 @@ func TestExportedServices_ToConsul(t *testing.T) { Partition: "third", }, { - PeerName: "second-peer", + Peer: "second-peer", }, }, }, @@ -193,7 +193,7 @@ func TestExportedServices_ToConsul(t *testing.T) { Partition: "fifth", }, { - PeerName: "third-peer", + Peer: "third-peer", }, }, }, @@ -253,8 +253,10 @@ func TestExportedServices_ToConsul(t *testing.T) { func TestExportedServices_Validate(t *testing.T) { cases := map[string]struct { - input *ExportedServices - expectedErrMsgs []string + input *ExportedServices + namespaceEnabled bool + partitionsEnabled bool + expectedErrMsgs []string }{ "valid": { input: &ExportedServices{ @@ -271,14 +273,16 @@ func TestExportedServices_Validate(t *testing.T) { Partition: "second", }, { - PeerName: "second-peer", + Peer: "second-peer", }, }, }, }, }, }, - expectedErrMsgs: []string{}, + namespaceEnabled: true, + partitionsEnabled: true, + expectedErrMsgs: []string{}, }, "no consumers specified": { input: &ExportedServices{ @@ -295,6 +299,8 @@ func TestExportedServices_Validate(t *testing.T) { }, }, }, + namespaceEnabled: true, + partitionsEnabled: true, expectedErrMsgs: []string{ `spec.services[0]: Invalid value: []v1alpha1.ServiceConsumer{}: service must have at least 1 consumer.`, }, @@ -312,15 +318,17 @@ func TestExportedServices_Validate(t *testing.T) { Consumers: []ServiceConsumer{ { Partition: "second", - PeerName: "second-peer", + Peer: "second-peer", }, }, }, }, }, }, + namespaceEnabled: true, + partitionsEnabled: true, expectedErrMsgs: []string{ - `spec.services[0].consumers[0]: Invalid value: v1alpha1.ServiceConsumer{Partition:"second", PeerName:"second-peer"}: both partition and peerName cannot be specified.`, + `spec.services[0].consumers[0]: Invalid value: v1alpha1.ServiceConsumer{Partition:"second", Peer:"second-peer"}: both partition and peer cannot be specified.`, }, }, "neither partition nor peer name specified": { @@ -340,8 +348,60 @@ func TestExportedServices_Validate(t *testing.T) { }, }, }, + namespaceEnabled: true, + partitionsEnabled: true, expectedErrMsgs: []string{ - `spec.services[0].consumers[0]: Invalid value: v1alpha1.ServiceConsumer{Partition:"", PeerName:""}: either partition or peerName must be specified.`, + `spec.services[0].consumers[0]: Invalid value: v1alpha1.ServiceConsumer{Partition:"", Peer:""}: either partition or peer must be specified.`, + }, + }, + "partition provided when partitions are disabled": { + input: &ExportedServices{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.DefaultConsulPartition, + }, + Spec: ExportedServicesSpec{ + Services: []ExportedService{ + { + Name: "service-frontend", + Namespace: "frontend", + Consumers: []ServiceConsumer{ + { + Partition: "test-partition", + }, + }, + }, + }, + }, + }, + namespaceEnabled: true, + partitionsEnabled: false, + expectedErrMsgs: []string{ + `spec.services[0].consumers[0].partitions: Invalid value: "test-partition": Consul Admin Partitions need to be enabled to specify partition.`, + }, + }, + "namespace provided when namespaces are disabled": { + input: &ExportedServices{ + ObjectMeta: metav1.ObjectMeta{ + Name: common.DefaultConsulPartition, + }, + Spec: ExportedServicesSpec{ + Services: []ExportedService{ + { + Name: "service-frontend", + Namespace: "frontend", + Consumers: []ServiceConsumer{ + { + Peer: "test-peer", + }, + }, + }, + }, + }, + }, + namespaceEnabled: false, + partitionsEnabled: false, + expectedErrMsgs: []string{ + `spec.services[0]: Invalid value: "frontend": Consul Namespaces must be enabled to specify service namespace.`, }, }, "multiple errors": { @@ -357,7 +417,7 @@ func TestExportedServices_Validate(t *testing.T) { Consumers: []ServiceConsumer{ { Partition: "second", - PeerName: "second-peer", + Peer: "second-peer", }, {}, }, @@ -365,16 +425,18 @@ func TestExportedServices_Validate(t *testing.T) { }, }, }, + namespaceEnabled: true, + partitionsEnabled: true, expectedErrMsgs: []string{ - `spec.services[0].consumers[0]: Invalid value: v1alpha1.ServiceConsumer{Partition:"second", PeerName:"second-peer"}: both partition and peerName cannot be specified.`, - `spec.services[0].consumers[1]: Invalid value: v1alpha1.ServiceConsumer{Partition:"", PeerName:""}: either partition or peerName must be specified.`, + `spec.services[0].consumers[0]: Invalid value: v1alpha1.ServiceConsumer{Partition:"second", Peer:"second-peer"}: both partition and peer cannot be specified.`, + `spec.services[0].consumers[1]: Invalid value: v1alpha1.ServiceConsumer{Partition:"", Peer:""}: either partition or peer must be specified.`, }, }, } for name, testCase := range cases { t.Run(name, func(t *testing.T) { - err := testCase.input.Validate(common.ConsulMeta{NamespacesEnabled: true, PartitionsEnabled: true, Partition: common.DefaultConsulPartition}) + err := testCase.input.Validate(common.ConsulMeta{NamespacesEnabled: testCase.namespaceEnabled, PartitionsEnabled: testCase.partitionsEnabled, Partition: common.DefaultConsulPartition}) if len(testCase.expectedErrMsgs) != 0 { require.Error(t, err) for _, s := range testCase.expectedErrMsgs { diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_exportedservices.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_exportedservices.yaml index ddcc51b2b8..da1a66fd74 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_exportedservices.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_exportedservices.yaml @@ -68,9 +68,9 @@ spec: description: Partition is the admin partition to export the service to. type: string - peerName: - description: '[Experimental] PeerName is the name of the - peer to export the service to.' + peer: + description: '[Experimental] Peer is the name of the peer + to export the service to.' type: string type: object type: array diff --git a/control-plane/connect-inject/annotations.go b/control-plane/connect-inject/annotations.go index d1c59d19dc..1139642a49 100644 --- a/control-plane/connect-inject/annotations.go +++ b/control-plane/connect-inject/annotations.go @@ -136,7 +136,7 @@ const ( annotationTransparentProxyOverwriteProbes = "consul.hashicorp.com/transparent-proxy-overwrite-probes" // annotationOriginalPod is the value of the pod before being overwritten by the consul - // webhook/handler. + // webhook/meshWebhook. annotationOriginalPod = "consul.hashicorp.com/original-pod" // labelServiceIgnore is a label that can be added to a service to prevent it from being diff --git a/control-plane/connect-inject/connect_webhook.go b/control-plane/connect-inject/connect_webhook.go deleted file mode 100644 index f2653c2bcc..0000000000 --- a/control-plane/connect-inject/connect_webhook.go +++ /dev/null @@ -1,657 +0,0 @@ -package connectinject - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "net/http" - "path/filepath" - "strconv" - "strings" - "time" - - mapset "github.com/deckarep/golang-set" - "github.com/go-logr/logr" - "github.com/hashicorp/consul-k8s/control-plane/namespaces" - "github.com/hashicorp/consul/api" - "gomodules.xyz/jsonpatch/v2" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/client-go/kubernetes" - _ "k8s.io/client-go/plugin/pkg/client/auth" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" -) - -var ( - // kubeSystemNamespaces is a set of namespaces that are considered - // "system" level namespaces and are always skipped (never injected). - kubeSystemNamespaces = mapset.NewSetWith(metav1.NamespaceSystem, metav1.NamespacePublic) -) - -// Handler is the HTTP handler for admission webhooks. -type ConnectWebhook struct { - ConsulClient *api.Client - Clientset kubernetes.Interface - - // ImageConsul is the container image for Consul to use. - // ImageEnvoy is the container image for Envoy to use. - // - // Both of these MUST be set. - ImageConsul string - ImageEnvoy string - - // ImageConsulK8S is the container image for consul-k8s to use. - // This image is used for the consul-sidecar container. - ImageConsulK8S string - - // Optional: set when you need extra options to be set when running envoy - // See a list of args here: https://www.envoyproxy.io/docs/envoy/latest/operations/cli - EnvoyExtraArgs string - - // RequireAnnotation means that the annotation must be given to inject. - // If this is false, injection is default. - RequireAnnotation bool - - // AuthMethod is the name of the Kubernetes Auth Method to - // use for identity with connectInjection if ACLs are enabled. - AuthMethod string - - // The PEM-encoded CA certificate string - // to use when communicating with Consul clients over HTTPS. - // If not set, will use HTTP. - ConsulCACert string - - // ConsulPartition is the name of the Admin Partition that the controller - // is deployed in. It is an enterprise feature requiring Consul Enterprise 1.11+. - // Its value is an empty string if partitions aren't enabled. - ConsulPartition string - - // EnableNamespaces indicates that a user is running Consul Enterprise - // with version 1.7+ which is namespace aware. It enables Consul namespaces, - // with injection into either a single Consul namespace or mirrored from - // k8s namespaces. - EnableNamespaces bool - - // AllowK8sNamespacesSet is a set of k8s namespaces to explicitly allow for - // injection. It supports the special character `*` which indicates that - // all k8s namespaces are eligible unless explicitly denied. This filter - // is applied before checking pod annotations. - AllowK8sNamespacesSet mapset.Set - - // DenyK8sNamespacesSet is a set of k8s namespaces to explicitly deny - // injection and thus service registration with Consul. An empty set - // means that no namespaces are removed from consideration. This filter - // takes precedence over AllowK8sNamespacesSet. - DenyK8sNamespacesSet mapset.Set - - // ConsulDestinationNamespace is the name of the Consul namespace to register all - // injected services into if Consul namespaces are enabled and mirroring - // is disabled. This may be set, but will not be used if mirroring is enabled. - ConsulDestinationNamespace string - - // EnableK8SNSMirroring causes Consul namespaces to be created to match the - // k8s namespace of any service being registered into Consul. Services are - // registered into the Consul namespace that mirrors their k8s namespace. - EnableK8SNSMirroring bool - - // K8SNSMirroringPrefix is an optional prefix that can be added to the Consul - // namespaces created while mirroring. For example, if it is set to "k8s-", - // then the k8s `default` namespace will be mirrored in Consul's - // `k8s-default` namespace. - K8SNSMirroringPrefix string - - // CrossNamespaceACLPolicy is the name of the ACL policy to attach to - // any created Consul namespaces to allow cross namespace service discovery. - // Only necessary if ACLs are enabled. - CrossNamespaceACLPolicy string - - // Default resource settings for sidecar proxies. Some of these - // fields may be empty. - DefaultProxyCPURequest resource.Quantity - DefaultProxyCPULimit resource.Quantity - DefaultProxyMemoryRequest resource.Quantity - DefaultProxyMemoryLimit resource.Quantity - - // MetricsConfig contains metrics configuration from the inject-connect command and has methods to determine whether - // configuration should come from the default flags or annotations. The handler uses this to configure prometheus - // annotations and the merged metrics server. - MetricsConfig MetricsConfig - - // Resource settings for init container. All of these fields - // will be populated by the defaults provided in the initial flags. - InitContainerResources corev1.ResourceRequirements - - // Resource settings for Consul sidecar. All of these fields - // will be populated by the defaults provided in the initial flags. - DefaultConsulSidecarResources corev1.ResourceRequirements - - // EnableTransparentProxy enables transparent proxy mode. - // This means that the injected init container will apply traffic redirection rules - // so that all traffic will go through the Envoy proxy. - EnableTransparentProxy bool - - // TProxyOverwriteProbes controls whether the webhook should mutate pod's HTTP probes - // to point them to the Envoy proxy. - TProxyOverwriteProbes bool - - // EnableConsulDNS enables traffic redirection so that DNS requests are directed to Consul - // from mesh services. - EnableConsulDNS bool - - // ResourcePrefix is the prefix used for the installation which is used to determine the Service - // name of the Consul DNS service. - ResourcePrefix string - - // EnableOpenShift indicates that when tproxy is enabled, the security context for the Envoy and init - // containers should not be added because OpenShift sets a random user for those and will not allow - // those containers to be created otherwise. - EnableOpenShift bool - - // ConsulAPITimeout is the duration that the consul API client will - // wait for a response from the API before cancelling the request. - ConsulAPITimeout time.Duration - - // Log - Log logr.Logger - // Log settings for consul-sidecar - LogLevel string - LogJSON bool - - decoder *admission.Decoder -} -type multiPortInfo struct { - serviceIndex int - serviceName string -} - -// Handle is the admission.Handler implementation that actually handles the -// webhook request for admission control. This should be registered or -// served via the controller runtime manager. -func (w *ConnectWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { - var pod corev1.Pod - - // Decode the pod from the request - if err := w.decoder.Decode(req, &pod); err != nil { - w.Log.Error(err, "could not unmarshal request to pod") - return admission.Errored(http.StatusBadRequest, err) - } - - // Marshall the contents of the pod that was received. This is compared with the - // marshalled contents of the pod after it has been updated to create the jsonpatch. - origPodJson, err := json.Marshal(pod) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - - if err := w.validatePod(pod); err != nil { - w.Log.Error(err, "error validating pod", "request name", req.Name) - return admission.Errored(http.StatusBadRequest, err) - } - - // Setup the default annotation values that are used for the container. - // This MUST be done before shouldInject is called since that function - // uses these annotations. - if err := w.defaultAnnotations(&pod, string(origPodJson)); err != nil { - w.Log.Error(err, "error creating default annotations", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error creating default annotations: %s", err)) - } - - // Check if we should inject, for example we don't inject in the - // system namespaces. - if shouldInject, err := w.shouldInject(pod, req.Namespace); err != nil { - w.Log.Error(err, "error checking if should inject", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error checking if should inject: %s", err)) - } else if !shouldInject { - return admission.Allowed(fmt.Sprintf("%s %s does not require injection", pod.Kind, pod.Name)) - } - - w.Log.Info("received pod", "name", req.Name, "ns", req.Namespace) - - // Add our volume that will be shared by the init container and - // the sidecar for passing data in the pod. - pod.Spec.Volumes = append(pod.Spec.Volumes, w.containerVolume()) - - // Optionally mount data volume to other containers - w.injectVolumeMount(pod) - - // Add the upstream services as environment variables for easy - // service discovery. - containerEnvVars := w.containerEnvVars(pod) - for i := range pod.Spec.InitContainers { - pod.Spec.InitContainers[i].Env = append(pod.Spec.InitContainers[i].Env, containerEnvVars...) - } - - for i := range pod.Spec.Containers { - pod.Spec.Containers[i].Env = append(pod.Spec.Containers[i].Env, containerEnvVars...) - } - - // Add the init container which copies the Consul binary to /consul/connect-inject/. - initCopyContainer := w.initCopyContainer() - pod.Spec.InitContainers = append(pod.Spec.InitContainers, initCopyContainer) - - // A user can enable/disable tproxy for an entire namespace via a label. - ns, err := w.Clientset.CoreV1().Namespaces().Get(ctx, req.Namespace, metav1.GetOptions{}) - if err != nil { - w.Log.Error(err, "error fetching namespace metadata for container", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error getting namespace metadata for container: %s", err)) - } - - // Get service names from the annotation. If theres 0-1 service names, it's a single port pod, otherwise it's multi - // port. - annotatedSvcNames := w.annotatedServiceNames(pod) - multiPort := len(annotatedSvcNames) > 1 - - // For single port pods, add the single init container and envoy sidecar. - if !multiPort { - // Add the init container that registers the service and sets up the Envoy configuration. - initContainer, err := w.containerInit(*ns, pod, multiPortInfo{}) - if err != nil { - w.Log.Error(err, "error configuring injection init container", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error configuring injection init container: %s", err)) - } - pod.Spec.InitContainers = append(pod.Spec.InitContainers, initContainer) - - // Add the Envoy sidecar. - envoySidecar, err := w.envoySidecar(*ns, pod, multiPortInfo{}) - if err != nil { - w.Log.Error(err, "error configuring injection sidecar container", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error configuring injection sidecar container: %s", err)) - } - pod.Spec.Containers = append(pod.Spec.Containers, envoySidecar) - } else { - // For multi port pods, check for unsupported cases, mount all relevant service account tokens, and mount an init - // container and envoy sidecar per port. Tproxy, metrics, and metrics merging are not supported for multi port pods. - // In a single port pod, the service account specified in the pod is sufficient for mounting the service account - // token to the pod. In a multi port pod, where multiple services are registered with Consul, we also require a - // service account per service. So, this will look for service accounts whose name matches the service and mount - // those tokens if not already specified via the pod's serviceAccountName. - - w.Log.Info("processing multiport pod") - err := w.checkUnsupportedMultiPortCases(*ns, pod) - if err != nil { - w.Log.Error(err, "checking unsupported cases for multi port pods") - return admission.Errored(http.StatusInternalServerError, err) - } - for i, svc := range annotatedSvcNames { - w.Log.Info(fmt.Sprintf("service: %s", svc)) - if w.AuthMethod != "" { - if svc != "" && pod.Spec.ServiceAccountName != svc { - sa, err := w.Clientset.CoreV1().ServiceAccounts(req.Namespace).Get(ctx, svc, metav1.GetOptions{}) - if err != nil { - w.Log.Error(err, "couldn't get service accounts") - return admission.Errored(http.StatusInternalServerError, err) - } - if len(sa.Secrets) == 0 { - w.Log.Info(fmt.Sprintf("service account %s has zero secrets exp at least 1", svc)) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("service account %s has zero secrets, expected at least one", svc)) - } - saSecret := sa.Secrets[0].Name - w.Log.Info("found service account, mounting service account secret to Pod", "serviceAccountName", sa.Name) - pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{ - Name: fmt.Sprintf("%s-service-account", svc), - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: saSecret, - }, - }, - }) - } - } - - // This will get passed to the init and sidecar containers so they are configured correctly. - mpi := multiPortInfo{ - serviceIndex: i, - serviceName: svc, - } - - // Add the init container that registers the service and sets up the Envoy configuration. - initContainer, err := w.containerInit(*ns, pod, mpi) - if err != nil { - w.Log.Error(err, "error configuring injection init container", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error configuring injection init container: %s", err)) - } - pod.Spec.InitContainers = append(pod.Spec.InitContainers, initContainer) - - // Add the Envoy sidecar. - envoySidecar, err := w.envoySidecar(*ns, pod, mpi) - if err != nil { - w.Log.Error(err, "error configuring injection sidecar container", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error configuring injection sidecar container: %s", err)) - } - pod.Spec.Containers = append(pod.Spec.Containers, envoySidecar) - } - } - - // Now that the consul-sidecar no longer needs to re-register services periodically - // (that functionality lives in the endpoints-controller), - // we only need the consul sidecar to run the metrics merging server. - // First, determine if we need to run the metrics merging server. - shouldRunMetricsMerging, err := w.MetricsConfig.shouldRunMergedMetricsServer(pod) - if err != nil { - w.Log.Error(err, "error determining if metrics merging server should be run", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error determining if metrics merging server should be run: %s", err)) - } - - // Add the consul-sidecar only if we need to run the metrics merging server. - if shouldRunMetricsMerging { - consulSidecar, err := w.consulSidecar(pod) - if err != nil { - w.Log.Error(err, "error configuring consul sidecar container", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error configuring consul sidecar container: %s", err)) - } - pod.Spec.Containers = append(pod.Spec.Containers, consulSidecar) - } - - // pod.Annotations has already been initialized by h.defaultAnnotations() - // and does not need to be checked for being a nil value. - pod.Annotations[keyInjectStatus] = injected - - // Add annotations for metrics. - if err = w.prometheusAnnotations(&pod); err != nil { - w.Log.Error(err, "error configuring prometheus annotations", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error configuring prometheus annotations: %s", err)) - } - - if pod.Labels == nil { - pod.Labels = make(map[string]string) - } - pod.Labels[keyInjectStatus] = injected - - // Add the managed-by label since services are now managed by endpoints controller. This is to support upgrading - // from consul-k8s without Endpoints controller to consul-k8s with Endpoints controller. - pod.Labels[keyManagedBy] = managedByValue - - // Consul-ENT only: Add the Consul destination namespace as an annotation to the pod. - if w.EnableNamespaces { - pod.Annotations[annotationConsulNamespace] = w.consulNamespace(req.Namespace) - } - - // Overwrite readiness/liveness probes if needed. - err = w.overwriteProbes(*ns, &pod) - if err != nil { - w.Log.Error(err, "error overwriting readiness or liveness probes", "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error overwriting readiness or liveness probes: %s", err)) - } - - // Marshall the pod into JSON after it has the desired envs, annotations, labels, - // sidecars and initContainers appended to it. - updatedPodJson, err := json.Marshal(pod) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - - // Create a patches based on the Pod that was received by the handler - // and the desired Pod spec. - patches, err := jsonpatch.CreatePatch(origPodJson, updatedPodJson) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - - // Check and potentially create Consul resources. This is done after - // all patches are created to guarantee no errors were encountered in - // that process before modifying the Consul cluster. - if w.EnableNamespaces { - if _, err := namespaces.EnsureExists(w.ConsulClient, w.consulNamespace(req.Namespace), w.CrossNamespaceACLPolicy); err != nil { - w.Log.Error(err, "error checking or creating namespace", - "ns", w.consulNamespace(req.Namespace), "request name", req.Name) - return admission.Errored(http.StatusInternalServerError, fmt.Errorf("error checking or creating namespace: %s", err)) - } - } - - // Return a Patched response along with the patches we intend on applying to the - // Pod received by the handler. - return admission.Patched(fmt.Sprintf("valid %s request", pod.Kind), patches...) -} - -// shouldOverwriteProbes returns true if we need to overwrite readiness/liveness probes for this pod. -// It returns an error when the annotation value cannot be parsed by strconv.ParseBool. -func shouldOverwriteProbes(pod corev1.Pod, globalOverwrite bool) (bool, error) { - if raw, ok := pod.Annotations[annotationTransparentProxyOverwriteProbes]; ok { - return strconv.ParseBool(raw) - } - - return globalOverwrite, nil -} - -// overwriteProbes overwrites readiness/liveness probes of this pod when -// both transparent proxy is enabled and overwrite probes is true for the pod. -func (w *ConnectWebhook) overwriteProbes(ns corev1.Namespace, pod *corev1.Pod) error { - tproxyEnabled, err := transparentProxyEnabled(ns, *pod, w.EnableTransparentProxy) - if err != nil { - return err - } - - overwriteProbes, err := shouldOverwriteProbes(*pod, w.TProxyOverwriteProbes) - if err != nil { - return err - } - - if tproxyEnabled && overwriteProbes { - for i, container := range pod.Spec.Containers { - // skip the "envoy-sidecar" container from having it's probes overridden - if container.Name == envoySidecarContainer { - continue - } - if container.LivenessProbe != nil && container.LivenessProbe.HTTPGet != nil { - container.LivenessProbe.HTTPGet.Port = intstr.FromInt(exposedPathsLivenessPortsRangeStart + i) - } - if container.ReadinessProbe != nil && container.ReadinessProbe.HTTPGet != nil { - container.ReadinessProbe.HTTPGet.Port = intstr.FromInt(exposedPathsReadinessPortsRangeStart + i) - } - if container.StartupProbe != nil && container.StartupProbe.HTTPGet != nil { - container.StartupProbe.HTTPGet.Port = intstr.FromInt(exposedPathsStartupPortsRangeStart + i) - } - } - } - return nil -} - -func (w *ConnectWebhook) injectVolumeMount(pod corev1.Pod) { - containersToInject := splitCommaSeparatedItemsFromAnnotation(annotationInjectMountVolumes, pod) - - for index, container := range pod.Spec.Containers { - if sliceContains(containersToInject, container.Name) { - pod.Spec.Containers[index].VolumeMounts = append(pod.Spec.Containers[index].VolumeMounts, corev1.VolumeMount{ - Name: volumeName, - MountPath: "/consul/connect-inject", - }) - } - } -} - -func (w *ConnectWebhook) shouldInject(pod corev1.Pod, namespace string) (bool, error) { - // Don't inject in the Kubernetes system namespaces - if kubeSystemNamespaces.Contains(namespace) { - return false, nil - } - - // Namespace logic - // If in deny list, don't inject - if w.DenyK8sNamespacesSet.Contains(namespace) { - return false, nil - } - - // If not in allow list or allow list is not *, don't inject - if !w.AllowK8sNamespacesSet.Contains("*") && !w.AllowK8sNamespacesSet.Contains(namespace) { - return false, nil - } - - // If we already injected then don't inject again - if pod.Annotations[keyInjectStatus] != "" { - return false, nil - } - - // If the explicit true/false is on, then take that value. Note that - // this has to be the last check since it sets a default value after - // all other checks. - if raw, ok := pod.Annotations[annotationInject]; ok { - return strconv.ParseBool(raw) - } - - return !w.RequireAnnotation, nil -} - -func (w *ConnectWebhook) defaultAnnotations(pod *corev1.Pod, podJson string) error { - if pod.Annotations == nil { - pod.Annotations = make(map[string]string) - } - - // Default service port is the first port exported in the container - if _, ok := pod.ObjectMeta.Annotations[annotationPort]; !ok { - if cs := pod.Spec.Containers; len(cs) > 0 { - if ps := cs[0].Ports; len(ps) > 0 { - if ps[0].Name != "" { - pod.Annotations[annotationPort] = ps[0].Name - } else { - pod.Annotations[annotationPort] = strconv.Itoa(int(ps[0].ContainerPort)) - } - } - } - } - pod.Annotations[annotationOriginalPod] = podJson - - return nil -} - -// prometheusAnnotations sets the Prometheus scraping configuration -// annotations on the Pod. -func (w *ConnectWebhook) prometheusAnnotations(pod *corev1.Pod) error { - enableMetrics, err := w.MetricsConfig.enableMetrics(*pod) - if err != nil { - return err - } - prometheusScrapePort, err := w.MetricsConfig.prometheusScrapePort(*pod) - if err != nil { - return err - } - prometheusScrapePath := w.MetricsConfig.prometheusScrapePath(*pod) - - if enableMetrics { - pod.Annotations[annotationPrometheusScrape] = "true" - pod.Annotations[annotationPrometheusPort] = prometheusScrapePort - pod.Annotations[annotationPrometheusPath] = prometheusScrapePath - } - return nil -} - -// consulNamespace returns the namespace that a service should be -// registered in based on the namespace options. It returns an -// empty string if namespaces aren't enabled. -func (w *ConnectWebhook) consulNamespace(ns string) string { - return namespaces.ConsulNamespace(ns, w.EnableNamespaces, w.ConsulDestinationNamespace, w.EnableK8SNSMirroring, w.K8SNSMirroringPrefix) -} - -func (w *ConnectWebhook) validatePod(pod corev1.Pod) error { - if _, ok := pod.Annotations[annotationProtocol]; ok { - return fmt.Errorf("the %q annotation is no longer supported. Instead, create a ServiceDefaults resource (see www.consul.io/docs/k8s/crds/upgrade-to-crds)", - annotationProtocol) - } - - if _, ok := pod.Annotations[annotationSyncPeriod]; ok { - return fmt.Errorf("the %q annotation is no longer supported because consul-sidecar is no longer injected to periodically register services", annotationSyncPeriod) - } - return nil -} - -func portValue(pod corev1.Pod, value string) (int32, error) { - value = strings.Split(value, ",")[0] - // First search for the named port. - for _, c := range pod.Spec.Containers { - for _, p := range c.Ports { - if p.Name == value { - return p.ContainerPort, nil - } - } - } - - // Named port not found, return the parsed value. - raw, err := strconv.ParseInt(value, 0, 32) - return int32(raw), err -} - -func findServiceAccountVolumeMount(pod corev1.Pod, multiPort bool, multiPortSvcName string) (corev1.VolumeMount, string, error) { - // In the case of a multiPort pod, there may be another service account - // token mounted as a different volume. Its name must be -serviceaccount. - // If not we'll fall back to the service account for the pod. - if multiPort { - for _, v := range pod.Spec.Volumes { - if v.Name == fmt.Sprintf("%s-service-account", multiPortSvcName) { - mountPath := fmt.Sprintf("/consul/serviceaccount-%s", multiPortSvcName) - return corev1.VolumeMount{ - Name: v.Name, - ReadOnly: true, - MountPath: mountPath, - }, filepath.Join(mountPath, "token"), nil - } - } - } - - // Find the volume mount that is mounted at the known - // service account token location - var volumeMount corev1.VolumeMount - for _, container := range pod.Spec.Containers { - for _, vm := range container.VolumeMounts { - if vm.MountPath == "/var/run/secrets/kubernetes.io/serviceaccount" { - volumeMount = vm - break - } - } - } - - // Return an error if volumeMount is still empty - if (corev1.VolumeMount{}) == volumeMount { - return volumeMount, "", errors.New("unable to find service account token volumeMount") - } - - return volumeMount, "/var/run/secrets/kubernetes.io/serviceaccount/token", nil -} - -func (w *ConnectWebhook) annotatedServiceNames(pod corev1.Pod) []string { - var annotatedSvcNames []string - if anno, ok := pod.Annotations[annotationService]; ok { - annotatedSvcNames = strings.Split(anno, ",") - } - return annotatedSvcNames -} - -func (w *ConnectWebhook) checkUnsupportedMultiPortCases(ns corev1.Namespace, pod corev1.Pod) error { - tproxyEnabled, err := transparentProxyEnabled(ns, pod, w.EnableTransparentProxy) - if err != nil { - return fmt.Errorf("couldn't check if tproxy is enabled: %s", err) - } - metricsEnabled, err := w.MetricsConfig.enableMetrics(pod) - if err != nil { - return fmt.Errorf("couldn't check if metrics is enabled: %s", err) - } - metricsMergingEnabled, err := w.MetricsConfig.enableMetricsMerging(pod) - if err != nil { - return fmt.Errorf("couldn't check if metrics merging is enabled: %s", err) - } - if tproxyEnabled { - return fmt.Errorf("multi port services are not compatible with transparent proxy") - } - if metricsEnabled { - return fmt.Errorf("multi port services are not compatible with metrics") - } - if metricsMergingEnabled { - return fmt.Errorf("multi port services are not compatible with metrics merging") - } - return nil -} - -func (w *ConnectWebhook) InjectDecoder(d *admission.Decoder) error { - w.decoder = d - return nil -} - -func sliceContains(slice []string, entry string) bool { - for _, s := range slice { - if entry == s { - return true - } - } - return false -} diff --git a/control-plane/connect-inject/connect_webhook_ent_test.go b/control-plane/connect-inject/connect_webhook_ent_test.go deleted file mode 100644 index 24e5ce7d11..0000000000 --- a/control-plane/connect-inject/connect_webhook_ent_test.go +++ /dev/null @@ -1,692 +0,0 @@ -//go:build enterprise - -package connectinject - -import ( - "context" - "testing" - "time" - - "github.com/deckarep/golang-set" - logrtest "github.com/go-logr/logr/testing" - "github.com/hashicorp/consul/api" - "github.com/hashicorp/consul/sdk/testutil" - "github.com/hashicorp/consul/sdk/testutil/retry" - "github.com/stretchr/testify/require" - admissionv1 "k8s.io/api/admission/v1" - corev1 "k8s.io/api/core/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" -) - -// This tests the checkAndCreate namespace function that is called -// in handler.Mutate. Patch generation is tested in the non-enterprise -// tests. Other namespace-specific logic is tested directly in the -// specific methods (shouldInject, consulNamespace). -func TestHandler_MutateWithNamespaces(t *testing.T) { - t.Parallel() - - basicSpec := corev1.PodSpec{ - Containers: []corev1.Container{ - corev1.Container{ - Name: "web", - }, - }, - } - s := runtime.NewScheme() - s.AddKnownTypes(schema.GroupVersion{Group: "", Version: "v1"}, &corev1.Pod{}) - decoder, err := admission.NewDecoder(s) - require.NoError(t, err) - - cases := []struct { - Name string - Handler ConnectWebhook - Req admission.Request - ExpectedNamespaces []string - }{ - { - Name: "single destination namespace 'default' from k8s 'default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "default", - }, - }, - ExpectedNamespaces: []string{"default"}, - }, - - { - Name: "single destination namespace 'default' from k8s 'non-default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", - decoder: decoder, - Clientset: clientWithNamespace("non-default"), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "non-default", - }, - }, - ExpectedNamespaces: []string{"default"}, - }, - - { - Name: "single destination namespace 'dest' from k8s 'default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "dest", - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "default", - }, - }, - ExpectedNamespaces: []string{"default", "dest"}, - }, - - { - Name: "single destination namespace 'dest' from k8s 'non-default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "dest", - decoder: decoder, - Clientset: clientWithNamespace("non-default"), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "non-default", - }, - }, - ExpectedNamespaces: []string{"default", "dest"}, - }, - - { - Name: "mirroring from k8s 'default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", // will be overridden - EnableK8SNSMirroring: true, - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "default", - }, - }, - ExpectedNamespaces: []string{"default"}, - }, - - { - Name: "mirroring from k8s 'dest'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", // will be overridden - EnableK8SNSMirroring: true, - decoder: decoder, - Clientset: clientWithNamespace("dest"), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "dest", - }, - }, - ExpectedNamespaces: []string{"default", "dest"}, - }, - - { - Name: "mirroring with prefix from k8s 'default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", // will be overridden - EnableK8SNSMirroring: true, - K8SNSMirroringPrefix: "k8s-", - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "default", - }, - }, - ExpectedNamespaces: []string{"default", "k8s-default"}, - }, - - { - Name: "mirroring with prefix from k8s 'dest'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", // will be overridden - EnableK8SNSMirroring: true, - K8SNSMirroringPrefix: "k8s-", - decoder: decoder, - Clientset: clientWithNamespace("dest"), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "dest", - }, - }, - ExpectedNamespaces: []string{"default", "k8s-dest"}, - }, - } - - for _, tt := range cases { - t.Run(tt.Name, func(t *testing.T) { - require := require.New(t) - - // Set up consul server - a, err := testutil.NewTestServerConfigT(t, nil) - require.NoError(err) - a.WaitForSerfCheck(t) - defer a.Stop() - - // Set up consul client - client, err := api.NewClient(&api.Config{ - Address: a.HTTPAddr, - }) - require.NoError(err) - - // Add the client to the test's handler - tt.Handler.ConsulClient = client - - // Mutate! - resp := tt.Handler.Handle(context.Background(), tt.Req) - require.Equal(resp.Allowed, true) - - // Check all the namespace things - // Check that we have the right number of namespaces - namespaces, _, err := client.Namespaces().List(&api.QueryOptions{}) - require.NoError(err) - require.Len(namespaces, len(tt.ExpectedNamespaces)) - - // Check the namespace details - for _, ns := range tt.ExpectedNamespaces { - actNamespace, _, err := client.Namespaces().Read(ns, &api.QueryOptions{}) - require.NoErrorf(err, "error getting namespace %s", ns) - require.NotNilf(actNamespace, "namespace %s was nil", ns) - require.Equalf(ns, actNamespace.Name, "namespace %s was improperly named", ns) - - // Check created namespace properties - if ns != "default" { - require.Equalf("Auto-generated by consul-k8s", actNamespace.Description, - "wrong namespace description for namespace %s", ns) - require.Containsf(actNamespace.Meta, "external-source", - "namespace %s does not contain external-source metadata key", ns) - require.Equalf("kubernetes", actNamespace.Meta["external-source"], - "namespace %s has wrong value for external-source metadata key", ns) - } - - } - }) - } -} - -// Tests that the correct cross-namespace policy is -// added to created namespaces. -func TestHandler_MutateWithNamespaces_ACLs(t *testing.T) { - basicSpec := corev1.PodSpec{ - Containers: []corev1.Container{ - corev1.Container{ - Name: "web", - }, - }, - } - - s := runtime.NewScheme() - s.AddKnownTypes(schema.GroupVersion{Group: "", Version: "v1"}, &corev1.Pod{}) - decoder, err := admission.NewDecoder(s) - require.NoError(t, err) - - cases := []struct { - Name string - Handler ConnectWebhook - Req admission.Request - ExpectedNamespaces []string - }{ - { - Name: "acls + single destination namespace 'default' from k8s 'default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", - CrossNamespaceACLPolicy: "cross-namespace-policy", - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "default", - }, - }, - ExpectedNamespaces: []string{"default"}, - }, - - { - Name: "acls + single destination namespace 'default' from k8s 'non-default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", - CrossNamespaceACLPolicy: "cross-namespace-policy", - decoder: decoder, - Clientset: clientWithNamespace("non-default"), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "non-default", - }, - }, - ExpectedNamespaces: []string{"default"}, - }, - - { - Name: "acls + single destination namespace 'dest' from k8s 'default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "dest", - CrossNamespaceACLPolicy: "cross-namespace-policy", - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "default", - }, - }, - ExpectedNamespaces: []string{"default", "dest"}, - }, - - { - Name: "acls + single destination namespace 'dest' from k8s 'non-default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "dest", - CrossNamespaceACLPolicy: "cross-namespace-policy", - decoder: decoder, - Clientset: clientWithNamespace("non-default"), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "non-default", - }, - }, - ExpectedNamespaces: []string{"default", "dest"}, - }, - - { - Name: "acls + mirroring from k8s 'default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", // will be overridden - EnableK8SNSMirroring: true, - CrossNamespaceACLPolicy: "cross-namespace-policy", - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "default", - }, - }, - ExpectedNamespaces: []string{"default"}, - }, - - { - Name: "acls + mirroring from k8s 'dest'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", // will be overridden - EnableK8SNSMirroring: true, - CrossNamespaceACLPolicy: "cross-namespace-policy", - decoder: decoder, - Clientset: clientWithNamespace("dest"), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "dest", - }, - }, - ExpectedNamespaces: []string{"default", "dest"}, - }, - - { - Name: "acls + mirroring with prefix from k8s 'default'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", // will be overridden - EnableK8SNSMirroring: true, - K8SNSMirroringPrefix: "k8s-", - CrossNamespaceACLPolicy: "cross-namespace-policy", - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "default", - }, - }, - ExpectedNamespaces: []string{"default", "k8s-default"}, - }, - - { - Name: "acls + mirroring with prefix from k8s 'dest'", - Handler: ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: "default", // will be overridden - EnableK8SNSMirroring: true, - K8SNSMirroringPrefix: "k8s-", - CrossNamespaceACLPolicy: "cross-namespace-policy", - decoder: decoder, - Clientset: clientWithNamespace("dest"), - }, - Req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - Namespace: "dest", - }, - }, - ExpectedNamespaces: []string{"default", "k8s-dest"}, - }, - } - - for _, tt := range cases { - t.Run(tt.Name, func(t *testing.T) { - // Set up consul server - a, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { - c.ACL.Enabled = true - }) - a.WaitForSerfCheck(t) - defer a.Stop() - - // Set up a client for bootstrapping - bootClient, err := api.NewClient(&api.Config{ - Address: a.HTTPAddr, - }) - require.NoError(t, err) - - // Bootstrap the server and get the bootstrap token - var bootstrapResp *api.ACLToken - timer := &retry.Timer{Timeout: 10 * time.Second, Wait: 500 * time.Millisecond} - retry.RunWith(timer, t, func(r *retry.R) { - bootstrapResp, _, err = bootClient.ACL().Bootstrap() - require.NoError(r, err) - }) - bootstrapToken := bootstrapResp.SecretID - require.NotEmpty(t, bootstrapToken) - - // Set up consul client - client, err := api.NewClient(&api.Config{ - Address: a.HTTPAddr, - Token: bootstrapToken, - }) - require.NoError(t, err) - - // Add the client to the test's handler - tt.Handler.ConsulClient = client - - // Create cross namespace policy - // This would have been created by the acl bootstrapper in the - // default namespace to be attached to all created namespaces. - crossNamespaceRules := `namespace_prefix "" { - service_prefix "" { - policy = "read" - } - node_prefix "" { - policy = "read" - } -} ` - - policyTmpl := api.ACLPolicy{ - Name: "cross-namespace-policy", - Description: "Policy to allow permissions to cross Consul namespaces for k8s services", - Rules: crossNamespaceRules, - } - - _, _, err = client.ACL().PolicyCreate(&policyTmpl, &api.WriteOptions{}) - require.NoError(t, err) - - // Mutate! - resp := tt.Handler.Handle(context.Background(), tt.Req) - require.Equal(t, resp.Allowed, true) - - // Check all the namespace things - // Check that we have the right number of namespaces - namespaces, _, err := client.Namespaces().List(&api.QueryOptions{}) - require.NoError(t, err) - require.Len(t, namespaces, len(tt.ExpectedNamespaces)) - - // Check the namespace details - for _, ns := range tt.ExpectedNamespaces { - actNamespace, _, err := client.Namespaces().Read(ns, &api.QueryOptions{}) - require.NoErrorf(t, err, "error getting namespace %s", ns) - require.NotNilf(t, actNamespace, "namespace %s was nil", ns) - require.Equalf(t, ns, actNamespace.Name, "namespace %s was improperly named", ns) - - // Check created namespace properties - if ns != "default" { - require.Equalf(t, "Auto-generated by consul-k8s", actNamespace.Description, - "wrong namespace description for namespace %s", ns) - require.Containsf(t, actNamespace.Meta, "external-source", - "namespace %s does not contain external-source metadata key", ns) - require.Equalf(t, "kubernetes", actNamespace.Meta["external-source"], - "namespace %s has wrong value for external-source metadata key", ns) - - // Check for ACL policy things - // The acl bootstrapper will update the `default` namespace, so that - // can't be tested here. - require.NotNilf(t, actNamespace.ACLs, "ACLs was nil for namespace %s", ns) - require.Lenf(t, actNamespace.ACLs.PolicyDefaults, 1, "wrong length for PolicyDefaults in namespace %s", ns) - require.Equalf(t, "cross-namespace-policy", actNamespace.ACLs.PolicyDefaults[0].Name, - "wrong policy name for namespace %s", ns) - } - - } - }) - } -} - -// Test that the annotation for the Consul namespace is added. -func TestHandler_MutateWithNamespaces_Annotation(t *testing.T) { - t.Parallel() - sourceKubeNS := "kube-ns" - - cases := map[string]struct { - ConsulDestinationNamespace string - Mirroring bool - MirroringPrefix string - ExpNamespaceAnnotation string - }{ - "dest: default": { - ConsulDestinationNamespace: "default", - ExpNamespaceAnnotation: "default", - }, - "dest: foo": { - ConsulDestinationNamespace: "foo", - ExpNamespaceAnnotation: "foo", - }, - "mirroring": { - Mirroring: true, - ExpNamespaceAnnotation: sourceKubeNS, - }, - "mirroring with prefix": { - Mirroring: true, - MirroringPrefix: "prefix-", - ExpNamespaceAnnotation: "prefix-" + sourceKubeNS, - }, - } - - for name, c := range cases { - t.Run(name, func(t *testing.T) { - require := require.New(t) - - // Set up consul server - a, err := testutil.NewTestServerConfigT(t, nil) - require.NoError(err) - a.WaitForSerfCheck(t) - defer a.Stop() - - s := runtime.NewScheme() - s.AddKnownTypes(schema.GroupVersion{Group: "", Version: "v1"}, &corev1.Pod{}) - decoder, err := admission.NewDecoder(s) - require.NoError(err) - - // Set up consul client - client, err := api.NewClient(&api.Config{ - Address: a.HTTPAddr, - }) - require.NoError(err) - - handler := ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSet("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableNamespaces: true, - ConsulDestinationNamespace: c.ConsulDestinationNamespace, - EnableK8SNSMirroring: c.Mirroring, - K8SNSMirroringPrefix: c.MirroringPrefix, - ConsulClient: client, - decoder: decoder, - Clientset: clientWithNamespace(sourceKubeNS), - } - - pod := corev1.Pod{ - ObjectMeta: v1.ObjectMeta{ - Namespace: sourceKubeNS, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - }, - }, - }, - } - request := admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &pod), - Namespace: sourceKubeNS, - }, - } - resp := handler.Handle(context.Background(), request) - require.Equal(resp.Allowed, true) - - // Check that the annotation was added as a patch. - var consulNamespaceAnnotationValue string - for _, patch := range resp.Patches { - if patch.Path == "/metadata/annotations" { - for annotationName, annotationValue := range patch.Value.(map[string]interface{}) { - if annotationName == annotationConsulNamespace { - consulNamespaceAnnotationValue = annotationValue.(string) - } - } - } - } - require.NotEmpty(consulNamespaceAnnotationValue, "no namespace annotation set") - require.Equal(c.ExpNamespaceAnnotation, consulNamespaceAnnotationValue) - }) - } -} diff --git a/control-plane/connect-inject/connect_webhook_test.go b/control-plane/connect-inject/connect_webhook_test.go deleted file mode 100644 index 92bae3d872..0000000000 --- a/control-plane/connect-inject/connect_webhook_test.go +++ /dev/null @@ -1,1851 +0,0 @@ -package connectinject - -import ( - "context" - "encoding/json" - "strings" - "testing" - - mapset "github.com/deckarep/golang-set" - logrtest "github.com/go-logr/logr/testing" - "github.com/hashicorp/consul-k8s/control-plane/namespaces" - "github.com/stretchr/testify/require" - "gomodules.xyz/jsonpatch/v2" - admissionv1 "k8s.io/api/admission/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/kubernetes/fake" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" -) - -func TestHandlerHandle(t *testing.T) { - t.Parallel() - basicSpec := corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - }, - }, - } - s := runtime.NewScheme() - s.AddKnownTypes(schema.GroupVersion{ - Group: "", - Version: "v1", - }, &corev1.Pod{}) - decoder, err := admission.NewDecoder(s) - require.NoError(t, err) - - cases := []struct { - Name string - Handler ConnectWebhook - Req admission.Request - Err string // expected error string, not exact - Patches []jsonpatch.Operation - }{ - { - "kube-system namespace", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: metav1.NamespaceSystem, - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - }, - }, - "", - nil, - }, - - { - "already injected", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Object: encodeRaw(t, &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - keyInjectStatus: injected, - }, - }, - Spec: basicSpec, - }), - }, - }, - "", - nil, - }, - - { - "empty pod basic", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - }), - }, - }, - "", - []jsonpatch.Operation{ - { - Operation: "add", - Path: "/metadata/labels", - }, - { - Operation: "add", - Path: "/metadata/annotations", - }, - { - Operation: "add", - Path: "/spec/volumes", - }, - { - Operation: "add", - Path: "/spec/initContainers", - }, - { - Operation: "add", - Path: "/spec/containers/1", - }, - }, - }, - - { - "pod with upstreams specified", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationUpstreams: "echo:1234,db:1234", - }, - }, - Spec: basicSpec, - }), - }, - }, - "", - []jsonpatch.Operation{ - { - Operation: "add", - Path: "/metadata/labels", - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(keyInjectStatus), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationOriginalPod), - }, - { - Operation: "add", - Path: "/spec/volumes", - }, - { - Operation: "add", - Path: "/spec/initContainers", - }, - { - Operation: "add", - Path: "/spec/containers/1", - }, - { - Operation: "add", - Path: "/spec/containers/0/env", - }, - }, - }, - - { - "empty pod with injection disabled", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationInject: "false", - }, - }, - Spec: basicSpec, - }), - }, - }, - "", - nil, - }, - - { - "empty pod with injection truthy", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationInject: "t", - }, - }, - Spec: basicSpec, - }), - }, - }, - "", - []jsonpatch.Operation{ - { - Operation: "add", - Path: "/spec/volumes", - }, - { - Operation: "add", - Path: "/spec/initContainers", - }, - { - Operation: "add", - Path: "/spec/containers/1", - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(keyInjectStatus), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationOriginalPod), - }, - { - Operation: "add", - Path: "/metadata/labels", - }, - }, - }, - - { - "pod with empty volume mount annotation", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationInjectMountVolumes: "", - }, - }, - Spec: basicSpec, - }), - }, - }, - "", - []jsonpatch.Operation{ - { - Operation: "add", - Path: "/spec/volumes", - }, - { - Operation: "add", - Path: "/spec/initContainers", - }, - { - Operation: "add", - Path: "/spec/containers/1", - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(keyInjectStatus), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationOriginalPod), - }, - { - Operation: "add", - Path: "/metadata/labels", - }, - }, - }, - { - "pod with volume mount annotation", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationInjectMountVolumes: "web,unknown,web_three_point_oh", - }, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - }, - { - Name: "web_two_point_oh", - }, - { - Name: "web_three_point_oh", - }, - }, - }, - }), - }, - }, - "", - []jsonpatch.Operation{ - { - Operation: "add", - Path: "/spec/volumes", - }, - { - Operation: "add", - Path: "/spec/containers/0/volumeMounts", - }, - { - Operation: "add", - Path: "/spec/containers/2/volumeMounts", - }, - { - Operation: "add", - Path: "/spec/initContainers", - }, - { - Operation: "add", - Path: "/spec/containers/3", - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(keyInjectStatus), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationOriginalPod), - }, - { - Operation: "add", - Path: "/metadata/labels", - }, - }, - }, - - { - "pod with service annotation", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "foo", - }, - }, - }), - }, - }, - "", - []jsonpatch.Operation{ - { - Operation: "add", - Path: "/spec/volumes", - }, - { - Operation: "add", - Path: "/spec/initContainers", - }, - { - Operation: "add", - Path: "/spec/containers/1", - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(keyInjectStatus), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationOriginalPod), - }, - { - Operation: "add", - Path: "/metadata/labels", - }, - }, - }, - - { - "pod with existing label", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "testLabel": "123", - }, - }, - Spec: basicSpec, - }), - }, - }, - "", - []jsonpatch.Operation{ - { - Operation: "add", - Path: "/spec/volumes", - }, - { - Operation: "add", - Path: "/spec/initContainers", - }, - { - Operation: "add", - Path: "/spec/containers/1", - }, - { - Operation: "add", - Path: "/metadata/annotations", - }, - { - Operation: "add", - Path: "/metadata/labels/" + escapeJSONPointer(keyInjectStatus), - }, - { - Operation: "add", - Path: "/metadata/labels/" + escapeJSONPointer(keyManagedBy), - }, - }, - }, - - { - "when metrics merging is enabled, we should inject the consul-sidecar and add prometheus annotations", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - MetricsConfig: MetricsConfig{ - DefaultEnableMetrics: true, - DefaultEnableMetricsMerging: true, - }, - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "testLabel": "123", - }, - Annotations: map[string]string{ - annotationServiceMetricsPort: "1234", - }, - }, - Spec: basicSpec, - }), - }, - }, - "", - []jsonpatch.Operation{ - { - Operation: "add", - Path: "/spec/volumes", - }, - { - Operation: "add", - Path: "/spec/initContainers", - }, - { - Operation: "add", - Path: "/spec/containers/1", - }, - { - Operation: "add", - Path: "/spec/containers/2", - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(keyInjectStatus), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationOriginalPod), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationPrometheusScrape), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationPrometheusPath), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationPrometheusPort), - }, - { - Operation: "add", - Path: "/metadata/labels/" + escapeJSONPointer(keyInjectStatus), - }, - { - Operation: "add", - Path: "/metadata/labels/" + escapeJSONPointer(keyManagedBy), - }, - }, - }, - - { - "tproxy with overwriteProbes is enabled", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - EnableTransparentProxy: true, - TProxyOverwriteProbes: true, - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{}, - // We're setting an existing annotation so that we can assert on the - // specific annotations that are set as a result of probes being overwritten. - Annotations: map[string]string{"foo": "bar"}, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - LivenessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8080), - }, - }, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8081), - }, - }, - }, - }, - }, - }, - }), - }, - }, - "", - []jsonpatch.Operation{ - { - Operation: "add", - Path: "/spec/volumes", - }, - { - Operation: "add", - Path: "/spec/initContainers", - }, - { - Operation: "add", - Path: "/spec/containers/1", - }, - { - Operation: "add", - Path: "/metadata/labels", - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(keyInjectStatus), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationOriginalPod), - }, - { - Operation: "replace", - Path: "/spec/containers/0/livenessProbe/httpGet/port", - }, - { - Operation: "replace", - Path: "/spec/containers/0/readinessProbe/httpGet/port", - }, - }, - }, - { - "multi port pod", - ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - Clientset: defaultTestClientWithNamespace(), - }, - admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: namespaces.DefaultNamespace, - Object: encodeRaw(t, &corev1.Pod{ - Spec: basicSpec, - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "web, web-admin", - }, - }, - }), - }, - }, - "", - []jsonpatch.Operation{ - { - Operation: "add", - Path: "/spec/volumes", - }, - { - Operation: "add", - Path: "/spec/initContainers", - }, - { - Operation: "add", - Path: "/spec/containers/1", - }, - { - Operation: "add", - Path: "/spec/containers/2", - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(keyInjectStatus), - }, - { - Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationOriginalPod), - }, - { - Operation: "add", - Path: "/metadata/labels", - }, - }, - }, - } - - for _, tt := range cases { - t.Run(tt.Name, func(t *testing.T) { - require := require.New(t) - ctx := context.Background() - resp := tt.Handler.Handle(ctx, tt.Req) - if (tt.Err == "") != resp.Allowed { - t.Fatalf("allowed: %v, expected err: %v", resp.Allowed, tt.Err) - } - if tt.Err != "" { - require.Contains(resp.Result.Message, tt.Err) - return - } - - actual := resp.Patches - if len(actual) > 0 { - for i := range actual { - actual[i].Value = nil - } - } - require.ElementsMatch(tt.Patches, actual) - }) - } -} - -// Test that we error out when deprecated annotations are set. -func TestHandler_ErrorsOnDeprecatedAnnotations(t *testing.T) { - cases := []struct { - name string - annotations map[string]string - expErr string - }{ - { - "default protocol annotation", - map[string]string{ - annotationProtocol: "http", - }, - "the \"consul.hashicorp.com/connect-service-protocol\" annotation is no longer supported. Instead, create a ServiceDefaults resource (see www.consul.io/docs/k8s/crds/upgrade-to-crds)", - }, - { - "sync period annotation", - map[string]string{ - annotationSyncPeriod: "30s", - }, - "the \"consul.hashicorp.com/connect-sync-period\" annotation is no longer supported because consul-sidecar is no longer injected to periodically register services", - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - require := require.New(t) - s := runtime.NewScheme() - s.AddKnownTypes(schema.GroupVersion{ - Group: "", - Version: "v1", - }, &corev1.Pod{}) - decoder, err := admission.NewDecoder(s) - require.NoError(err) - - handler := ConnectWebhook{ - Log: logrtest.TestLogger{T: t}, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSet(), - decoder: decoder, - } - - request := admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Namespace: "default", - Object: encodeRaw(t, &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: c.annotations, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - }, - }, - }, - }), - }, - } - - response := handler.Handle(context.Background(), request) - require.False(response.Allowed) - require.Equal(c.expErr, response.Result.Message) - }) - } -} - -func TestHandlerDefaultAnnotations(t *testing.T) { - cases := []struct { - Name string - Pod *corev1.Pod - Expected map[string]string - Err string - }{ - { - "empty", - &corev1.Pod{}, - map[string]string{ - annotationOriginalPod: "{\"metadata\":{\"creationTimestamp\":null},\"spec\":{\"containers\":null},\"status\":{}}", - }, - "", - }, - - { - "basic pod, no ports", - &corev1.Pod{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - }, - { - Name: "web-side", - }, - }, - }, - }, - map[string]string{ - annotationOriginalPod: "{\"metadata\":{\"creationTimestamp\":null},\"spec\":{\"containers\":[{\"name\":\"web\",\"resources\":{}},{\"name\":\"web-side\",\"resources\":{}}]},\"status\":{}}", - }, - "", - }, - - { - "basic pod, name annotated", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "foo", - }, - }, - - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - }, - { - Name: "web-side", - }, - }, - }, - }, - map[string]string{ - "consul.hashicorp.com/connect-service": "foo", - annotationOriginalPod: "{\"metadata\":{\"creationTimestamp\":null,\"annotations\":{\"consul.hashicorp.com/connect-service\":\"foo\"}},\"spec\":{\"containers\":[{\"name\":\"web\",\"resources\":{}},{\"name\":\"web-side\",\"resources\":{}}]},\"status\":{}}", - }, - - "", - }, - - { - "basic pod, with ports", - &corev1.Pod{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - Ports: []corev1.ContainerPort{ - { - Name: "http", - ContainerPort: 8080, - }, - }, - }, - { - Name: "web-side", - }, - }, - }, - }, - map[string]string{ - annotationPort: "http", - annotationOriginalPod: "{\"metadata\":{\"creationTimestamp\":null},\"spec\":{\"containers\":[{\"name\":\"web\",\"ports\":[{\"name\":\"http\",\"containerPort\":8080}],\"resources\":{}},{\"name\":\"web-side\",\"resources\":{}}]},\"status\":{}}", - }, - "", - }, - - { - "basic pod, with unnamed ports", - &corev1.Pod{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - Ports: []corev1.ContainerPort{ - { - ContainerPort: 8080, - }, - }, - }, - { - Name: "web-side", - }, - }, - }, - }, - map[string]string{ - annotationPort: "8080", - annotationOriginalPod: "{\"metadata\":{\"creationTimestamp\":null},\"spec\":{\"containers\":[{\"name\":\"web\",\"ports\":[{\"containerPort\":8080}],\"resources\":{}},{\"name\":\"web-side\",\"resources\":{}}]},\"status\":{}}", - }, - "", - }, - } - - for _, tt := range cases { - t.Run(tt.Name, func(t *testing.T) { - require := require.New(t) - - podJson, err := json.Marshal(tt.Pod) - require.NoError(err) - - var h ConnectWebhook - err = h.defaultAnnotations(tt.Pod, string(podJson)) - if (tt.Err != "") != (err != nil) { - t.Fatalf("actual: %v, expected err: %v", err, tt.Err) - } - if tt.Err != "" { - require.Contains(err.Error(), tt.Err) - return - } - - actual := tt.Pod.Annotations - if len(actual) == 0 { - actual = nil - } - require.Equal(tt.Expected, actual) - }) - } -} - -func TestHandlerPrometheusAnnotations(t *testing.T) { - cases := []struct { - Name string - Handler ConnectWebhook - Expected map[string]string - }{ - { - Name: "Sets the correct prometheus annotations on the pod if metrics are enabled", - Handler: ConnectWebhook{ - MetricsConfig: MetricsConfig{ - DefaultEnableMetrics: true, - DefaultPrometheusScrapePort: "20200", - DefaultPrometheusScrapePath: "/metrics", - }, - }, - Expected: map[string]string{ - annotationPrometheusScrape: "true", - annotationPrometheusPort: "20200", - annotationPrometheusPath: "/metrics", - }, - }, - { - Name: "Does not set annotations if metrics are not enabled", - Handler: ConnectWebhook{ - MetricsConfig: MetricsConfig{ - DefaultEnableMetrics: false, - DefaultPrometheusScrapePort: "20200", - DefaultPrometheusScrapePath: "/metrics", - }, - }, - Expected: map[string]string{}, - }, - } - - for _, tt := range cases { - t.Run(tt.Name, func(t *testing.T) { - require := require.New(t) - h := tt.Handler - pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}}} - - err := h.prometheusAnnotations(pod) - require.NoError(err) - - require.Equal(pod.Annotations, tt.Expected) - }) - } -} - -// Test portValue function. -func TestHandlerPortValue(t *testing.T) { - cases := []struct { - Name string - Pod *corev1.Pod - Value string - Expected int32 - Err string - }{ - { - "empty", - &corev1.Pod{}, - "", - 0, - "strconv.ParseInt: parsing \"\": invalid syntax", - }, - - { - "basic pod, with ports", - &corev1.Pod{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - Ports: []corev1.ContainerPort{ - { - Name: "http", - ContainerPort: 8080, - }, - }, - }, - - { - Name: "web-side", - }, - }, - }, - }, - "http", - int32(8080), - "", - }, - - { - "basic pod, with unnamed ports", - &corev1.Pod{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "web", - Ports: []corev1.ContainerPort{ - { - ContainerPort: 8080, - }, - }, - }, - - { - Name: "web-side", - }, - }, - }, - }, - "8080", - int32(8080), - "", - }, - } - - for _, tt := range cases { - t.Run(tt.Name, func(t *testing.T) { - require := require.New(t) - - port, err := portValue(*tt.Pod, tt.Value) - if (tt.Err != "") != (err != nil) { - t.Fatalf("actual: %v, expected err: %v", err, tt.Err) - } - if tt.Err != "" { - require.Contains(err.Error(), tt.Err) - return - } - - require.Equal(tt.Expected, port) - }) - } -} - -// Test consulNamespace function. -func TestConsulNamespace(t *testing.T) { - cases := []struct { - Name string - EnableNamespaces bool - ConsulDestinationNamespace string - EnableK8SNSMirroring bool - K8SNSMirroringPrefix string - K8sNamespace string - Expected string - }{ - { - "namespaces disabled", - false, - "default", - false, - "", - "namespace", - "", - }, - - { - "namespaces disabled, mirroring enabled", - false, - "default", - true, - "", - "namespace", - "", - }, - - { - "namespaces disabled, mirroring enabled, prefix defined", - false, - "default", - true, - "test-", - "namespace", - "", - }, - - { - "namespaces enabled, mirroring disabled", - true, - "default", - false, - "", - "namespace", - "default", - }, - - { - "namespaces enabled, mirroring disabled, prefix defined", - true, - "default", - false, - "test-", - "namespace", - "default", - }, - - { - "namespaces enabled, mirroring enabled", - true, - "default", - true, - "", - "namespace", - "namespace", - }, - - { - "namespaces enabled, mirroring enabled, prefix defined", - true, - "default", - true, - "test-", - "namespace", - "test-namespace", - }, - } - - for _, tt := range cases { - t.Run(tt.Name, func(t *testing.T) { - require := require.New(t) - - h := ConnectWebhook{ - EnableNamespaces: tt.EnableNamespaces, - ConsulDestinationNamespace: tt.ConsulDestinationNamespace, - EnableK8SNSMirroring: tt.EnableK8SNSMirroring, - K8SNSMirroringPrefix: tt.K8SNSMirroringPrefix, - } - - ns := h.consulNamespace(tt.K8sNamespace) - - require.Equal(tt.Expected, ns) - }) - } -} - -// Test shouldInject function. -func TestShouldInject(t *testing.T) { - cases := []struct { - Name string - Pod *corev1.Pod - K8sNamespace string - EnableNamespaces bool - AllowK8sNamespacesSet mapset.Set - DenyK8sNamespacesSet mapset.Set - Expected bool - }{ - { - "kube-system not injected", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - // Service annotation is required for injection - annotationService: "testing", - }, - }, - }, - "kube-system", - false, - mapset.NewSet(), - mapset.NewSet(), - false, - }, - { - "kube-public not injected", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "kube-public", - false, - mapset.NewSet(), - mapset.NewSet(), - false, - }, - { - "namespaces disabled, empty allow/deny lists", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - false, - mapset.NewSet(), - mapset.NewSet(), - false, - }, - { - "namespaces disabled, allow *", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - false, - mapset.NewSetWith("*"), - mapset.NewSet(), - true, - }, - { - "namespaces disabled, allow default", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - false, - mapset.NewSetWith("default"), - mapset.NewSet(), - true, - }, - { - "namespaces disabled, allow * and default", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - false, - mapset.NewSetWith("*", "default"), - mapset.NewSet(), - true, - }, - { - "namespaces disabled, allow only ns1 and ns2", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - false, - mapset.NewSetWith("ns1", "ns2"), - mapset.NewSet(), - false, - }, - { - "namespaces disabled, deny default ns", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - false, - mapset.NewSet(), - mapset.NewSetWith("default"), - false, - }, - { - "namespaces disabled, allow *, deny default ns", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - false, - mapset.NewSetWith("*"), - mapset.NewSetWith("default"), - false, - }, - { - "namespaces disabled, default ns in both allow and deny lists", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - false, - mapset.NewSetWith("default"), - mapset.NewSetWith("default"), - false, - }, - { - "namespaces enabled, empty allow/deny lists", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - true, - mapset.NewSet(), - mapset.NewSet(), - false, - }, - { - "namespaces enabled, allow *", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - true, - mapset.NewSetWith("*"), - mapset.NewSet(), - true, - }, - { - "namespaces enabled, allow default", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - true, - mapset.NewSetWith("default"), - mapset.NewSet(), - true, - }, - { - "namespaces enabled, allow * and default", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - true, - mapset.NewSetWith("*", "default"), - mapset.NewSet(), - true, - }, - { - "namespaces enabled, allow only ns1 and ns2", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - true, - mapset.NewSetWith("ns1", "ns2"), - mapset.NewSet(), - false, - }, - { - "namespaces enabled, deny default ns", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - true, - mapset.NewSet(), - mapset.NewSetWith("default"), - false, - }, - { - "namespaces enabled, allow *, deny default ns", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - true, - mapset.NewSetWith("*"), - mapset.NewSetWith("default"), - false, - }, - { - "namespaces enabled, default ns in both allow and deny lists", - &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - annotationService: "testing", - }, - }, - }, - "default", - true, - mapset.NewSetWith("default"), - mapset.NewSetWith("default"), - false, - }, - } - - for _, tt := range cases { - t.Run(tt.Name, func(t *testing.T) { - require := require.New(t) - - h := ConnectWebhook{ - RequireAnnotation: false, - EnableNamespaces: tt.EnableNamespaces, - AllowK8sNamespacesSet: tt.AllowK8sNamespacesSet, - DenyK8sNamespacesSet: tt.DenyK8sNamespacesSet, - } - - injected, err := h.shouldInject(*tt.Pod, tt.K8sNamespace) - - require.Equal(nil, err) - require.Equal(tt.Expected, injected) - }) - } -} - -func TestOverwriteProbes(t *testing.T) { - t.Parallel() - - cases := map[string]struct { - tproxyEnabled bool - overwriteProbes bool - podContainers []corev1.Container - expLivenessPort []int - expReadinessPort []int - expStartupPort []int - additionalAnnotations map[string]string - }{ - "transparent proxy disabled; overwrites probes disabled": { - tproxyEnabled: false, - podContainers: []corev1.Container{ - { - Name: "test", - Ports: []corev1.ContainerPort{ - { - Name: "tcp", - ContainerPort: 8081, - }, - { - Name: "http", - ContainerPort: 8080, - }, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8080), - }, - }, - }, - }, - }, - expReadinessPort: []int{8080}, - }, - "transparent proxy enabled; overwrite probes disabled": { - tproxyEnabled: true, - podContainers: []corev1.Container{ - { - Name: "test", - Ports: []corev1.ContainerPort{ - { - Name: "tcp", - ContainerPort: 8081, - }, - { - Name: "http", - ContainerPort: 8080, - }, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8080), - }, - }, - }, - }, - }, - expReadinessPort: []int{8080}, - }, - "transparent proxy disabled; overwrite probes enabled": { - tproxyEnabled: false, - overwriteProbes: true, - podContainers: []corev1.Container{ - { - Name: "test", - Ports: []corev1.ContainerPort{ - { - Name: "tcp", - ContainerPort: 8081, - }, - { - Name: "http", - ContainerPort: 8080, - }, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8080), - }, - }, - }, - }, - }, - expReadinessPort: []int{8080}, - }, - "just the readiness probe": { - tproxyEnabled: true, - overwriteProbes: true, - podContainers: []corev1.Container{ - { - Name: "test", - Ports: []corev1.ContainerPort{ - { - Name: "tcp", - ContainerPort: 8081, - }, - { - Name: "http", - ContainerPort: 8080, - }, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8080), - }, - }, - }, - }, - }, - expReadinessPort: []int{exposedPathsReadinessPortsRangeStart}, - }, - "just the liveness probe": { - tproxyEnabled: true, - overwriteProbes: true, - podContainers: []corev1.Container{ - { - Name: "test", - Ports: []corev1.ContainerPort{ - { - Name: "tcp", - ContainerPort: 8081, - }, - { - Name: "http", - ContainerPort: 8080, - }, - }, - LivenessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8081), - }, - }, - }, - }, - }, - expLivenessPort: []int{exposedPathsLivenessPortsRangeStart}, - }, - "skips envoy sidecar": { - tproxyEnabled: true, - overwriteProbes: true, - podContainers: []corev1.Container{ - { - Name: envoySidecarContainer, - }, - }, - }, - "readiness, liveness and startup probes": { - tproxyEnabled: true, - overwriteProbes: true, - podContainers: []corev1.Container{ - { - Name: "test", - Ports: []corev1.ContainerPort{ - { - Name: "tcp", - ContainerPort: 8081, - }, - { - Name: "http", - ContainerPort: 8080, - }, - }, - LivenessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8081), - }, - }, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8080), - }, - }, - }, - StartupProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8082), - }, - }, - }, - }, - }, - expLivenessPort: []int{exposedPathsLivenessPortsRangeStart}, - expReadinessPort: []int{exposedPathsReadinessPortsRangeStart}, - expStartupPort: []int{exposedPathsStartupPortsRangeStart}, - }, - "readiness, liveness and startup probes multiple containers": { - tproxyEnabled: true, - overwriteProbes: true, - podContainers: []corev1.Container{ - { - Name: "test", - Ports: []corev1.ContainerPort{ - { - Name: "tcp", - ContainerPort: 8081, - }, - { - Name: "http", - ContainerPort: 8080, - }, - }, - LivenessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8081), - }, - }, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8080), - }, - }, - }, - StartupProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8080), - }, - }, - }, - }, - { - Name: "test-2", - Ports: []corev1.ContainerPort{ - { - Name: "tcp", - ContainerPort: 8083, - }, - { - Name: "http", - ContainerPort: 8082, - }, - }, - LivenessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8083), - }, - }, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8082), - }, - }, - }, - StartupProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(8082), - }, - }, - }, - }, - }, - expLivenessPort: []int{exposedPathsLivenessPortsRangeStart, exposedPathsLivenessPortsRangeStart + 1}, - expReadinessPort: []int{exposedPathsReadinessPortsRangeStart, exposedPathsReadinessPortsRangeStart + 1}, - expStartupPort: []int{exposedPathsStartupPortsRangeStart, exposedPathsStartupPortsRangeStart + 1}, - }, - } - - for name, c := range cases { - t.Run(name, func(t *testing.T) { - pod := &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{}, - Annotations: map[string]string{}, - }, - Spec: corev1.PodSpec{ - Containers: c.podContainers, - }, - } - if c.additionalAnnotations != nil { - pod.ObjectMeta.Annotations = c.additionalAnnotations - } - - h := ConnectWebhook{ - EnableTransparentProxy: c.tproxyEnabled, - TProxyOverwriteProbes: c.overwriteProbes, - } - err := h.overwriteProbes(corev1.Namespace{}, pod) - require.NoError(t, err) - for i, container := range pod.Spec.Containers { - if container.ReadinessProbe != nil { - require.Equal(t, c.expReadinessPort[i], container.ReadinessProbe.HTTPGet.Port.IntValue()) - } - if container.LivenessProbe != nil { - require.Equal(t, c.expLivenessPort[i], container.LivenessProbe.HTTPGet.Port.IntValue()) - } - if container.StartupProbe != nil { - require.Equal(t, c.expStartupPort[i], container.StartupProbe.HTTPGet.Port.IntValue()) - } - } - - }) - } -} - -func TestHandler_checkUnsupportedMultiPortCases(t *testing.T) { - cases := []struct { - name string - annotations map[string]string - expErr string - }{ - { - name: "tproxy", - annotations: map[string]string{keyTransparentProxy: "true"}, - expErr: "multi port services are not compatible with transparent proxy", - }, - { - name: "metrics", - annotations: map[string]string{annotationEnableMetrics: "true"}, - expErr: "multi port services are not compatible with metrics", - }, - { - name: "metrics merging", - annotations: map[string]string{annotationEnableMetricsMerging: "true"}, - expErr: "multi port services are not compatible with metrics merging", - }, - } - for _, tt := range cases { - t.Run(tt.name, func(t *testing.T) { - h := ConnectWebhook{} - pod := minimal() - pod.Annotations = tt.annotations - err := h.checkUnsupportedMultiPortCases(corev1.Namespace{}, *pod) - require.Error(t, err) - require.Equal(t, tt.expErr, err.Error()) - }) - - } - -} - -// encodeRaw is a helper to encode some data into a RawExtension. -func encodeRaw(t *testing.T, input interface{}) runtime.RawExtension { - data, err := json.Marshal(input) - require.NoError(t, err) - return runtime.RawExtension{Raw: data} -} - -// https://tools.ietf.org/html/rfc6901 -func escapeJSONPointer(s string) string { - s = strings.Replace(s, "~", "~0", -1) - s = strings.Replace(s, "/", "~1", -1) - return s -} - -func defaultTestClientWithNamespace() kubernetes.Interface { - return clientWithNamespace("default") -} - -func clientWithNamespace(name string) kubernetes.Interface { - ns := corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - } - return fake.NewSimpleClientset(&ns) -} diff --git a/control-plane/connect-inject/consul_sidecar.go b/control-plane/connect-inject/consul_sidecar.go index 2f2bac34b8..a19eebb5ef 100644 --- a/control-plane/connect-inject/consul_sidecar.go +++ b/control-plane/connect-inject/consul_sidecar.go @@ -11,7 +11,7 @@ import ( // the metrics merging server when metrics merging feature is enabled. // It always disables service registration because for connect we no longer // need to keep services registered as this is handled in the endpoints-controller. -func (w *ConnectWebhook) consulSidecar(pod corev1.Pod) (corev1.Container, error) { +func (w *MeshWebhook) consulSidecar(pod corev1.Pod) (corev1.Container, error) { metricsPorts, err := w.MetricsConfig.mergedMetricsServerConfiguration(pod) if err != nil { return corev1.Container{}, err @@ -48,7 +48,7 @@ func (w *ConnectWebhook) consulSidecar(pod corev1.Pod) (corev1.Container, error) }, nil } -func (w *ConnectWebhook) consulSidecarResources(pod corev1.Pod) (corev1.ResourceRequirements, error) { +func (w *MeshWebhook) consulSidecarResources(pod corev1.Pod) (corev1.ResourceRequirements, error) { resources := corev1.ResourceRequirements{ Limits: corev1.ResourceList{}, Requests: corev1.ResourceList{}, diff --git a/control-plane/connect-inject/consul_sidecar_test.go b/control-plane/connect-inject/consul_sidecar_test.go index fac2454991..bafaad104a 100644 --- a/control-plane/connect-inject/consul_sidecar_test.go +++ b/control-plane/connect-inject/consul_sidecar_test.go @@ -13,7 +13,7 @@ import ( // Test that if the conditions for running a merged metrics server are true, // that we pass the metrics flags to consul sidecar. func TestConsulSidecar_MetricsFlags(t *testing.T) { - handler := ConnectWebhook{ + meshWebhook := MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -21,7 +21,7 @@ func TestConsulSidecar_MetricsFlags(t *testing.T) { DefaultEnableMetricsMerging: true, }, } - container, err := handler.consulSidecar(corev1.Pod{ + container, err := meshWebhook.consulSidecar(corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ annotationMergedMetricsPort: "20100", @@ -53,13 +53,13 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { zero := resource.MustParse("0") cases := map[string]struct { - handler ConnectWebhook + meshWebhook MeshWebhook annotations map[string]string expResources corev1.ResourceRequirements expErr string }{ "no defaults, no annotations": { - handler: ConnectWebhook{ + meshWebhook: MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -78,7 +78,7 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { }, }, "all defaults, no annotations": { - handler: ConnectWebhook{ + meshWebhook: MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -113,7 +113,7 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { }, }, "no defaults, all annotations": { - handler: ConnectWebhook{ + meshWebhook: MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -142,7 +142,7 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { }, }, "annotations override defaults": { - handler: ConnectWebhook{ + meshWebhook: MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -181,7 +181,7 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { }, }, "defaults set to zero, no annotations": { - handler: ConnectWebhook{ + meshWebhook: MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -216,7 +216,7 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { }, }, "annotations set to 0": { - handler: ConnectWebhook{ + meshWebhook: MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -245,7 +245,7 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { }, }, "invalid cpu request": { - handler: ConnectWebhook{ + meshWebhook: MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -262,7 +262,7 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { expErr: "parsing annotation consul.hashicorp.com/consul-sidecar-cpu-request:\"invalid\": quantities must match the regular expression", }, "invalid cpu limit": { - handler: ConnectWebhook{ + meshWebhook: MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -279,7 +279,7 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { expErr: "parsing annotation consul.hashicorp.com/consul-sidecar-cpu-limit:\"invalid\": quantities must match the regular expression", }, "invalid memory request": { - handler: ConnectWebhook{ + meshWebhook: MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -296,7 +296,7 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { expErr: "parsing annotation consul.hashicorp.com/consul-sidecar-memory-request:\"invalid\": quantities must match the regular expression", }, "invalid memory limit": { - handler: ConnectWebhook{ + meshWebhook: MeshWebhook{ Log: logrtest.TestLogger{T: t}, ImageConsulK8S: "hashicorp/consul-k8s:9.9.9", MetricsConfig: MetricsConfig{ @@ -330,7 +330,7 @@ func TestHandlerConsulSidecar_Resources(t *testing.T) { }, }, } - container, err := c.handler.consulSidecar(pod) + container, err := c.meshWebhook.consulSidecar(pod) if c.expErr != "" { require.NotNil(err) require.Contains(err.Error(), c.expErr) diff --git a/control-plane/connect-inject/container_env.go b/control-plane/connect-inject/container_env.go index f20976d5f9..4ad2b156cd 100644 --- a/control-plane/connect-inject/container_env.go +++ b/control-plane/connect-inject/container_env.go @@ -8,7 +8,7 @@ import ( corev1 "k8s.io/api/core/v1" ) -func (w *ConnectWebhook) containerEnvVars(pod corev1.Pod) []corev1.EnvVar { +func (w *MeshWebhook) containerEnvVars(pod corev1.Pod) []corev1.EnvVar { raw, ok := pod.Annotations[annotationUpstreams] if !ok || raw == "" { return []corev1.EnvVar{} diff --git a/control-plane/connect-inject/container_env_test.go b/control-plane/connect-inject/container_env_test.go index 41eb45d71b..cb29b6e742 100644 --- a/control-plane/connect-inject/container_env_test.go +++ b/control-plane/connect-inject/container_env_test.go @@ -28,8 +28,8 @@ func TestContainerEnvVars(t *testing.T) { t.Run(tt.Name, func(t *testing.T) { require := require.New(t) - var h ConnectWebhook - envVars := h.containerEnvVars(corev1.Pod{ + var w MeshWebhook + envVars := w.containerEnvVars(corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ annotationService: "foo", diff --git a/control-plane/connect-inject/container_init.go b/control-plane/connect-inject/container_init.go index 3c4974784a..66ab836863 100644 --- a/control-plane/connect-inject/container_init.go +++ b/control-plane/connect-inject/container_init.go @@ -93,7 +93,7 @@ type initContainerCommandData struct { // initCopyContainer returns the init container spec for the copy container which places // the consul binary into the shared volume. -func (w *ConnectWebhook) initCopyContainer() corev1.Container { +func (w *MeshWebhook) initCopyContainer() corev1.Container { // Copy the Consul binary from the image to the shared volume. cmd := "cp /bin/consul /consul/connect-inject/consul" container := corev1.Container{ @@ -123,7 +123,7 @@ func (w *ConnectWebhook) initCopyContainer() corev1.Container { // containerInit returns the init container spec for connect-init that polls for the service and the connect proxy service to be registered // so that it can save the proxy service id to the shared volume and boostrap Envoy with the proxy-id. -func (w *ConnectWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod, mpi multiPortInfo) (corev1.Container, error) { +func (w *MeshWebhook) containerInit(namespace corev1.Namespace, pod corev1.Pod, mpi multiPortInfo) (corev1.Container, error) { // Check if tproxy is enabled on this pod. tproxyEnabled, err := transparentProxyEnabled(namespace, pod, w.EnableTransparentProxy) if err != nil { @@ -283,7 +283,7 @@ func (w *ConnectWebhook) containerInit(namespace corev1.Namespace, pod corev1.Po // constructDNSServiceHostName use the resource prefix and the DNS Service hostname suffix to construct the // key of the env variable whose value is the cluster IP of the Consul DNS Service. // It translates "resource-prefix" into "RESOURCE_PREFIX_DNS_SERVICE_HOST". -func (w *ConnectWebhook) constructDNSServiceHostName() string { +func (w *MeshWebhook) constructDNSServiceHostName() string { upcaseResourcePrefix := strings.ToUpper(w.ResourcePrefix) upcaseResourcePrefixWithUnderscores := strings.ReplaceAll(upcaseResourcePrefix, "-", "_") return strings.Join([]string{upcaseResourcePrefixWithUnderscores, dnsServiceHostEnvSuffix}, "_") diff --git a/control-plane/connect-inject/container_init_test.go b/control-plane/connect-inject/container_init_test.go index 1954ea2f61..f97868f603 100644 --- a/control-plane/connect-inject/container_init_test.go +++ b/control-plane/connect-inject/container_init_test.go @@ -46,7 +46,7 @@ func TestHandlerContainerInit(t *testing.T) { cases := []struct { Name string Pod func(*corev1.Pod) *corev1.Pod - Handler ConnectWebhook + Webhook MeshWebhook Cmd string // Strings.Contains test CmdNot string // Not contains }{ @@ -58,7 +58,7 @@ func TestHandlerContainerInit(t *testing.T) { pod.Annotations[annotationService] = "web" return pod }, - ConnectWebhook{}, + MeshWebhook{}, `/bin/sh -ec export CONSUL_HTTP_ADDR="${HOST_IP}:8500" export CONSUL_GRPC_ADDR="${HOST_IP}:8502" @@ -85,7 +85,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD } return pod }, - ConnectWebhook{ + MeshWebhook{ AuthMethod: "an-auth-method", ConsulAPITimeout: 5 * time.Second, }, @@ -118,7 +118,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD pod.Annotations[annotationPrometheusScrapePath] = "/scrape-path" return pod }, - ConnectWebhook{ + MeshWebhook{ ConsulAPITimeout: 5 * time.Second, }, `# Generate the envoy bootstrap code @@ -135,7 +135,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD t.Run(tt.Name, func(t *testing.T) { require := require.New(t) - h := tt.Handler + h := tt.Webhook pod := *tt.Pod(minimal()) container, err := h.containerInit(testNS, pod, multiPortInfo{}) require.NoError(err) @@ -287,7 +287,7 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) { } for name, c := range cases { t.Run(name, func(t *testing.T) { - h := ConnectWebhook{ + w := MeshWebhook{ EnableTransparentProxy: c.globalEnabled, ConsulAPITimeout: 5 * time.Second, } @@ -305,7 +305,7 @@ func TestHandlerContainerInit_transparentProxy(t *testing.T) { } ns := testNS ns.Labels = c.namespaceLabel - container, err := h.containerInit(ns, *pod, multiPortInfo{}) + container, err := w.containerInit(ns, *pod, multiPortInfo{}) require.NoError(t, err) actualCmd := strings.Join(container.Command, " ") @@ -384,7 +384,7 @@ func TestHandlerContainerInit_consulDNS(t *testing.T) { } for name, c := range cases { t.Run(name, func(t *testing.T) { - h := ConnectWebhook{ + w := MeshWebhook{ EnableConsulDNS: c.globalEnabled, EnableTransparentProxy: true, ResourcePrefix: "consul-consul", @@ -398,7 +398,7 @@ func TestHandlerContainerInit_consulDNS(t *testing.T) { ns := testNS ns.Labels = c.namespaceLabel - container, err := h.containerInit(ns, *pod, multiPortInfo{}) + container, err := w.containerInit(ns, *pod, multiPortInfo{}) require.NoError(t, err) actualCmd := strings.Join(container.Command, " ") @@ -428,8 +428,8 @@ func TestHandler_constructDNSServiceHostName(t *testing.T) { for _, c := range cases { t.Run(c.prefix, func(t *testing.T) { - h := ConnectWebhook{ResourcePrefix: c.prefix, ConsulAPITimeout: 5 * time.Second} - require.Equal(t, c.result, h.constructDNSServiceHostName()) + w := MeshWebhook{ResourcePrefix: c.prefix, ConsulAPITimeout: 5 * time.Second} + require.Equal(t, c.result, w.constructDNSServiceHostName()) }) } } @@ -469,7 +469,7 @@ func TestHandlerContainerInit_namespacesAndPartitionsEnabled(t *testing.T) { cases := []struct { Name string Pod func(*corev1.Pod) *corev1.Pod - Handler ConnectWebhook + Webhook MeshWebhook Cmd string // Strings.Contains test }{ { @@ -478,7 +478,7 @@ func TestHandlerContainerInit_namespacesAndPartitionsEnabled(t *testing.T) { pod.Annotations[annotationService] = "web" return pod }, - ConnectWebhook{ + MeshWebhook{ EnableNamespaces: true, ConsulDestinationNamespace: "default", ConsulPartition: "", @@ -503,7 +503,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD pod.Annotations[annotationService] = "web" return pod }, - ConnectWebhook{ + MeshWebhook{ EnableNamespaces: true, ConsulDestinationNamespace: "default", ConsulPartition: "default", @@ -530,7 +530,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD pod.Annotations[annotationService] = "web" return pod }, - ConnectWebhook{ + MeshWebhook{ EnableNamespaces: true, ConsulDestinationNamespace: "non-default", ConsulPartition: "", @@ -555,7 +555,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD pod.Annotations[annotationService] = "web" return pod }, - ConnectWebhook{ + MeshWebhook{ EnableNamespaces: true, ConsulDestinationNamespace: "non-default", ConsulPartition: "non-default-part", @@ -582,7 +582,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD pod.Annotations[annotationService] = "" return pod }, - ConnectWebhook{ + MeshWebhook{ AuthMethod: "auth-method", EnableNamespaces: true, ConsulDestinationNamespace: "non-default", @@ -616,7 +616,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD pod.Annotations[annotationService] = "" return pod }, - ConnectWebhook{ + MeshWebhook{ AuthMethod: "auth-method", EnableNamespaces: true, ConsulDestinationNamespace: "non-default", // Overridden by mirroring @@ -651,7 +651,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD pod.Annotations[annotationService] = "web" return pod }, - ConnectWebhook{ + MeshWebhook{ EnableNamespaces: true, ConsulDestinationNamespace: "default", ConsulPartition: "", @@ -683,7 +683,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD pod.Annotations[annotationService] = "web" return pod }, - ConnectWebhook{ + MeshWebhook{ EnableNamespaces: true, ConsulPartition: "default", ConsulDestinationNamespace: "non-default", @@ -719,7 +719,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD pod.Annotations[annotationService] = "web" return pod }, - ConnectWebhook{ + MeshWebhook{ AuthMethod: "auth-method", EnableNamespaces: true, ConsulPartition: "non-default", @@ -763,8 +763,8 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD t.Run(tt.Name, func(t *testing.T) { require := require.New(t) - h := tt.Handler - container, err := h.containerInit(testNS, *tt.Pod(minimal()), multiPortInfo{}) + w := tt.Webhook + container, err := w.containerInit(testNS, *tt.Pod(minimal()), multiPortInfo{}) require.NoError(err) actual := strings.Join(container.Command, " ") require.Equal(tt.Cmd, actual) @@ -818,7 +818,7 @@ func TestHandlerContainerInit_Multiport(t *testing.T) { cases := []struct { Name string Pod func(*corev1.Pod) *corev1.Pod - Handler ConnectWebhook + Webhook MeshWebhook NumInitContainers int MultiPortInfos []multiPortInfo Cmd []string // Strings.Contains test @@ -828,7 +828,7 @@ func TestHandlerContainerInit_Multiport(t *testing.T) { func(pod *corev1.Pod) *corev1.Pod { return pod }, - ConnectWebhook{ConsulAPITimeout: 5 * time.Second}, + MeshWebhook{ConsulAPITimeout: 5 * time.Second}, 2, []multiPortInfo{ { @@ -876,7 +876,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD func(pod *corev1.Pod) *corev1.Pod { return pod }, - ConnectWebhook{ + MeshWebhook{ AuthMethod: "auth-method", ConsulAPITimeout: 5 * time.Second, }, @@ -938,9 +938,9 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD t.Run(tt.Name, func(t *testing.T) { require := require.New(t) - h := tt.Handler + w := tt.Webhook for i := 0; i < tt.NumInitContainers; i++ { - container, err := h.containerInit(testNS, *tt.Pod(minimal()), tt.MultiPortInfos[i]) + container, err := w.containerInit(testNS, *tt.Pod(minimal()), tt.MultiPortInfos[i]) require.NoError(err) actual := strings.Join(container.Command, " ") require.Equal(tt.Cmd[i], actual) @@ -951,7 +951,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD func TestHandlerContainerInit_authMethod(t *testing.T) { require := require.New(t) - h := ConnectWebhook{ + w := MeshWebhook{ AuthMethod: "release-name-consul-k8s-auth-method", ConsulAPITimeout: 5 * time.Second, } @@ -978,7 +978,7 @@ func TestHandlerContainerInit_authMethod(t *testing.T) { ServiceAccountName: "foo", }, } - container, err := h.containerInit(testNS, *pod, multiPortInfo{}) + container, err := w.containerInit(testNS, *pod, multiPortInfo{}) require.NoError(err) actual := strings.Join(container.Command, " ") require.Contains(actual, ` @@ -998,7 +998,7 @@ consul-k8s-control-plane connect-init -pod-name=${POD_NAME} -pod-namespace=${POD // and CA cert should be set as env variable. func TestHandlerContainerInit_WithTLS(t *testing.T) { require := require.New(t) - h := ConnectWebhook{ + w := MeshWebhook{ ConsulCACert: "consul-ca-cert", ConsulAPITimeout: 5 * time.Second, } @@ -1017,7 +1017,7 @@ func TestHandlerContainerInit_WithTLS(t *testing.T) { }, }, } - container, err := h.containerInit(testNS, *pod, multiPortInfo{}) + container, err := w.containerInit(testNS, *pod, multiPortInfo{}) require.NoError(err) actual := strings.Join(container.Command, " ") require.Contains(actual, ` @@ -1034,7 +1034,7 @@ export CONSUL_GRPC_ADDR="${HOST_IP}:8502"`) func TestHandlerContainerInit_Resources(t *testing.T) { require := require.New(t) - h := ConnectWebhook{ + w := MeshWebhook{ InitContainerResources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("10m"), @@ -1062,7 +1062,7 @@ func TestHandlerContainerInit_Resources(t *testing.T) { }, }, } - container, err := h.containerInit(testNS, *pod, multiPortInfo{}) + container, err := w.containerInit(testNS, *pod, multiPortInfo{}) require.NoError(err) require.Equal(corev1.ResourceRequirements{ Limits: corev1.ResourceList{ @@ -1082,9 +1082,9 @@ func TestHandlerInitCopyContainer(t *testing.T) { for _, openShiftEnabled := range openShiftEnabledCases { t.Run(fmt.Sprintf("openshift enabled: %t", openShiftEnabled), func(t *testing.T) { - h := ConnectWebhook{EnableOpenShift: openShiftEnabled, ConsulAPITimeout: 5 * time.Second} + w := MeshWebhook{EnableOpenShift: openShiftEnabled, ConsulAPITimeout: 5 * time.Second} - container := h.initCopyContainer() + container := w.initCopyContainer() if openShiftEnabled { require.Nil(t, container.SecurityContext) diff --git a/control-plane/connect-inject/container_volume.go b/control-plane/connect-inject/container_volume.go index c9e794b45b..53ba985f3e 100644 --- a/control-plane/connect-inject/container_volume.go +++ b/control-plane/connect-inject/container_volume.go @@ -10,7 +10,7 @@ const volumeName = "consul-connect-inject-data" // containerVolume returns the volume data to add to the pod. This volume // is used for shared data between containers. -func (w *ConnectWebhook) containerVolume() corev1.Volume { +func (w *MeshWebhook) containerVolume() corev1.Volume { return corev1.Volume{ Name: volumeName, VolumeSource: corev1.VolumeSource{ diff --git a/control-plane/connect-inject/endpoints_controller.go b/control-plane/connect-inject/endpoints_controller.go index 2fdd912b16..f91b306571 100644 --- a/control-plane/connect-inject/endpoints_controller.go +++ b/control-plane/connect-inject/endpoints_controller.go @@ -405,7 +405,7 @@ func getProxyServiceID(pod corev1.Pod, serviceEndpoints corev1.Endpoints) string func (r *EndpointsController) createServiceRegistrations(pod corev1.Pod, serviceEndpoints corev1.Endpoints) (*api.AgentServiceRegistration, *api.AgentServiceRegistration, error) { // If a port is specified, then we determine the value of that port // and register that port for the host service. - // The handler will always set the port annotation if one is not provided on the pod. + // The meshWebhook will always set the port annotation if one is not provided on the pod. var consulServicePort int if raw, ok := pod.Annotations[annotationPort]; ok && raw != "" { if multiPort := strings.Split(raw, ","); len(multiPort) > 1 { @@ -807,6 +807,62 @@ func (r *EndpointsController) deleteACLTokensForServiceInstance(client *api.Clie return nil } +// processUpstreams reads the list of upstreams from the Pod annotation and converts them into a list of api.Upstream +// objects. +func (r *EndpointsController) processUpstreams(pod corev1.Pod, endpoints corev1.Endpoints) ([]api.Upstream, error) { + // In a multiport pod, only the first service's proxy should have upstreams configured. This skips configuring + // upstreams on additional services on the pod. + mpIdx := getMultiPortIdx(pod, endpoints) + if mpIdx > 0 { + return []api.Upstream{}, nil + } + + var upstreams []api.Upstream + if raw, ok := pod.Annotations[annotationUpstreams]; ok && raw != "" { + for _, raw := range strings.Split(raw, ",") { + var upstream api.Upstream + + // parts separates out the port, and determines whether it's a prepared query or not, since parts[0] would + // be "prepared_query" if it is. + parts := strings.SplitN(raw, ":", 3) + + // serviceParts helps determine which format of upstream we're processing, + // [service-name].[service-namespace].[service-partition]:[port]:[optional datacenter] + // or + // [service-name].svc.[service-namespace].ns.[service-peer].peer:[port] + // [service-name].svc.[service-namespace].ns.[service-partition].ap:[port] + // [service-name].svc.[service-namespace].ns.[service-datacenter].dc:[port] + labeledFormat := false + serviceParts := strings.Split(parts[0], ".") + if len(serviceParts) >= 2 { + if serviceParts[1] == "svc" { + labeledFormat = true + } + } + + if strings.TrimSpace(parts[0]) == "prepared_query" { + upstream = processPreparedQueryUpstream(pod, raw) + } else if labeledFormat { + var err error + upstream, err = r.processLabeledUpstream(pod, raw) + if err != nil { + return []api.Upstream{}, err + } + } else { + var err error + upstream, err = r.processUnlabeledUpstream(pod, raw) + if err != nil { + return []api.Upstream{}, err + } + } + + upstreams = append(upstreams, upstream) + } + } + + return upstreams, nil +} + // getTokenMetaFromDescription parses JSON metadata from token's description. func getTokenMetaFromDescription(description string) (map[string]string, error) { re := regexp.MustCompile(`.*({.+})`) @@ -855,9 +911,9 @@ func processPreparedQueryUpstream(pod corev1.Pod, rawUpstream string) api.Upstre return upstream } -// processNonAnnotatedUpstream processes an upstream in the format: +// processUnlabeledUpstream processes an upstream in the format: // [service-name].[service-namespace].[service-partition]:[port]:[optional datacenter]. -func (r *EndpointsController) processNonAnnotatedUpstream(pod corev1.Pod, rawUpstream string) (api.Upstream, error) { +func (r *EndpointsController) processUnlabeledUpstream(pod corev1.Pod, rawUpstream string) (api.Upstream, error) { var datacenter, serviceName, namespace, partition, peer string var port int32 var upstream api.Upstream @@ -921,11 +977,11 @@ func (r *EndpointsController) processNonAnnotatedUpstream(pod corev1.Pod, rawUps } -// processAnnotatedUpstream processes an upstream in the format: +// processLabeledUpstream processes an upstream in the format: // [service-name].svc.[service-namespace].ns.[service-peer].peer:[port] // [service-name].svc.[service-namespace].ns.[service-partition].ap:[port] // [service-name].svc.[service-namespace].ns.[service-datacenter].dc:[port]. -func (r *EndpointsController) processAnnotatedUpstream(pod corev1.Pod, rawUpstream string) (api.Upstream, error) { +func (r *EndpointsController) processLabeledUpstream(pod corev1.Pod, rawUpstream string) (api.Upstream, error) { var datacenter, serviceName, namespace, partition, peer string var port int32 var upstream api.Upstream @@ -1004,64 +1060,6 @@ func (r *EndpointsController) processAnnotatedUpstream(pod corev1.Pod, rawUpstre } -// processUpstreams reads the list of upstreams from the Pod annotation and converts them into a list of api.Upstream -// objects. -func (r *EndpointsController) processUpstreams(pod corev1.Pod, endpoints corev1.Endpoints) ([]api.Upstream, error) { - // In a multiport pod, only the first service's proxy should have upstreams configured. This skips configuring - // upstreams on additional services on the pod. - mpIdx := getMultiPortIdx(pod, endpoints) - if mpIdx > 0 { - return []api.Upstream{}, nil - } - - var upstreams []api.Upstream - if raw, ok := pod.Annotations[annotationUpstreams]; ok && raw != "" { - for _, raw := range strings.Split(raw, ",") { - //var datacenter, serviceName, namespace, partition, peer string - //var port int32 - var upstream api.Upstream - - // parts separates out the port, and determines whether it's a prepared query or not, since parts[0] would - // be "prepared_query" if it is. - parts := strings.SplitN(raw, ":", 3) - - // serviceParts helps determine which format of upstream we're processing, - // [service-name].[service-namespace].[service-partition]:[port]:[optional datacenter] - // or - // [service-name].svc.[service-namespace].ns.[service-peer].peer:[port] - // [service-name].svc.[service-namespace].ns.[service-partition].ap:[port] - // [service-name].svc.[service-namespace].ns.[service-datacenter].dc:[port] - annotatedFormat := false - serviceParts := strings.Split(parts[0], ".") - if len(serviceParts) >= 2 { - if serviceParts[1] == "svc" { - annotatedFormat = true - } - } - - if strings.TrimSpace(parts[0]) == "prepared_query" { - upstream = processPreparedQueryUpstream(pod, raw) - } else if annotatedFormat { - var err error - upstream, err = r.processAnnotatedUpstream(pod, raw) - if err != nil { - return []api.Upstream{}, err - } - } else { - var err error - upstream, err = r.processNonAnnotatedUpstream(pod, raw) - if err != nil { - return []api.Upstream{}, err - } - } - - upstreams = append(upstreams, upstream) - } - } - - return upstreams, nil -} - // remoteConsulClient returns an *api.Client that points at the consul agent local to the pod for a provided namespace. func (r *EndpointsController) remoteConsulClient(ip string, namespace string) (*api.Client, error) { newAddr := fmt.Sprintf("%s://%s:%s", r.ConsulScheme, ip, r.ConsulPort) diff --git a/control-plane/connect-inject/endpoints_controller_test.go b/control-plane/connect-inject/endpoints_controller_test.go index c1ded649e8..741c7571ee 100644 --- a/control-plane/connect-inject/endpoints_controller_test.go +++ b/control-plane/connect-inject/endpoints_controller_test.go @@ -443,6 +443,39 @@ func TestProcessUpstreams(t *testing.T) { consulNamespacesEnabled: false, consulPartitionsEnabled: false, }, + { + name: "annotated upstream error: both peer and partition provided", + pod: func() *corev1.Pod { + pod1 := createPod("pod1", "1.2.3.4", true, true) + pod1.Annotations[annotationUpstreams] = "upstream1.svc.ns1.ns.part1.partition.peer1.peer:1234" + return pod1 + }, + expErr: "upstream structured incorrectly: upstream1.svc.ns1.ns.part1.partition.peer1.peer:1234", + consulNamespacesEnabled: true, + consulPartitionsEnabled: true, + }, + { + name: "annotated upstream error: both peer and dc provided", + pod: func() *corev1.Pod { + pod1 := createPod("pod1", "1.2.3.4", true, true) + pod1.Annotations[annotationUpstreams] = "upstream1.svc.ns1.ns.peer1.peer.dc1.dc:1234" + return pod1 + }, + expErr: "upstream structured incorrectly: upstream1.svc.ns1.ns.peer1.peer.dc1.dc:1234", + consulNamespacesEnabled: true, + consulPartitionsEnabled: false, + }, + { + name: "annotated upstream error: both dc and partition provided", + pod: func() *corev1.Pod { + pod1 := createPod("pod1", "1.2.3.4", true, true) + pod1.Annotations[annotationUpstreams] = "upstream1.svc.ns1.ns.part1.partition.dc1.dc:1234" + return pod1 + }, + expErr: "upstream structured incorrectly: upstream1.svc.ns1.ns.part1.partition.dc1.dc:1234", + consulNamespacesEnabled: true, + consulPartitionsEnabled: true, + }, { name: "upstream with datacenter with ProxyDefaults and mesh gateway is in local mode", pod: func() *corev1.Pod { @@ -5754,7 +5787,7 @@ func TestCreateServiceRegistrations_withTransparentProxy(t *testing.T) { pod.Spec.Containers = c.podContainers } - // We set these annotations explicitly as these are set by the handler and we + // We set these annotations explicitly as these are set by the meshWebhook and we // need these values to determine which port to use for the service registration. pod.Annotations[annotationPort] = "tcp" diff --git a/control-plane/connect-inject/envoy_sidecar.go b/control-plane/connect-inject/envoy_sidecar.go index 716b4d4ef5..074855f2ce 100644 --- a/control-plane/connect-inject/envoy_sidecar.go +++ b/control-plane/connect-inject/envoy_sidecar.go @@ -10,7 +10,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" ) -func (w *ConnectWebhook) envoySidecar(namespace corev1.Namespace, pod corev1.Pod, mpi multiPortInfo) (corev1.Container, error) { +func (w *MeshWebhook) envoySidecar(namespace corev1.Namespace, pod corev1.Pod, mpi multiPortInfo) (corev1.Container, error) { resources, err := w.envoySidecarResources(pod) if err != nil { return corev1.Container{}, err @@ -64,7 +64,7 @@ func (w *ConnectWebhook) envoySidecar(namespace corev1.Namespace, pod corev1.Pod return corev1.Container{}, fmt.Errorf("pod security context cannot have the same uid as envoy: %v", envoyUserAndGroupID) } } - // Ensure that none of the user's containers have the same UID as Envoy. At this point in injection the handler + // Ensure that none of the user's containers have the same UID as Envoy. At this point in injection the meshWebhook // has only injected init containers so all containers defined in pod.Spec.Containers are from the user. for _, c := range pod.Spec.Containers { // User container and Envoy container cannot have the same UID. @@ -82,7 +82,7 @@ func (w *ConnectWebhook) envoySidecar(namespace corev1.Namespace, pod corev1.Pod return container, nil } -func (w *ConnectWebhook) getContainerSidecarCommand(pod corev1.Pod, multiPortSvcName string, multiPortSvcIdx int) ([]string, error) { +func (w *MeshWebhook) getContainerSidecarCommand(pod corev1.Pod, multiPortSvcName string, multiPortSvcIdx int) ([]string, error) { bootstrapFile := "/consul/connect-inject/envoy-bootstrap.yaml" if multiPortSvcName != "" { bootstrapFile = fmt.Sprintf("/consul/connect-inject/envoy-bootstrap-%s.yaml", multiPortSvcName) @@ -122,7 +122,7 @@ func (w *ConnectWebhook) getContainerSidecarCommand(pod corev1.Pod, multiPortSvc return cmd, nil } -func (w *ConnectWebhook) envoySidecarResources(pod corev1.Pod) (corev1.ResourceRequirements, error) { +func (w *MeshWebhook) envoySidecarResources(pod corev1.Pod) (corev1.ResourceRequirements, error) { resources := corev1.ResourceRequirements{ Limits: corev1.ResourceList{}, Requests: corev1.ResourceList{}, diff --git a/control-plane/connect-inject/envoy_sidecar_test.go b/control-plane/connect-inject/envoy_sidecar_test.go index d631b0f481..e907ae6533 100644 --- a/control-plane/connect-inject/envoy_sidecar_test.go +++ b/control-plane/connect-inject/envoy_sidecar_test.go @@ -12,7 +12,7 @@ import ( func TestHandlerEnvoySidecar(t *testing.T) { require := require.New(t) - h := ConnectWebhook{} + w := MeshWebhook{} pod := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ @@ -28,7 +28,7 @@ func TestHandlerEnvoySidecar(t *testing.T) { }, }, } - container, err := h.envoySidecar(testNS, pod, multiPortInfo{}) + container, err := w.envoySidecar(testNS, pod, multiPortInfo{}) require.NoError(err) require.Equal(container.Command, []string{ "envoy", @@ -45,7 +45,7 @@ func TestHandlerEnvoySidecar(t *testing.T) { func TestHandlerEnvoySidecar_Multiport(t *testing.T) { require := require.New(t) - h := ConnectWebhook{} + w := MeshWebhook{} pod := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ @@ -79,7 +79,7 @@ func TestHandlerEnvoySidecar_Multiport(t *testing.T) { 1: {"envoy", "--config-path", "/consul/connect-inject/envoy-bootstrap-web-admin.yaml", "--base-id", "1"}, } for i := 0; i < 2; i++ { - container, err := h.envoySidecar(testNS, pod, multiPortInfos[i]) + container, err := w.envoySidecar(testNS, pod, multiPortInfos[i]) require.NoError(err) require.Equal(expCommand[i], container.Command) @@ -136,7 +136,7 @@ func TestHandlerEnvoySidecar_withSecurityContext(t *testing.T) { } for name, c := range cases { t.Run(name, func(t *testing.T) { - h := ConnectWebhook{ + w := MeshWebhook{ EnableTransparentProxy: c.tproxyEnabled, EnableOpenShift: c.openShiftEnabled, } @@ -155,7 +155,7 @@ func TestHandlerEnvoySidecar_withSecurityContext(t *testing.T) { }, }, } - ec, err := h.envoySidecar(testNS, pod, multiPortInfo{}) + ec, err := w.envoySidecar(testNS, pod, multiPortInfo{}) require.NoError(t, err) require.Equal(t, c.expSecurityContext, ec.SecurityContext) }) @@ -163,10 +163,10 @@ func TestHandlerEnvoySidecar_withSecurityContext(t *testing.T) { } // Test that if the user specifies a pod security context with the same uid as `envoyUserAndGroupID` that we return -// an error to the handler. +// an error to the meshWebhook. func TestHandlerEnvoySidecar_FailsWithDuplicatePodSecurityContextUID(t *testing.T) { require := require.New(t) - h := ConnectWebhook{} + w := MeshWebhook{} pod := corev1.Pod{ Spec: corev1.PodSpec{ Containers: []corev1.Container{ @@ -179,18 +179,18 @@ func TestHandlerEnvoySidecar_FailsWithDuplicatePodSecurityContextUID(t *testing. }, }, } - _, err := h.envoySidecar(testNS, pod, multiPortInfo{}) + _, err := w.envoySidecar(testNS, pod, multiPortInfo{}) require.Error(err, fmt.Sprintf("pod security context cannot have the same uid as envoy: %v", envoyUserAndGroupID)) } // Test that if the user specifies a container with security context with the same uid as `envoyUserAndGroupID` that we -// return an error to the handler. If a container using the envoy image has the same uid, we don't return an error +// return an error to the meshWebhook. If a container using the envoy image has the same uid, we don't return an error // because in multiport pod there can be multiple envoy sidecars. func TestHandlerEnvoySidecar_FailsWithDuplicateContainerSecurityContextUID(t *testing.T) { cases := []struct { name string pod corev1.Pod - handler ConnectWebhook + webhook MeshWebhook expErr bool expErrMessage error }{ @@ -217,7 +217,7 @@ func TestHandlerEnvoySidecar_FailsWithDuplicateContainerSecurityContextUID(t *te }, }, }, - handler: ConnectWebhook{}, + webhook: MeshWebhook{}, expErr: true, expErrMessage: fmt.Errorf("container app has runAsUser set to the same uid %q as envoy which is not allowed", envoyUserAndGroupID), }, @@ -244,7 +244,7 @@ func TestHandlerEnvoySidecar_FailsWithDuplicateContainerSecurityContextUID(t *te }, }, }, - handler: ConnectWebhook{ + webhook: MeshWebhook{ ImageEnvoy: "envoy", }, expErr: false, @@ -253,7 +253,7 @@ func TestHandlerEnvoySidecar_FailsWithDuplicateContainerSecurityContextUID(t *te for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - _, err := tc.handler.envoySidecar(testNS, tc.pod, multiPortInfo{}) + _, err := tc.webhook.envoySidecar(testNS, tc.pod, multiPortInfo{}) if tc.expErr { require.Error(t, err, tc.expErrMessage) } else { @@ -341,7 +341,7 @@ func TestHandlerEnvoySidecar_EnvoyExtraArgs(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - h := ConnectWebhook{ + h := MeshWebhook{ ImageConsul: "hashicorp/consul:latest", ImageEnvoy: "hashicorp/consul-k8s:latest", EnvoyExtraArgs: tc.envoyExtraArgs, @@ -362,13 +362,13 @@ func TestHandlerEnvoySidecar_Resources(t *testing.T) { zero := resource.MustParse("0") cases := map[string]struct { - handler ConnectWebhook + webhook MeshWebhook annotations map[string]string expResources corev1.ResourceRequirements expErr string }{ "no defaults, no annotations": { - handler: ConnectWebhook{}, + webhook: MeshWebhook{}, annotations: nil, expResources: corev1.ResourceRequirements{ Limits: corev1.ResourceList{}, @@ -376,7 +376,7 @@ func TestHandlerEnvoySidecar_Resources(t *testing.T) { }, }, "all defaults, no annotations": { - handler: ConnectWebhook{ + webhook: MeshWebhook{ DefaultProxyCPURequest: cpu1, DefaultProxyCPULimit: cpu2, DefaultProxyMemoryRequest: mem1, @@ -395,7 +395,7 @@ func TestHandlerEnvoySidecar_Resources(t *testing.T) { }, }, "no defaults, all annotations": { - handler: ConnectWebhook{}, + webhook: MeshWebhook{}, annotations: map[string]string{ annotationSidecarProxyCPURequest: "100m", annotationSidecarProxyMemoryRequest: "100Mi", @@ -414,7 +414,7 @@ func TestHandlerEnvoySidecar_Resources(t *testing.T) { }, }, "annotations override defaults": { - handler: ConnectWebhook{ + webhook: MeshWebhook{ DefaultProxyCPURequest: zero, DefaultProxyCPULimit: zero, DefaultProxyMemoryRequest: zero, @@ -438,7 +438,7 @@ func TestHandlerEnvoySidecar_Resources(t *testing.T) { }, }, "defaults set to zero, no annotations": { - handler: ConnectWebhook{ + webhook: MeshWebhook{ DefaultProxyCPURequest: zero, DefaultProxyCPULimit: zero, DefaultProxyMemoryRequest: zero, @@ -457,7 +457,7 @@ func TestHandlerEnvoySidecar_Resources(t *testing.T) { }, }, "annotations set to 0": { - handler: ConnectWebhook{}, + webhook: MeshWebhook{}, annotations: map[string]string{ annotationSidecarProxyCPURequest: "0", annotationSidecarProxyMemoryRequest: "0", @@ -476,28 +476,28 @@ func TestHandlerEnvoySidecar_Resources(t *testing.T) { }, }, "invalid cpu request": { - handler: ConnectWebhook{}, + webhook: MeshWebhook{}, annotations: map[string]string{ annotationSidecarProxyCPURequest: "invalid", }, expErr: "parsing annotation consul.hashicorp.com/sidecar-proxy-cpu-request:\"invalid\": quantities must match the regular expression", }, "invalid cpu limit": { - handler: ConnectWebhook{}, + webhook: MeshWebhook{}, annotations: map[string]string{ annotationSidecarProxyCPULimit: "invalid", }, expErr: "parsing annotation consul.hashicorp.com/sidecar-proxy-cpu-limit:\"invalid\": quantities must match the regular expression", }, "invalid memory request": { - handler: ConnectWebhook{}, + webhook: MeshWebhook{}, annotations: map[string]string{ annotationSidecarProxyMemoryRequest: "invalid", }, expErr: "parsing annotation consul.hashicorp.com/sidecar-proxy-memory-request:\"invalid\": quantities must match the regular expression", }, "invalid memory limit": { - handler: ConnectWebhook{}, + webhook: MeshWebhook{}, annotations: map[string]string{ annotationSidecarProxyMemoryLimit: "invalid", }, @@ -521,7 +521,7 @@ func TestHandlerEnvoySidecar_Resources(t *testing.T) { }, }, } - container, err := c.handler.envoySidecar(testNS, pod, multiPortInfo{}) + container, err := c.webhook.envoySidecar(testNS, pod, multiPortInfo{}) if c.expErr != "" { require.NotNil(err) require.Contains(err.Error(), c.expErr) diff --git a/control-plane/connect-inject/metrics_configuration.go b/control-plane/connect-inject/metrics_configuration.go index cdfc5d0b6e..fc8c5d574a 100644 --- a/control-plane/connect-inject/metrics_configuration.go +++ b/control-plane/connect-inject/metrics_configuration.go @@ -35,7 +35,7 @@ func (mc MetricsConfig) mergedMetricsServerConfiguration(pod corev1.Pod) (metric return metricsPorts{}, err } - // This should never happen because we only call this function in the handler if + // This should never happen because we only call this function in the meshWebhook if // we need to run the metrics merging server. This check is here just in case. if !run { return metricsPorts{}, errors.New("metrics merging should be enabled in order to return the metrics server configuration") @@ -61,7 +61,7 @@ func (mc MetricsConfig) mergedMetricsServerConfiguration(pod corev1.Pod) (metric return metricsPorts, nil } -// enableMetrics returns whether metrics are enabled either via the default value in the handler, or if it's been +// enableMetrics returns whether metrics are enabled either via the default value in the meshWebhook, or if it's been // overridden via the annotation. func (mc MetricsConfig) enableMetrics(pod corev1.Pod) (bool, error) { enabled := mc.DefaultEnableMetrics @@ -76,7 +76,7 @@ func (mc MetricsConfig) enableMetrics(pod corev1.Pod) (bool, error) { } // enableMetricsMerging returns whether metrics merging functionality is enabled either via the default value in the -// handler, or if it's been overridden via the annotation. +// meshWebhook, or if it's been overridden via the annotation. func (mc MetricsConfig) enableMetricsMerging(pod corev1.Pod) (bool, error) { enabled := mc.DefaultEnableMetricsMerging if raw, ok := pod.Annotations[annotationEnableMetricsMerging]; ok && raw != "" { @@ -89,19 +89,19 @@ func (mc MetricsConfig) enableMetricsMerging(pod corev1.Pod) (bool, error) { return enabled, nil } -// mergedMetricsPort returns the port to run the merged metrics server on, either via the default value in the handler, +// mergedMetricsPort returns the port to run the merged metrics server on, either via the default value in the meshWebhook, // or if it's been overridden via the annotation. It also validates the port is in the unprivileged port range. func (mc MetricsConfig) mergedMetricsPort(pod corev1.Pod) (string, error) { return determineAndValidatePort(pod, annotationMergedMetricsPort, mc.DefaultMergedMetricsPort, false) } -// prometheusScrapePort returns the port for Prometheus to scrape from, either via the default value in the handler, or +// prometheusScrapePort returns the port for Prometheus to scrape from, either via the default value in the meshWebhook, or // if it's been overridden via the annotation. It also validates the port is in the unprivileged port range. func (mc MetricsConfig) prometheusScrapePort(pod corev1.Pod) (string, error) { return determineAndValidatePort(pod, annotationPrometheusScrapePort, mc.DefaultPrometheusScrapePort, false) } -// prometheusScrapePath returns the path for Prometheus to scrape from, either via the default value in the handler, or +// prometheusScrapePath returns the path for Prometheus to scrape from, either via the default value in the meshWebhook, or // if it's been overridden via the annotation. func (mc MetricsConfig) prometheusScrapePath(pod corev1.Pod) string { if raw, ok := pod.Annotations[annotationPrometheusScrapePath]; ok && raw != "" { diff --git a/control-plane/connect-inject/metrics_configuration_test.go b/control-plane/connect-inject/metrics_configuration_test.go index 9fcbccc336..9564b2190c 100644 --- a/control-plane/connect-inject/metrics_configuration_test.go +++ b/control-plane/connect-inject/metrics_configuration_test.go @@ -18,7 +18,7 @@ func TestMetricsConfigEnableMetrics(t *testing.T) { Err string }{ { - Name: "Metrics enabled via handler", + Name: "Metrics enabled via meshWebhook", Pod: func(pod *corev1.Pod) *corev1.Pod { return pod }, @@ -80,7 +80,7 @@ func TestMetricsConfigEnableMetricsMerging(t *testing.T) { Err string }{ { - Name: "Metrics merging enabled via handler", + Name: "Metrics merging enabled via meshWebhook", Pod: func(pod *corev1.Pod) *corev1.Pod { return pod }, @@ -221,7 +221,7 @@ func TestMetricsConfigPrometheusScrapePath(t *testing.T) { Expected string }{ { - Name: "Defaults to the handler's value", + Name: "Defaults to the meshWebhook's value", Pod: func(pod *corev1.Pod) *corev1.Pod { return pod }, diff --git a/control-plane/connect-inject/peering_acceptor_controller.go b/control-plane/connect-inject/peering_acceptor_controller.go index 522f8b9ade..ca14b2f89c 100644 --- a/control-plane/connect-inject/peering_acceptor_controller.go +++ b/control-plane/connect-inject/peering_acceptor_controller.go @@ -74,16 +74,15 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ } else { if containsString(acceptor.Finalizers, FinalizerName) { r.Log.Info("PeeringAcceptor was deleted, deleting from Consul", "name", req.Name, "ns", req.Namespace) - if err := r.deletePeering(ctx, req.Name); err != nil { - return ctrl.Result{}, err - } + err := r.deletePeering(ctx, req.Name) if acceptor.Secret().Backend == "kubernetes" { - if err := r.deleteK8sSecret(ctx, acceptor); err != nil { - return ctrl.Result{}, err - } + err = r.deleteK8sSecret(ctx, acceptor) + } + if err != nil { + return ctrl.Result{}, err } controllerutil.RemoveFinalizer(acceptor, FinalizerName) - err := r.Update(ctx, acceptor) + err = r.Update(ctx, acceptor) return ctrl.Result{}, err } } @@ -112,10 +111,11 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ // If the peering doesn't exist in Consul, generate a new token, and store it in the specified backend. Store the // current state in the status. if peering == nil { - r.Log.Info("peering doesn't exist in Consul", "name", acceptor.Name) + r.Log.Info("peering doesn't exist in Consul; creating new peering", "name", acceptor.Name) if statusSecretSet { if existingStatusSecret != nil { + r.Log.Info("stale secret in status; deleting stale secret", "name", acceptor.Name) err := r.Client.Delete(ctx, existingStatusSecret) if err != nil { r.updateStatusError(ctx, acceptor, err) @@ -130,7 +130,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, err } if acceptor.Secret().Backend == "kubernetes" { - secretResourceVersion, err = r.createK8sSecret(ctx, acceptor, resp) + secretResourceVersion, err = r.createOrUpdateK8sSecret(ctx, acceptor, resp) if err != nil { r.updateStatusError(ctx, acceptor, err) return ctrl.Result{}, err @@ -147,7 +147,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ // TODO(peering): Verify that the existing peering in Consul is an acceptor peer. If it is a dialing peer, an error should be thrown. // If the peering does exist in Consul, figure out whether to generate and store a new token by comparing the secret - // in the status to the actual contents of the secret. If no secret is specified in the status, shouldGenerate will + // in the status to the resource version of the secret. If no secret is specified in the status, shouldGenerate will // be set to true. var shouldGenerate bool var nameChanged bool @@ -168,7 +168,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, err } if acceptor.Secret().Backend == "kubernetes" { - secretResourceVersion, err = r.createK8sSecret(ctx, acceptor, resp) + secretResourceVersion, err = r.createOrUpdateK8sSecret(ctx, acceptor, resp) if err != nil { return ctrl.Result{}, err } @@ -193,8 +193,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ } // shouldGenerateToken returns whether a token should be generated, and whether the name of the secret has changed. It -// compares the spec secret's name/key/backend and contents to the status secret's name/key/backend and contents. The -// contents are compared by taking a SHA256 sum of the secret. +// compares the spec secret's name/key/backend and resource version with the name/key/backend and resource version of the status secret's. func shouldGenerateToken(acceptor *consulv1alpha1.PeeringAcceptor, existingStatusSecret *corev1.Secret) (shouldGenerate bool, nameChanged bool, err error) { if acceptor.SecretRef() == nil { return false, false, errors.New("shouldGenerateToken was called with an empty fields in the existing status") @@ -210,8 +209,8 @@ func shouldGenerateToken(acceptor *consulv1alpha1.PeeringAcceptor, existingStatu if acceptor.SecretRef().Backend != acceptor.Secret().Backend { return false, false, errors.New("PeeringAcceptor backend cannot be changed") } - // Compare the existing secret hash. - // Get the secret specified by the status, make sure it matches the status' secret.latestHash. + // Compare the existing secret resource version. + // Get the secret specified by the status, make sure it matches the status' secret.ResourceVersion. if existingStatusSecret != nil { if existingStatusSecret.ResourceVersion != acceptor.SecretRef().ResourceVersion { return true, false, nil @@ -270,9 +269,9 @@ func (r *PeeringAcceptorController) getExistingSecret(ctx context.Context, name return existingSecret, nil } -// createK8sSecret creates a secret and uses the controller's K8s client to apply the secret. It checks if +// createOrUpdateK8sSecret creates a secret and uses the controller's K8s client to apply the secret. It checks if // there's an existing secret with the same name and makes sure to update the existing secret if so. -func (r *PeeringAcceptorController) createK8sSecret(ctx context.Context, acceptor *consulv1alpha1.PeeringAcceptor, resp *api.PeeringGenerateTokenResponse) (string, error) { +func (r *PeeringAcceptorController) createOrUpdateK8sSecret(ctx context.Context, acceptor *consulv1alpha1.PeeringAcceptor, resp *api.PeeringGenerateTokenResponse) (string, error) { secretName := acceptor.Secret().Name secretNamespace := acceptor.Namespace secret := createSecret(secretName, secretNamespace, acceptor.Secret().Key, resp.PeeringToken) @@ -345,16 +344,13 @@ func (r *PeeringAcceptorController) deletePeering(ctx context.Context, peerName return nil } -// createSecret is a helper function that creates a corev1.SecretRef when provided inputs. +// createSecret is a helper function that creates a corev1.Secret when provided inputs. func createSecret(name, namespace, key, value string) *corev1.Secret { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, }, - StringData: map[string]string{ - key: value, - }, Data: map[string][]byte{ key: []byte(value), }, diff --git a/control-plane/connect-inject/peering_acceptor_controller_test.go b/control-plane/connect-inject/peering_acceptor_controller_test.go index b872d1cfd7..177f644501 100644 --- a/control-plane/connect-inject/peering_acceptor_controller_test.go +++ b/control-plane/connect-inject/peering_acceptor_controller_test.go @@ -402,7 +402,7 @@ func TestReconcileCreateUpdatePeeringAcceptor(t *testing.T) { // exists". Leaving this here documents that the entire contents of an existing secret should // be replaced. require.Equal(t, "", createdSecret.StringData["some-old-key"]) - decodedTokenData, err := base64.StdEncoding.DecodeString(createdSecret.StringData["data"]) + decodedTokenData, err := base64.StdEncoding.DecodeString(string(createdSecret.Data["data"])) require.NoError(t, err) require.Contains(t, string(decodedTokenData), "\"CA\":null") @@ -437,101 +437,81 @@ func TestReconcileCreateUpdatePeeringAcceptor(t *testing.T) { // TestReconcileDeletePeeringAcceptor reconciles a PeeringAcceptor resource that is no longer in Kubernetes, but still // exists in Consul. func TestReconcileDeletePeeringAcceptor(t *testing.T) { - t.Parallel() - nodeName := "test-node" - cases := []struct { - name string - initialConsulPeerName string - expErr string - }{ - { - name: "PeeringAcceptor ", - initialConsulPeerName: "acceptor-deleted", + // Add the default namespace. + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + acceptor := &v1alpha1.PeeringAcceptor{ + ObjectMeta: metav1.ObjectMeta{ + Name: "acceptor-deleted", + Namespace: "default", + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + Finalizers: []string{FinalizerName}, }, - } - for _, tt := range cases { - t.Run(tt.name, func(t *testing.T) { - // Add the default namespace. - ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}} - - acceptor := &v1alpha1.PeeringAcceptor{ - ObjectMeta: metav1.ObjectMeta{ - Name: "acceptor-deleted", - Namespace: "default", - DeletionTimestamp: &metav1.Time{Time: time.Now()}, - Finalizers: []string{FinalizerName}, - }, - Spec: v1alpha1.PeeringAcceptorSpec{ - Peer: &v1alpha1.Peer{ - Secret: &v1alpha1.Secret{ - Name: "acceptor-deleted-secret", - Key: "data", - Backend: "kubernetes", - }, - }, + Spec: v1alpha1.PeeringAcceptorSpec{ + Peer: &v1alpha1.Peer{ + Secret: &v1alpha1.Secret{ + Name: "acceptor-deleted-secret", + Key: "data", + Backend: "kubernetes", }, - } - k8sObjects := []runtime.Object{&ns, acceptor} - - // Add peering types to the scheme. - s := scheme.Scheme - s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringAcceptor{}, &v1alpha1.PeeringAcceptorList{}) - fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build() - - // Create test consul server. - consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { - c.NodeName = nodeName - }) - require.NoError(t, err) - defer consul.Stop() - consul.WaitForServiceIntentions(t) - - cfg := &api.Config{ - Address: consul.HTTPAddr, - } - consulClient, err := api.NewClient(cfg) - require.NoError(t, err) - - // Add the initial peerings into Consul by calling the Generate token endpoint. - _, _, err = consulClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: tt.initialConsulPeerName}, nil) - require.NoError(t, err) - - // Create the peering acceptor controller. - controller := &PeeringAcceptorController{ - Client: fakeClient, - Log: logrtest.TestLogger{T: t}, - ConsulClient: consulClient, - Scheme: s, - } - namespacedName := types.NamespacedName{ - Name: "acceptor-deleted", - Namespace: "default", - } - - // Reconcile a resource that is not in K8s, but is still in Consul. - resp, err := controller.Reconcile(context.Background(), ctrl.Request{ - NamespacedName: namespacedName, - }) - if tt.expErr != "" { - require.EqualError(t, err, tt.expErr) - } else { - require.NoError(t, err) - } - require.False(t, resp.Requeue) - - // After reconciliation, Consul should not have the peering. - peering, _, err := consulClient.Peerings().Read(context.Background(), "acceptor-deleted", nil) - require.Nil(t, peering) - require.NoError(t, err) - - err = fakeClient.Get(context.Background(), namespacedName, acceptor) - require.EqualError(t, err, `peeringacceptors.consul.hashicorp.com "acceptor-deleted" not found`) - - oldSecret := &corev1.Secret{} - err = fakeClient.Get(context.Background(), namespacedName, oldSecret) - require.EqualError(t, err, `secrets "acceptor-deleted" not found`) - }) + }, + }, } + k8sObjects := []runtime.Object{&ns, acceptor} + + // Add peering types to the scheme. + s := scheme.Scheme + s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringAcceptor{}, &v1alpha1.PeeringAcceptorList{}) + fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build() + + // Create test consul server. + consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { + c.NodeName = "test-node" + }) + require.NoError(t, err) + defer consul.Stop() + consul.WaitForServiceIntentions(t) + + cfg := &api.Config{ + Address: consul.HTTPAddr, + } + consulClient, err := api.NewClient(cfg) + require.NoError(t, err) + + // Add the initial peerings into Consul by calling the Generate token endpoint. + _, _, err = consulClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: "acceptor-deleted"}, nil) + require.NoError(t, err) + + // Create the peering acceptor controller. + controller := &PeeringAcceptorController{ + Client: fakeClient, + Log: logrtest.TestLogger{T: t}, + ConsulClient: consulClient, + Scheme: s, + } + namespacedName := types.NamespacedName{ + Name: "acceptor-deleted", + Namespace: "default", + } + + // Reconcile a resource that is not in K8s, but is still in Consul. + resp, err := controller.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: namespacedName, + }) + require.NoError(t, err) + require.False(t, resp.Requeue) + + // After reconciliation, Consul should have a peering with its DeletedAt set. + peering, _, err := consulClient.Peerings().Read(context.Background(), "acceptor-deleted", nil) + require.NotNil(t, peering.DeletedAt) + require.Equal(t, api.PeeringStateUndefined, peering.State) + require.NoError(t, err) + + err = fakeClient.Get(context.Background(), namespacedName, acceptor) + require.EqualError(t, err, `peeringacceptors.consul.hashicorp.com "acceptor-deleted" not found`) + + oldSecret := &corev1.Secret{} + err = fakeClient.Get(context.Background(), namespacedName, oldSecret) + require.EqualError(t, err, `secrets "acceptor-deleted" not found`) } func TestShouldGenerateToken(t *testing.T) { diff --git a/control-plane/connect-inject/peering_dialer_controller.go b/control-plane/connect-inject/peering_dialer_controller.go index 8ab18ab146..5ca50c8442 100644 --- a/control-plane/connect-inject/peering_dialer_controller.go +++ b/control-plane/connect-inject/peering_dialer_controller.go @@ -72,15 +72,6 @@ func (r *PeeringDialerController) Reconcile(ctx context.Context, req ctrl.Reques } } - // Get the status secret and the spec secret. - // Cases need to handle secretRefSet, statusSecret, secretSet, specSecret. - // no secretSet --> error bc spec needs to be set. - // no specSecret --> error bc waiting for spec secret to exist. - // no secretRefSet, yes secretSet, no statusSecret, yes specSecret --> initiate peering. - // yes secretRefSet, yes secretSet, no statusSecret, yes specSecret --> initiate peering. - // yes secretRefSet, yes secretSet, yes statusSecret, yes specSecret --> compare contents, if - // different initiate peering. - // TODO(peering): remove this once CRD validation exists. secretSet := false if dialer.Secret() != nil { @@ -162,7 +153,7 @@ func (r *PeeringDialerController) Reconcile(ctx context.Context, req ctrl.Reques } } - // Or, if the peering in Consul does exist, compare it to the contents of the spec's secret. If there's any + // Or, if the peering in Consul does exist, compare it to the spec's secret. If there's any // differences, initiate peering. if r.specStatusSecretsDifferent(dialer, specSecret) { r.Log.Info("the secret in status.secretRef exists and is different from spec.peer.secret; establishing peering with the existing spec.peer.secret", "secret-name", dialer.Secret().Name, "secret-namespace", dialer.Namespace) diff --git a/control-plane/connect-inject/peering_dialer_controller_test.go b/control-plane/connect-inject/peering_dialer_controller_test.go index 069c35cc32..1fe30d9f03 100644 --- a/control-plane/connect-inject/peering_dialer_controller_test.go +++ b/control-plane/connect-inject/peering_dialer_controller_test.go @@ -487,97 +487,76 @@ func TestSpecStatusSecretsDifferent(t *testing.T) { // TestReconcileDeletePeeringDialer reconciles a PeeringDialer resource that is no longer in Kubernetes, but still // exists in Consul. func TestReconcileDeletePeeringDialer(t *testing.T) { - t.Parallel() - nodeName := "test-node" - cases := []struct { - name string - initialConsulPeerNames []string - expErr string - }{ - { - name: "PeeringDialer no longer in K8s, still exists in Consul", - initialConsulPeerNames: []string{ - "dialer-deleted", + // Add the default namespace. + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + + dialer := &v1alpha1.PeeringDialer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dialer-deleted", + Namespace: "default", + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + Finalizers: []string{FinalizerName}, + }, + Spec: v1alpha1.PeeringDialerSpec{ + Peer: &v1alpha1.Peer{ + Secret: nil, }, }, } - for _, tt := range cases { - t.Run(tt.name, func(t *testing.T) { - // Add the default namespace. - ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}} - - dialer := &v1alpha1.PeeringDialer{ - ObjectMeta: metav1.ObjectMeta{ - Name: "dialer-deleted", - Namespace: "default", - DeletionTimestamp: &metav1.Time{Time: time.Now()}, - Finalizers: []string{FinalizerName}, - }, - Spec: v1alpha1.PeeringDialerSpec{ - Peer: &v1alpha1.Peer{ - Secret: nil, - }, - }, - } - - // Create fake k8s client. - k8sObjects := []runtime.Object{ns, dialer} - - // Add peering types to the scheme. - s := scheme.Scheme - s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringDialer{}, &v1alpha1.PeeringDialerList{}) - fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build() - - // Create test consul server. - consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { - c.NodeName = nodeName - }) - require.NoError(t, err) - defer consul.Stop() - consul.WaitForServiceIntentions(t) - - cfg := &api.Config{ - Address: consul.HTTPAddr, - } - consulClient, err := api.NewClient(cfg) - require.NoError(t, err) - - // Add the initial peerings into Consul by calling the Generate token endpoint. - _, _, err = consulClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: tt.initialConsulPeerNames[0]}, nil) - require.NoError(t, err) - // Create the peering dialer controller. - pdc := &PeeringDialerController{ - Client: fakeClient, - Log: logrtest.TestLogger{T: t}, - ConsulClient: consulClient, - Scheme: s, - } - namespacedName := types.NamespacedName{ - Name: "dialer-deleted", - Namespace: "default", - } + // Create fake k8s client. + k8sObjects := []runtime.Object{ns, dialer} - // Reconcile a resource that is not in K8s, but is still in Consul. - resp, err := pdc.Reconcile(context.Background(), ctrl.Request{ - NamespacedName: namespacedName, - }) - if tt.expErr != "" { - require.EqualError(t, err, tt.expErr) - } else { - require.NoError(t, err) - } - require.False(t, resp.Requeue) + // Add peering types to the scheme. + s := scheme.Scheme + s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.PeeringDialer{}, &v1alpha1.PeeringDialerList{}) + fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(k8sObjects...).Build() - // After reconciliation, Consul should not have the peering. - peering, _, err := consulClient.Peerings().Read(context.Background(), "dialer-deleted", nil) - require.Nil(t, peering) - require.NoError(t, err) + // Create test consul server. + consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { + c.NodeName = "test-node" + }) + require.NoError(t, err) + defer consul.Stop() + consul.WaitForServiceIntentions(t) - err = fakeClient.Get(context.Background(), namespacedName, dialer) - require.EqualError(t, err, `peeringdialers.consul.hashicorp.com "dialer-deleted" not found`) - }) + cfg := &api.Config{ + Address: consul.HTTPAddr, } + consulClient, err := api.NewClient(cfg) + require.NoError(t, err) + + // Add the initial peerings into Consul by calling the Generate token endpoint. + _, _, err = consulClient.Peerings().GenerateToken(context.Background(), api.PeeringGenerateTokenRequest{PeerName: "dialer-deleted"}, nil) + require.NoError(t, err) + + // Create the peering dialer controller. + pdc := &PeeringDialerController{ + Client: fakeClient, + Log: logrtest.TestLogger{T: t}, + ConsulClient: consulClient, + Scheme: s, + } + namespacedName := types.NamespacedName{ + Name: "dialer-deleted", + Namespace: "default", + } + + // Reconcile a resource that is not in K8s, but is still in Consul. + resp, err := pdc.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: namespacedName, + }) + require.NoError(t, err) + require.False(t, resp.Requeue) + + // After reconciliation, Consul should have a peering with its DeletedAt set. + peering, _, err := consulClient.Peerings().Read(context.Background(), "dialer-deleted", nil) + require.NotNil(t, peering.DeletedAt) + require.Equal(t, api.PeeringStateUndefined, peering.State) + require.NoError(t, err) + + err = fakeClient.Get(context.Background(), namespacedName, dialer) + require.EqualError(t, err, `peeringdialers.consul.hashicorp.com "dialer-deleted" not found`) } func TestDialerUpdateStatus(t *testing.T) { diff --git a/control-plane/go.mod b/control-plane/go.mod index db7b2d9ec7..cd7368ec7a 100644 --- a/control-plane/go.mod +++ b/control-plane/go.mod @@ -6,7 +6,7 @@ require ( github.com/go-logr/logr v0.4.0 github.com/google/go-cmp v0.5.7 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 - github.com/hashicorp/consul/api v1.10.1-0.20220610161046-7001e1151cbe + github.com/hashicorp/consul/api v1.10.1-0.20220613182022-20955742a7ab github.com/hashicorp/consul/sdk v0.9.0 github.com/hashicorp/go-discover v0.0.0-20200812215701-c4b85f6ed31f github.com/hashicorp/go-hclog v0.16.1 @@ -131,6 +131,4 @@ require ( replace github.com/hashicorp/consul/sdk v0.9.0 => github.com/hashicorp/consul/sdk v0.4.1-0.20220531155537-364758ef2f50 -//replace github.com/hashicorp/consul/api v1.10.1-0.20220425143126-6d0162a58a94 => /Users/nitya/workspace/hashicorp/consul/api - go 1.17 diff --git a/control-plane/go.sum b/control-plane/go.sum index 8d26a90944..87852188ce 100644 --- a/control-plane/go.sum +++ b/control-plane/go.sum @@ -297,8 +297,8 @@ github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgf github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= -github.com/hashicorp/consul/api v1.10.1-0.20220610161046-7001e1151cbe h1:YQSxqFG8IsG/qCQaPLnimycM8bpU6UYVJ5fURrJmDS4= -github.com/hashicorp/consul/api v1.10.1-0.20220610161046-7001e1151cbe/go.mod h1:ZlVrynguJKcYr54zGaDbaL3fOvKC9m72FhPvA8T35KQ= +github.com/hashicorp/consul/api v1.10.1-0.20220613182022-20955742a7ab h1:1WA23MKExkzduP1S85DdPGHfWDN0jf8sPEINq8cPIHg= +github.com/hashicorp/consul/api v1.10.1-0.20220613182022-20955742a7ab/go.mod h1:ZlVrynguJKcYr54zGaDbaL3fOvKC9m72FhPvA8T35KQ= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= github.com/hashicorp/consul/sdk v0.4.1-0.20220531155537-364758ef2f50 h1:GwbRRT+QxMRbYI608FGwTfcZ0iOVLX69B2ePjpQoyXw= github.com/hashicorp/consul/sdk v0.4.1-0.20220531155537-364758ef2f50/go.mod h1:yPkX5Q6CsxTFMjQQDJwzeNmUUF5NUGGbrDsv9wTb8cw= diff --git a/control-plane/main.go b/control-plane/main.go index f093901baf..7ec1340290 100644 --- a/control-plane/main.go +++ b/control-plane/main.go @@ -4,9 +4,8 @@ import ( "log" "os" - "github.com/mitchellh/cli" - "github.com/hashicorp/consul-k8s/control-plane/version" + "github.com/mitchellh/cli" ) func main() { diff --git a/control-plane/subcommand/inject-connect/command.go b/control-plane/subcommand/inject-connect/command.go index 32248f61c4..a39f8f44e9 100644 --- a/control-plane/subcommand/inject-connect/command.go +++ b/control-plane/subcommand/inject-connect/command.go @@ -462,7 +462,7 @@ func (c *Command) Run(args []string) int { mgr.GetWebhookServer().CertDir = c.flagCertDir mgr.GetWebhookServer().Register("/mutate", - &webhook.Admission{Handler: &connectinject.ConnectWebhook{ + &webhook.Admission{Handler: &connectinject.MeshWebhook{ Clientset: c.clientset, ConsulClient: c.consulClient, ImageConsul: c.flagConsulImage,