Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nomad agent reload TLS configuration on SIGHUP #3479

Merged
merged 24 commits into from
Nov 15, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
25a86ac
Allow server TLS configuration to be reloaded via SIGHUP
chelseakomlo Oct 27, 2017
c65e23b
dynamic tls reloading for nomad agents
chelseakomlo Oct 31, 2017
c9acc5f
code cleanup and refactoring
chelseakomlo Nov 1, 2017
256f5a0
ensure keyloader is initialized, add comments
chelseakomlo Nov 1, 2017
cbd18ae
allow downgrading from TLS
chelseakomlo Nov 1, 2017
50a1336
initalize keyloader if necessary
chelseakomlo Nov 1, 2017
ff1d0dd
integration test for tls reload
chelseakomlo Nov 1, 2017
d7ff9f4
fix up test to assert success on reloaded TLS configuration
chelseakomlo Nov 2, 2017
0be15b0
failure in loading a new TLS config should remain at current
chelseakomlo Nov 2, 2017
0c4336e
reload agent configuration before specific server/client
chelseakomlo Nov 2, 2017
2990eb8
introduce a get-or-set method for keyloader
chelseakomlo Nov 3, 2017
b2ef194
fixups from code review
chelseakomlo Nov 3, 2017
fea24fe
fix up linting errors
chelseakomlo Nov 4, 2017
a40aafb
fixups from code review
chelseakomlo Nov 7, 2017
2cade96
add lock for config updates; improve copy of tls config
chelseakomlo Nov 13, 2017
12cb657
GetCertificate only reloads certificates dynamically for the server
chelseakomlo Nov 14, 2017
885c175
config updates/copies should be on agent
chelseakomlo Nov 14, 2017
d46eb3b
improve http integration test
chelseakomlo Nov 14, 2017
3c7bb25
simplify agent reloading storing a local copy of config
dadgar Nov 15, 2017
ae63199
reuse the same keyloader when reloading
dadgar Nov 15, 2017
d7d25b4
Test that server and client get reloaded but keep keyloader
dadgar Nov 15, 2017
9f954dc
Keyloader exposes GetClientCertificate as well for outgoing connections
dadgar Nov 15, 2017
98d6358
Fix spelling
dadgar Nov 15, 2017
75a0c60
correct changelog style
dadgar Nov 15, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
code cleanup and refactoring
  • Loading branch information
chelseakomlo committed Nov 14, 2017
commit c9acc5f98793b283d4a4fbaeaf8d2d52c7eb7c09
11 changes: 2 additions & 9 deletions helper/tlsutil/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,11 @@ type Config struct {
// Must be provided to serve TLS connections.
CertFile string

// Stores a TLS certificate that has been loaded given the information in the
// configuration. This can be updated via config.Reload()
Certificate *tls.Certificate

// KeyFile is used to provide a TLS key that is used for serving TLS connections.
// Must be provided to serve TLS connections.
KeyFile string

// Utility to dynamically reload TLS configuration.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go's style is to put the name of the var/func as the first word in the description so that searching for terms jumps to their comment first (and some tools even take advantage of this idiom!):

// KeyLoader dynamically reloads TLS configuration

or similar.

KeyLoader *config.KeyLoader
}

Expand Down Expand Up @@ -97,7 +94,7 @@ func (c *Config) LoadKeyPair() (*tls.Certificate, error) {
}

if c.KeyLoader == nil {
return nil, nil // TODO make sure this is the correct behavior
return nil, fmt.Errorf("No Keyloader object to perform LoadKeyPair")
}

cert, err := c.KeyLoader.LoadKeyPair(c.CertFile, c.KeyFile)
Expand Down Expand Up @@ -149,10 +146,6 @@ func (c *Config) OutgoingTLSConfig() (*tls.Config, error) {
return tlsConfig, nil
}

func (c *Config) getOutgoingCertificate(*tls.ClientHelloInfo) (*tls.Certificate, error) {
return c.Certificate, nil
}

// OutgoingTLSWrapper returns a a Wrapper based on the OutgoingTLS
// configuration. If hostname verification is on, the wrapper
// will properly generate the dynamic server name for verification.
Expand Down
27 changes: 8 additions & 19 deletions helper/tlsutil/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ func TestConfig_OutgoingTLS_VerifyHostname(t *testing.T) {
}

func TestConfig_OutgoingTLS_WithKeyPair(t *testing.T) {
assert := assert.New(t)

conf := &Config{
VerifyOutgoing: true,
CAFile: cacert,
Expand All @@ -151,28 +153,15 @@ func TestConfig_OutgoingTLS_WithKeyPair(t *testing.T) {
KeyLoader: &config.KeyLoader{},
}
tlsConf, err := conf.OutgoingTLSConfig()
if err != nil {
t.Fatalf("err: %v", err)
}
if tlsConf == nil {
t.Fatalf("expected config")
}
if len(tlsConf.RootCAs.Subjects()) != 1 {
t.Fatalf("expect root cert")
}
if !tlsConf.InsecureSkipVerify {
t.Fatalf("should skip verification")
}
assert.Nil(err)
assert.NotNil(tlsConf)
assert.Equal(len(tlsConf.RootCAs.Subjects()), 1)
assert.True(tlsConf.InsecureSkipVerify)

clientHelloInfo := &tls.ClientHelloInfo{}
cert, err := tlsConf.GetCertificate(clientHelloInfo)
// TODO add asert package
if err != nil {
t.Fatalf("expected no error")
}
if cert == nil {
t.Fatalf("expected client cert")
}
assert.Nil(err)
assert.NotNil(cert)
}

func startTLSServer(config *Config) (net.Conn, chan error) {
Expand Down
3 changes: 0 additions & 3 deletions nomad/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"net"
"os"
"runtime"
"sync"
"time"

"github.com/hashicorp/memberlist"
Expand Down Expand Up @@ -58,8 +57,6 @@ type Config struct {
// must be handled via `atomic.*Int32()` calls.
BootstrapExpect int32

configLock sync.RWMutex

// DataDir is the directory to store our state in
DataDir string

Expand Down