From 4aeab3897cb4df6bc480f7a46d02efa4bc07a670 Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Thu, 10 May 2018 17:04:33 +0100 Subject: [PATCH] Fixed many tests after rebase. Some still failing and seem unrelated to any connect changes. --- agent/agent.go | 53 ++++++++++++++++----- agent/agent_endpoint.go | 5 +- agent/agent_endpoint_test.go | 60 +++++++----------------- agent/agent_test.go | 40 ++++++++++++++++ agent/cache-types/connect_ca_leaf.go | 7 ++- agent/connect/testing_ca.go | 12 ++--- agent/connect/testing_spiffe.go | 2 +- agent/connect/uri_signing_test.go | 14 +++--- agent/consul/connect_ca_endpoint_test.go | 40 +++++++--------- agent/consul/leader.go | 12 +++-- agent/consul/server_test.go | 6 +++ agent/testagent.go | 4 ++ api/agent_test.go | 6 +-- api/connect_ca.go | 1 + api/connect_ca_test.go | 41 +++++++++++++--- connect/tls_test.go | 6 +-- testutil/server.go | 7 +++ 17 files changed, 199 insertions(+), 117 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 77045c69e9..eb9e203dc5 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -932,6 +932,25 @@ func (a *Agent) consulConfig() (*consul.Config, error) { if a.config.ConnectEnabled { base.ConnectEnabled = true + // Allow config to specify cluster_id provided it's a valid UUID. This is + // meant only for tests where a deterministic ID makes fixtures much simpler + // to work with but since it's only read on initial cluster bootstrap it's not + // that much of a liability in production. The worst a user could do is + // configure logically separate clusters with same ID by mistake but we can + // avoid documenting this is even an option. + if clusterID, ok := a.config.ConnectCAConfig["cluster_id"]; ok { + if cIDStr, ok := clusterID.(string); ok { + if _, err := uuid.ParseUUID(cIDStr); err == nil { + // Valid UUID configured, use that + base.CAConfig.ClusterID = cIDStr + } + } + if base.CAConfig.ClusterID == "" { + a.logger.Println("[WARN] connect CA config cluster_id specified but ", + "is not a valid UUID, ignoring") + } + } + if a.config.ConnectCAProvider != "" { base.CAConfig.Provider = a.config.ConnectCAProvider @@ -2116,20 +2135,25 @@ func (a *Agent) RemoveProxy(proxyID string, persist bool) error { } // verifyProxyToken takes a token and attempts to verify it against the -// targetService name. If targetProxy is specified, then the local proxy -// token must exactly match the given proxy ID. -// cert, config, etc.). +// targetService name. If targetProxy is specified, then the local proxy token +// must exactly match the given proxy ID. cert, config, etc.). // -// The given token may be a local-only proxy token or it may be an ACL -// token. We will attempt to verify the local proxy token first. -func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) error { +// The given token may be a local-only proxy token or it may be an ACL token. We +// will attempt to verify the local proxy token first. +// +// The effective ACL token is returned along with any error. In the case the +// token matches a proxy token, then the ACL token used to register that proxy's +// target service is returned for use in any RPC calls the proxy needs to make +// on behalf of that service. If the token was an ACL token already then it is +// always returned. Provided error is nil, a valid ACL token is always returned. +func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) (string, error) { // If we specify a target proxy, we look up that proxy directly. Otherwise, // we resolve with any proxy we can find. var proxy *local.ManagedProxy if targetProxy != "" { proxy = a.State.Proxy(targetProxy) if proxy == nil { - return fmt.Errorf("unknown proxy service ID: %q", targetProxy) + return "", fmt.Errorf("unknown proxy service ID: %q", targetProxy) } // If the token DOESN'T match, then we reset the proxy which will @@ -2148,10 +2172,13 @@ func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) error // service. if proxy != nil { if proxy.Proxy.TargetServiceID != targetService { - return acl.ErrPermissionDenied + return "", acl.ErrPermissionDenied } - return nil + // Resolve the actual ACL token used to register the proxy/service and + // return that for use in RPC calls. + aclToken := a.State.ServiceToken(targetService) + return aclToken, nil } // Retrieve the service specified. This should always exist because @@ -2159,7 +2186,7 @@ func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) error // only be called for local services. service := a.State.Service(targetService) if service == nil { - return fmt.Errorf("unknown service ID: %s", targetService) + return "", fmt.Errorf("unknown service ID: %s", targetService) } // Doesn't match, we have to do a full token resolution. The required @@ -2168,13 +2195,13 @@ func (a *Agent) verifyProxyToken(token, targetService, targetProxy string) error // is usually present in the configuration. rule, err := a.resolveToken(token) if err != nil { - return err + return "", err } if rule != nil && !rule.ServiceWrite(service.Service, nil) { - return acl.ErrPermissionDenied + return "", acl.ErrPermissionDenied } - return nil + return token, nil } func (a *Agent) cancelCheckMonitors(checkID types.CheckID) { diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index c9afa55db7..6a5126fa27 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -927,10 +927,11 @@ func (s *HTTPServer) AgentConnectCALeafCert(resp http.ResponseWriter, req *http. // Verify the proxy token. This will check both the local proxy token // as well as the ACL if the token isn't local. - err := s.agent.verifyProxyToken(qOpts.Token, id, "") + effectiveToken, err := s.agent.verifyProxyToken(qOpts.Token, id, "") if err != nil { return nil, err } + args.Token = effectiveToken raw, err := s.agent.cache.Get(cachetype.ConnectCALeafName, &args) if err != nil { @@ -982,7 +983,7 @@ func (s *HTTPServer) AgentConnectProxyConfig(resp http.ResponseWriter, req *http } // Validate the ACL token - err := s.agent.verifyProxyToken(token, proxy.Proxy.TargetServiceID, id) + _, err := s.agent.verifyProxyToken(token, proxy.Proxy.TargetServiceID, id) if err != nil { return "", nil, err } diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index c1fd16eb9d..ac2d28d009 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -3286,17 +3286,6 @@ func TestAgentConnectAuthorize_idNotService(t *testing.T) { assert.Contains(obj.Reason, "must be a valid") } -func testFetchTrustDomain(t *testing.T, a *TestAgent) string { - req, _ := http.NewRequest("GET", "/v1/agent/connect/ca/roots", nil) - resp := httptest.NewRecorder() - obj, err := a.srv.AgentConnectCARoots(resp, req) - require.NoError(t, err) - - value := obj.(structs.IndexedCARoots) - require.NotEmpty(t, value.TrustDomain) - return value.TrustDomain -} - // Test when there is an intention allowing the connection func TestAgentConnectAuthorize_allow(t *testing.T) { t.Parallel() @@ -3307,8 +3296,6 @@ func TestAgentConnectAuthorize_allow(t *testing.T) { target := "db" - trustDomain := testFetchTrustDomain(t, a) - // Create some intentions var ixnId string { @@ -3330,9 +3317,8 @@ func TestAgentConnectAuthorize_allow(t *testing.T) { cacheHits := a.cache.Hits() args := &structs.ConnectAuthorizeRequest{ - Target: target, - ClientCertURI: connect.TestSpiffeIDServiceWithHost(t, "web", trustDomain). - URI().String(), + Target: target, + ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(), } req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args)) resp := httptest.NewRecorder() @@ -3344,13 +3330,9 @@ func TestAgentConnectAuthorize_allow(t *testing.T) { require.True(obj.Authorized) require.Contains(obj.Reason, "Matched") - // That should've been a cache miss, so no hit change, however since - // testFetchTrustDomain already called Roots and caused it to be in cache, the - // authorize call above will also call it and see a cache hit for the Roots - // RPC. In other words, there are 2 cached calls in /authorize and we always - // expect one of them to be a hit. So asserting only 1 happened is as close as - // we can get to verifying that the intention match RPC was a hit. - require.Equal(cacheHits+1, a.cache.Hits()) + // That should've been a cache miss (for both Intentions and Roots, so no hit + // change). + require.Equal(cacheHits, a.cache.Hits()) // Make the request again { @@ -3365,10 +3347,9 @@ func TestAgentConnectAuthorize_allow(t *testing.T) { require.Contains(obj.Reason, "Matched") } - // That should've been a cache hit. We add the one hit from Roots from first - // call as well as the 2 from this call (Roots + Intentions). - require.Equal(cacheHits+1+2, a.cache.Hits()) - cacheHits = a.cache.Hits() + // That should've been a cache hit. We add 2 (Roots + Intentions). + require.Equal(cacheHits+2, a.cache.Hits()) + cacheHits += 2 // Change the intention { @@ -3419,8 +3400,6 @@ func TestAgentConnectAuthorize_deny(t *testing.T) { target := "db" - trustDomain := testFetchTrustDomain(t, a) - // Create some intentions { req := structs.IntentionRequest{ @@ -3439,9 +3418,8 @@ func TestAgentConnectAuthorize_deny(t *testing.T) { } args := &structs.ConnectAuthorizeRequest{ - Target: target, - ClientCertURI: connect.TestSpiffeIDServiceWithHost(t, "web", trustDomain). - URI().String(), + Target: target, + ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(), } req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args)) resp := httptest.NewRecorder() @@ -3484,10 +3462,8 @@ func TestAgentConnectAuthorize_denyTrustDomain(t *testing.T) { { args := &structs.ConnectAuthorizeRequest{ - Target: target, - // Rely on the test trust domain this will choose to not match the random - // one picked on agent startup. - ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(), + Target: target, + ClientCertURI: "spiffe://fake-domain.consul/ns/default/dc/dc1/svc/web", } req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args)) resp := httptest.NewRecorder() @@ -3510,8 +3486,6 @@ func TestAgentConnectAuthorize_denyWildcard(t *testing.T) { target := "db" - trustDomain := testFetchTrustDomain(t, a) - // Create some intentions { // Deny wildcard to DB @@ -3549,9 +3523,8 @@ func TestAgentConnectAuthorize_denyWildcard(t *testing.T) { // Web should be allowed { args := &structs.ConnectAuthorizeRequest{ - Target: target, - ClientCertURI: connect.TestSpiffeIDServiceWithHost(t, "web", trustDomain). - URI().String(), + Target: target, + ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(), } req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args)) resp := httptest.NewRecorder() @@ -3567,9 +3540,8 @@ func TestAgentConnectAuthorize_denyWildcard(t *testing.T) { // API should be denied { args := &structs.ConnectAuthorizeRequest{ - Target: target, - ClientCertURI: connect.TestSpiffeIDServiceWithHost(t, "api", trustDomain). - URI().String(), + Target: target, + ClientCertURI: connect.TestSpiffeIDService(t, "api").URI().String(), } req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args)) resp := httptest.NewRecorder() diff --git a/agent/agent_test.go b/agent/agent_test.go index 6219b70daf..911ed63a03 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -15,7 +15,10 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/hashicorp/consul/agent/checks" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/testutil" @@ -52,6 +55,43 @@ func TestAgent_MultiStartStop(t *testing.T) { } } +func TestAgent_ConnectClusterIDConfig(t *testing.T) { + tests := []struct { + name string + hcl string + wantClusterID string + }{ + { + name: "default TestAgent has fixed cluster id", + hcl: "", + wantClusterID: connect.TestClusterID, + }, + { + name: "no cluster ID specified remains null", + hcl: "connect { enabled = true }", + wantClusterID: "", + }, + { + name: "non-UUID cluster_id is ignored", + hcl: `connect { + enabled = true + ca_config { + cluster_id = "fake-id" + } + }`, + wantClusterID: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := NewTestAgent("test", tt.hcl) + cfg := a.consulConfig() + assert.Equal(t, tt.wantClusterID, cfg.CAConfig.ClusterID) + }) + } +} + func TestAgent_StartStop(t *testing.T) { t.Parallel() a := NewTestAgent(t.Name(), "") diff --git a/agent/cache-types/connect_ca_leaf.go b/agent/cache-types/connect_ca_leaf.go index ef354c9cef..2316acab1a 100644 --- a/agent/cache-types/connect_ca_leaf.go +++ b/agent/cache-types/connect_ca_leaf.go @@ -134,8 +134,9 @@ func (c *ConnectCALeaf) Fetch(opts cache.FetchOptions, req cache.Request) (cache // Request signing var reply structs.IssuedCert args := structs.CASignRequest{ - Datacenter: reqReal.Datacenter, - CSR: csr, + WriteRequest: structs.WriteRequest{Token: reqReal.Token}, + Datacenter: reqReal.Datacenter, + CSR: csr, } if err := c.RPC.RPC("ConnectCA.Sign", &args, &reply); err != nil { return result, err @@ -217,6 +218,7 @@ func (c *ConnectCALeaf) waitNewRootCA(datacenter string, ch chan<- error, // since this is only used for cache-related requests and not forwarded // directly to any Consul servers. type ConnectCALeafRequest struct { + Token string Datacenter string Service string // Service name, not ID MinQueryIndex uint64 @@ -224,6 +226,7 @@ type ConnectCALeafRequest struct { func (r *ConnectCALeafRequest) CacheInfo() cache.RequestInfo { return cache.RequestInfo{ + Token: r.Token, Key: r.Service, Datacenter: r.Datacenter, MinIndex: r.MinQueryIndex, diff --git a/agent/connect/testing_ca.go b/agent/connect/testing_ca.go index ba2d29203b..cc015af81e 100644 --- a/agent/connect/testing_ca.go +++ b/agent/connect/testing_ca.go @@ -20,12 +20,8 @@ import ( "github.com/mitchellh/go-testing-interface" ) -// testClusterID is the Consul cluster ID for testing. -// -// NOTE(mitchellh): This might have to change some other constant for -// real testing once we integrate the Cluster ID into the core. For now it -// is unchecked. -const testClusterID = "11111111-2222-3333-4444-555555555555" +// TestClusterID is the Consul cluster ID for testing. +const TestClusterID = "11111111-2222-3333-4444-555555555555" // testCACounter is just an atomically incremented counter for creating // unique names for the CA certs. @@ -53,7 +49,7 @@ func TestCA(t testing.T, xc *structs.CARoot) *structs.CARoot { } // The URI (SPIFFE compatible) for the cert - id := &SpiffeIDSigning{ClusterID: testClusterID, Domain: "consul"} + id := &SpiffeIDSigning{ClusterID: TestClusterID, Domain: "consul"} // Create the CA cert template := x509.Certificate{ @@ -148,7 +144,7 @@ func TestLeaf(t testing.T, service string, root *structs.CARoot) (string, string // Build the SPIFFE ID spiffeId := &SpiffeIDService{ - Host: fmt.Sprintf("%s.consul", testClusterID), + Host: fmt.Sprintf("%s.consul", TestClusterID), Namespace: "default", Datacenter: "dc1", Service: service, diff --git a/agent/connect/testing_spiffe.go b/agent/connect/testing_spiffe.go index 42db76495d..c7fa6f753f 100644 --- a/agent/connect/testing_spiffe.go +++ b/agent/connect/testing_spiffe.go @@ -6,7 +6,7 @@ import ( // TestSpiffeIDService returns a SPIFFE ID representing a service. func TestSpiffeIDService(t testing.T, service string) *SpiffeIDService { - return TestSpiffeIDServiceWithHost(t, service, testClusterID+".consul") + return TestSpiffeIDServiceWithHost(t, service, TestClusterID+".consul") } // TestSpiffeIDServiceWithHost returns a SPIFFE ID representing a service with diff --git a/agent/connect/uri_signing_test.go b/agent/connect/uri_signing_test.go index 2d8975858c..6d04a5fab8 100644 --- a/agent/connect/uri_signing_test.go +++ b/agent/connect/uri_signing_test.go @@ -21,10 +21,10 @@ func TestSpiffeIDSigningAuthorize(t *testing.T) { func TestSpiffeIDSigningForCluster(t *testing.T) { // For now it should just append .consul to the ID. config := &structs.CAConfiguration{ - ClusterID: testClusterID, + ClusterID: TestClusterID, } id := SpiffeIDSigningForCluster(config) - assert.Equal(t, id.URI().String(), "spiffe://"+testClusterID+".consul") + assert.Equal(t, id.URI().String(), "spiffe://"+TestClusterID+".consul") } // fakeCertURI is a CertURI implementation that our implementation doesn't know @@ -42,7 +42,7 @@ func (f fakeCertURI) URI() *url.URL { func TestSpiffeIDSigning_CanSign(t *testing.T) { testSigning := &SpiffeIDSigning{ - ClusterID: testClusterID, + ClusterID: TestClusterID, Domain: "consul", } @@ -71,7 +71,7 @@ func TestSpiffeIDSigning_CanSign(t *testing.T) { name: "different TLD signing ID", id: testSigning, input: &SpiffeIDSigning{ - ClusterID: testClusterID, + ClusterID: TestClusterID, Domain: "evil", }, want: false, @@ -91,13 +91,13 @@ func TestSpiffeIDSigning_CanSign(t *testing.T) { { name: "service - good", id: testSigning, - input: &SpiffeIDService{testClusterID + ".consul", "default", "dc1", "web"}, + input: &SpiffeIDService{TestClusterID + ".consul", "default", "dc1", "web"}, want: true, }, { name: "service - good midex case", id: testSigning, - input: &SpiffeIDService{strings.ToUpper(testClusterID) + ".CONsuL", "defAUlt", "dc1", "WEB"}, + input: &SpiffeIDService{strings.ToUpper(TestClusterID) + ".CONsuL", "defAUlt", "dc1", "WEB"}, want: true, }, { @@ -109,7 +109,7 @@ func TestSpiffeIDSigning_CanSign(t *testing.T) { { name: "service - different TLD", id: testSigning, - input: &SpiffeIDService{testClusterID + ".fake", "default", "dc1", "web"}, + input: &SpiffeIDService{TestClusterID + ".fake", "default", "dc1", "web"}, want: false, }, } diff --git a/agent/consul/connect_ca_endpoint_test.go b/agent/consul/connect_ca_endpoint_test.go index ac64ceb303..eb7176b678 100644 --- a/agent/consul/connect_ca_endpoint_test.go +++ b/agent/consul/connect_ca_endpoint_test.go @@ -252,7 +252,6 @@ func TestConnectCASign(t *testing.T) { // Generate a CSR and request signing spiffeId := connect.TestSpiffeIDService(t, "web") - spiffeId.Host = testGetClusterTrustDomain(t, s1) csr, _ := connect.TestCSR(t, spiffeId) args := &structs.CASignRequest{ Datacenter: "dc1", @@ -281,16 +280,6 @@ func TestConnectCASign(t *testing.T) { assert.Equal(spiffeId.URI().String(), reply.ServiceURI) } -func testGetClusterTrustDomain(t *testing.T, s *Server) string { - t.Helper() - state := s.fsm.State() - _, config, err := state.CAConfig() - require.NoError(t, err) - // Build TrustDomain based on the ClusterID stored. - signingID := connect.SpiffeIDSigningForCluster(config) - return signingID.Host() -} - func TestConnectCASignValidation(t *testing.T) { t.Parallel() @@ -325,7 +314,7 @@ func TestConnectCASignValidation(t *testing.T) { require.NoError(t, msgpackrpc.CallWithCodec(codec, "ACL.Apply", &arg, &webToken)) } - trustDomain := testGetClusterTrustDomain(t, s1) + testWebID := connect.TestSpiffeIDService(t, "web") tests := []struct { name string @@ -335,29 +324,36 @@ func TestConnectCASignValidation(t *testing.T) { { name: "different cluster", id: &connect.SpiffeIDService{ - "55555555-4444-3333-2222-111111111111.consul", - "default", "dc1", "web"}, + Host: "55555555-4444-3333-2222-111111111111.consul", + Namespace: testWebID.Namespace, + Datacenter: testWebID.Datacenter, + Service: testWebID.Service, + }, wantErr: "different trust domain", }, { - name: "same cluster should validate", - id: &connect.SpiffeIDService{ - trustDomain, - "default", "dc1", "web"}, + name: "same cluster should validate", + id: testWebID, wantErr: "", }, { name: "same cluster, CSR for a different DC should NOT validate", id: &connect.SpiffeIDService{ - trustDomain, - "default", "dc2", "web"}, + Host: testWebID.Host, + Namespace: testWebID.Namespace, + Datacenter: "dc2", + Service: testWebID.Service, + }, wantErr: "different datacenter", }, { name: "same cluster and DC, different service should not have perms", id: &connect.SpiffeIDService{ - trustDomain, - "default", "dc1", "db"}, + Host: testWebID.Host, + Namespace: testWebID.Namespace, + Datacenter: testWebID.Datacenter, + Service: "db", + }, wantErr: "Permission denied", }, } diff --git a/agent/consul/leader.go b/agent/consul/leader.go index 579ff4b1dc..f47dde83fd 100644 --- a/agent/consul/leader.go +++ b/agent/consul/leader.go @@ -383,13 +383,15 @@ func (s *Server) initializeCAConfig() (*structs.CAConfiguration, error) { return config, nil } - id, err := uuid.GenerateUUID() - if err != nil { - return nil, err + config = s.config.CAConfig + if config.ClusterID == "" { + id, err := uuid.GenerateUUID() + if err != nil { + return nil, err + } + config.ClusterID = id } - config = s.config.CAConfig - config.ClusterID = id req := structs.CARequest{ Op: structs.CAOpSetConfig, Config: config, diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 84ec6743af..0359c847fb 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -10,7 +10,9 @@ import ( "testing" "time" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/metadata" + "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/token" "github.com/hashicorp/consul/lib/freeport" "github.com/hashicorp/consul/testrpc" @@ -92,6 +94,10 @@ func testServerConfig(t *testing.T) (string, *Config) { config.RPCHoldTimeout = 5 * time.Second config.ConnectEnabled = true + config.CAConfig = &structs.CAConfiguration{ + ClusterID: connect.TestClusterID, + Provider: structs.ConsulCAProvider, + } return dir, config } diff --git a/agent/testagent.go b/agent/testagent.go index 724b0c80e0..26c81a81dc 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -19,6 +19,7 @@ import ( uuid "github.com/hashicorp/go-uuid" "github.com/hashicorp/consul/agent/config" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" @@ -337,6 +338,9 @@ func TestConfig(sources ...config.Source) *config.RuntimeConfig { node_name = "Node ` + nodeID + `" connect { enabled = true + ca_config { + cluster_id = "` + connect.TestClusterID + `" + } } performance { raft_multiplier = 1 diff --git a/api/agent_test.go b/api/agent_test.go index 1f816c23a3..ed73672de0 100644 --- a/api/agent_test.go +++ b/api/agent_test.go @@ -203,7 +203,7 @@ func TestAPI_AgentServices_ManagedConnectProxy(t *testing.T) { Connect: &AgentServiceConnect{ Proxy: &AgentServiceConnectProxy{ ExecMode: ProxyExecModeScript, - Command: "foo.rb", + Command: []string{"foo.rb"}, Config: map[string]interface{}{ "foo": "bar", }, @@ -1123,7 +1123,7 @@ func TestAPI_AgentConnectAuthorize(t *testing.T) { Target: "foo", ClientCertSerial: "fake", // Importing connect.TestSpiffeIDService creates an import cycle - ClientCertURI: "spiffe://123.consul/ns/default/dc/ny1/svc/web", + ClientCertURI: "spiffe://11111111-2222-3333-4444-555555555555.consul/ns/default/dc/ny1/svc/web", } auth, err := agent.ConnectAuthorize(params) require.Nil(err) @@ -1169,7 +1169,7 @@ func TestAPI_AgentConnectProxyConfig(t *testing.T) { TargetServiceName: "foo", ContentHash: "e662ea8600d84cf0", ExecMode: "daemon", - Command: "consul connect proxy", + Command: []string{"consul connect proxy"}, Config: map[string]interface{}{ "bind_address": "127.0.0.1", "bind_port": float64(20000), diff --git a/api/connect_ca.go b/api/connect_ca.go index 00951c75dc..ed0ac5e8fa 100644 --- a/api/connect_ca.go +++ b/api/connect_ca.go @@ -7,6 +7,7 @@ import ( // CARootList is the structure for the results of listing roots. type CARootList struct { ActiveRootID string + TrustDomain string Roots []*CARoot } diff --git a/api/connect_ca_test.go b/api/connect_ca_test.go index 3ad7cb0789..36fb12b56d 100644 --- a/api/connect_ca_test.go +++ b/api/connect_ca_test.go @@ -3,24 +3,51 @@ package api import ( "testing" + "github.com/hashicorp/consul/testutil" + "github.com/hashicorp/consul/testutil/retry" "github.com/stretchr/testify/require" ) -// NOTE(mitchellh): we don't have a way to test CA roots yet since there -// is no API public way to configure the root certs. This wll be resolved -// in the future and we can write tests then. This is tested in agent and -// agent/consul which do have internal access to manually create roots. - func TestAPI_ConnectCARoots_empty(t *testing.T) { t.Parallel() require := require.New(t) - c, s := makeClient(t) + c, s := makeClientWithConfig(t, nil, func(c *testutil.TestServerConfig) { + // Don't bootstrap CA + c.Connect = nil + }) defer s.Stop() connect := c.Connect() list, meta, err := connect.CARoots(nil) - require.Nil(err) + require.NoError(err) require.Equal(uint64(0), meta.LastIndex) require.Len(list.Roots, 0) + require.Empty(list.TrustDomain) +} + +func TestAPI_ConnectCARoots_list(t *testing.T) { + t.Parallel() + + c, s := makeClient(t) + defer s.Stop() + + // This fails occasionally if server doesn't have time to bootstrap CA so + // retry + retry.Run(t, func(r *retry.R) { + connect := c.Connect() + list, meta, err := connect.CARoots(nil) + r.Check(err) + if meta.LastIndex <= 0 { + r.Fatalf("expected roots raft index to be > 0") + } + if v := len(list.Roots); v != 1 { + r.Fatalf("expected 1 root, got %d", v) + } + // connect.TestClusterID causes import cycle so hard code it + if list.TrustDomain != "11111111-2222-3333-4444-555555555555.consul" { + r.Fatalf("expected fixed trust domain got '%s'", list.TrustDomain) + } + }) + } diff --git a/connect/tls_test.go b/connect/tls_test.go index a9fd6fe8c1..5df491866f 100644 --- a/connect/tls_test.go +++ b/connect/tls_test.go @@ -147,7 +147,7 @@ func TestServerSideVerifier(t *testing.T) { cfg := api.DefaultConfig() cfg.Address = agent.HTTPAddr() client, err := api.NewClient(cfg) - require.Nil(t, err) + require.NoError(t, err) // Setup intentions to validate against. We actually default to allow so first // setup a blanket deny rule for db, then only allow web. @@ -162,7 +162,7 @@ func TestServerSideVerifier(t *testing.T) { Meta: map[string]string{}, } id, _, err := connect.IntentionCreate(ixn, nil) - require.Nil(t, err) + require.NoError(t, err) require.NotEmpty(t, id) ixn = &api.Intention{ @@ -175,7 +175,7 @@ func TestServerSideVerifier(t *testing.T) { Meta: map[string]string{}, } id, _, err = connect.IntentionCreate(ixn, nil) - require.Nil(t, err) + require.NoError(t, err) require.NotEmpty(t, id) tests := []struct { diff --git a/testutil/server.go b/testutil/server.go index f188079d70..e80b0e7fd2 100644 --- a/testutil/server.go +++ b/testutil/server.go @@ -135,6 +135,13 @@ func defaultServerConfig() *TestServerConfig { Server: ports[5], }, ReadyTimeout: 10 * time.Second, + Connect: map[string]interface{}{ + "enabled": true, + "ca_config": map[string]interface{}{ + // const TestClusterID causes import cycle so hard code it here. + "cluster_id": "11111111-2222-3333-4444-555555555555", + }, + }, } }