diff --git a/agent/auto-config/config_translate.go b/agent/auto-config/config_translate.go index 84b04adc94..2506c70252 100644 --- a/agent/auto-config/config_translate.go +++ b/agent/auto-config/config_translate.go @@ -2,7 +2,6 @@ package autoconf import ( pbconfig "github.com/hashicorp/consul/agent/agentpb/config" - "github.com/hashicorp/consul/agent/config" ) // translateAgentConfig is meant to take in a agent/agentpb/config.Config type @@ -13,81 +12,149 @@ import ( // // Why is this function not in the agent/agentpb/config package? The answer, that // package cannot import the agent/config package without running into import cycles. -func translateConfig(c *pbconfig.Config) *config.Config { - out := config.Config{ - Datacenter: &c.Datacenter, - PrimaryDatacenter: &c.PrimaryDatacenter, - NodeName: &c.NodeName, - SegmentName: &c.SegmentName, +// +// If this function is meant to output an agent/config.Config then why does it output +// a map[string]interface{}? The answer is that our config and command line option +// parsing is messed up and it would require major changes to fix (we probably should +// do them but not for the auto-config feature). To understand this we need to work +// backwards. What we want to be able to do is persist the config settings from an +// auto-config response configuration to disk. We then want that configuration +// to be able to be parsed with the normal configuration parser/builder. It sort of was +// working with returning a filled out agent/config.Config but the problem was that +// the struct has a lot of non-pointer struct members. Thus, JSON serializtion caused +// these to always be emitted even if they contained no non-empty fields. The +// configuration would then seem to parse okay, but in OSS we would get warnings for +// setting a bunch of enterprise fields like "audit" at the top level. In an attempt +// to quiet those warnings, I had converted all the existing non-pointer struct fields +// to pointers. Then there were issues with the builder code expecting concrete values. +// I could add nil checks **EVERYWHERE** in builder.go or take a different approach. +// I then made a function utilizing github.com/mitchellh/reflectwalk to un-nil all the +// struct pointers after parsing to prevent any nil pointer dereferences. At first +// glance this seemed like it was going to work but then I saw that nearly all of the +// tests in runtime_test.go were failing. The first issues was that we were not merging +// pointers to struct fields properly. It was simply taking the new pointer if non-nil +// and defaulting to the original. So I updated that code, to properly merge pointers +// to structs. That fixed a bunch of tests but then there was another issue with +// the runtime tests where it was emitting warnings for using consul enterprise only +// configuration. After spending some time tracking this down it turns out that it +// was coming from our CLI option parsing. Our CLI option parsing works by filling +// in a agent/config.Config struct. Along the way when converting to pointers to +// structs I had to add a call to that function to un-nil various pointers to prevent +// the CLI from segfaulting. However this un-nil operation was causing the various +// enterprise only keys to be materialized. Thus we were back to where we were before +// the conversion to pointers to structs and mostly stuck. +// +// Therefore, this function will create a map[string]interface{} that should be +// compatible with the agent/config.Config struct but where we can more tightly +// control which fields are output. Its not a nice solution. It has a non-trivial +// maintenance burden. In the long run we should unify the protobuf Config and +// the normal agent/config.Config so that we can just serialize the protobuf version +// without any translation. For now, this hack is necessary :( +func translateConfig(c *pbconfig.Config) map[string]interface{} { + out := map[string]interface{}{ + "datacenter": c.Datacenter, + "primary_datacenter": c.PrimaryDatacenter, + "node_name": c.NodeName, + } + + // only output the SegmentName in the configuration if its non-empty + // this will avoid a warning later when parsing the persisted configuration + if c.SegmentName != "" { + out["segment"] = c.SegmentName } // Translate Auto Encrypt settings if a := c.AutoEncrypt; a != nil { - out.AutoEncrypt = config.AutoEncrypt{ - TLS: &a.TLS, - DNSSAN: a.DNSSAN, - IPSAN: a.IPSAN, - AllowTLS: &a.AllowTLS, + autoEncryptConfig := map[string]interface{}{ + "tls": a.TLS, + "allow_tls": a.AllowTLS, } + + if len(a.DNSSAN) > 0 { + autoEncryptConfig["dns_san"] = a.DNSSAN + } + if len(a.IPSAN) > 0 { + autoEncryptConfig["ip_san"] = a.IPSAN + } + + out["auto_encrypt"] = autoEncryptConfig } // Translate all the ACL settings if a := c.ACL; a != nil { - out.ACL = config.ACL{ - Enabled: &a.Enabled, - PolicyTTL: &a.PolicyTTL, - RoleTTL: &a.RoleTTL, - TokenTTL: &a.TokenTTL, - DownPolicy: &a.DownPolicy, - DefaultPolicy: &a.DefaultPolicy, - EnableKeyListPolicy: &a.EnableKeyListPolicy, - DisabledTTL: &a.DisabledTTL, - EnableTokenPersistence: &a.EnableTokenPersistence, - MSPDisableBootstrap: &a.MSPDisableBootstrap, + aclConfig := map[string]interface{}{ + "enabled": a.Enabled, + "policy_ttl": a.PolicyTTL, + "role_ttl": a.RoleTTL, + "token_ttl": a.TokenTTL, + "down_policy": a.DownPolicy, + "default_policy": a.DefaultPolicy, + "enable_key_list_policy": a.EnableKeyListPolicy, + "disabled_ttl": a.DisabledTTL, + "enable_token_persistence": a.EnableTokenPersistence, } if t := c.ACL.Tokens; t != nil { - var tokens []config.ServiceProviderToken + var mspTokens []map[string]string // create the slice of msp tokens if any for _, mspToken := range t.ManagedServiceProvider { - tokens = append(tokens, config.ServiceProviderToken{ - AccessorID: &mspToken.AccessorID, - SecretID: &mspToken.SecretID, + mspTokens = append(mspTokens, map[string]string{ + "accessor_id": mspToken.AccessorID, + "secret_id": mspToken.SecretID, }) } - out.ACL.Tokens = config.Tokens{ - Master: &t.Master, - Replication: &t.Replication, - AgentMaster: &t.AgentMaster, - Default: &t.Default, - Agent: &t.Agent, - ManagedServiceProvider: tokens, + tokenConfig := make(map[string]interface{}) + + if t.Master != "" { + tokenConfig["master"] = t.Master } + if t.Replication != "" { + tokenConfig["replication"] = t.Replication + } + if t.AgentMaster != "" { + tokenConfig["agent_master"] = t.AgentMaster + } + if t.Default != "" { + tokenConfig["default"] = t.Default + } + if t.Agent != "" { + tokenConfig["agent"] = t.Agent + } + if len(mspTokens) > 0 { + tokenConfig["managed_service_provider"] = mspTokens + } + + aclConfig["tokens"] = tokenConfig } + out["acl"] = aclConfig } // Translate the Gossip settings if g := c.Gossip; g != nil { - out.RetryJoinLAN = g.RetryJoinLAN + out["retry_join"] = g.RetryJoinLAN // Translate the Gossip Encryption settings if e := c.Gossip.Encryption; e != nil { - out.EncryptKey = &e.Key - out.EncryptVerifyIncoming = &e.VerifyIncoming - out.EncryptVerifyOutgoing = &e.VerifyOutgoing + out["encrypt"] = e.Key + out["encrypt_verify_incoming"] = e.VerifyIncoming + out["encrypt_verify_outgoing"] = e.VerifyOutgoing } } // Translate the Generic TLS settings if t := c.TLS; t != nil { - out.VerifyOutgoing = &t.VerifyOutgoing - out.VerifyServerHostname = &t.VerifyServerHostname - out.TLSMinVersion = &t.MinVersion - out.TLSCipherSuites = &t.CipherSuites - out.TLSPreferServerCipherSuites = &t.PreferServerCipherSuites + out["verify_outgoing"] = t.VerifyOutgoing + out["verify_server_hostname"] = t.VerifyServerHostname + if t.MinVersion != "" { + out["tls_min_version"] = t.MinVersion + } + if t.CipherSuites != "" { + out["tls_cipher_suites"] = t.CipherSuites + } + out["tls_prefer_server_cipher_suites"] = t.PreferServerCipherSuites } - return &out + return out } diff --git a/agent/auto-config/config_translate_test.go b/agent/auto-config/config_translate_test.go index 202fb3a5e5..92a0ad162f 100644 --- a/agent/auto-config/config_translate_test.go +++ b/agent/auto-config/config_translate_test.go @@ -1,6 +1,7 @@ package autoconf import ( + "encoding/json" "testing" pbconfig "github.com/hashicorp/consul/agent/agentpb/config" @@ -94,7 +95,6 @@ func TestConfig_translateConfig(t *testing.T) { EnableKeyListPolicy: boolPointer(true), DisabledTTL: stringPointer("4s"), EnableTokenPersistence: boolPointer(true), - MSPDisableBootstrap: boolPointer(false), Tokens: config.Tokens{ Master: stringPointer("99e7e490-6baf-43fc-9010-78b6aa9a6813"), Replication: stringPointer("51308d40-465c-4ac6-a636-7c0747edec89"), @@ -117,6 +117,11 @@ func TestConfig_translateConfig(t *testing.T) { }, } - actual := translateConfig(&original) - require.Equal(t, expected, actual) + translated := translateConfig(&original) + data, err := json.Marshal(translated) + require.NoError(t, err) + + actual, _, err := config.Parse(string(data), "json") + require.NoError(t, err) + require.Equal(t, expected, &actual) }