diff --git a/command/agent/agent.go b/command/agent/agent.go index 62f1b47cad9..8080591e694 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -699,7 +699,7 @@ func (a *Agent) setupConsul(consulConfig *config.ConsulConfig) error { a.consulCatalog = client.Catalog() // Create Consul Service client for service advertisement and checks. - a.consulService = consul.NewServiceClient(client.Agent(), a.logger) + a.consulService = consul.NewServiceClient(client.Agent(), a.consulSupportsTLSSkipVerify, a.logger) go a.consulService.Run() return nil } diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go index 246d4f47803..2fd2542e422 100644 --- a/command/agent/consul/client.go +++ b/command/agent/consul/client.go @@ -89,6 +89,9 @@ type ServiceClient struct { retryInterval time.Duration maxRetryInterval time.Duration + // skipVerifySupport is true if the local Consul agent suppots TLSSkipVerify + skipVerifySupport bool + // exitCh is closed when the main Run loop exits exitCh chan struct{} @@ -115,22 +118,23 @@ type ServiceClient struct { // NewServiceClient creates a new Consul ServiceClient from an existing Consul API // Client and logger. -func NewServiceClient(consulClient AgentAPI, logger *log.Logger) *ServiceClient { +func NewServiceClient(consulClient AgentAPI, skipVerifySupport bool, logger *log.Logger) *ServiceClient { return &ServiceClient{ - client: consulClient, - logger: logger, - retryInterval: defaultRetryInterval, - maxRetryInterval: defaultMaxRetryInterval, - exitCh: make(chan struct{}), - shutdownCh: make(chan struct{}), - shutdownWait: defaultShutdownWait, - opCh: make(chan *operations, 8), - services: make(map[string]*api.AgentServiceRegistration), - checks: make(map[string]*api.AgentCheckRegistration), - scripts: make(map[string]*scriptCheck), - runningScripts: make(map[string]*scriptHandle), - agentServices: make(map[string]struct{}), - agentChecks: make(map[string]struct{}), + client: consulClient, + skipVerifySupport: skipVerifySupport, + logger: logger, + retryInterval: defaultRetryInterval, + maxRetryInterval: defaultMaxRetryInterval, + exitCh: make(chan struct{}), + shutdownCh: make(chan struct{}), + shutdownWait: defaultShutdownWait, + opCh: make(chan *operations, 8), + services: make(map[string]*api.AgentServiceRegistration), + checks: make(map[string]*api.AgentCheckRegistration), + scripts: make(map[string]*scriptCheck), + runningScripts: make(map[string]*scriptHandle), + agentServices: make(map[string]struct{}), + agentChecks: make(map[string]struct{}), } } @@ -432,6 +436,11 @@ func (c *ServiceClient) serviceRegs(ops *operations, allocID string, service *st ops.regServices = append(ops.regServices, serviceReg) for _, check := range service.Checks { + if check.TLSSkipVerify && !c.skipVerifySupport { + c.logger.Printf("[WARN] consul.sync: skipping check %q for task %q alloc %q because Consul doesn't support tls_skip_verify. Please upgrade to Consul >= 0.7.2.", + check.Name, task.Name, allocID) + continue + } checkID := createCheckID(id, check) if check.Type == structs.ServiceCheckScript { if exec == nil { @@ -441,6 +450,7 @@ func (c *ServiceClient) serviceRegs(ops *operations, allocID string, service *st allocID, task.Name, checkID, check, exec, c.client, c.logger, c.shutdownCh)) } + host, port := serviceReg.Address, serviceReg.Port if check.PortLabel != "" { host, port = task.FindHostAndPortFor(check.PortLabel) diff --git a/command/agent/consul/int_test.go b/command/agent/consul/int_test.go index 8a5635e179e..eb64cf76096 100644 --- a/command/agent/consul/int_test.go +++ b/command/agent/consul/int_test.go @@ -122,7 +122,7 @@ func TestConsul_Integration(t *testing.T) { if err != nil { t.Fatalf("error creating consul client: %v", err) } - serviceClient := consul.NewServiceClient(consulClient.Agent(), logger) + serviceClient := consul.NewServiceClient(consulClient.Agent(), true, logger) defer serviceClient.Shutdown() // just-in-case cleanup consulRan := make(chan struct{}) go func() { diff --git a/command/agent/consul/unit_test.go b/command/agent/consul/unit_test.go index 622b4628aed..96794e0c1b8 100644 --- a/command/agent/consul/unit_test.go +++ b/command/agent/consul/unit_test.go @@ -98,7 +98,7 @@ func (t *testFakeCtx) syncOnce() error { func setupFake() *testFakeCtx { fc := newFakeConsul() return &testFakeCtx{ - ServiceClient: NewServiceClient(fc, testLogger()), + ServiceClient: NewServiceClient(fc, true, testLogger()), FakeConsul: fc, Task: testTask(), execs: make(chan int, 100), @@ -445,7 +445,6 @@ func TestConsul_ChangePorts(t *testing.T) { // TestConsul_RegServices tests basic service registration. func TestConsul_RegServices(t *testing.T) { ctx := setupFake() - port := ctx.Task.Resources.Networks[0].DynamicPorts[0].Value if err := ctx.ServiceClient.RegisterTask("allocid", ctx.Task, nil); err != nil { t.Fatalf("unexpected error registering task: %v", err) @@ -465,8 +464,8 @@ func TestConsul_RegServices(t *testing.T) { if !reflect.DeepEqual(v.Tags, ctx.Task.Services[0].Tags) { t.Errorf("expected Tags=%v != %v", ctx.Task.Services[0].Tags, v.Tags) } - if v.Port != port { - t.Errorf("expected Port=%d != %d", port, v.Port) + if v.Port != xPort { + t.Errorf("expected Port=%d != %d", xPort, v.Port) } } @@ -723,3 +722,48 @@ func TestConsul_ShutdownBlocked(t *testing.T) { } } } + +// TestConsul_NoTLSSkipVerifySupport asserts that checks with +// TLSSkipVerify=true are skipped when Consul doesn't support TLSSkipVerify. +func TestConsul_NoTLSSkipVerifySupport(t *testing.T) { + ctx := setupFake() + ctx.ServiceClient = NewServiceClient(ctx.FakeConsul, false, testLogger()) + ctx.Task.Services[0].Checks = []*structs.ServiceCheck{ + // This check sets TLSSkipVerify so it should get dropped + { + Name: "tls-check-skip", + Type: "http", + Protocol: "https", + Path: "/", + TLSSkipVerify: true, + }, + // This check doesn't set TLSSkipVerify so it should work fine + { + Name: "tls-check-noskip", + Type: "http", + Protocol: "https", + Path: "/", + TLSSkipVerify: false, + }, + } + + if err := ctx.ServiceClient.RegisterTask("allocid", ctx.Task, nil); err != nil { + t.Fatalf("unexpected error registering task: %v", err) + } + + if err := ctx.syncOnce(); err != nil { + t.Fatalf("unexpected error syncing task: %v", err) + } + + if len(ctx.FakeConsul.checks) != 1 { + t.Errorf("expected 1 check but found %d", len(ctx.FakeConsul.checks)) + } + for _, v := range ctx.FakeConsul.checks { + if expected := "tls-check-noskip"; v.Name != expected { + t.Errorf("only expected %q but found: %q", expected, v.Name) + } + if v.TLSSkipVerify { + t.Errorf("TLSSkipVerify=true when TLSSkipVerify not supported!") + } + } +}