From c4ab8211b633cb47433279cc17a560776f4914c4 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Tue, 2 May 2017 17:37:09 -0700 Subject: [PATCH] Skip https health check if verify_https_client is true --- command/agent/agent.go | 89 ++++++++++++++++------------------- command/agent/agent_test.go | 94 +++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 49 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index 967740ce0..d09ee4147 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -351,43 +351,22 @@ func (a *Agent) setupServer() error { a.server = server // Consul check addresses default to bind but can be toggled to use advertise - httpCheckAddr := a.config.normalizedAddrs.HTTP rpcCheckAddr := a.config.normalizedAddrs.RPC serfCheckAddr := a.config.normalizedAddrs.Serf if *a.config.Consul.ChecksUseAdvertise { - httpCheckAddr = a.config.AdvertiseAddrs.HTTP rpcCheckAddr = a.config.AdvertiseAddrs.RPC serfCheckAddr = a.config.AdvertiseAddrs.Serf } // Create the Nomad Server services for Consul - // TODO re-introduce HTTP/S checks when Consul 0.7.1 comes out if *a.config.Consul.AutoAdvertise { httpServ := &structs.Service{ Name: a.config.Consul.ServerServiceName, PortLabel: a.config.AdvertiseAddrs.HTTP, Tags: []string{consul.ServiceTagHTTP}, - Checks: []*structs.ServiceCheck{ - &structs.ServiceCheck{ - Name: "Nomad Server HTTP Check", - Type: "http", - Path: "/v1/status/peers", - Protocol: "http", - Interval: serverHttpCheckInterval, - Timeout: serverHttpCheckTimeout, - PortLabel: httpCheckAddr, - }, - }, } - if conf.TLSConfig.EnableHTTP { - if a.consulSupportsTLSSkipVerify { - httpServ.Checks[0].Protocol = "https" - httpServ.Checks[0].TLSSkipVerify = true - } else { - // No TLSSkipVerify support, don't register https check - a.logger.Printf("[WARN] agent: not registering Nomad HTTPS Health Check because it requires Consul>=0.7.2") - httpServ.Checks = []*structs.ServiceCheck{} - } + if check := a.agentHTTPCheck(); check != nil { + httpServ.Checks = []*structs.ServiceCheck{check} } rpcServ := &structs.Service{ Name: a.config.Consul.ServerServiceName, @@ -482,39 +461,15 @@ func (a *Agent) setupClient() error { } a.client = client - // Resolve the http check address - httpCheckAddr := a.config.normalizedAddrs.HTTP - if *a.config.Consul.ChecksUseAdvertise { - httpCheckAddr = a.config.AdvertiseAddrs.HTTP - } - // Create the Nomad Client services for Consul if *a.config.Consul.AutoAdvertise { httpServ := &structs.Service{ Name: a.config.Consul.ClientServiceName, PortLabel: a.config.AdvertiseAddrs.HTTP, Tags: []string{consul.ServiceTagHTTP}, - Checks: []*structs.ServiceCheck{ - &structs.ServiceCheck{ - Name: "Nomad Client HTTP Check", - Type: "http", - Path: "/v1/agent/servers", - Protocol: "http", - Interval: clientHttpCheckInterval, - Timeout: clientHttpCheckTimeout, - PortLabel: httpCheckAddr, - }, - }, } - if conf.TLSConfig.EnableHTTP { - if a.consulSupportsTLSSkipVerify { - httpServ.Checks[0].Protocol = "https" - httpServ.Checks[0].TLSSkipVerify = true - } else { - // No TLSSkipVerify support, don't register https check - a.logger.Printf("[WARN] agent: not registering Nomad HTTPS Health Check because it requires Consul>=0.7.2") - httpServ.Checks = []*structs.ServiceCheck{} - } + if check := a.agentHTTPCheck(); check != nil { + httpServ.Checks = []*structs.ServiceCheck{check} } if err := a.consulService.RegisterAgent(consulRoleClient, []*structs.Service{httpServ}); err != nil { return err @@ -524,6 +479,42 @@ func (a *Agent) setupClient() error { return nil } +// agentHTTPCheck returns a health check for the agent's HTTP API if possible. +// If no HTTP health check can be supported nil is returned. +func (a *Agent) agentHTTPCheck() *structs.ServiceCheck { + // Resolve the http check address + httpCheckAddr := a.config.normalizedAddrs.HTTP + if *a.config.Consul.ChecksUseAdvertise { + httpCheckAddr = a.config.AdvertiseAddrs.HTTP + } + check := structs.ServiceCheck{ + Name: "Nomad Client HTTP Check", + Type: "http", + Path: "/v1/agent/servers", + Protocol: "http", + Interval: clientHttpCheckInterval, + Timeout: clientHttpCheckTimeout, + PortLabel: httpCheckAddr, + } + if !a.config.TLSConfig.EnableHTTP { + // No HTTPS, return a plain http check + return &check + } + if !a.consulSupportsTLSSkipVerify { + a.logger.Printf("[WARN] agent: not registering Nomad HTTPS Health Check because it requires Consul>=0.7.2") + return nil + } + if a.config.TLSConfig.VerifyHTTPSClient { + a.logger.Printf("[WARN] agent: not registering Nomad HTTPS Health Check because verify_https_client enabled") + return nil + } + + // HTTPS enabled; skip verification + check.Protocol = "https" + check.TLSSkipVerify = true + return &check +} + // reservePortsForClient reserves a range of ports for the client to use when // it creates various plugins for log collection, executors, drivers, etc func (a *Agent) reservePortsForClient(conf *clientconfig.Config) error { diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index 24d45ee8d..c1803d071 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -4,12 +4,14 @@ import ( "encoding/json" "fmt" "io/ioutil" + "log" "net" "os" "strings" "testing" "time" + "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/nomad" sconfig "github.com/hashicorp/nomad/nomad/structs/config" ) @@ -360,6 +362,98 @@ func TestAgent_ClientConfig(t *testing.T) { } } +// TestAgent_HTTPCheck asserts Agent.agentHTTPCheck properly alters the HTTP +// API health check depending on configuration. +func TestAgent_HTTPCheck(t *testing.T) { + logger := log.New(ioutil.Discard, "", 0) + if testing.Verbose() { + logger = log.New(os.Stdout, "[TestAgent_HTTPCheck] ", log.Lshortfile) + } + agent := func() *Agent { + return &Agent{ + logger: logger, + config: &Config{ + AdvertiseAddrs: &AdvertiseAddrs{HTTP: "advertise:4646"}, + normalizedAddrs: &Addresses{HTTP: "normalized:4646"}, + Consul: &sconfig.ConsulConfig{ + ChecksUseAdvertise: helper.BoolToPtr(false), + }, + TLSConfig: &sconfig.TLSConfig{EnableHTTP: false}, + }, + } + } + + t.Run("Plain HTTP Check", func(t *testing.T) { + a := agent() + check := a.agentHTTPCheck() + if check == nil { + t.Fatalf("expected non-nil check") + } + if check.Type != "http" { + t.Errorf("expected http check not: %q", check.Type) + } + if expected := "/v1/agent/servers"; check.Path != expected { + t.Errorf("expected %q path not: %q", expected, check.Path) + } + if check.Protocol != "http" { + t.Errorf("expected http proto not: %q", check.Protocol) + } + if expected := a.config.normalizedAddrs.HTTP; check.PortLabel != expected { + t.Errorf("expected normalized addr not %q", check.PortLabel) + } + }) + + t.Run("Plain HTTP + ChecksUseAdvertise", func(t *testing.T) { + a := agent() + a.config.Consul.ChecksUseAdvertise = helper.BoolToPtr(true) + check := a.agentHTTPCheck() + if check == nil { + t.Fatalf("expected non-nil check") + } + if expected := a.config.AdvertiseAddrs.HTTP; check.PortLabel != expected { + t.Errorf("expected advertise addr not %q", check.PortLabel) + } + }) + + t.Run("HTTPS + consulSupportsTLSSkipVerify", func(t *testing.T) { + a := agent() + a.consulSupportsTLSSkipVerify = true + a.config.TLSConfig.EnableHTTP = true + + check := a.agentHTTPCheck() + if check == nil { + t.Fatalf("expected non-nil check") + } + if !check.TLSSkipVerify { + t.Errorf("expected tls skip verify") + } + if check.Protocol != "https" { + t.Errorf("expected https not: %q", check.Protocol) + } + }) + + t.Run("HTTPS w/o TLSSkipVerify", func(t *testing.T) { + a := agent() + a.consulSupportsTLSSkipVerify = false + a.config.TLSConfig.EnableHTTP = true + + if check := a.agentHTTPCheck(); check != nil { + t.Fatalf("expected nil check not: %#v", check) + } + }) + + t.Run("HTTPS + VerifyHTTPSClient", func(t *testing.T) { + a := agent() + a.consulSupportsTLSSkipVerify = true + a.config.TLSConfig.EnableHTTP = true + a.config.TLSConfig.VerifyHTTPSClient = true + + if check := a.agentHTTPCheck(); check != nil { + t.Fatalf("expected nil check not: %#v", check) + } + }) +} + func TestAgent_ConsulSupportsTLSSkipVerify(t *testing.T) { assertSupport := func(expected bool, blob string) { self := map[string]map[string]interface{}{}