diff --git a/CHANGELOG.md b/CHANGELOG.md index 23efbd4e2d..454786554a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ BUG FIXES: * Connect: Use `AdmissionregistrationV1` instead of `AdmissionregistrationV1beta1` API as it was deprecated in k8s 1.16. [[GH-558](https://github.com/hashicorp/consul-k8s/pull/558)] * Connect: Fix bug where environment variables `_CONNECT_SERVICE_HOST` and `_CONNECT_SERVICE_PORT` weren't being set when the upstream annotation was used. [[GH-549](https://github.com/hashicorp/consul-k8s/issues/549)] +* Connect: Fix a bug with leaving around ACL tokens after a service has been deregistered. [[GH-571](https://github.com/hashicorp/consul-k8s/issues/540)] * CRDs: Fix ProxyDefaults and ServiceDefaults resources not syncing with Consul < 1.10.0 [[GH-1023](https://github.com/hashicorp/consul-helm/issues/1023)] ## 0.26.0 (June 22, 2021) diff --git a/connect-inject/endpoints_controller.go b/connect-inject/endpoints_controller.go index 7b2ba8edb4..3f47fe03a9 100644 --- a/connect-inject/endpoints_controller.go +++ b/connect-inject/endpoints_controller.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net" + "regexp" "strings" "github.com/deckarep/golang-set" @@ -34,13 +35,13 @@ const ( MetaKeyKubeServiceName = "k8s-service-name" MetaKeyKubeNS = "k8s-namespace" MetaKeyManagedBy = "managed-by" + TokenMetaPodNameKey = "pod" kubernetesSuccessReasonMsg = "Kubernetes health checks passing" envoyPrometheusBindAddr = "envoy_prometheus_bind_addr" envoySidecarContainer = "envoy-sidecar" // clusterIPTaggedAddressName is the key for the tagged address to store the service's cluster IP and service port // in Consul. Note: This value should not be changed without a corresponding change in Consul. - // TODO: change this to a constant shared with Consul to avoid accidentally changing this. clusterIPTaggedAddressName = "virtual" // exposedPathsLivenessPortsRangeStart is the start of the port range that we will use as @@ -100,6 +101,11 @@ type EndpointsController struct { // TProxyOverwriteProbes controls whether the endpoints controller should expose pod's HTTP probes // via Envoy proxy. TProxyOverwriteProbes bool + // AuthMethod is the name of the Kubernetes Auth Method that + // was used to login with Consul. The Endpoints controller + // will delete any tokens associated with this auth method + // whenever service instances are deregistered. + AuthMethod string MetricsConfig MetricsConfig Log logr.Logger @@ -718,11 +724,78 @@ func (r *EndpointsController) deregisterServiceOnAllAgents(ctx context.Context, return err } } + + if r.AuthMethod != "" { + r.Log.Info("reconciling ACL tokens for service", "svc", serviceRegistration.Service) + err = r.reconcileACLTokensForService(client, serviceRegistration.Service, k8sSvcNamespace) + if err != nil { + r.Log.Error(err, "failed to reconcile ACL tokens for service", "svc", serviceRegistration.Service) + return err + } + } } } return nil } +// reconcileACLTokensForService finds the ACL tokens that belongs to the service and deletes it from Consul. +// It will only check for ACL tokens that have been created with the auth method this controller +// has been configured with. +func (r *EndpointsController) reconcileACLTokensForService(client *api.Client, serviceName, k8sNS string) error { + tokens, _, err := client.ACL().TokenList(nil) + if err != nil { + return fmt.Errorf("failed to get a list of tokens from Consul: %s", err) + } + + for _, token := range tokens { + // Only delete tokens that: + // * have been created with the auth method configured for this endpoints controller + // * have a single service identity whose service name is the same as 'serviceName' + if token.AuthMethod == r.AuthMethod && + len(token.ServiceIdentities) == 1 && + token.ServiceIdentities[0].ServiceName == serviceName { + tokenMeta, err := getTokenMetaFromDescription(token.Description) + if err != nil { + return fmt.Errorf("failed to parse token metadata: %s", err) + } + + podName := strings.TrimPrefix(tokenMeta[TokenMetaPodNameKey], k8sNS+"/") + err = r.Client.Get(r.Context, types.NamespacedName{Name: podName, Namespace: k8sNS}, &corev1.Pod{}) + // If we can't find token's pod, delete it. + if err != nil && k8serrors.IsNotFound(err) { + r.Log.Info("deleting ACL token for pod", "name", podName) + _, err = client.ACL().TokenDelete(token.AccessorID, nil) + if err != nil { + return fmt.Errorf("failed to delete token from Consul: %s", err) + } + } else if err != nil { + return err + } + } + } + + return nil +} + +// getTokenMetaFromDescription parses JSON metadata from token's description. +func getTokenMetaFromDescription(description string) (map[string]string, error) { + re := regexp.MustCompile(`.*({.+})`) + + matches := re.FindStringSubmatch(description) + if len(matches) != 2 { + return nil, fmt.Errorf("failed to extract token metadata from description: %s", description) + } + tokenMetaJSON := matches[1] + + var tokenMeta map[string]string + err := json.Unmarshal([]byte(tokenMetaJSON), &tokenMeta) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal token metadata '%s': %s", tokenMetaJSON, err) + } + + return tokenMeta, nil +} + // serviceInstancesForK8SServiceNameAndNamespace calls Consul's ServicesWithFilter to get the list // of services instances that have the provided k8sServiceName and k8sServiceNamespace in their metadata. func serviceInstancesForK8SServiceNameAndNamespace(k8sServiceName, k8sServiceNamespace string, client *api.Client) (map[string]*api.AgentService, error) { diff --git a/connect-inject/endpoints_controller_ent_test.go b/connect-inject/endpoints_controller_ent_test.go index 1a9c2f8d13..1bfd7a009f 100644 --- a/connect-inject/endpoints_controller_ent_test.go +++ b/connect-inject/endpoints_controller_ent_test.go @@ -12,8 +12,8 @@ import ( logrtest "github.com/go-logr/logr/testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/hashicorp/consul-k8s/helper/test" "github.com/hashicorp/consul-k8s/namespaces" - "github.com/hashicorp/consul-k8s/subcommand/common" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/sdk/testutil" "github.com/stretchr/testify/require" @@ -331,7 +331,7 @@ func TestReconcileCreateEndpointWithNamespaces(t *testing.T) { // For the register and deregister codepath, this also tests that they work when the Consul service name is different // from the K8s service name. // This test covers EndpointsController.deregisterServiceOnAllAgents when services should be selectively deregistered -// since the map will not be nil. This test also runs each test with ACLs+TLS enabled and disabled, since it covers all the cases where a Consul client is created. +// since the map will not be nil. func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { t.Parallel() nodeName := "test-node" @@ -385,10 +385,10 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { consulSvcName string k8sObjects func() []runtime.Object initialConsulSvcs []*api.AgentServiceRegistration - expectedNumSvcInstances int expectedConsulSvcInstances []*api.CatalogService expectedProxySvcInstances []*api.CatalogService expectedAgentHealthChecks []*api.AgentCheck + enableACLs bool }{ { name: "Legacy service: Health check is added to the correct namespace", @@ -439,7 +439,6 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { Namespace: ts.ExpConsulNS, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -517,7 +516,6 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { Namespace: ts.ExpConsulNS, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -584,7 +582,6 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { Namespace: ts.ExpConsulNS, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-different-consul-svc-name", @@ -660,7 +657,6 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { Namespace: ts.ExpConsulNS, }, }, - expectedNumSvcInstances: 2, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -687,7 +683,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { }, }, { - name: "Consul has instances that are not in the Endpoints addresses.", + name: "Consul has instances that are not in the Endpoints addresses", consulSvcName: "service-updated", k8sObjects: func() []runtime.Object { pod1 := createPodWithNamespace("pod1", ts.SourceKubeNS, "1.2.3.4", true, true) @@ -758,7 +754,6 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { Namespace: ts.ExpConsulNS, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -775,7 +770,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { }, }, { - name: "Different Consul service name: Consul has instances that are not in the Endpoints addresses.", + name: "Different Consul service name: Consul has instances that are not in the Endpoints addresses", consulSvcName: "different-consul-svc-name", k8sObjects: func() []runtime.Object { pod1 := createPodWithNamespace("pod1", ts.SourceKubeNS, "1.2.3.4", true, true) @@ -847,7 +842,6 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { Namespace: ts.ExpConsulNS, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-different-consul-svc-name", @@ -921,7 +915,6 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { Namespace: ts.ExpConsulNS, }, }, - expectedNumSvcInstances: 0, expectedConsulSvcInstances: []*api.CatalogService{}, expectedProxySvcInstances: []*api.CatalogService{}, }, @@ -983,134 +976,372 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) { Namespace: ts.ExpConsulNS, }, }, - expectedNumSvcInstances: 0, expectedConsulSvcInstances: []*api.CatalogService{}, expectedProxySvcInstances: []*api.CatalogService{}, }, + { + name: "ACLs enabled: Endpoints has an updated address because the target pod changes", + consulSvcName: "service-updated", + k8sObjects: func() []runtime.Object { + pod2 := createPodWithNamespace("pod2", ts.SourceKubeNS, "4.4.4.4", true, true) + endpoint := &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-updated", + Namespace: ts.SourceKubeNS, + }, + Subsets: []corev1.EndpointSubset{ + { + Addresses: []corev1.EndpointAddress{ + { + IP: "4.4.4.4", + NodeName: &nodeName, + TargetRef: &corev1.ObjectReference{ + Kind: "Pod", + Name: "pod2", + Namespace: ts.SourceKubeNS, + }, + }, + }, + }, + }, + } + return []runtime.Object{pod2, endpoint} + }, + initialConsulSvcs: []*api.AgentServiceRegistration{ + { + ID: "pod1-service-updated", + Name: "service-updated", + Port: 80, + Address: "1.2.3.4", + Meta: map[string]string{ + MetaKeyManagedBy: managedByValue, + MetaKeyKubeServiceName: "service-updated", + MetaKeyPodName: "pod1", + MetaKeyKubeNS: ts.SourceKubeNS, + }, + Namespace: ts.ExpConsulNS, + }, + { + Kind: api.ServiceKindConnectProxy, + ID: "pod1-service-updated-sidecar-proxy", + Name: "service-updated-sidecar-proxy", + Port: 20000, + Address: "1.2.3.4", + Proxy: &api.AgentServiceConnectProxyConfig{ + DestinationServiceName: "service-updated", + DestinationServiceID: "pod1-service-updated", + }, + Meta: map[string]string{ + MetaKeyManagedBy: managedByValue, + MetaKeyKubeServiceName: "service-updated", + MetaKeyPodName: "pod1", + MetaKeyKubeNS: ts.SourceKubeNS, + }, + Namespace: ts.ExpConsulNS, + }, + }, + expectedConsulSvcInstances: []*api.CatalogService{ + { + ServiceID: "pod2-service-updated", + ServiceAddress: "4.4.4.4", + Namespace: ts.ExpConsulNS, + }, + }, + expectedProxySvcInstances: []*api.CatalogService{ + { + ServiceID: "pod2-service-updated-sidecar-proxy", + ServiceAddress: "4.4.4.4", + Namespace: ts.ExpConsulNS, + }, + }, + enableACLs: true, + }, + { + name: "ACLs enabled: Consul has instances that are not in the Endpoints addresses", + consulSvcName: "service-updated", + k8sObjects: func() []runtime.Object { + pod1 := createPodWithNamespace("pod1", ts.SourceKubeNS, "1.2.3.4", true, true) + endpoint := &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-updated", + Namespace: ts.SourceKubeNS, + }, + Subsets: []corev1.EndpointSubset{ + { + Addresses: []corev1.EndpointAddress{ + { + IP: "1.2.3.4", + NodeName: &nodeName, + TargetRef: &corev1.ObjectReference{ + Kind: "Pod", + Name: "pod1", + Namespace: ts.SourceKubeNS, + }, + }, + }, + }, + }, + } + return []runtime.Object{pod1, endpoint} + }, + initialConsulSvcs: []*api.AgentServiceRegistration{ + { + ID: "pod1-service-updated", + Name: "service-updated", + Port: 80, + Address: "1.2.3.4", + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: ts.SourceKubeNS, + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod1", + }, + Namespace: ts.ExpConsulNS, + }, + { + Kind: api.ServiceKindConnectProxy, + ID: "pod1-service-updated-sidecar-proxy", + Name: "service-updated-sidecar-proxy", + Port: 20000, + Address: "1.2.3.4", + Proxy: &api.AgentServiceConnectProxyConfig{ + DestinationServiceName: "service-updated", + DestinationServiceID: "pod1-service-updated", + }, + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: ts.SourceKubeNS, + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod1", + }, + Namespace: ts.ExpConsulNS, + }, + { + ID: "pod2-service-updated", + Name: "service-updated", + Port: 80, + Address: "2.2.3.4", + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: ts.SourceKubeNS, + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod2", + }, + Namespace: ts.ExpConsulNS, + }, + { + Kind: api.ServiceKindConnectProxy, + ID: "pod2-service-updated-sidecar-proxy", + Name: "service-updated-sidecar-proxy", + Port: 20000, + Address: "2.2.3.4", + Proxy: &api.AgentServiceConnectProxyConfig{ + DestinationServiceName: "service-updated", + DestinationServiceID: "pod2-service-updated", + }, + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: ts.SourceKubeNS, + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod2", + }, + Namespace: ts.ExpConsulNS, + }, + }, + expectedConsulSvcInstances: []*api.CatalogService{ + { + ServiceID: "pod1-service-updated", + ServiceAddress: "1.2.3.4", + Namespace: ts.ExpConsulNS, + }, + }, + expectedProxySvcInstances: []*api.CatalogService{ + { + ServiceID: "pod1-service-updated-sidecar-proxy", + ServiceAddress: "1.2.3.4", + Namespace: ts.ExpConsulNS, + }, + }, + enableACLs: true, + }, } - for _, secure := range []bool{true, false} { - for _, tt := range cases { - t.Run(fmt.Sprintf("%s: %s - secure: %v", name, tt.name, secure), func(t *testing.T) { - // The agent pod needs to have the address 127.0.0.1 so when the - // code gets the agent pods via the label component=client, and - // makes requests against the agent API, it will actually hit the - // test server we have on localhost. - fakeClientPod := createPod("fake-consul-client", "127.0.0.1", false, true) - fakeClientPod.Labels = map[string]string{"component": "client", "app": "consul", "release": "consul"} - - // Add the pods namespace. - ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ts.SourceKubeNS}} - // Create fake k8s client. - k8sObjects := append(tt.k8sObjects(), fakeClientPod, &ns) - fakeClient := fake.NewClientBuilder().WithRuntimeObjects(k8sObjects...).Build() + for _, tt := range cases { + t.Run(fmt.Sprintf("%s: %s", name, tt.name), func(t *testing.T) { + // The agent pod needs to have the address 127.0.0.1 so when the + // code gets the agent pods via the label component=client, and + // makes requests against the agent API, it will actually hit the + // test server we have on localhost. + fakeClientPod := createPod("fake-consul-client", "127.0.0.1", false, true) + fakeClientPod.Labels = map[string]string{"component": "client", "app": "consul", "release": "consul"} - masterToken := "b78d37c7-0ca7-5f4d-99ee-6d9975ce4586" - caFile, certFile, keyFile := common.GenerateServerCerts(t) - // Create test consul server, with ACLs+TLS if necessary. - consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { - if secure { - c.ACL.Enabled = true - c.ACL.DefaultPolicy = "deny" - c.ACL.Tokens.Master = masterToken - c.CAFile = caFile - c.CertFile = certFile - c.KeyFile = keyFile - } - c.NodeName = nodeName - }) - require.NoError(t, err) - defer consul.Stop() - consul.WaitForSerfCheck(t) + // Add the pods namespace. + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ts.SourceKubeNS}} + // Create fake k8s client. + k8sObjects := append(tt.k8sObjects(), fakeClientPod, &ns) + fakeClient := fake.NewClientBuilder().WithRuntimeObjects(k8sObjects...).Build() - cfg := &api.Config{ - Scheme: "http", - Address: consul.HTTPAddr, - Namespace: ts.ExpConsulNS, - } - if secure { - cfg.Address = consul.HTTPSAddr - cfg.Scheme = "https" - cfg.TLSConfig = api.TLSConfig{ - CAFile: caFile, - } - cfg.Token = masterToken + adminToken := "123e4567-e89b-12d3-a456-426614174000" + consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { + if tt.enableACLs { + c.ACL.Enabled = true + c.ACL.Tokens.Master = adminToken } - consulClient, err := api.NewClient(cfg) - require.NoError(t, err) - addr := strings.Split(cfg.Address, ":") - consulPort := addr[1] + c.NodeName = nodeName + }) + require.NoError(t, err) + defer consul.Stop() + consul.WaitForSerfCheck(t) + + cfg := &api.Config{ + Scheme: "http", + Address: consul.HTTPAddr, + Namespace: ts.ExpConsulNS, + } + if tt.enableACLs { + cfg.Token = adminToken + } - _, err = namespaces.EnsureExists(consulClient, ts.ExpConsulNS, "") + consulClient, err := api.NewClient(cfg) + require.NoError(t, err) + addr := strings.Split(cfg.Address, ":") + consulPort := addr[1] + + _, err = namespaces.EnsureExists(consulClient, ts.ExpConsulNS, "") + require.NoError(t, err) + + // Holds token accessorID for each service ID. + tokensForServices := make(map[string]string) + + // Register service and proxy in Consul. + for _, svc := range tt.initialConsulSvcs { + err = consulClient.Agent().ServiceRegister(svc) require.NoError(t, err) + // Create a token for this service if ACLs are enabled. + if tt.enableACLs { + if svc.Kind != api.ServiceKindConnectProxy { + var writeOpts api.WriteOptions + // When mirroring is enabled, the auth method will be created in the "default" Consul namespace. + if ts.Mirror { + writeOpts.Namespace = "default" + } + test.SetupK8sAuthMethodWithNamespaces(t, consulClient, svc.Name, svc.Meta[MetaKeyKubeNS], ts.ExpConsulNS, ts.Mirror, ts.MirrorPrefix) + token, _, err := consulClient.ACL().Login(&api.ACLLoginParams{ + AuthMethod: test.AuthMethod, + BearerToken: test.ServiceAccountJWTToken, + Meta: map[string]string{ + TokenMetaPodNameKey: fmt.Sprintf("%s/%s", svc.Meta[MetaKeyKubeNS], svc.Meta[MetaKeyPodName]), + }, + }, &writeOpts) - // Register service and proxy in Consul. - for _, svc := range tt.initialConsulSvcs { - err = consulClient.Agent().ServiceRegister(svc) - require.NoError(t, err) - } + require.NoError(t, err) - // Create the endpoints controller. - ep := &EndpointsController{ - Client: fakeClient, - Log: logrtest.TestLogger{T: t}, - ConsulClient: consulClient, - ConsulPort: consulPort, - ConsulScheme: cfg.Scheme, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSetWith(), - ReleaseName: "consul", - ReleaseNamespace: "default", - ConsulClientCfg: cfg, - EnableConsulNamespaces: true, - EnableNSMirroring: ts.Mirror, - NSMirroringPrefix: ts.MirrorPrefix, - ConsulDestinationNamespace: ts.DestConsulNS, - } - namespacedName := types.NamespacedName{ - Namespace: ts.SourceKubeNS, - Name: "service-updated", + tokensForServices[svc.ID] = token.AccessorID + + // Create another token for the same service but a pod that no longer exists. + // This is to test a scenario with orphaned tokens + // where we have a token for the pod but the service instance + // for that pod no longer exists in Consul. + // In that case, the token should still be deleted. + token, _, err = consulClient.ACL().Login(&api.ACLLoginParams{ + AuthMethod: test.AuthMethod, + BearerToken: test.ServiceAccountJWTToken, + Meta: map[string]string{ + TokenMetaPodNameKey: fmt.Sprintf("%s/%s", svc.Meta[MetaKeyKubeNS], "does-not-exist"), + }, + }, &writeOpts) + require.NoError(t, err) + tokensForServices["does-not-exist"+svc.Name] = token.AccessorID + } } + } - resp, err := ep.Reconcile(context.Background(), ctrl.Request{ - NamespacedName: namespacedName, - }) - require.NoError(t, err) - require.False(t, resp.Requeue) + // Create the endpoints controller. + ep := &EndpointsController{ + Client: fakeClient, + Log: logrtest.TestLogger{T: t}, + ConsulClient: consulClient, + ConsulPort: consulPort, + ConsulScheme: cfg.Scheme, + AllowK8sNamespacesSet: mapset.NewSetWith("*"), + DenyK8sNamespacesSet: mapset.NewSetWith(), + ReleaseName: "consul", + ReleaseNamespace: "default", + ConsulClientCfg: cfg, + EnableConsulNamespaces: true, + EnableNSMirroring: ts.Mirror, + NSMirroringPrefix: ts.MirrorPrefix, + ConsulDestinationNamespace: ts.DestConsulNS, + } + if tt.enableACLs { + ep.AuthMethod = test.AuthMethod + } + namespacedName := types.NamespacedName{ + Namespace: ts.SourceKubeNS, + Name: "service-updated", + } - // After reconciliation, Consul should have service-updated with the correct number of instances. - serviceInstances, _, err := consulClient.Catalog().Service(tt.consulSvcName, "", &api.QueryOptions{Namespace: ts.ExpConsulNS}) - require.NoError(t, err) - require.Len(t, serviceInstances, tt.expectedNumSvcInstances) - for i, instance := range serviceInstances { - require.Equal(t, tt.expectedConsulSvcInstances[i].ServiceID, instance.ServiceID) - require.Equal(t, tt.expectedConsulSvcInstances[i].ServiceAddress, instance.ServiceAddress) + resp, err := ep.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: namespacedName, + }) + require.NoError(t, err) + require.False(t, resp.Requeue) + + // After reconciliation, Consul should have service-updated with the correct number of instances. + serviceInstances, _, err := consulClient.Catalog().Service(tt.consulSvcName, "", &api.QueryOptions{Namespace: ts.ExpConsulNS}) + require.NoError(t, err) + require.Len(t, serviceInstances, len(tt.expectedProxySvcInstances)) + for i, instance := range serviceInstances { + require.Equal(t, tt.expectedConsulSvcInstances[i].ServiceID, instance.ServiceID) + require.Equal(t, tt.expectedConsulSvcInstances[i].ServiceAddress, instance.ServiceAddress) + } + proxyServiceInstances, _, err := consulClient.Catalog().Service(fmt.Sprintf("%s-sidecar-proxy", tt.consulSvcName), "", &api.QueryOptions{Namespace: ts.ExpConsulNS}) + require.NoError(t, err) + require.Len(t, proxyServiceInstances, len(tt.expectedProxySvcInstances)) + for i, instance := range proxyServiceInstances { + require.Equal(t, tt.expectedProxySvcInstances[i].ServiceID, instance.ServiceID) + require.Equal(t, tt.expectedProxySvcInstances[i].ServiceAddress, instance.ServiceAddress) + } + + // Check that the Consul health check was created for the k8s pod. + if tt.expectedAgentHealthChecks != nil { + for i := range tt.expectedConsulSvcInstances { + filter := fmt.Sprintf("CheckID == `%s`", tt.expectedAgentHealthChecks[i].CheckID) + newChecks, _ := consulClient.Agent().Checks() + for key, value := range newChecks { + fmt.Printf("%s:%v\n", key, value) + } + check, err := consulClient.Agent().ChecksWithFilter(filter) + require.NoError(t, err) + require.EqualValues(t, 1, len(check)) + // Ignoring Namespace because the response from ENT includes it and OSS does not. + var ignoredFields = []string{"Node", "Definition", "Namespace"} + require.True(t, cmp.Equal(check[tt.expectedAgentHealthChecks[i].CheckID], tt.expectedAgentHealthChecks[i], cmpopts.IgnoreFields(api.AgentCheck{}, ignoredFields...))) } - proxyServiceInstances, _, err := consulClient.Catalog().Service(fmt.Sprintf("%s-sidecar-proxy", tt.consulSvcName), "", &api.QueryOptions{Namespace: ts.ExpConsulNS}) - require.NoError(t, err) - require.Len(t, proxyServiceInstances, tt.expectedNumSvcInstances) - for i, instance := range proxyServiceInstances { - require.Equal(t, tt.expectedProxySvcInstances[i].ServiceID, instance.ServiceID) - require.Equal(t, tt.expectedProxySvcInstances[i].ServiceAddress, instance.ServiceAddress) + } + + if tt.enableACLs { + // Put expected services into a map to make it easier to find service IDs. + expectedServices := make(map[string]struct{}) + for _, svc := range tt.expectedConsulSvcInstances { + expectedServices[svc.ServiceID] = struct{}{} } - // Check that the Consul health check was created for the k8s pod. - if tt.expectedAgentHealthChecks != nil { - for i := range tt.expectedConsulSvcInstances { - filter := fmt.Sprintf("CheckID == `%s`", tt.expectedAgentHealthChecks[i].CheckID) - newChecks, _ := consulClient.Agent().Checks() - for key, value := range newChecks { - fmt.Printf("%s:%v\n", key, value) - } - check, err := consulClient.Agent().ChecksWithFilter(filter) + // Look through the tokens we've created and check that only + // tokens for the deregistered services have been deleted. + for serviceID, tokenID := range tokensForServices { + // Read the token from Consul. + token, _, err := consulClient.ACL().TokenRead(tokenID, nil) + if _, ok := expectedServices[serviceID]; ok { + // If service is expected to still exist in Consul, then the ACL token for it should not be deleted. require.NoError(t, err) - require.EqualValues(t, 1, len(check)) - // Ignoring Namespace because the response from ENT includes it and OSS does not. - var ignoredFields = []string{"Node", "Definition", "Namespace"} - require.True(t, cmp.Equal(check[tt.expectedAgentHealthChecks[i].CheckID], tt.expectedAgentHealthChecks[i], cmpopts.IgnoreFields(api.AgentCheck{}, ignoredFields...))) + require.NotNil(t, token) + } else { + // If service should no longer exist, then ACL token for it should be deleted. + require.EqualError(t, err, "Unexpected response code: 403 (ACL not found)") } } - }) - } + } + }) } } } @@ -1169,6 +1400,7 @@ func TestReconcileDeleteEndpointWithNamespaces(t *testing.T) { name string consulSvcName string initialConsulSvcs []*api.AgentServiceRegistration + enableACLs bool }{ { name: "Consul service name matches K8s service name", @@ -1225,6 +1457,44 @@ func TestReconcileDeleteEndpointWithNamespaces(t *testing.T) { }, }, }, + { + name: "When ACLs are enabled, the ACL token should be deleted", + consulSvcName: "service-deleted", + initialConsulSvcs: []*api.AgentServiceRegistration{ + { + ID: "pod1-service-deleted", + Name: "service-deleted", + Port: 80, + Address: "1.2.3.4", + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-deleted", + MetaKeyKubeNS: ts.SourceKubeNS, + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod1", + }, + Namespace: ts.ExpConsulNS, + }, + { + Kind: api.ServiceKindConnectProxy, + ID: "pod1-service-deleted-sidecar-proxy", + Name: "service-deleted-sidecar-proxy", + Port: 20000, + Address: "1.2.3.4", + Proxy: &api.AgentServiceConnectProxyConfig{ + DestinationServiceName: "service-deleted", + DestinationServiceID: "pod1-service-deleted", + }, + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-deleted", + MetaKeyKubeNS: ts.SourceKubeNS, + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod1", + }, + Namespace: ts.ExpConsulNS, + }, + }, + enableACLs: true, + }, } for _, tt := range cases { t.Run(fmt.Sprintf("%s:%s", name, tt.name), func(t *testing.T) { @@ -1239,7 +1509,12 @@ func TestReconcileDeleteEndpointWithNamespaces(t *testing.T) { fakeClient := fake.NewClientBuilder().WithRuntimeObjects(fakeClientPod).Build() // Create test Consul server. + adminToken := "123e4567-e89b-12d3-a456-426614174000" consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { + if tt.enableACLs { + c.ACL.Enabled = true + c.ACL.Tokens.Master = adminToken + } c.NodeName = nodeName }) require.NoError(t, err) @@ -1250,6 +1525,9 @@ func TestReconcileDeleteEndpointWithNamespaces(t *testing.T) { Address: consul.HTTPAddr, Namespace: ts.ExpConsulNS, } + if tt.enableACLs { + cfg.Token = adminToken + } consulClient, err := api.NewClient(cfg) require.NoError(t, err) addr := strings.Split(consul.HTTPAddr, ":") @@ -1259,9 +1537,30 @@ func TestReconcileDeleteEndpointWithNamespaces(t *testing.T) { require.NoError(t, err) // Register service and proxy in consul. + var token *api.ACLToken for _, svc := range tt.initialConsulSvcs { err = consulClient.Agent().ServiceRegister(svc) require.NoError(t, err) + // Create a token for it if ACLs are enabled. + if tt.enableACLs { + if svc.Kind != api.ServiceKindConnectProxy { + var writeOpts api.WriteOptions + // When mirroring is enabled, the auth method will be created in the "default" Consul namespace. + if ts.Mirror { + writeOpts.Namespace = "default" + } + test.SetupK8sAuthMethodWithNamespaces(t, consulClient, svc.Name, svc.Meta[MetaKeyKubeNS], ts.ExpConsulNS, ts.Mirror, ts.MirrorPrefix) + token, _, err = consulClient.ACL().Login(&api.ACLLoginParams{ + AuthMethod: test.AuthMethod, + BearerToken: test.ServiceAccountJWTToken, + Meta: map[string]string{ + TokenMetaPodNameKey: fmt.Sprintf("%s/%s", svc.Meta[MetaKeyKubeNS], svc.Meta[MetaKeyPodName]), + }, + }, &writeOpts) + + require.NoError(t, err) + } + } } // Create the endpoints controller. @@ -1281,6 +1580,9 @@ func TestReconcileDeleteEndpointWithNamespaces(t *testing.T) { NSMirroringPrefix: ts.MirrorPrefix, ConsulDestinationNamespace: ts.DestConsulNS, } + if tt.enableACLs { + ep.AuthMethod = test.AuthMethod + } // Set up the Endpoint that will be reconciled, and reconcile. namespacedName := types.NamespacedName{ @@ -1301,6 +1603,10 @@ func TestReconcileDeleteEndpointWithNamespaces(t *testing.T) { require.NoError(t, err) require.Empty(t, proxyServiceInstances) + if tt.enableACLs { + _, _, err = consulClient.ACL().TokenRead(token.AccessorID, nil) + require.EqualError(t, err, "Unexpected response code: 403 (ACL not found)") + } }) } } diff --git a/connect-inject/endpoints_controller_test.go b/connect-inject/endpoints_controller_test.go index db4617ecec..d9ce5d0915 100644 --- a/connect-inject/endpoints_controller_test.go +++ b/connect-inject/endpoints_controller_test.go @@ -11,7 +11,7 @@ import ( logrtest "github.com/go-logr/logr/testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/hashicorp/consul-k8s/subcommand/common" + "github.com/hashicorp/consul-k8s/helper/test" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/sdk/testutil" "github.com/stretchr/testify/require" @@ -123,7 +123,7 @@ func TestProcessUpstreamsTLSandACLs(t *testing.T) { nodeName := "test-node" masterToken := "b78d37c7-0ca7-5f4d-99ee-6d9975ce4586" - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) // Create test consul server with ACLs and TLS consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { c.ACL.Enabled = true @@ -1042,7 +1042,7 @@ func TestReconcileCreateEndpoint(t *testing.T) { // For the register and deregister codepath, this also tests that they work when the Consul service name is different // from the K8s service name. // This test covers EndpointsController.deregisterServiceOnAllAgents when services should be selectively deregistered -// since the map will not be nil. This test also runs each test with ACLs+TLS enabled and disabled, since it covers all the cases where a Consul client is created. +// since the map will not be nil. func TestReconcileUpdateEndpoint(t *testing.T) { t.Parallel() nodeName := "test-node" @@ -1051,10 +1051,10 @@ func TestReconcileUpdateEndpoint(t *testing.T) { consulSvcName string k8sObjects func() []runtime.Object initialConsulSvcs []*api.AgentServiceRegistration - expectedNumSvcInstances int expectedConsulSvcInstances []*api.CatalogService expectedProxySvcInstances []*api.CatalogService expectedAgentHealthChecks []*api.AgentCheck + enableACLs bool }{ // Legacy services are not managed by endpoints controller, but endpoints controller // will still add/update the legacy service's health checks. @@ -1105,7 +1105,6 @@ func TestReconcileUpdateEndpoint(t *testing.T) { }, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -1177,7 +1176,6 @@ func TestReconcileUpdateEndpoint(t *testing.T) { }, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -1257,7 +1255,6 @@ func TestReconcileUpdateEndpoint(t *testing.T) { }, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -1337,7 +1334,6 @@ func TestReconcileUpdateEndpoint(t *testing.T) { }, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -1419,7 +1415,6 @@ func TestReconcileUpdateEndpoint(t *testing.T) { }, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -1501,7 +1496,6 @@ func TestReconcileUpdateEndpoint(t *testing.T) { }, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -1560,7 +1554,12 @@ func TestReconcileUpdateEndpoint(t *testing.T) { Name: "service-updated", Port: 80, Address: "1.2.3.4", - Meta: map[string]string{MetaKeyKubeNS: "default"}, + Meta: map[string]string{ + MetaKeyKubeNS: "default", + MetaKeyPodName: "pod1", + MetaKeyKubeServiceName: "service-updated", + MetaKeyManagedBy: managedByValue, + }, }, { Kind: api.ServiceKindConnectProxy, @@ -1568,14 +1567,18 @@ func TestReconcileUpdateEndpoint(t *testing.T) { Name: "service-updated-sidecar-proxy", Port: 20000, Address: "1.2.3.4", - Meta: map[string]string{MetaKeyKubeNS: "default"}, + Meta: map[string]string{ + MetaKeyKubeNS: "default", + MetaKeyPodName: "pod1", + MetaKeyKubeServiceName: "service-updated", + MetaKeyManagedBy: managedByValue, + }, Proxy: &api.AgentServiceConnectProxyConfig{ DestinationServiceName: "service-updated", DestinationServiceID: "pod1-service-updated", }, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -1624,7 +1627,12 @@ func TestReconcileUpdateEndpoint(t *testing.T) { Name: "different-consul-svc-name", Port: 80, Address: "1.2.3.4", - Meta: map[string]string{MetaKeyManagedBy: managedByValue, MetaKeyKubeNS: "default"}, + Meta: map[string]string{ + MetaKeyManagedBy: managedByValue, + MetaKeyKubeNS: "default", + MetaKeyPodName: "pod1", + MetaKeyKubeServiceName: "service-updated", + }, }, { Kind: api.ServiceKindConnectProxy, @@ -1636,9 +1644,14 @@ func TestReconcileUpdateEndpoint(t *testing.T) { DestinationServiceName: "different-consul-svc-name", DestinationServiceID: "pod1-different-consul-svc-name", }, + Meta: map[string]string{ + MetaKeyManagedBy: managedByValue, + MetaKeyKubeNS: "default", + MetaKeyPodName: "pod1", + MetaKeyKubeServiceName: "service-updated", + }, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-different-consul-svc-name", @@ -1653,7 +1666,7 @@ func TestReconcileUpdateEndpoint(t *testing.T) { }, }, { - name: "Endpoints has additional address not in Consul.", + name: "Endpoints has additional address not in Consul", consulSvcName: "service-updated", k8sObjects: func() []runtime.Object { pod1 := createPod("pod1", "1.2.3.4", true, true) @@ -1710,7 +1723,6 @@ func TestReconcileUpdateEndpoint(t *testing.T) { }, }, }, - expectedNumSvcInstances: 2, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -1753,7 +1765,7 @@ func TestReconcileUpdateEndpoint(t *testing.T) { }, }, { - name: "Consul has instances that are not in the Endpoints addresses.", + name: "Consul has instances that are not in the Endpoints addresses", consulSvcName: "service-updated", k8sObjects: func() []runtime.Object { pod1 := createPod("pod1", "1.2.3.4", true, true) @@ -1820,7 +1832,6 @@ func TestReconcileUpdateEndpoint(t *testing.T) { Meta: map[string]string{"k8s-service-name": "service-updated", "k8s-namespace": "default", MetaKeyManagedBy: managedByValue}, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-service-updated", @@ -1835,7 +1846,7 @@ func TestReconcileUpdateEndpoint(t *testing.T) { }, }, { - name: "Different Consul service name: Consul has instances that are not in the Endpoints addresses.", + name: "Different Consul service name: Consul has instances that are not in the Endpoints addresses", consulSvcName: "different-consul-svc-name", k8sObjects: func() []runtime.Object { pod1 := createPod("pod1", "1.2.3.4", true, true) @@ -1903,7 +1914,6 @@ func TestReconcileUpdateEndpoint(t *testing.T) { Meta: map[string]string{"k8s-service-name": "service-updated", "k8s-namespace": "default", MetaKeyManagedBy: managedByValue}, }, }, - expectedNumSvcInstances: 1, expectedConsulSvcInstances: []*api.CatalogService{ { ServiceID: "pod1-different-consul-svc-name", @@ -1971,7 +1981,6 @@ func TestReconcileUpdateEndpoint(t *testing.T) { Meta: map[string]string{"k8s-service-name": "service-updated", "k8s-namespace": "default", MetaKeyManagedBy: managedByValue}, }, }, - expectedNumSvcInstances: 0, expectedConsulSvcInstances: []*api.CatalogService{}, expectedProxySvcInstances: []*api.CatalogService{}, }, @@ -2029,123 +2038,362 @@ func TestReconcileUpdateEndpoint(t *testing.T) { Meta: map[string]string{"k8s-service-name": "service-updated", "k8s-namespace": "default", MetaKeyManagedBy: managedByValue}, }, }, - expectedNumSvcInstances: 0, expectedConsulSvcInstances: []*api.CatalogService{}, expectedProxySvcInstances: []*api.CatalogService{}, }, + { + name: "ACLs enabled: Endpoints has an updated address because the target pod changes", + consulSvcName: "service-updated", + k8sObjects: func() []runtime.Object { + pod2 := createPod("pod2", "4.4.4.4", true, true) + endpoint := &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-updated", + Namespace: "default", + }, + Subsets: []corev1.EndpointSubset{ + { + Addresses: []corev1.EndpointAddress{ + { + IP: "4.4.4.4", + NodeName: &nodeName, + TargetRef: &corev1.ObjectReference{ + Kind: "Pod", + Name: "pod2", + Namespace: "default", + }, + }, + }, + }, + }, + } + return []runtime.Object{pod2, endpoint} + }, + initialConsulSvcs: []*api.AgentServiceRegistration{ + { + ID: "pod1-service-updated", + Name: "service-updated", + Port: 80, + Address: "1.2.3.4", + Meta: map[string]string{ + MetaKeyKubeNS: "default", + MetaKeyPodName: "pod1", + MetaKeyKubeServiceName: "service-updated", + MetaKeyManagedBy: managedByValue, + }, + }, + { + Kind: api.ServiceKindConnectProxy, + ID: "pod1-service-updated-sidecar-proxy", + Name: "service-updated-sidecar-proxy", + Port: 20000, + Address: "1.2.3.4", + Meta: map[string]string{ + MetaKeyKubeNS: "default", + MetaKeyPodName: "pod1", + MetaKeyKubeServiceName: "service-updated", + MetaKeyManagedBy: managedByValue, + }, + Proxy: &api.AgentServiceConnectProxyConfig{ + DestinationServiceName: "service-updated", + DestinationServiceID: "pod1-service-updated", + }, + }, + }, + expectedConsulSvcInstances: []*api.CatalogService{ + { + ServiceID: "pod2-service-updated", + ServiceAddress: "4.4.4.4", + ServiceMeta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: "default", + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod2", + }, + }, + }, + expectedProxySvcInstances: []*api.CatalogService{ + { + ServiceID: "pod2-service-updated-sidecar-proxy", + ServiceAddress: "4.4.4.4", + ServiceMeta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: "default", + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod2", + }, + }, + }, + enableACLs: true, + }, + { + name: "ACLs enabled: Consul has instances that are not in the Endpoints addresses", + consulSvcName: "service-updated", + k8sObjects: func() []runtime.Object { + pod1 := createPod("pod1", "1.2.3.4", true, true) + endpoint := &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-updated", + Namespace: "default", + }, + Subsets: []corev1.EndpointSubset{ + { + Addresses: []corev1.EndpointAddress{ + { + IP: "1.2.3.4", + NodeName: &nodeName, + TargetRef: &corev1.ObjectReference{ + Kind: "Pod", + Name: "pod1", + Namespace: "default", + }, + }, + }, + }, + }, + } + return []runtime.Object{pod1, endpoint} + }, + initialConsulSvcs: []*api.AgentServiceRegistration{ + { + ID: "pod1-service-updated", + Name: "service-updated", + Port: 80, + Address: "1.2.3.4", + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: "default", + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod1", + }, + }, + { + Kind: api.ServiceKindConnectProxy, + ID: "pod1-service-updated-sidecar-proxy", + Name: "service-updated-sidecar-proxy", + Port: 20000, + Address: "1.2.3.4", + Proxy: &api.AgentServiceConnectProxyConfig{ + DestinationServiceName: "service-updated", + DestinationServiceID: "pod1-service-updated", + }, + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: "default", + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod1", + }, + }, + { + ID: "pod2-service-updated", + Name: "service-updated", + Port: 80, + Address: "2.2.3.4", + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: "default", + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod2", + }, + }, + { + Kind: api.ServiceKindConnectProxy, + ID: "pod2-service-updated-sidecar-proxy", + Name: "service-updated-sidecar-proxy", + Port: 20000, + Address: "2.2.3.4", + Proxy: &api.AgentServiceConnectProxyConfig{ + DestinationServiceName: "service-updated", + DestinationServiceID: "pod2-service-updated", + }, + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: "default", + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod2", + }, + }, + }, + expectedConsulSvcInstances: []*api.CatalogService{ + { + ServiceID: "pod1-service-updated", + ServiceName: "service-updated", + ServiceAddress: "1.2.3.4", + ServiceMeta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: "default", + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod1", + }, + }, + }, + expectedProxySvcInstances: []*api.CatalogService{ + { + ServiceID: "pod1-service-updated-sidecar-proxy", + ServiceName: "service-updated-sidecar-proxy", + ServiceAddress: "1.2.3.4", + ServiceMeta: map[string]string{ + MetaKeyKubeServiceName: "service-updated", + MetaKeyKubeNS: "default", + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod1", + }, + }, + }, + enableACLs: true, + }, } - // Each test is run with ACLs+TLS (secure) enabled and disabled. - for _, secure := range []bool{true, false} { - for _, tt := range cases { - t.Run(fmt.Sprintf("%s - secure: %v", tt.name, secure), func(t *testing.T) { - // The agent pod needs to have the address 127.0.0.1 so when the - // code gets the agent pods via the label component=client, and - // makes requests against the agent API, it will actually hit the - // test server we have on localhost. - fakeClientPod := createPod("fake-consul-client", "127.0.0.1", false, true) - fakeClientPod.Labels = map[string]string{"component": "client", "app": "consul", "release": "consul"} + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + // The agent pod needs to have the address 127.0.0.1 so when the + // code gets the agent pods via the label component=client, and + // makes requests against the agent API, it will actually hit the + // test server we have on localhost. + fakeClientPod := createPod("fake-consul-client", "127.0.0.1", false, true) + fakeClientPod.Labels = map[string]string{"component": "client", "app": "consul", "release": "consul"} - // Add the default namespace. - ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}} - // Create fake k8s client - k8sObjects := append(tt.k8sObjects(), fakeClientPod, &ns) - fakeClient := fake.NewClientBuilder().WithRuntimeObjects(k8sObjects...).Build() + // Add the default namespace. + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + // Create fake k8s client. + k8sObjects := append(tt.k8sObjects(), fakeClientPod, &ns) + fakeClient := fake.NewClientBuilder().WithRuntimeObjects(k8sObjects...).Build() - masterToken := "b78d37c7-0ca7-5f4d-99ee-6d9975ce4586" - caFile, certFile, keyFile := common.GenerateServerCerts(t) - // Create test consul server, with ACLs+TLS if necessary - consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { - if secure { - c.ACL.Enabled = true - c.ACL.DefaultPolicy = "deny" - c.ACL.Tokens.Master = masterToken - c.CAFile = caFile - c.CertFile = certFile - c.KeyFile = keyFile - } - c.NodeName = nodeName - }) + // Create test consul server. + adminToken := "123e4567-e89b-12d3-a456-426614174000" + consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { + if tt.enableACLs { + c.ACL.Enabled = tt.enableACLs + c.ACL.Tokens.Master = adminToken + } + c.NodeName = nodeName + }) + require.NoError(t, err) + defer consul.Stop() + consul.WaitForServiceIntentions(t) + addr := strings.Split(consul.HTTPAddr, ":") + consulPort := addr[1] + + cfg := &api.Config{Scheme: "http", Address: consul.HTTPAddr} + if tt.enableACLs { + cfg.Token = adminToken + } + consulClient, err := api.NewClient(cfg) + require.NoError(t, err) + + // Holds token accessorID for each service ID. + tokensForServices := make(map[string]string) + + // Register service and proxy in consul. + for _, svc := range tt.initialConsulSvcs { + err = consulClient.Agent().ServiceRegister(svc) require.NoError(t, err) - defer consul.Stop() - consul.WaitForServiceIntentions(t) - addr := strings.Split(consul.HTTPAddr, ":") - consulPort := addr[1] - cfg := &api.Config{ - Scheme: "http", - Address: consul.HTTPAddr, - } - if secure { - consulPort = strings.Split(consul.HTTPSAddr, ":")[1] - cfg.Address = consul.HTTPSAddr - cfg.Scheme = "https" - cfg.TLSConfig = api.TLSConfig{ - CAFile: caFile, + // Create a token for this service if ACLs are enabled. + if tt.enableACLs { + if svc.Kind != api.ServiceKindConnectProxy { + test.SetupK8sAuthMethod(t, consulClient, svc.Name, svc.Meta[MetaKeyKubeNS]) + token, _, err := consulClient.ACL().Login(&api.ACLLoginParams{ + AuthMethod: test.AuthMethod, + BearerToken: test.ServiceAccountJWTToken, + Meta: map[string]string{ + TokenMetaPodNameKey: fmt.Sprintf("%s/%s", svc.Meta[MetaKeyKubeNS], svc.Meta[MetaKeyPodName]), + }, + }, nil) + // Record each token we create. + require.NoError(t, err) + tokensForServices[svc.ID] = token.AccessorID + + // Create another token for the same service but a pod that no longer exists. + // This is to test a scenario with orphaned tokens + // where we have a token for the pod but the service instance + // for that pod no longer exists in Consul. + // In that case, the token should still be deleted. + token, _, err = consulClient.ACL().Login(&api.ACLLoginParams{ + AuthMethod: test.AuthMethod, + BearerToken: test.ServiceAccountJWTToken, + Meta: map[string]string{ + TokenMetaPodNameKey: fmt.Sprintf("%s/%s", svc.Meta[MetaKeyKubeNS], "does-not-exist"), + }, + }, nil) + require.NoError(t, err) + tokensForServices["does-not-exist"+svc.Name] = token.AccessorID } - cfg.Token = masterToken } - consulClient, err := api.NewClient(cfg) - require.NoError(t, err) + } + + // Create the endpoints controller. + ep := &EndpointsController{ + Client: fakeClient, + Log: logrtest.TestLogger{T: t}, + ConsulClient: consulClient, + ConsulPort: consulPort, + ConsulScheme: cfg.Scheme, + AllowK8sNamespacesSet: mapset.NewSetWith("*"), + DenyK8sNamespacesSet: mapset.NewSetWith(), + ReleaseName: "consul", + ReleaseNamespace: "default", + ConsulClientCfg: cfg, + } + if tt.enableACLs { + ep.AuthMethod = test.AuthMethod + } + namespacedName := types.NamespacedName{Namespace: "default", Name: "service-updated"} - // Register service and proxy in consul - for _, svc := range tt.initialConsulSvcs { - err = consulClient.Agent().ServiceRegister(svc) + resp, err := ep.Reconcile(context.Background(), ctrl.Request{NamespacedName: namespacedName}) + require.NoError(t, err) + require.False(t, resp.Requeue) + + // After reconciliation, Consul should have service-updated with the correct number of instances. + serviceInstances, _, err := consulClient.Catalog().Service(tt.consulSvcName, "", nil) + require.NoError(t, err) + require.Len(t, serviceInstances, len(tt.expectedConsulSvcInstances)) + for i, instance := range serviceInstances { + require.Equal(t, tt.expectedConsulSvcInstances[i].ServiceID, instance.ServiceID) + require.Equal(t, tt.expectedConsulSvcInstances[i].ServiceAddress, instance.ServiceAddress) + } + proxyServiceInstances, _, err := consulClient.Catalog().Service(fmt.Sprintf("%s-sidecar-proxy", tt.consulSvcName), "", nil) + require.NoError(t, err) + require.Len(t, proxyServiceInstances, len(tt.expectedProxySvcInstances)) + for i, instance := range proxyServiceInstances { + require.Equal(t, tt.expectedProxySvcInstances[i].ServiceID, instance.ServiceID) + require.Equal(t, tt.expectedProxySvcInstances[i].ServiceAddress, instance.ServiceAddress) + } + // Check that the Consul health check was created for the k8s pod. + if tt.expectedAgentHealthChecks != nil { + for i := range tt.expectedConsulSvcInstances { + filter := fmt.Sprintf("CheckID == `%s`", tt.expectedAgentHealthChecks[i].CheckID) + check, err := consulClient.Agent().ChecksWithFilter(filter) require.NoError(t, err) + require.EqualValues(t, len(check), 1) + // Ignoring Namespace because the response from ENT includes it and OSS does not. + var ignoredFields = []string{"Node", "Definition", "Namespace"} + require.True(t, cmp.Equal(check[tt.expectedAgentHealthChecks[i].CheckID], tt.expectedAgentHealthChecks[i], cmpopts.IgnoreFields(api.AgentCheck{}, ignoredFields...))) } + } - // Create the endpoints controller - ep := &EndpointsController{ - Client: fakeClient, - Log: logrtest.TestLogger{T: t}, - ConsulClient: consulClient, - ConsulPort: consulPort, - ConsulScheme: cfg.Scheme, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSetWith(), - ReleaseName: "consul", - ReleaseNamespace: "default", - ConsulClientCfg: cfg, - } - namespacedName := types.NamespacedName{ - Namespace: "default", - Name: "service-updated", + if tt.enableACLs { + // Put expected services into a map to make it easier to find service IDs. + expectedServices := make(map[string]struct{}) + for _, svc := range tt.expectedConsulSvcInstances { + expectedServices[svc.ServiceID] = struct{}{} } - resp, err := ep.Reconcile(context.Background(), ctrl.Request{ - NamespacedName: namespacedName, - }) - require.NoError(t, err) - require.False(t, resp.Requeue) - - // After reconciliation, Consul should have service-updated with the correct number of instances - serviceInstances, _, err := consulClient.Catalog().Service(tt.consulSvcName, "", nil) - require.NoError(t, err) - require.Len(t, serviceInstances, tt.expectedNumSvcInstances) - for i, instance := range serviceInstances { - require.Equal(t, tt.expectedConsulSvcInstances[i].ServiceID, instance.ServiceID) - require.Equal(t, tt.expectedConsulSvcInstances[i].ServiceAddress, instance.ServiceAddress) - } - proxyServiceInstances, _, err := consulClient.Catalog().Service(fmt.Sprintf("%s-sidecar-proxy", tt.consulSvcName), "", nil) - require.NoError(t, err) - require.Len(t, proxyServiceInstances, tt.expectedNumSvcInstances) - for i, instance := range proxyServiceInstances { - require.Equal(t, tt.expectedProxySvcInstances[i].ServiceID, instance.ServiceID) - require.Equal(t, tt.expectedProxySvcInstances[i].ServiceAddress, instance.ServiceAddress) - } - // Check that the Consul health check was created for the k8s pod. - if tt.expectedAgentHealthChecks != nil { - for i, _ := range tt.expectedConsulSvcInstances { - filter := fmt.Sprintf("CheckID == `%s`", tt.expectedAgentHealthChecks[i].CheckID) - check, err := consulClient.Agent().ChecksWithFilter(filter) - require.NoError(t, err) - require.EqualValues(t, len(check), 1) - // Ignoring Namespace because the response from ENT includes it and OSS does not. - var ignoredFields = []string{"Node", "Definition", "Namespace"} - require.True(t, cmp.Equal(check[tt.expectedAgentHealthChecks[i].CheckID], tt.expectedAgentHealthChecks[i], cmpopts.IgnoreFields(api.AgentCheck{}, ignoredFields...))) + // Look through the tokens we've created and check that only + // tokens for the deregistered services have been deleted. + for serviceID, tokenID := range tokensForServices { + // Read the token from Consul. + token, _, err := consulClient.ACL().TokenRead(tokenID, nil) + if _, ok := expectedServices[serviceID]; ok { + // If service is expected to still exist in Consul, then the ACL token for it should not be deleted. + require.NoError(t, err, "token should exist for service instance: "+serviceID) + require.NotNil(t, token) + } else { + // If service should no longer exist, then ACL token for it should be deleted. + require.EqualError(t, err, "Unexpected response code: 403 (ACL not found)") } } - }) - } + } + }) } } @@ -2159,6 +2407,7 @@ func TestReconcileDeleteEndpoint(t *testing.T) { consulSvcName string legacyService bool initialConsulSvcs []*api.AgentServiceRegistration + enableACLs bool }{ { name: "Legacy service: does not delete", @@ -2236,6 +2485,42 @@ func TestReconcileDeleteEndpoint(t *testing.T) { }, }, }, + { + name: "When ACLs are enabled, the token should be deleted", + consulSvcName: "service-deleted", + initialConsulSvcs: []*api.AgentServiceRegistration{ + { + ID: "pod1-service-deleted", + Name: "service-deleted", + Port: 80, + Address: "1.2.3.4", + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-deleted", + MetaKeyKubeNS: "default", + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod1", + }, + }, + { + Kind: api.ServiceKindConnectProxy, + ID: "pod1-service-deleted-sidecar-proxy", + Name: "service-deleted-sidecar-proxy", + Port: 20000, + Address: "1.2.3.4", + Proxy: &api.AgentServiceConnectProxyConfig{ + DestinationServiceName: "service-deleted", + DestinationServiceID: "pod1-service-deleted", + }, + Meta: map[string]string{ + MetaKeyKubeServiceName: "service-deleted", + MetaKeyKubeNS: "default", + MetaKeyManagedBy: managedByValue, + MetaKeyPodName: "pod1", + }, + }, + }, + enableACLs: true, + }, } for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { @@ -2248,19 +2533,25 @@ func TestReconcileDeleteEndpoint(t *testing.T) { // Add the default namespace. ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}} - // Create fake k8s client + // Create fake k8s client. fakeClient := fake.NewClientBuilder().WithRuntimeObjects(fakeClientPod, &ns).Build() - // Create test consul server + // Create test consul server. + adminToken := "123e4567-e89b-12d3-a456-426614174000" consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { + if tt.enableACLs { + c.ACL.Enabled = true + c.ACL.Tokens.Master = adminToken + } c.NodeName = nodeName }) require.NoError(t, err) defer consul.Stop() consul.WaitForServiceIntentions(t) - cfg := &api.Config{ - Address: consul.HTTPAddr, + cfg := &api.Config{Address: consul.HTTPAddr} + if tt.enableACLs { + cfg.Token = adminToken } consulClient, err := api.NewClient(cfg) require.NoError(t, err) @@ -2268,9 +2559,26 @@ func TestReconcileDeleteEndpoint(t *testing.T) { consulPort := addr[1] // Register service and proxy in consul + var token *api.ACLToken for _, svc := range tt.initialConsulSvcs { err = consulClient.Agent().ServiceRegister(svc) require.NoError(t, err) + + // Create a token for it if ACLs are enabled. + if tt.enableACLs { + test.SetupK8sAuthMethod(t, consulClient, svc.Name, "default") + if svc.Kind != api.ServiceKindConnectProxy { + token, _, err = consulClient.ACL().Login(&api.ACLLoginParams{ + AuthMethod: test.AuthMethod, + BearerToken: test.ServiceAccountJWTToken, + Meta: map[string]string{ + "pod": fmt.Sprintf("%s/%s", svc.Meta[MetaKeyKubeNS], svc.Meta[MetaKeyPodName]), + }, + }, nil) + + require.NoError(t, err) + } + } } // Create the endpoints controller @@ -2286,6 +2594,9 @@ func TestReconcileDeleteEndpoint(t *testing.T) { ReleaseNamespace: "default", ConsulClientCfg: cfg, } + if tt.enableACLs { + ep.AuthMethod = test.AuthMethod + } // Set up the Endpoint that will be reconciled, and reconcile namespacedName := types.NamespacedName{ @@ -2312,6 +2623,10 @@ func TestReconcileDeleteEndpoint(t *testing.T) { require.NoError(t, err) require.Empty(t, proxyServiceInstances) + if tt.enableACLs { + _, _, err = consulClient.ACL().TokenRead(token.AccessorID, nil) + require.EqualError(t, err, "Unexpected response code: 403 (ACL not found)") + } }) } } @@ -4430,6 +4745,31 @@ func TestRegisterServicesAndHealthCheck_skipsWhenDuplicateServiceFound(t *testin } } +func TestGetTokenMetaFromDescription(t *testing.T) { + t.Parallel() + cases := map[string]struct { + description string + expectedTokenMeta map[string]string + }{ + "no description prefix": { + description: `{"pod":"default/pod"}`, + expectedTokenMeta: map[string]string{"pod": "default/pod"}, + }, + "consul's default description prefix": { + description: `token created via login: {"pod":"default/pod"}`, + expectedTokenMeta: map[string]string{"pod": "default/pod"}, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + tokenMeta, err := getTokenMetaFromDescription(c.description) + require.NoError(t, err) + require.Equal(t, c.expectedTokenMeta, tokenMeta) + }) + } +} + func createPod(name, ip string, inject bool, managedByEndpointsController bool) *corev1.Pod { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ diff --git a/connect-inject/handler.go b/connect-inject/handler.go index 6de88d8979..4c6d2c7b2d 100644 --- a/connect-inject/handler.go +++ b/connect-inject/handler.go @@ -58,7 +58,7 @@ type Handler struct { RequireAnnotation bool // AuthMethod is the name of the Kubernetes Auth Method to - // use for identity with connectInjection if ACLs are enabled + // use for identity with connectInjection if ACLs are enabled. AuthMethod string // The PEM-encoded CA certificate string diff --git a/helper/test/test_util.go b/helper/test/test_util.go new file mode 100644 index 0000000000..1db22103f4 --- /dev/null +++ b/helper/test/test_util.go @@ -0,0 +1,187 @@ +package test + +import ( + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "os" + "strconv" + "testing" + "time" + + "github.com/hashicorp/consul-k8s/helper/cert" + "github.com/hashicorp/consul/api" + "github.com/stretchr/testify/require" +) + +// GenerateServerCerts generates Consul CA +// and a server certificate and saves them to temp files. +// It returns file names in this order: +// CA certificate, server certificate, and server key. +func GenerateServerCerts(t *testing.T) (string, string, string) { + require := require.New(t) + + caFile, err := ioutil.TempFile("", "ca") + require.NoError(err) + + certFile, err := ioutil.TempFile("", "cert") + require.NoError(err) + + certKeyFile, err := ioutil.TempFile("", "key") + require.NoError(err) + + // Generate CA + signer, _, caCertPem, caCertTemplate, err := cert.GenerateCA("Consul Agent CA - Test") + require.NoError(err) + + // Generate Server Cert + name := "server.dc1.consul" + hosts := []string{name, "localhost", "127.0.0.1"} + certPem, keyPem, err := cert.GenerateCert(name, 1*time.Hour, caCertTemplate, signer, hosts) + require.NoError(err) + + // Write certs and key to files + _, err = caFile.WriteString(caCertPem) + require.NoError(err) + _, err = certFile.WriteString(certPem) + require.NoError(err) + _, err = certKeyFile.WriteString(keyPem) + require.NoError(err) + + t.Cleanup(func() { + os.Remove(caFile.Name()) + os.Remove(certFile.Name()) + os.Remove(certKeyFile.Name()) + }) + return caFile.Name(), certFile.Name(), certKeyFile.Name() +} + +// SetupK8sAuthMethod create a k8s auth method and a binding rule in Consul for the +// given k8s service and namespace. +func SetupK8sAuthMethod(t *testing.T, consulClient *api.Client, serviceName, k8sServiceNS string) { + SetupK8sAuthMethodWithNamespaces(t, consulClient, serviceName, k8sServiceNS, "", false, "") +} + +// SetupK8sAuthMethodWithNamespaces creates a k8s auth method and binding rule +// in Consul for the k8s service name and namespace. It sets up the auth method and the binding +// rule so that it works with consul namespaces. +func SetupK8sAuthMethodWithNamespaces(t *testing.T, consulClient *api.Client, serviceName, k8sServiceNS, consulNS string, mirrorNS bool, nsPrefix string) { + t.Helper() + // Start the mock k8s server. + k8sMockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("content-type", "application/json") + if r != nil && r.URL.Path == "/apis/authentication.k8s.io/v1/tokenreviews" && r.Method == "POST" { + w.Write([]byte(tokenReviewsResponse(serviceName, k8sServiceNS))) + } + if r != nil && r.URL.Path == fmt.Sprintf("/api/v1/namespaces/%s/serviceaccounts/%s", k8sServiceNS, serviceName) && + r.Method == "GET" { + w.Write([]byte(serviceAccountGetResponse(serviceName, k8sServiceNS))) + } + })) + t.Cleanup(k8sMockServer.Close) + + // Set up Consul's auth method. + authMethodTmpl := api.ACLAuthMethod{ + Name: AuthMethod, + Type: "kubernetes", + Description: "Kubernetes Auth Method", + Config: map[string]interface{}{ + "Host": k8sMockServer.URL, + "CACert": serviceAccountCACert, + "ServiceAccountJWT": ServiceAccountJWTToken, + }, + Namespace: consulNS, + } + if mirrorNS { + authMethodTmpl.Namespace = "default" + authMethodTmpl.Config["MapNamespaces"] = strconv.FormatBool(mirrorNS) + authMethodTmpl.Config["ConsulNamespacePrefix"] = nsPrefix + } + // This API call will idempotently create the auth method (it won't fail if it already exists). + _, _, err := consulClient.ACL().AuthMethodCreate(&authMethodTmpl, nil) + require.NoError(t, err) + + // Create the binding rule. + aclBindingRule := api.ACLBindingRule{ + Description: "Kubernetes binding rule", + AuthMethod: AuthMethod, + BindType: api.BindingRuleBindTypeService, + BindName: "${serviceaccount.name}", + Selector: "serviceaccount.name!=default", + Namespace: consulNS, + } + if mirrorNS { + aclBindingRule.Namespace = "default" + } + // This API call will idempotently create the binding rule (it won't fail if it already exists). + _, _, err = consulClient.ACL().BindingRuleCreate(&aclBindingRule, nil) + require.NoError(t, err) +} + +func tokenReviewsResponse(name, ns string) string { + return fmt.Sprintf(`{ + "kind": "TokenReview", + "apiVersion": "authentication.k8s.io/v1", + "metadata": { + "creationTimestamp": null + }, + "spec": { + "token": "eyJhbGciOiJSUzI1NiIsImtpZCI6IiJ9.eyJpc3MiOiJrdWJlcm5ldGVzL3NlcnZpY2VhY2NvdW50Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9uYW1lc3BhY2UiOiJkZWZhdWx0Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9zZWNyZXQubmFtZSI6ImRlbW8tdG9rZW4tbTljdm4iLCJrdWJlcm5ldGVzLmlvL3NlcnZpY2VhY2NvdW50L3NlcnZpY2UtYWNjb3VudC5uYW1lIjoiZGVtbyIsImt1YmVybmV0ZXMuaW8vc2VydmljZWFjY291bnQvc2VydmljZS1hY2NvdW50LnVpZCI6IjlmZjUxZmY0LTU1N2UtMTFlOS05Njg3LTQ4ZTZjOGI4ZWNiNSIsInN1YiI6InN5c3RlbTpzZXJ2aWNlYWNjb3VudDpkZWZhdWx0OmRlbW8ifQ.UJEphtrN261gy9WCl4ZKjm2PRDLDkc3Xg9VcDGfzyroOqFQ6sog5dVAb9voc5Nc0-H5b1yGwxDViEMucwKvZpA5pi7VEx_OskK-KTWXSmafM0Xg_AvzpU9Ed5TSRno-OhXaAraxdjXoC4myh1ay2DMeHUusJg_ibqcYJrWx-6MO1bH_ObORtAKhoST_8fzkqNAlZmsQ87FinQvYN5mzDXYukl-eeRdBgQUBkWvEb-Ju6cc0-QE4sUQ4IH_fs0fUyX_xc0om0SZGWLP909FTz4V8LxV8kr6L7irxROiS1jn3Fvyc9ur1PamVf3JOPPrOyfmKbaGRiWJM32b3buQw7cg" + }, + "status": { + "authenticated": true, + "user": { + "username": "system:serviceaccount:%s:%s", + "uid": "9ff51ff4-557e-11e9-9687-48e6c8b8ecb5", + "groups": [ + "system:serviceaccounts", + "system:serviceaccounts:%s", + "system:authenticated" + ] + } + } +}`, ns, name, ns) +} + +func serviceAccountGetResponse(name, ns string) string { + return fmt.Sprintf(`{ + "kind": "ServiceAccount", + "apiVersion": "v1", + "metadata": { + "name": "%s", + "namespace": "%s", + "selfLink": "/api/v1/namespaces/%s/serviceaccounts/%s", + "uid": "9ff51ff4-557e-11e9-9687-48e6c8b8ecb5", + "resourceVersion": "2101", + "creationTimestamp": "2019-04-02T19:36:34Z" + }, + "secrets": [ + { + "name": "%s-token-m9cvn" + } + ] +}`, name, ns, ns, name, name) +} + +const AuthMethod = "consul-k8s-auth-method" +const ServiceAccountJWTToken = `eyJhbGciOiJSUzI1NiIsImtpZCI6IiJ9.eyJpc3MiOiJrdWJlcm5ldGVzL3NlcnZpY2VhY2NvdW50Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9uYW1lc3BhY2UiOiJkZWZhdWx0Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9zZWNyZXQubmFtZSI6ImtoYWtpLWFyYWNobmlkLWNvbnN1bC1jb25uZWN0LWluamVjdG9yLWF1dGhtZXRob2Qtc3ZjLWFjY29obmRidiIsImt1YmVybmV0ZXMuaW8vc2VydmljZWFjY291bnQvc2VydmljZS1hY2NvdW50Lm5hbWUiOiJraGFraS1hcmFjaG5pZC1jb25zdWwtY29ubmVjdC1pbmplY3Rvci1hdXRobWV0aG9kLXN2Yy1hY2NvdW50Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9zZXJ2aWNlLWFjY291bnQudWlkIjoiN2U5NWUxMjktZTQ3My0xMWU5LThmYWEtNDIwMTBhODAwMTIyIiwic3ViIjoic3lzdGVtOnNlcnZpY2VhY2NvdW50OmRlZmF1bHQ6a2hha2ktYXJhY2huaWQtY29uc3VsLWNvbm5lY3QtaW5qZWN0b3ItYXV0aG1ldGhvZC1zdmMtYWNjb3VudCJ9.Yi63MMtzh5MBWKKd3a7dzCJjTITE15ikFy_Tnpdk_AwdwA9J4AMSGEeHN5vWtCuuFjo_lMJqBBPHkK2AqbnoFUj9m5CopWyqICJQlvEOP4fUQ-Rc0W1P_JjU1rZERHG39b5TMLgKPQguyhaiZEJ6CjVtm9wUTagrgiuqYV2iUqLuF6SYNm6SrKtkPS-lqIO-u7C06wVk5m5uqwIVQNpZSIC_5Ls5aLmyZU3nHvH-V7E3HmBhVyZAB76jgKB0TyVX1IOskt9PDFarNtU3suZyCjvqC-UJA6sYeySe4dBNKsKlSZ6YuxUUmn1Rgv32YMdImnsWg8khf-zJvqgWk7B5EA` +const serviceAccountCACert = `-----BEGIN CERTIFICATE----- +MIIDCzCCAfOgAwIBAgIQKzs7Njl9Hs6Xc8EXou25hzANBgkqhkiG9w0BAQsFADAv +MS0wKwYDVQQDEyQ1OWU2ZGM0MS0yMDhmLTQwOTUtYTI4OS0xZmM3MDBhYzFjYzgw +HhcNMTkwNjA3MTAxNzMxWhcNMjQwNjA1MTExNzMxWjAvMS0wKwYDVQQDEyQ1OWU2 +ZGM0MS0yMDhmLTQwOTUtYTI4OS0xZmM3MDBhYzFjYzgwggEiMA0GCSqGSIb3DQEB +AQUAA4IBDwAwggEKAoIBAQDZjHzwqofzTpGpc0MdICS7euvfujUKE3PC/apfDAgB +4jzEFKA78/9+KUGw/c/0SHeSQhN+a8gwlHRnAz1NJcfOIXy4dweUuOkAiFxH8pht +ECwkeNO7z8DoV8ceminCRHGjaRmoMxpZ7g2pZAJNZePxi3y1aNkFAXe9gSUSdjRZ +RXYka7wh2AO9k2dlGFAYB+t3vWwJ6twjG0TtKQrhYM9Od1/oN0E01LzBcZuxkN1k +8gfIHy7bOFCBM2WTEDW/0aAvcAPrO8DLqDJ+6Mjc3r7+zlzl8aQspb0S08pVzki5 +Dz//83kyu0phJuij5eB88V7UfPXxXF/EtV6fvrL7MN4fAgMBAAGjIzAhMA4GA1Ud +DwEB/wQEAwICBDAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBv +QsaG6qlcaRktJ0zGhxxJ52NnRV2GcIYPeN3Zv2VXe3ML3Vd6G32PV7lIOhjx3KmA +/uMh6NhqBzsekkTz0PuC3wJyM2OGonVQisFlqx9sFQ3fU2mIGXCa3wC8e/qP8BHS +w7/VeA7lzmj3TQRE/W0U0ZGeoAxn9b6JtT0iMucYvP0hXKTPBWlnzIijamU50r2Y +7ia065Ug2xUN5FLX/vxOA3y4rjpkjWoVQcu1p8TZrVoM3dsGFWp10fDMRiAHTvOH +Z23jGuk6rn9DUHC2xPj3wCTmd8SGEJoV31noJV5dVeQ90wusXz3vTG7ficKnvHFS +xtr5PSwH1DusYfVaGH2O +-----END CERTIFICATE-----` diff --git a/subcommand/common/test_util.go b/subcommand/common/test_util.go index 94ede7a5ad..cf53239841 100644 --- a/subcommand/common/test_util.go +++ b/subcommand/common/test_util.go @@ -4,54 +4,10 @@ import ( "io/ioutil" "os" "testing" - "time" - "github.com/hashicorp/consul-k8s/helper/cert" "github.com/stretchr/testify/require" ) -// GenerateServerCerts generates Consul CA -// and a server certificate and saves them to temp files. -// It returns file names in this order: -// CA certificate, server certificate, and server key. -func GenerateServerCerts(t *testing.T) (string, string, string) { - require := require.New(t) - - caFile, err := ioutil.TempFile("", "ca") - require.NoError(err) - - certFile, err := ioutil.TempFile("", "cert") - require.NoError(err) - - certKeyFile, err := ioutil.TempFile("", "key") - require.NoError(err) - - // Generate CA - signer, _, caCertPem, caCertTemplate, err := cert.GenerateCA("Consul Agent CA - Test") - require.NoError(err) - - // Generate Server Cert - name := "server.dc1.consul" - hosts := []string{name, "localhost", "127.0.0.1"} - certPem, keyPem, err := cert.GenerateCert(name, 1*time.Hour, caCertTemplate, signer, hosts) - require.NoError(err) - - // Write certs and key to files - _, err = caFile.WriteString(caCertPem) - require.NoError(err) - _, err = certFile.WriteString(certPem) - require.NoError(err) - _, err = certKeyFile.WriteString(keyPem) - require.NoError(err) - - t.Cleanup(func() { - os.Remove(caFile.Name()) - os.Remove(certFile.Name()) - os.Remove(certKeyFile.Name()) - }) - return caFile.Name(), certFile.Name(), certKeyFile.Name() -} - // WriteTempFile writes contents to a temporary file and returns the file // name. It will remove the file once the test completes. func WriteTempFile(t *testing.T, contents string) string { diff --git a/subcommand/connect-init/command_ent_test.go b/subcommand/connect-init/command_ent_test.go index dd32753aaf..1e81dab548 100644 --- a/subcommand/connect-init/command_ent_test.go +++ b/subcommand/connect-init/command_ent_test.go @@ -6,11 +6,10 @@ import ( "fmt" "io/ioutil" "math/rand" - "net/http" - "net/http/httptest" "os" "testing" + "github.com/hashicorp/consul-k8s/helper/test" "github.com/hashicorp/consul-k8s/namespaces" "github.com/hashicorp/consul-k8s/subcommand/common" "github.com/hashicorp/consul/api" @@ -25,7 +24,7 @@ func TestRun_ServicePollingWithACLsAndTLSWithNamespaces(t *testing.T) { name string tls bool consulServiceNamespace string - authMethod string + acls bool authMethodNamespace string }{ { @@ -33,42 +32,42 @@ func TestRun_ServicePollingWithACLsAndTLSWithNamespaces(t *testing.T) { tls: false, consulServiceNamespace: "default", authMethodNamespace: "default", - authMethod: "consul-k8s-auth-method", + acls: true, }, { name: "ACLs enabled, tls, serviceNS=default, authMethodNS=default", tls: true, consulServiceNamespace: "default", authMethodNamespace: "default", - authMethod: "consul-k8s-auth-method", + acls: true, }, { name: "ACLs enabled, no tls, serviceNS=default-ns, authMethodNS=default", tls: false, consulServiceNamespace: "default-ns", authMethodNamespace: "default", - authMethod: "consul-k8s-auth-method", + acls: true, }, { name: "ACLs enabled, tls, serviceNS=default-ns, authMethodNS=default", tls: true, consulServiceNamespace: "default-ns", authMethodNamespace: "default", - authMethod: "consul-k8s-auth-method", + acls: true, }, { name: "ACLs enabled, no tls, serviceNS=other, authMethodNS=other", tls: false, consulServiceNamespace: "other", authMethodNamespace: "other", - authMethod: "consul-k8s-auth-method", + acls: true, }, { name: "ACLs enabled, tls, serviceNS=other, authMethodNS=other", tls: true, consulServiceNamespace: "other", authMethodNamespace: "other", - authMethod: "consul-k8s-auth-method", + acls: true, }, { name: "ACLs disabled, no tls, serviceNS=default, authMethodNS=default", @@ -107,9 +106,9 @@ func TestRun_ServicePollingWithACLsAndTLSWithNamespaces(t *testing.T) { authMethodNamespace: "other", }, } - for _, test := range cases { - t.Run(test.name, func(t *testing.T) { - bearerFile := common.WriteTempFile(t, serviceAccountJWTToken) + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + bearerFile := common.WriteTempFile(t, test.ServiceAccountJWTToken) tokenFile := fmt.Sprintf("/tmp/%d1", rand.Int()) proxyFile := fmt.Sprintf("/tmp/%d2", rand.Int()) t.Cleanup(func() { @@ -120,17 +119,17 @@ func TestRun_ServicePollingWithACLsAndTLSWithNamespaces(t *testing.T) { var caFile, certFile, keyFile string // Start Consul server with ACLs enabled and default deny policy. masterToken := "b78d37c7-0ca7-5f4d-99ee-6d9975ce4586" - server, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { - if test.authMethod != "" { - c.ACL.Enabled = true - c.ACL.DefaultPolicy = "deny" - c.ACL.Tokens.Master = masterToken + server, err := testutil.NewTestServerConfigT(t, func(cfg *testutil.TestServerConfig) { + if c.acls { + cfg.ACL.Enabled = true + cfg.ACL.DefaultPolicy = "deny" + cfg.ACL.Tokens.Master = masterToken } - if test.tls { - caFile, certFile, keyFile = common.GenerateServerCerts(t) - c.CAFile = caFile - c.CertFile = certFile - c.KeyFile = keyFile + if c.tls { + caFile, certFile, keyFile = test.GenerateServerCerts(t) + cfg.CAFile = caFile + cfg.CertFile = certFile + cfg.KeyFile = keyFile } }) require.NoError(t, err) @@ -139,12 +138,12 @@ func TestRun_ServicePollingWithACLsAndTLSWithNamespaces(t *testing.T) { cfg := &api.Config{ Scheme: "http", Address: server.HTTPAddr, - Namespace: test.consulServiceNamespace, + Namespace: c.consulServiceNamespace, } - if test.authMethod != "" { + if c.acls { cfg.Token = masterToken } - if test.tls { + if c.tls { cfg.Address = server.HTTPSAddr cfg.Scheme = "https" cfg.TLSConfig = api.TLSConfig{ @@ -155,53 +154,11 @@ func TestRun_ServicePollingWithACLsAndTLSWithNamespaces(t *testing.T) { consulClient, err := api.NewClient(cfg) require.NoError(t, err) - _, err = namespaces.EnsureExists(consulClient, test.consulServiceNamespace, "") + _, err = namespaces.EnsureExists(consulClient, c.consulServiceNamespace, "") require.NoError(t, err) - // Start the mock k8s server. - k8sMockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("content-type", "application/json") - if r != nil && r.URL.Path == "/apis/authentication.k8s.io/v1/tokenreviews" && r.Method == "POST" { - w.Write([]byte(tokenReviewFoundResponseForNamespaces)) - } - if r != nil && r.URL.Path == "/api/v1/namespaces/default-ns/serviceaccounts/counting" && r.Method == "GET" { - w.Write([]byte(readServiceAccountFoundForNamespaces)) - } - })) - defer k8sMockServer.Close() - - if test.authMethod != "" { - // Set up Consul's auth method. - authMethod := &api.ACLAuthMethod{ - Name: testAuthMethod, - Type: "kubernetes", - Description: "Kubernetes Auth Method", - Config: map[string]interface{}{ - "Host": k8sMockServer.URL, - "CACert": serviceAccountCACert, - "ServiceAccountJWT": serviceAccountJWTToken, - }, - Namespace: test.authMethodNamespace, - } - // This will be the case when we are emulating "namespace mirroring" where the - // authMethodNamespace is not equal to the consulServiceNamespace. - if test.authMethodNamespace != test.consulServiceNamespace { - authMethod.Config["MapNamespaces"] = true - } - _, _, err = consulClient.ACL().AuthMethodCreate(authMethod, &api.WriteOptions{Namespace: test.authMethodNamespace}) - require.NoError(t, err) - - // Create the binding rule. - aclBindingRule := api.ACLBindingRule{ - Description: "Kubernetes binding rule", - AuthMethod: testAuthMethod, - BindType: api.BindingRuleBindTypeService, - BindName: "${serviceaccount.name}", - Selector: "serviceaccount.name!=default", - Namespace: test.authMethodNamespace, - } - _, _, err = consulClient.ACL().BindingRuleCreate(&aclBindingRule, &api.WriteOptions{Namespace: test.authMethodNamespace}) - require.NoError(t, err) + if c.acls { + test.SetupK8sAuthMethodWithNamespaces(t, consulClient, testServiceAccountName, "default-ns", c.authMethodNamespace, c.authMethodNamespace != c.consulServiceNamespace, "") } // Register Consul services. @@ -222,30 +179,31 @@ func TestRun_ServicePollingWithACLsAndTLSWithNamespaces(t *testing.T) { // CONSUL_HTTP_ADDR when it processes the command template. flags := []string{"-pod-name", testPodName, "-pod-namespace", testPodNamespace, - "-acl-auth-method", test.authMethod, "-service-account-name", testServiceAccountName, "-http-addr", fmt.Sprintf("%s://%s", cfg.Scheme, cfg.Address), - "-consul-service-namespace", test.consulServiceNamespace, - "-auth-method-namespace", test.authMethodNamespace, + "-consul-service-namespace", c.consulServiceNamespace, + } + if c.acls { + flags = append(flags, "-acl-auth-method", test.AuthMethod, "-auth-method-namespace", c.authMethodNamespace) } // Add the CA File if necessary since we're not setting CONSUL_CACERT in test ENV. - if test.tls { + if c.tls { flags = append(flags, "-ca-file", caFile) } // Run the command. code := cmd.Run(flags) require.Equal(t, 0, code, ui.ErrorWriter.String()) - if test.authMethod != "" { + if c.acls { // Validate the ACL token was written. tokenData, err := ioutil.ReadFile(tokenFile) require.NoError(t, err) require.NotEmpty(t, tokenData) // Check that the token has the metadata with pod name and pod namespace. - consulClient, err = api.NewClient(&api.Config{Address: server.HTTPAddr, Token: string(tokenData), Namespace: test.consulServiceNamespace}) + consulClient, err = api.NewClient(&api.Config{Address: server.HTTPAddr, Token: string(tokenData), Namespace: c.consulServiceNamespace}) require.NoError(t, err) - token, _, err := consulClient.ACL().TokenReadSelf(&api.QueryOptions{Namespace: test.authMethodNamespace}) + token, _, err := consulClient.ACL().TokenReadSelf(&api.QueryOptions{Namespace: c.authMethodNamespace}) require.NoError(t, err) require.Equal(t, "token created via login: {\"pod\":\"default-ns/counting-pod\"}", token.Description) } @@ -257,52 +215,3 @@ func TestRun_ServicePollingWithACLsAndTLSWithNamespaces(t *testing.T) { }) } } - -// The namespace here is default-ns as the k8s-auth method -// relies on the namespace in the response from Kubernetes to -// correctly create the token in the same namespace as the Kubernetes -// namespace which is required when namespace mirroring is enabled. -// Note that this namespace is incorrect for other test cases but -// Consul only cares about this namespace when mirroring is enabled. -const ( - readServiceAccountFoundForNamespaces = `{ - "kind": "ServiceAccount", - "apiVersion": "v1", - "metadata": { - "name": "counting", - "namespace": "default-ns", - "selfLink": "/api/v1/namespaces/default-ns/serviceaccounts/counting", - "uid": "9ff51ff4-557e-11e9-9687-48e6c8b8ecb5", - "resourceVersion": "2101", - "creationTimestamp": "2019-04-02T19:36:34Z" - }, - "secrets": [ - { - "name": "counting-token-m9cvn" - } - ] -}` - - tokenReviewFoundResponseForNamespaces = `{ - "kind": "TokenReview", - "apiVersion": "authentication.k8s.io/v1", - "metadata": { - "creationTimestamp": null - }, - "spec": { - "token": "eyJhbGciOiJSUzI1NiIsImtpZCI6IiJ9.eyJpc3MiOiJrdWJlcm5ldGVzL3NlcnZpY2VhY2NvdW50Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9uYW1lc3BhY2UiOiJkZWZhdWx0Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9zZWNyZXQubmFtZSI6ImRlbW8tdG9rZW4tbTljdm4iLCJrdWJlcm5ldGVzLmlvL3NlcnZpY2VhY2NvdW50L3NlcnZpY2UtYWNjb3VudC5uYW1lIjoiZGVtbyIsImt1YmVybmV0ZXMuaW8vc2VydmljZWFjY291bnQvc2VydmljZS1hY2NvdW50LnVpZCI6IjlmZjUxZmY0LTU1N2UtMTFlOS05Njg3LTQ4ZTZjOGI4ZWNiNSIsInN1YiI6InN5c3RlbTpzZXJ2aWNlYWNjb3VudDpkZWZhdWx0OmRlbW8ifQ.UJEphtrN261gy9WCl4ZKjm2PRDLDkc3Xg9VcDGfzyroOqFQ6sog5dVAb9voc5Nc0-H5b1yGwxDViEMucwKvZpA5pi7VEx_OskK-KTWXSmafM0Xg_AvzpU9Ed5TSRno-OhXaAraxdjXoC4myh1ay2DMeHUusJg_ibqcYJrWx-6MO1bH_ObORtAKhoST_8fzkqNAlZmsQ87FinQvYN5mzDXYukl-eeRdBgQUBkWvEb-Ju6cc0-QE4sUQ4IH_fs0fUyX_xc0om0SZGWLP909FTz4V8LxV8kr6L7irxROiS1jn3Fvyc9ur1PamVf3JOPPrOyfmKbaGRiWJM32b3buQw7cg" - }, - "status": { - "authenticated": true, - "user": { - "username": "system:serviceaccount:default-ns:counting", - "uid": "9ff51ff4-557e-11e9-9687-48e6c8b8ecb5", - "groups": [ - "system:serviceaccounts", - "system:serviceaccounts:default-ns", - "system:authenticated" - ] - } - } -}` -) diff --git a/subcommand/connect-init/command_test.go b/subcommand/connect-init/command_test.go index 54862c619d..608fb5f161 100644 --- a/subcommand/connect-init/command_test.go +++ b/subcommand/connect-init/command_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/hashicorp/consul-k8s/helper/test" "github.com/hashicorp/consul-k8s/subcommand/common" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/sdk/testutil" @@ -33,11 +34,11 @@ func TestRun_FlagValidation(t *testing.T) { expErr: "-pod-namespace must be set", }, { - flags: []string{"-pod-name", testPodName, "-pod-namespace", testPodNamespace, "-acl-auth-method", testAuthMethod}, + flags: []string{"-pod-name", testPodName, "-pod-namespace", testPodNamespace, "-acl-auth-method", test.AuthMethod}, expErr: "-service-account-name must be set when ACLs are enabled", }, { - flags: []string{"-pod-name", testPodName, "-pod-namespace", testPodNamespace, "-acl-auth-method", testAuthMethod, "-service-account-name", "foo", "-log-level", "invalid"}, + flags: []string{"-pod-name", testPodName, "-pod-namespace", testPodNamespace, "-acl-auth-method", test.AuthMethod, "-service-account-name", "foo", "-log-level", "invalid"}, expErr: "unknown log level: invalid", }, } @@ -105,9 +106,9 @@ func TestRun_ServicePollingWithACLsAndTLS(t *testing.T) { expFail: true, }, } - for _, test := range cases { - t.Run(test.name, func(t *testing.T) { - bearerFile := common.WriteTempFile(t, serviceAccountJWTToken) + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + bearerFile := common.WriteTempFile(t, test.ServiceAccountJWTToken) tokenFile := fmt.Sprintf("/tmp/%d1", rand.Int()) proxyFile := fmt.Sprintf("/tmp/%d2", rand.Int()) t.Cleanup(func() { @@ -122,8 +123,8 @@ func TestRun_ServicePollingWithACLsAndTLS(t *testing.T) { c.ACL.Enabled = true c.ACL.DefaultPolicy = "deny" c.ACL.Tokens.Master = masterToken - if test.tls { - caFile, certFile, keyFile = common.GenerateServerCerts(t) + if tt.tls { + caFile, certFile, keyFile = test.GenerateServerCerts(t) c.CAFile = caFile c.CertFile = certFile c.KeyFile = keyFile @@ -137,7 +138,7 @@ func TestRun_ServicePollingWithACLsAndTLS(t *testing.T) { Address: server.HTTPAddr, Token: masterToken, } - if test.tls { + if tt.tls { cfg.Address = server.HTTPSAddr cfg.Scheme = "https" cfg.TLSConfig = api.TLSConfig{ @@ -147,42 +148,7 @@ func TestRun_ServicePollingWithACLsAndTLS(t *testing.T) { consulClient, err := api.NewClient(cfg) require.NoError(t, err) - // Start the mock k8s server. - k8sMockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("content-type", "application/json") - if r != nil && r.URL.Path == "/apis/authentication.k8s.io/v1/tokenreviews" && r.Method == "POST" { - w.Write([]byte(tokenReviewFoundResponse)) - } - if r != nil && r.URL.Path == "/api/v1/namespaces/default/serviceaccounts/counting" && r.Method == "GET" { - w.Write([]byte(readServiceAccountFound)) - } - })) - defer k8sMockServer.Close() - - // Set up Consul's auth method. - authMethodTmpl := api.ACLAuthMethod{ - Name: testAuthMethod, - Type: "kubernetes", - Description: "Kubernetes Auth Method", - Config: map[string]interface{}{ - "Host": k8sMockServer.URL, - "CACert": serviceAccountCACert, - "ServiceAccountJWT": serviceAccountJWTToken, - }, - } - _, _, err = consulClient.ACL().AuthMethodCreate(&authMethodTmpl, nil) - require.NoError(t, err) - - // Create the binding rule. - aclBindingRule := api.ACLBindingRule{ - Description: "Kubernetes binding rule", - AuthMethod: testAuthMethod, - BindType: api.BindingRuleBindTypeService, - BindName: "${serviceaccount.name}", - Selector: "serviceaccount.name!=default", - } - _, _, err = consulClient.ACL().BindingRuleCreate(&aclBindingRule, nil) - require.NoError(t, err) + test.SetupK8sAuthMethod(t, consulClient, testServiceAccountName, "default") // Register Consul services. testConsulServices := []api.AgentServiceRegistration{consulCountingSvc, consulCountingSvcSidecar} @@ -198,22 +164,23 @@ func TestRun_ServicePollingWithACLsAndTLS(t *testing.T) { proxyIDFile: proxyFile, serviceRegistrationPollingAttempts: 3, } + // We build the http-addr because normally it's defined by the init container setting // CONSUL_HTTP_ADDR when it processes the command template. flags := []string{"-pod-name", testPodName, "-pod-namespace", testPodNamespace, - "-acl-auth-method", testAuthMethod, - "-service-account-name", test.serviceAccountName, - "-service-name", test.serviceName, + "-acl-auth-method", test.AuthMethod, + "-service-account-name", tt.serviceAccountName, + "-service-name", tt.serviceName, "-http-addr", fmt.Sprintf("%s://%s", cfg.Scheme, cfg.Address), } - // Add the CA File if necessary since we're not setting CONSUL_CACERT in test ENV. - if test.tls { + // Add the CA File if necessary since we're not setting CONSUL_CACERT in tt ENV. + if tt.tls { flags = append(flags, "-ca-file", caFile) } // Run the command. code := cmd.Run(flags) - if test.expFail { + if tt.expFail { require.Equal(t, 1, code) return } @@ -255,8 +222,8 @@ func TestRun_ServicePollingOnly(t *testing.T) { tls: true, }, } - for _, test := range cases { - t.Run(test.name, func(t *testing.T) { + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { proxyFile := fmt.Sprintf("/tmp/%d", rand.Int()) t.Cleanup(func() { os.Remove(proxyFile) @@ -265,8 +232,8 @@ func TestRun_ServicePollingOnly(t *testing.T) { var caFile, certFile, keyFile string // Start Consul server with TLS enabled if required. server, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { - if test.tls { - caFile, certFile, keyFile = common.GenerateServerCerts(t) + if tt.tls { + caFile, certFile, keyFile = test.GenerateServerCerts(t) c.CAFile = caFile c.CertFile = certFile c.KeyFile = keyFile @@ -281,7 +248,7 @@ func TestRun_ServicePollingOnly(t *testing.T) { Scheme: "http", Address: server.HTTPAddr, } - if test.tls { + if tt.tls { cfg.Address = server.HTTPSAddr cfg.Scheme = "https" cfg.TLSConfig = api.TLSConfig{ @@ -309,8 +276,8 @@ func TestRun_ServicePollingOnly(t *testing.T) { "-pod-name", testPodName, "-pod-namespace", testPodNamespace, "-http-addr", fmt.Sprintf("%s://%s", cfg.Scheme, cfg.Address)} - // Add the CA File if necessary since we're not setting CONSUL_CACERT in test ENV. - if test.tls { + // Add the CA File if necessary since we're not setting CONSUL_CACERT in tt ENV. + if tt.tls { flags = append(flags, "-ca-file", caFile) } @@ -647,7 +614,7 @@ func TestRun_FailsWithBadServerResponses(t *testing.T) { require.NoError(t, err) flags := []string{ "-pod-name", testPodName, "-pod-namespace", testPodNamespace, - "-acl-auth-method", testAuthMethod, + "-acl-auth-method", test.AuthMethod, "-service-account-name", testServiceAccountName, "-http-addr", serverURL.String()} code := cmd.Run(flags) @@ -717,7 +684,7 @@ func TestRun_LoginWithRetries(t *testing.T) { code := cmd.Run([]string{ "-pod-name", testPodName, "-pod-namespace", testPodNamespace, - "-acl-auth-method", testAuthMethod, + "-acl-auth-method", test.AuthMethod, "-service-account-name", testServiceAccountName, "-http-addr", serverURL.String()}) fmt.Println(ui.ErrorWriter.String()) @@ -742,70 +709,8 @@ const ( metaKeyKubeServiceName = "k8s-service-name" testPodNamespace = "default-ns" testPodName = "counting-pod" - testAuthMethod = "consul-k8s-auth-method" testServiceAccountName = "counting" - serviceAccountJWTToken = `eyJhbGciOiJSUzI1NiIsImtpZCI6IiJ9.eyJpc3MiOiJrdWJlcm5ldGVzL3NlcnZpY2VhY2NvdW50Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9uYW1lc3BhY2UiOiJkZWZhdWx0Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9zZWNyZXQubmFtZSI6ImtoYWtpLWFyYWNobmlkLWNvbnN1bC1jb25uZWN0LWluamVjdG9yLWF1dGhtZXRob2Qtc3ZjLWFjY29obmRidiIsImt1YmVybmV0ZXMuaW8vc2VydmljZWFjY291bnQvc2VydmljZS1hY2NvdW50Lm5hbWUiOiJraGFraS1hcmFjaG5pZC1jb25zdWwtY29ubmVjdC1pbmplY3Rvci1hdXRobWV0aG9kLXN2Yy1hY2NvdW50Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9zZXJ2aWNlLWFjY291bnQudWlkIjoiN2U5NWUxMjktZTQ3My0xMWU5LThmYWEtNDIwMTBhODAwMTIyIiwic3ViIjoic3lzdGVtOnNlcnZpY2VhY2NvdW50OmRlZmF1bHQ6a2hha2ktYXJhY2huaWQtY29uc3VsLWNvbm5lY3QtaW5qZWN0b3ItYXV0aG1ldGhvZC1zdmMtYWNjb3VudCJ9.Yi63MMtzh5MBWKKd3a7dzCJjTITE15ikFy_Tnpdk_AwdwA9J4AMSGEeHN5vWtCuuFjo_lMJqBBPHkK2AqbnoFUj9m5CopWyqICJQlvEOP4fUQ-Rc0W1P_JjU1rZERHG39b5TMLgKPQguyhaiZEJ6CjVtm9wUTagrgiuqYV2iUqLuF6SYNm6SrKtkPS-lqIO-u7C06wVk5m5uqwIVQNpZSIC_5Ls5aLmyZU3nHvH-V7E3HmBhVyZAB76jgKB0TyVX1IOskt9PDFarNtU3suZyCjvqC-UJA6sYeySe4dBNKsKlSZ6YuxUUmn1Rgv32YMdImnsWg8khf-zJvqgWk7B5EA` - serviceAccountCACert = `-----BEGIN CERTIFICATE----- -MIIDCzCCAfOgAwIBAgIQKzs7Njl9Hs6Xc8EXou25hzANBgkqhkiG9w0BAQsFADAv -MS0wKwYDVQQDEyQ1OWU2ZGM0MS0yMDhmLTQwOTUtYTI4OS0xZmM3MDBhYzFjYzgw -HhcNMTkwNjA3MTAxNzMxWhcNMjQwNjA1MTExNzMxWjAvMS0wKwYDVQQDEyQ1OWU2 -ZGM0MS0yMDhmLTQwOTUtYTI4OS0xZmM3MDBhYzFjYzgwggEiMA0GCSqGSIb3DQEB -AQUAA4IBDwAwggEKAoIBAQDZjHzwqofzTpGpc0MdICS7euvfujUKE3PC/apfDAgB -4jzEFKA78/9+KUGw/c/0SHeSQhN+a8gwlHRnAz1NJcfOIXy4dweUuOkAiFxH8pht -ECwkeNO7z8DoV8ceminCRHGjaRmoMxpZ7g2pZAJNZePxi3y1aNkFAXe9gSUSdjRZ -RXYka7wh2AO9k2dlGFAYB+t3vWwJ6twjG0TtKQrhYM9Od1/oN0E01LzBcZuxkN1k -8gfIHy7bOFCBM2WTEDW/0aAvcAPrO8DLqDJ+6Mjc3r7+zlzl8aQspb0S08pVzki5 -Dz//83kyu0phJuij5eB88V7UfPXxXF/EtV6fvrL7MN4fAgMBAAGjIzAhMA4GA1Ud -DwEB/wQEAwICBDAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBv -QsaG6qlcaRktJ0zGhxxJ52NnRV2GcIYPeN3Zv2VXe3ML3Vd6G32PV7lIOhjx3KmA -/uMh6NhqBzsekkTz0PuC3wJyM2OGonVQisFlqx9sFQ3fU2mIGXCa3wC8e/qP8BHS -w7/VeA7lzmj3TQRE/W0U0ZGeoAxn9b6JtT0iMucYvP0hXKTPBWlnzIijamU50r2Y -7ia065Ug2xUN5FLX/vxOA3y4rjpkjWoVQcu1p8TZrVoM3dsGFWp10fDMRiAHTvOH -Z23jGuk6rn9DUHC2xPj3wCTmd8SGEJoV31noJV5dVeQ90wusXz3vTG7ficKnvHFS -xtr5PSwH1DusYfVaGH2O ------END CERTIFICATE-----` - - readServiceAccountFound = `{ - "kind": "ServiceAccount", - "apiVersion": "v1", - "metadata": { - "name": "counting", - "namespace": "default", - "selfLink": "/api/v1/namespaces/default/serviceaccounts/counting", - "uid": "9ff51ff4-557e-11e9-9687-48e6c8b8ecb5", - "resourceVersion": "2101", - "creationTimestamp": "2019-04-02T19:36:34Z" - }, - "secrets": [ - { - "name": "counting-token-m9cvn" - } - ] -}` - - tokenReviewFoundResponse = `{ - "kind": "TokenReview", - "apiVersion": "authentication.k8s.io/v1", - "metadata": { - "creationTimestamp": null - }, - "spec": { - "token": "eyJhbGciOiJSUzI1NiIsImtpZCI6IiJ9.eyJpc3MiOiJrdWJlcm5ldGVzL3NlcnZpY2VhY2NvdW50Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9uYW1lc3BhY2UiOiJkZWZhdWx0Iiwia3ViZXJuZXRlcy5pby9zZXJ2aWNlYWNjb3VudC9zZWNyZXQubmFtZSI6ImRlbW8tdG9rZW4tbTljdm4iLCJrdWJlcm5ldGVzLmlvL3NlcnZpY2VhY2NvdW50L3NlcnZpY2UtYWNjb3VudC5uYW1lIjoiZGVtbyIsImt1YmVybmV0ZXMuaW8vc2VydmljZWFjY291bnQvc2VydmljZS1hY2NvdW50LnVpZCI6IjlmZjUxZmY0LTU1N2UtMTFlOS05Njg3LTQ4ZTZjOGI4ZWNiNSIsInN1YiI6InN5c3RlbTpzZXJ2aWNlYWNjb3VudDpkZWZhdWx0OmRlbW8ifQ.UJEphtrN261gy9WCl4ZKjm2PRDLDkc3Xg9VcDGfzyroOqFQ6sog5dVAb9voc5Nc0-H5b1yGwxDViEMucwKvZpA5pi7VEx_OskK-KTWXSmafM0Xg_AvzpU9Ed5TSRno-OhXaAraxdjXoC4myh1ay2DMeHUusJg_ibqcYJrWx-6MO1bH_ObORtAKhoST_8fzkqNAlZmsQ87FinQvYN5mzDXYukl-eeRdBgQUBkWvEb-Ju6cc0-QE4sUQ4IH_fs0fUyX_xc0om0SZGWLP909FTz4V8LxV8kr6L7irxROiS1jn3Fvyc9ur1PamVf3JOPPrOyfmKbaGRiWJM32b3buQw7cg" - }, - "status": { - "authenticated": true, - "user": { - "username": "system:serviceaccount:default:counting", - "uid": "9ff51ff4-557e-11e9-9687-48e6c8b8ecb5", - "groups": [ - "system:serviceaccounts", - "system:serviceaccounts:default", - "system:authenticated" - ] - } - } -}` // sample response from https://consul.io/api-docs/acl#sample-response testLoginResponse = `{ "AccessorID": "926e2bd2-b344-d91b-0c83-ae89f372cd9b", diff --git a/subcommand/create-federation-secret/command_test.go b/subcommand/create-federation-secret/command_test.go index 7f7c4cc23f..424c83dac2 100644 --- a/subcommand/create-federation-secret/command_test.go +++ b/subcommand/create-federation-secret/command_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/hashicorp/consul-k8s/helper/test" "github.com/hashicorp/consul-k8s/subcommand/common" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/sdk/freeport" @@ -300,7 +301,7 @@ func TestRun_ACLs_K8SNamespaces_ResourcePrefixes(tt *testing.T) { tt.Run(name, func(t *testing.T) { // Set up Consul server with TLS. - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) a, err := testutil.NewTestServerConfigT(t, func(cfg *testutil.TestServerConfig) { cfg.CAFile = caFile cfg.CertFile = certFile @@ -481,7 +482,7 @@ func TestRun_WaitsForMeshGatewayInstances(t *testing.T) { k8s := fake.NewSimpleClientset() // Set up Consul server with TLS. - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) a, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { c.CAFile = caFile c.CertFile = certFile @@ -549,7 +550,7 @@ func TestRun_MeshGatewayNoWANAddr(t *testing.T) { t.Parallel() // Set up Consul server with TLS. - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) a, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { c.CAFile = caFile c.CertFile = certFile @@ -619,7 +620,7 @@ func TestRun_MeshGatewayUniqueAddrs(tt *testing.T) { k8s := fake.NewSimpleClientset() // Set up Consul server with TLS. - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) a, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { c.CAFile = caFile c.CertFile = certFile @@ -696,7 +697,7 @@ func TestRun_ReplicationSecretDelay(t *testing.T) { t.Parallel() // Set up Consul server with TLS. - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) a, err := testutil.NewTestServerConfigT(t, func(cfg *testutil.TestServerConfig) { cfg.CAFile = caFile cfg.CertFile = certFile @@ -829,7 +830,7 @@ func TestRun_UpdatesSecret(t *testing.T) { k8s := fake.NewSimpleClientset() // Set up Consul server with TLS. - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) a, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { c.CAFile = caFile c.CertFile = certFile @@ -940,7 +941,7 @@ func TestRun_ConsulClientDelay(t *testing.T) { // We need to reserve all 6 ports to avoid potential // port collisions with other tests. randomPorts := freeport.MustTake(6) - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) // Create fake k8s. k8s := fake.NewSimpleClientset() @@ -1031,7 +1032,7 @@ func TestRun_Autoencrypt(t *testing.T) { k8s := fake.NewSimpleClientset() // Set up Consul server with TLS. - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) a, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { c.CAFile = caFile c.CertFile = certFile diff --git a/subcommand/get-consul-client-ca/command_test.go b/subcommand/get-consul-client-ca/command_test.go index 7ab82204ff..772cde6f52 100644 --- a/subcommand/get-consul-client-ca/command_test.go +++ b/subcommand/get-consul-client-ca/command_test.go @@ -11,7 +11,7 @@ import ( "github.com/hashicorp/consul-k8s/helper/cert" "github.com/hashicorp/consul-k8s/helper/go-discover/mocks" - "github.com/hashicorp/consul-k8s/subcommand/common" + "github.com/hashicorp/consul-k8s/helper/test" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/sdk/freeport" "github.com/hashicorp/consul/sdk/testutil" @@ -71,7 +71,7 @@ func TestRun(t *testing.T) { require.NoError(t, err) defer os.Remove(outputFile.Name()) - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) ui := cli.NewMockUi() cmd := Command{ @@ -132,7 +132,7 @@ func TestRun_ConsulServerAvailableLater(t *testing.T) { require.NoError(t, err) defer os.Remove(outputFile.Name()) - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) ui := cli.NewMockUi() cmd := Command{ @@ -218,7 +218,7 @@ func TestRun_GetsOnlyActiveRoot(t *testing.T) { require.NoError(t, err) defer os.Remove(outputFile.Name()) - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) ui := cli.NewMockUi() cmd := Command{ @@ -315,7 +315,7 @@ func TestRun_WithProvider(t *testing.T) { providers: map[string]discover.Provider{"mock": provider}, } - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) // start the test server a, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { diff --git a/subcommand/inject-connect/command.go b/subcommand/inject-connect/command.go index ab9994c924..f1bdf30209 100644 --- a/subcommand/inject-connect/command.go +++ b/subcommand/inject-connect/command.go @@ -231,7 +231,7 @@ func (c *Command) Run(args []string) int { return 1 } - // Proxy resources + // Proxy resources. var sidecarProxyCPULimit, sidecarProxyCPURequest, sidecarProxyMemoryLimit, sidecarProxyMemoryRequest resource.Quantity var err error if c.flagDefaultSidecarProxyCPURequest != "" { @@ -276,7 +276,7 @@ func (c *Command) Run(args []string) int { return 1 } - // Validate ports in metrics flags + // Validate ports in metrics flags. err = common.ValidateUnprivilegedPort("-default-merged-metrics-port", c.flagDefaultMergedMetricsPort) if err != nil { c.UI.Error(err.Error()) @@ -295,7 +295,7 @@ func (c *Command) Run(args []string) int { return 1 } - // We must have an in-cluster K8S client + // We must have an in-cluster K8S client. if c.clientset == nil { config, err := rest.InClusterConfig() if err != nil { @@ -309,7 +309,7 @@ func (c *Command) Run(args []string) int { } } - // create Consul API config object + // Create Consul API config object. cfg := api.DefaultConfig() c.http.MergeOntoConfig(cfg) if cfg.TLSConfig.CAFile == "" && c.flagConsulCACert != "" { @@ -326,7 +326,7 @@ func (c *Command) Run(args []string) int { return 1 } - // load CA file contents + // Load CA file contents. var consulCACert []byte if cfg.TLSConfig.CAFile != "" { var err error @@ -337,7 +337,7 @@ func (c *Command) Run(args []string) int { } } - // Set up Consul client + // Set up Consul client. if c.consulClient == nil { var err error c.consulClient, err = consul.NewClient(cfg) @@ -351,7 +351,7 @@ func (c *Command) Run(args []string) int { ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() - // Convert allow/deny lists to sets + // Convert allow/deny lists to sets. allowK8sNamespaces := flags.ToSet(c.flagAllowK8sNamespacesList) denyK8sNamespaces := flags.ToSet(c.flagDenyK8sNamespacesList) @@ -412,6 +412,7 @@ func (c *Command) Run(args []string) int { CrossNSACLPolicy: c.flagCrossNamespaceACLPolicy, EnableTransparentProxy: c.flagDefaultEnableTransparentProxy, TProxyOverwriteProbes: c.flagTransparentProxyDefaultOverwriteProbes, + AuthMethod: c.flagACLAuthMethod, Log: ctrl.Log.WithName("controller").WithName("endpoints"), Scheme: mgr.GetScheme(), ReleaseName: c.flagReleaseName, diff --git a/subcommand/server-acl-init/command_test.go b/subcommand/server-acl-init/command_test.go index 1b78a1a8fb..d877780c8c 100644 --- a/subcommand/server-acl-init/command_test.go +++ b/subcommand/server-acl-init/command_test.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/consul-k8s/helper/cert" "github.com/hashicorp/consul-k8s/helper/go-discover/mocks" + "github.com/hashicorp/consul-k8s/helper/test" "github.com/hashicorp/consul-k8s/subcommand/common" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/sdk/freeport" @@ -1741,7 +1742,7 @@ func TestRun_HTTPS(t *testing.T) { require := require.New(t) k8s := fake.NewSimpleClientset() - caFile, certFile, keyFile := common.GenerateServerCerts(t) + caFile, certFile, keyFile := test.GenerateServerCerts(t) srv, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { c.ACL.Enabled = true diff --git a/subcommand/server-acl-init/rules.go b/subcommand/server-acl-init/rules.go index a324a1731a..1b8a64b704 100644 --- a/subcommand/server-acl-init/rules.go +++ b/subcommand/server-acl-init/rules.go @@ -206,6 +206,8 @@ namespace "{{ .SyncConsulDestNS }}" { func (c *Command) injectRules() (string, error) { // The Connect injector needs permissions to create namespaces when namespaces are enabled. // It must also create/update service health checks via the endpoints controller. + // When ACLs are enabled, the endpoints controller needs "acl:write" permissions + // to delete ACL tokens created via "consul login". injectRulesTpl := ` {{- if .EnableNamespaces }} operator = "write" @@ -216,6 +218,7 @@ node_prefix "" { {{- if .EnableNamespaces }} namespace_prefix "" { {{- end }} + acl = "write" service_prefix "" { policy = "write" } diff --git a/subcommand/server-acl-init/rules_test.go b/subcommand/server-acl-init/rules_test.go index be0a434fd4..1160236eef 100644 --- a/subcommand/server-acl-init/rules_test.go +++ b/subcommand/server-acl-init/rules_test.go @@ -506,6 +506,7 @@ func TestInjectRules(t *testing.T) { node_prefix "" { policy = "write" } + acl = "write" service_prefix "" { policy = "write" }`, @@ -518,6 +519,7 @@ node_prefix "" { policy = "write" } namespace_prefix "" { + acl = "write" service_prefix "" { policy = "write" }