From 5bd6c607b8ce23caea0782345b66668f1846a9fb Mon Sep 17 00:00:00 2001 From: Curt Bushko Date: Wed, 17 May 2023 09:50:41 -0400 Subject: [PATCH] NET-2619 - save ClusterIPs to manual vips table (#2124) --- .changelog/2124.txt | 3 + .../endpoints/endpoints_controller.go | 29 ++- .../endpoints/endpoints_controller_test.go | 51 ++++ .../controllers/configentry_controller.go | 61 +++++ .../configentry_controller_test.go | 233 ++++++++++++++++++ control-plane/go.mod | 6 +- control-plane/go.sum | 12 +- 7 files changed, 385 insertions(+), 10 deletions(-) create mode 100644 .changelog/2124.txt diff --git a/.changelog/2124.txt b/.changelog/2124.txt new file mode 100644 index 0000000000..b65c23db2e --- /dev/null +++ b/.changelog/2124.txt @@ -0,0 +1,3 @@ +``release-note:improvement +control-plane: Transparent proxy enhancements for failover and virtual Services +``` diff --git a/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go b/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go index f408a21ea1..13f75f1156 100644 --- a/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go +++ b/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go @@ -1,6 +1,5 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: MPL-2.0 - package endpoints import ( @@ -295,6 +294,14 @@ func (r *Controller) registerServicesAndHealthCheck(apiClient *api.Client, pod c return err } + // Add manual ip to the VIP table + r.Log.Info("adding manual ip to virtual ip table in Consul", "name", serviceRegistration.Service.Service, + "id", serviceRegistration.ID) + err = assignServiceVirtualIP(r.Context, apiClient, serviceRegistration.Service) + if err != nil { + r.Log.Error(err, "failed to add ip to virtual ip table", "name", serviceRegistration.Service.Service) + } + // Register the proxy service instance with Consul. r.Log.Info("registering proxy service with Consul", "name", proxyServiceRegistration.Service.Service) _, err = apiClient.Catalog().Register(proxyServiceRegistration, nil) @@ -1258,6 +1265,26 @@ func (r *Controller) appendNodeMeta(registration *api.CatalogRegistration) { } } +// assignServiceVirtualIPs manually assigns the ClusterIP to the virtual IP table so that transparent proxy routing works. +func assignServiceVirtualIP(ctx context.Context, apiClient *api.Client, svc *api.AgentService) error { + ip := svc.TaggedAddresses[clusterIPTaggedAddressName].Address + if ip == "" { + return nil + } + + _, _, err := apiClient.Internal().AssignServiceVirtualIP(ctx, svc.Service, []string{ip}, &api.WriteOptions{Namespace: svc.Namespace, Partition: svc.Partition}) + if err != nil { + // Maintain backwards compatibility with older versions of Consul that do not support the VIP improvements. Tproxy + // will not work 100% correctly but the mesh will still work + if strings.Contains(err.Error(), "404") { + return fmt.Errorf("failed to add ip for service %s to virtual ip table. Please upgrade Consul to version 1.16 or higher", svc.Service) + } else { + return err + } + } + return nil +} + // hasBeenInjected checks the value of the status annotation and returns true if the Pod has been injected. func hasBeenInjected(pod corev1.Pod) bool { if anno, ok := pod.Annotations[constants.KeyInjectStatus]; ok && anno == constants.Injected { diff --git a/control-plane/connect-inject/controllers/endpoints/endpoints_controller_test.go b/control-plane/connect-inject/controllers/endpoints/endpoints_controller_test.go index 7ea04a4e52..e9ae243a12 100644 --- a/control-plane/connect-inject/controllers/endpoints/endpoints_controller_test.go +++ b/control-plane/connect-inject/controllers/endpoints/endpoints_controller_test.go @@ -6414,3 +6414,54 @@ func createGatewayPod(name, ip string, annotations map[string]string) *corev1.Po } return pod } + +func TestReconcileAssignServiceVirtualIP(t *testing.T) { + t.Parallel() + ctx := context.Background() + cases := []struct { + name string + service *api.AgentService + expectErr bool + }{ + { + name: "valid service", + service: &api.AgentService{ + ID: "", + Service: "foo", + Port: 80, + Address: "1.2.3.4", + TaggedAddresses: map[string]api.ServiceAddress{ + "virtual": { + Address: "1.2.3.4", + Port: 80, + }, + }, + Meta: map[string]string{constants.MetaKeyKubeNS: "default"}, + }, + expectErr: false, + }, + { + name: "service missing IP should not error", + service: &api.AgentService{ + ID: "", + Service: "bar", + Meta: map[string]string{constants.MetaKeyKubeNS: "default"}, + }, + expectErr: false, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + + // Create test consulServer server. + testClient := test.TestServerWithMockConnMgrWatcher(t, nil) + apiClient := testClient.APIClient + err := assignServiceVirtualIP(ctx, apiClient, c.service) + if err != nil { + require.True(t, c.expectErr) + } else { + require.False(t, c.expectErr) + } + }) + } +} diff --git a/control-plane/controllers/configentry_controller.go b/control-plane/controllers/configentry_controller.go index c2c6da9071..f35d2e88e7 100644 --- a/control-plane/controllers/configentry_controller.go +++ b/control-plane/controllers/configentry_controller.go @@ -199,6 +199,7 @@ func (r *ConfigEntryController) ReconcileEntry(ctx context.Context, crdCtrl Cont return r.syncFailed(ctx, logger, crdCtrl, configEntry, ConsulAgentError, fmt.Errorf("writing config entry to consul: %w", err)) } + logger.Info("config entry created", "request-time", writeMeta.RequestTime) return r.syncSuccessful(ctx, crdCtrl, configEntry) } @@ -267,6 +268,15 @@ func (r *ConfigEntryController) ReconcileEntry(ctx context.Context, crdCtrl Cont return r.syncSuccessful(ctx, crdCtrl, configEntry) } + // For resolvers and splitters, we need to set the ClusterIP of the matching service to Consul so that transparent + // proxy works correctly. Do not fail the reconcile if assigning the virtual IP returns an error. + if needsVirtualIPAssignment(configEntry) { + err = assignServiceVirtualIP(ctx, logger, consulClient, crdCtrl, req.NamespacedName, configEntry, r.DatacenterName) + if err != nil { + logger.Error(err, "failed assigning service virtual ip") + } + } + return ctrl.Result{}, nil } @@ -381,6 +391,57 @@ func (r *ConfigEntryController) nonMatchingMigrationError(kubeEntry common.Confi return fmt.Errorf("migration failed: Kubernetes resource does not match existing Consul config entry: consul=%s, kube=%s", consulJSON, kubeJSON) } +// needsVirtualIPAssignment checks to see if a configEntry type needs to be assigned a virtual IP. +func needsVirtualIPAssignment(configEntry common.ConfigEntryResource) bool { + kubeKind := configEntry.KubeKind() + if kubeKind == common.ServiceResolver || kubeKind == common.ServiceRouter || kubeKind == common.ServiceSplitter { + return true + } + return false +} + +// assignServiceVirtualIPs manually sends the ClusterIP for a matching service for ServiceRouter or ServiceSplitter +// CRDs to Consul so that it can be added to the virtual IP table. The assignment is skipped if the matching service +// does not exist or if an older version of Consul is being used. Endpoints Controller, on service registration, also +// manually sends a ClusterIP when a service is created. This increases the chance of a real IP ending up in the +// discovery chain. +func assignServiceVirtualIP(ctx context.Context, logger logr.Logger, consulClient *capi.Client, crdCtrl Controller, namespacedName types.NamespacedName, configEntry common.ConfigEntryResource, datacenter string) error { + service := corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: configEntry.KubernetesName(), + Namespace: namespacedName.Namespace, + }, + } + if err := crdCtrl.Get(ctx, namespacedName, &service); err != nil { + // It is non-fatal if the service does not exist. The ClusterIP will get added when the service is registered in + // the endpoints controller + if k8serr.IsNotFound(err) { + return nil + } + // Something is really wrong with the service + return err + } + + wo := &capi.WriteOptions{ + Namespace: configEntry.ToConsul(datacenter).GetNamespace(), + Partition: configEntry.ToConsul(datacenter).GetPartition(), + } + + logger.Info("adding manual ip to virtual ip table in Consul", "name", service.Name) + _, _, err := consulClient.Internal().AssignServiceVirtualIP(ctx, configEntry.KubernetesName(), []string{service.Spec.ClusterIP}, wo) + if err != nil { + // Maintain backwards compatibility with older versions of Consul that do not support the manual VIP improvements. With the older version, the mesh + // will still work. + if isNotFoundErr(err) { + logger.Error(err, "failed to add ip to virtual ip table. Please upgrade Consul to version 1.16 or higher", "name", service.Name) + return nil + } else { + return err + } + } + return nil +} + func isNotFoundErr(err error) bool { return err != nil && strings.Contains(err.Error(), "404") } diff --git a/control-plane/controllers/configentry_controller_test.go b/control-plane/controllers/configentry_controller_test.go index 494715cf4f..d548f78069 100644 --- a/control-plane/controllers/configentry_controller_test.go +++ b/control-plane/controllers/configentry_controller_test.go @@ -22,6 +22,7 @@ import ( 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/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -1891,3 +1892,235 @@ func TestConfigEntryController_Migration(t *testing.T) { }) } } + +func TestConfigEntryControllers_assignServiceVirtualIP(t *testing.T) { + t.Parallel() + kubeNS := "default" + + cases := []struct { + name string + kubeKind string + consulKind string + consulPrereqs []capi.ConfigEntry + configEntryResource common.ConfigEntryResource + service corev1.Service + reconciler func(client.Client, *consul.Config, consul.ServerConnectionManager, logr.Logger) Controller + expectErr bool + }{ + { + name: "ServiceResolver no error and vip should be assigned", + kubeKind: "ServiceResolver", + consulKind: capi.ServiceRouter, + configEntryResource: &v1alpha1.ServiceRouter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: kubeNS, + }, + Spec: v1alpha1.ServiceRouterSpec{ + Routes: []v1alpha1.ServiceRoute{ + { + Match: &v1alpha1.ServiceRouteMatch{ + HTTP: &v1alpha1.ServiceRouteHTTPMatch{ + PathPrefix: "/admin", + }, + }, + }, + }, + }, + }, + service: corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: kubeNS, + }, + Spec: corev1.ServiceSpec{ + ClusterIP: "10.0.0.1", + Ports: []corev1.ServicePort{ + { + Port: 8081, + }, + }, + }, + }, + reconciler: func(client client.Client, cfg *consul.Config, watcher consul.ServerConnectionManager, logger logr.Logger) Controller { + return &ServiceRouterController{ + Client: client, + Log: logger, + ConfigEntryController: &ConfigEntryController{ + ConsulClientConfig: cfg, + ConsulServerConnMgr: watcher, + DatacenterName: datacenterName, + }, + } + }, + expectErr: false, + }, + { + name: "ServiceRouter no error and vip should be assigned", + kubeKind: "ServiceRouter", + consulKind: capi.ServiceRouter, + consulPrereqs: []capi.ConfigEntry{ + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "bar", + Protocol: "http", + }, + }, + configEntryResource: &v1alpha1.ServiceRouter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Namespace: kubeNS, + }, + Spec: v1alpha1.ServiceRouterSpec{ + Routes: []v1alpha1.ServiceRoute{ + { + Match: &v1alpha1.ServiceRouteMatch{ + HTTP: &v1alpha1.ServiceRouteHTTPMatch{ + PathPrefix: "/admin", + }, + }, + }, + }, + }, + }, + service: corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Namespace: kubeNS, + }, + Spec: corev1.ServiceSpec{ + ClusterIP: "10.0.0.2", + Ports: []corev1.ServicePort{ + { + Port: 8081, + }, + }, + }, + }, + reconciler: func(client client.Client, cfg *consul.Config, watcher consul.ServerConnectionManager, logger logr.Logger) Controller { + return &ServiceRouterController{ + Client: client, + Log: logger, + ConfigEntryController: &ConfigEntryController{ + ConsulClientConfig: cfg, + ConsulServerConnMgr: watcher, + DatacenterName: datacenterName, + }, + } + }, + expectErr: false, + }, + { + name: "ServiceRouter should fail because service does not have a valid IP address", + kubeKind: "ServiceRouter", + consulKind: capi.ServiceRouter, + consulPrereqs: []capi.ConfigEntry{ + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "bar", + Protocol: "http", + }, + }, + configEntryResource: &v1alpha1.ServiceRouter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Namespace: kubeNS, + }, + Spec: v1alpha1.ServiceRouterSpec{ + Routes: []v1alpha1.ServiceRoute{ + { + Match: &v1alpha1.ServiceRouteMatch{ + HTTP: &v1alpha1.ServiceRouteHTTPMatch{ + PathPrefix: "/admin", + }, + }, + }, + }, + }, + }, + service: corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Namespace: kubeNS, + }, + }, + reconciler: func(client client.Client, cfg *consul.Config, watcher consul.ServerConnectionManager, logger logr.Logger) Controller { + return &ServiceRouterController{ + Client: client, + Log: logger, + ConfigEntryController: &ConfigEntryController{ + ConsulClientConfig: cfg, + ConsulServerConnMgr: watcher, + DatacenterName: datacenterName, + }, + } + }, + expectErr: true, + }, + { + name: "ServiceSplitter no error because a matching service does not exist", + kubeKind: "ServiceSplitter", + consulKind: capi.ServiceSplitter, + consulPrereqs: []capi.ConfigEntry{ + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "foo", + Protocol: "http", + }, + }, + configEntryResource: &v1alpha1.ServiceSplitter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: kubeNS, + }, + Spec: v1alpha1.ServiceSplitterSpec{ + Splits: []v1alpha1.ServiceSplit{ + { + Weight: 100, + }, + }, + }, + }, + reconciler: func(client client.Client, cfg *consul.Config, watcher consul.ServerConnectionManager, logger logr.Logger) Controller { + return &ServiceSplitterController{ + Client: client, + Log: logger, + ConfigEntryController: &ConfigEntryController{ + ConsulClientConfig: cfg, + ConsulServerConnMgr: watcher, + DatacenterName: datacenterName, + }, + } + }, + expectErr: false, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + ctx := context.Background() + + s := runtime.NewScheme() + s.AddKnownTypes(v1alpha1.GroupVersion, c.configEntryResource) + s.AddKnownTypes(schema.GroupVersion{Group: "", Version: "v1"}, &corev1.Service{}) + fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(&c.service, c.configEntryResource).Build() + + testClient := test.TestServerWithMockConnMgrWatcher(t, nil) + testClient.TestServer.WaitForLeader(t) + consulClient := testClient.APIClient + + ctrl := c.reconciler(fakeClient, testClient.Cfg, testClient.Watcher, logrtest.TestLogger{T: t}) + namespacedName := types.NamespacedName{ + Namespace: kubeNS, + Name: c.configEntryResource.KubernetesName(), + } + + err := assignServiceVirtualIP(ctx, ctrl.Logger(namespacedName), consulClient, ctrl, namespacedName, c.configEntryResource, "dc1") + if err != nil { + require.True(t, c.expectErr) + } else { + require.False(t, c.expectErr) + } + }) + } +} diff --git a/control-plane/go.mod b/control-plane/go.mod index 4cf30f5816..6a565a080d 100644 --- a/control-plane/go.mod +++ b/control-plane/go.mod @@ -8,9 +8,9 @@ require ( github.com/go-logr/logr v0.4.0 github.com/google/go-cmp v0.5.8 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 - github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20220831174802-b8af65262de8 - github.com/hashicorp/consul-server-connection-manager v0.1.0 - github.com/hashicorp/consul/api v1.10.1-0.20230427155444-391ed069c461 + github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20230511143918-bd16ab83383d + github.com/hashicorp/consul-server-connection-manager v0.1.2 + github.com/hashicorp/consul/api v1.10.1-0.20230512003852-bd0eb07ed3ca github.com/hashicorp/consul/sdk v0.13.1 github.com/hashicorp/go-bexpr v0.1.11 github.com/hashicorp/go-discover v0.0.0-20200812215701-c4b85f6ed31f diff --git a/control-plane/go.sum b/control-plane/go.sum index f49bc4b2d5..9d5bf5e90c 100644 --- a/control-plane/go.sum +++ b/control-plane/go.sum @@ -348,13 +348,13 @@ github.com/grpc-ecosystem/go-grpc-middleware v1.3.0/go.mod h1:z0ButlSOZa5vEBq9m2 github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= 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-k8s/control-plane/cni v0.0.0-20220831174802-b8af65262de8 h1:TQY0oKtLV15UNYWeSkTxi4McBIyLecsEtbc/VfxvbYA= -github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20220831174802-b8af65262de8/go.mod h1:aw35GB76URgbtxaSSMxbOetbG7YEHHPkIX3/SkTBaWc= -github.com/hashicorp/consul-server-connection-manager v0.1.0 h1:XCweGvMHzra88rYv2zxwwuUOjBUdcQmNKVrnQmt/muo= -github.com/hashicorp/consul-server-connection-manager v0.1.0/go.mod h1:XVVlO+Yk7aiRpspiHZkrrFVn9BJIiOPnQIzqytPxGaU= +github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20230511143918-bd16ab83383d h1:RJ1MZ8JKnfgKQ1kR3IBQAMpOpzXrdseZAYN/QR//MFM= +github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20230511143918-bd16ab83383d/go.mod h1:IHIHMzkoMwlv6rLsgwcoFBVYupR7/1pKEOHBMjD4L0k= +github.com/hashicorp/consul-server-connection-manager v0.1.2 h1:tNVQHUPuMbd+cMdD8kd+qkZUYpmLmrHMAV/49f4L53I= +github.com/hashicorp/consul-server-connection-manager v0.1.2/go.mod h1:NzQoVi1KcxGI2SangsDue8+ZPuXZWs+6BKAKrDNyg+w= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= -github.com/hashicorp/consul/api v1.10.1-0.20230427155444-391ed069c461 h1:cbsTR88ShbvcRMqLU8K0atm4GmRr8UH4x4jX4e12RYE= -github.com/hashicorp/consul/api v1.10.1-0.20230427155444-391ed069c461/go.mod h1:tXfrC6o0yFTgAW46xd5Ic8STHc9oIBcRVBcwhX5KNCQ= +github.com/hashicorp/consul/api v1.10.1-0.20230512003852-bd0eb07ed3ca h1:5UPVYOlJg/HBEJ2q82rkkQ3ZLzeMnF5MOpGcw2kh+XU= +github.com/hashicorp/consul/api v1.10.1-0.20230512003852-bd0eb07ed3ca/go.mod h1:tXfrC6o0yFTgAW46xd5Ic8STHc9oIBcRVBcwhX5KNCQ= github.com/hashicorp/consul/proto-public v0.1.0 h1:O0LSmCqydZi363hsqc6n2v5sMz3usQMXZF6ziK3SzXU= github.com/hashicorp/consul/proto-public v0.1.0/go.mod h1:vs2KkuWwtjkIgA5ezp4YKPzQp4GitV+q/+PvksrA92k= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=