Skip checks with TLSSkipVerify if it's unsupported

Fixes #2218

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
 }

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)

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() {

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!")
+		}
+	}
+}