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

Add an option to disable keyring file #3145

Merged
merged 2 commits into from
Jun 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 8 additions & 4 deletions agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -887,8 +887,10 @@ func (a *Agent) makeServer() (*consul.Server, error) {
if err != nil {
return nil, err
}
if err := a.setupKeyrings(config); err != nil {
return nil, fmt.Errorf("Failed to configure keyring: %v", err)
if !a.config.DisableKeyringFile {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor readability thing - Any reason why this config param is not enableKeyRingFile, set to false by default? I find negative boolean feature flags always take a couple of seconds more to follow when you are reading a config file that has them, because its like if its set to true the file won't be written

Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't a bad idea since we are creating the config and not worrying about BC. If we switch to a bool* we could make this EnableKeyringFile and default it to true.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok leaving it disable too, since it defaults to enabled and we have a lot of other things like that in the config. I'll leave it up to you @kyhavlov - either way works.

if err := a.setupKeyrings(config); err != nil {
return nil, fmt.Errorf("Failed to configure keyring: %v", err)
}
}
server, err := consul.NewServerLogger(config, a.logger)
if err != nil {
Expand All @@ -903,8 +905,10 @@ func (a *Agent) makeClient() (*consul.Client, error) {
if err != nil {
return nil, err
}
if err := a.setupKeyrings(config); err != nil {
return nil, fmt.Errorf("Failed to configure keyring: %v", err)
if !a.config.DisableKeyringFile {
if err := a.setupKeyrings(config); err != nil {
return nil, fmt.Errorf("Failed to configure keyring: %v", err)
}
}
client, err := consul.NewClientLogger(config, a.logger)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,9 @@ type Config struct {
// Encryption key to use for the Serf communication
EncryptKey string `mapstructure:"encrypt" json:"-"`

// Disables writing the keyring to a file.
DisableKeyringFile bool `mapstructure:"disable_keyring_file"`

// EncryptVerifyIncoming and EncryptVerifyOutgoing are used to enforce
// incoming/outgoing gossip encryption and can be used to upshift to
// encrypted gossip on a running cluster.
Expand Down Expand Up @@ -952,6 +955,7 @@ func DevConfig() *Config {
conf.DisableAnonymousSignature = true
conf.EnableUI = true
conf.BindAddr = "127.0.0.1"
conf.DisableKeyringFile = true

conf.ConsulConfig = consul.DefaultConfig()
conf.ConsulConfig.SerfLANConfig.MemberlistConfig.ProbeTimeout = 100 * time.Millisecond
Expand Down Expand Up @@ -1561,6 +1565,9 @@ func MergeConfig(a, b *Config) *Config {
if b.EncryptKey != "" {
result.EncryptKey = b.EncryptKey
}
if b.DisableKeyringFile {
result.DisableKeyringFile = true
}
if b.EncryptVerifyIncoming != nil {
result.EncryptVerifyIncoming = b.EncryptVerifyIncoming
}
Expand Down
4 changes: 4 additions & 0 deletions agent/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ func TestDecodeConfig(t *testing.T) {
in: `{"enable_syslog":true}`,
c: &Config{EnableSyslog: true},
},
{
in: `{"disable_keyring_file":true}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to update the website with the new option.

c: &Config{DisableKeyringFile: true},
},
{
in: `{"encrypt_verify_incoming":true}`,
c: &Config{EncryptVerifyIncoming: Bool(true)},
Expand Down
2 changes: 2 additions & 0 deletions command/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ func (cmd *AgentCommand) readConfig() *agent.Config {
f.StringVar(&cmdCfg.UIDir, "ui-dir", "", "Path to directory containing the web UI resources.")
f.StringVar(&cmdCfg.PidFile, "pid-file", "", "Path to file to store agent PID.")
f.StringVar(&cmdCfg.EncryptKey, "encrypt", "", "Provides the gossip encryption key.")
f.BoolVar(&cmdCfg.DisableKeyringFile, "disable-keyring-file", false, "Disables the backing up "+
"of the keyring to a file.")

f.BoolVar(&cmdCfg.Server, "server", false, "Switches agent to server mode.")
f.BoolVar(&cmdCfg.NonVotingServer, "non-voting-server", false,
Expand Down
10 changes: 6 additions & 4 deletions vendor/github.com/hashicorp/serf/serf/internal_query.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions vendor/vendor.json
Original file line number Diff line number Diff line change
Expand Up @@ -675,11 +675,11 @@
"revisionTime": "2017-05-25T23:15:04Z"
},
{
"checksumSHA1": "ZkJRgexeNzNZzpw6YnedwoJl7pE=",
"checksumSHA1": "3LFg00GII0KbMRpqi38MRkMhoyM=",
"comment": "v0.7.0-66-g6c4672d",
"path": "github.com/hashicorp/serf/serf",
"revision": "dfab144618a063232d5753eaa4250a09865106c5",
"revisionTime": "2017-05-26T05:01:28Z"
"revision": "91fd53b1d3e624389ed9a295a3fa380e5c7b9dfc",
"revisionTime": "2017-06-14T22:59:51Z"
},
{
"checksumSHA1": "ZhK6IO2XN81Y+3RAjTcVm1Ic7oU=",
Expand Down
7 changes: 7 additions & 0 deletions website/source/docs/agent/options.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ will exit with an error at startup.
initialized with an encryption key, then the provided key is ignored and
a warning will be displayed.

* <a name="_disable_keyring_file"></a><a href="#_disable_keyring_file">`-disable-keyring-file`</a> - If set,
the keyring will not be persisted to a file. Any installed keys will be lost on shutdown, and only the given
`-encrypt` key will be available on startup. This defaults to false.

* <a name="_http_port"></a><a href="#_http_port">`-http-port`</a> - the HTTP API port to listen on.
This overrides the default port 8500. This option is very useful when deploying Consul
to an environment which communicates the HTTP port through the environment e.g. PaaS like CloudFoundry, allowing
Expand Down Expand Up @@ -720,6 +724,9 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass
(/docs/agent/encryption.html#configuring-gossip-encryption-on-an-existing-cluster) for more information.
Defaults to true.

* <a name="disable_keyring_file"></a><a href="#disable_keyring_file">`disable_keyring_file`</a> - Equivalent to the
[`-disable-keyring-file` command-line flag](#_disable_keyring_file).

* <a name="key_file"></a><a href="#key_file">`key_file`</a> This provides a the file path to a
PEM-encoded private key. The key is used with the certificate to verify the agent's authenticity.
This must be provided along with [`cert_file`](#cert_file).
Expand Down