Allow setting TLS for gRPC with deprecated options [1.13.x]

.changelog/14668.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+tls: undo breaking change that prevented setting TLS for gRPC when using config flags available in Consul v1.11.
+```

agent/agent.go

@@ -761,7 +761,7 @@ func (a *Agent) Failed() <-chan struct{} {
 }
 
 func (a *Agent) buildExternalGRPCServer() {
-	a.externalGRPCServer = external.NewServer(a.logger.Named("grpc.external"), a.tlsConfigurator)
+	a.externalGRPCServer = external.NewServer(a.logger.Named("grpc.external"), a.tlsConfigurator, a.config.HTTPSPort > 0)
 }
 
 func (a *Agent) listenAndServeGRPC() error {

agent/config/builder.go

@@ -189,9 +189,10 @@ func newBuilder(opts LoadOpts) (*builder, error) {
 	b.Tail = append(b.Tail, LiteralSource{Name: "flags.values", Config: values})
 	for i, s := range opts.HCL {
 		b.Tail = append(b.Tail, FileSource{
-			Name:   fmt.Sprintf("flags-%d.hcl", i),
-			Format: "hcl",
-			Data:   s,
+			Name:     fmt.Sprintf("flags-%d.hcl", i),
+			Format:   "hcl",
+			Data:     s,
+			FromUser: true,
 		})
 	}
 	b.Tail = append(b.Tail, NonUserSource(), DefaultConsulSource(), OverrideEnterpriseSource(), defaultVersionSource())
@@ -282,7 +283,7 @@ func newSourceFromFile(path string, format string) (Source, error) {
 	if format == "" {
 		format = formatFromFileExtension(path)
 	}
-	return FileSource{Name: path, Data: string(data), Format: format}, nil
+	return FileSource{Name: path, Data: string(data), Format: format, FromUser: true}, nil
 }
 
 // shouldParse file determines whether the file to be read is of a supported extension
@@ -2517,6 +2518,12 @@ func validateAbsoluteURLPath(p string) error {
 func (b *builder) buildTLSConfig(rt RuntimeConfig, t TLS) (tlsutil.Config, error) {
 	var c tlsutil.Config
 
+	// This flag indicates whether any of the per-listener configuration was set.
+	// The flag was populated earlier when applying deprecated options.
+	if t.SpecifiedTLSStanza != nil {
+		c.SpecifiedTLSStanza = *t.SpecifiedTLSStanza
+	}
+
 	// Consul makes no outgoing connections to the public gRPC port (internal gRPC
 	// traffic goes through the multiplexed internal RPC port) so return an error
 	// rather than let the user think this setting is going to do anything useful.

agent/config/builder_test.go

@@ -75,10 +75,10 @@ func TestNewBuilder_PopulatesSourcesFromConfigFiles(t *testing.T) {
 	require.NoError(t, err)
 
 	expected := []Source{
-		FileSource{Name: paths[0], Format: "hcl", Data: "content a"},
-		FileSource{Name: paths[1], Format: "json", Data: "content b"},
-		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
-		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b"},
+		FileSource{Name: paths[0], Format: "hcl", Data: "content a", FromUser: true},
+		FileSource{Name: paths[1], Format: "json", Data: "content b", FromUser: true},
+		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a", FromUser: true},
+		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b", FromUser: true},
 	}
 	require.Equal(t, expected, b.Sources)
 	require.Len(t, b.Warnings, 2)
@@ -91,12 +91,12 @@ func TestNewBuilder_PopulatesSourcesFromConfigFiles_WithConfigFormat(t *testing.
 	require.NoError(t, err)
 
 	expected := []Source{
-		FileSource{Name: paths[0], Format: "hcl", Data: "content a"},
-		FileSource{Name: paths[1], Format: "hcl", Data: "content b"},
-		FileSource{Name: paths[2], Format: "hcl", Data: "content c"},
-		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
-		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "hcl", Data: "content b"},
-		FileSource{Name: filepath.Join(paths[3], "c.yaml"), Format: "hcl", Data: "content c"},
+		FileSource{Name: paths[0], Format: "hcl", Data: "content a", FromUser: true},
+		FileSource{Name: paths[1], Format: "hcl", Data: "content b", FromUser: true},
+		FileSource{Name: paths[2], Format: "hcl", Data: "content c", FromUser: true},
+		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a", FromUser: true},
+		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "hcl", Data: "content b", FromUser: true},
+		FileSource{Name: filepath.Join(paths[3], "c.yaml"), Format: "hcl", Data: "content c", FromUser: true},
 	}
 	require.Equal(t, expected, b.Sources)
 }

agent/config/config.go

@@ -3,6 +3,7 @@ package config
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"time"
 
 	"github.com/hashicorp/consul/agent/consul"
@@ -30,6 +31,10 @@ type FileSource struct {
 	Name   string
 	Format string
 	Data   string
+
+	// FromUser indicates whether the the file source was provided by the user.
+	// This distinguishes from synthetic file sources that Consul will generate.
+	FromUser bool
 }
 
 func (f FileSource) Source() string {
@@ -79,7 +84,7 @@ func (f FileSource) Parse() (Config, Metadata, error) {
 		return Config{}, m, err
 	}
 
-	c, warns := applyDeprecatedConfig(&target)
+	c, warns := applyDeprecatedConfig(&target, f.FromUser)
 	m.Unused = md.Unused
 	m.Keys = md.Keys
 	m.Warnings = warns
@@ -870,12 +875,28 @@ type TLSProtocolConfig struct {
 	UseAutoCert          *bool   `mapstructure:"use_auto_cert"`
 }
 
+func (c TLSProtocolConfig) IsZero() bool {
+	v := reflect.ValueOf(c)
+
+	for i := 0; i < v.NumField(); i++ {
+		if !v.Field(i).IsNil() {
+			return false
+		}
+	}
+	return true
+}
+
 type TLS struct {
 	Defaults    TLSProtocolConfig `mapstructure:"defaults"`
 	InternalRPC TLSProtocolConfig `mapstructure:"internal_rpc"`
 	HTTPS       TLSProtocolConfig `mapstructure:"https"`
 	GRPC        TLSProtocolConfig `mapstructure:"grpc"`
 
+	// SpecifiedTLSStanza indicates whether the per-protocol tls stanza from configuration was used.
+	// If unspecified, and TLS is configured, that implies that the deprecated flags were used.
+	// The flag was added exclusively for the 1.13 patch series for backwards compatibility purposes.
+	SpecifiedTLSStanza *bool `mapstructure:"-"`
+
 	// GRPCModifiedByDeprecatedConfig is a flag used to indicate that GRPC was
 	// modified by the deprecated field mapping (as apposed to a user-provided
 	// a grpc stanza). This prevents us from emitting a warning about an
@@ -890,6 +911,11 @@ type TLS struct {
 	GRPCModifiedByDeprecatedConfig *struct{} `mapstructure:"-"`
 }
 
+// ContainsDefaults indicates whether the user-settable values in this type are the defaults.
+func (t *TLS) ContainsDefaults() bool {
+	return t.Defaults.IsZero() && t.InternalRPC.IsZero() && t.HTTPS.IsZero() && t.GRPC.IsZero()
+}
+
 type Peering struct {
 	Enabled *bool `mapstructure:"enabled"`
 }

agent/config/deprecated.go

@@ -70,7 +70,7 @@ type DeprecatedConfig struct {
 	TLSPreferServerCipherSuites *bool `mapstructure:"tls_prefer_server_cipher_suites"`
 }
 
-func applyDeprecatedConfig(d *decodeTarget) (Config, []string) {
+func applyDeprecatedConfig(d *decodeTarget, fromUser bool) (Config, []string) {
 	dep := d.DeprecatedConfig
 	var warns []string
 
@@ -172,15 +172,27 @@ func applyDeprecatedConfig(d *decodeTarget) (Config, []string) {
 		warns = append(warns, deprecationWarning("acl_enable_key_list_policy", "acl.enable_key_list_policy"))
 	}
 
-	warns = append(warns, applyDeprecatedTLSConfig(dep, &d.Config)...)
+	warns = append(warns, applyDeprecatedTLSConfig(dep, &d.Config, fromUser)...)
 
 	return d.Config, warns
 }
 
-func applyDeprecatedTLSConfig(dep DeprecatedConfig, cfg *Config) []string {
+func applyDeprecatedTLSConfig(dep DeprecatedConfig, cfg *Config, fromUser bool) []string {
 	var warns []string
 
 	tls := &cfg.TLS
+
+	// If the TLS stanza was specified by the user then we set a flag to indicate that.
+	// This check MUST happen before applying the deprecated options below, or else
+	// the tls struct will never be empty.
+	//
+	// This check was added exclusively to the 1.13 patch series for compatibility with
+	// Consul 1.11 style TLS configuration. Consul 1.14 does not require it since 1.12
+	// is the earliest major version supported once 1.14 is released.
+	if fromUser && !tls.ContainsDefaults() {
+		tls.SpecifiedTLSStanza = pBool(true)
+	}
+
 	defaults := &tls.Defaults
 	internalRPC := &tls.InternalRPC
 	https := &tls.HTTPS

agent/config/runtime_test.go

@@ -3051,6 +3051,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		expected: func(rt *RuntimeConfig) {
 			rt.DataDir = dataDir
 			rt.TLS.InternalRPC.VerifyIncoming = true
+			rt.TLS.SpecifiedTLSStanza = true
 			rt.AutoEncryptAllowTLS = true
 			rt.ConnectEnabled = true
 
@@ -3082,6 +3083,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.ConnectEnabled = true
 
 			rt.TLS.InternalRPC.VerifyIncoming = true
+			rt.TLS.SpecifiedTLSStanza = true
 			rt.TLS.GRPC.VerifyIncoming = true
 			rt.TLS.HTTPS.VerifyIncoming = true
 
@@ -3112,6 +3114,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.AutoEncryptAllowTLS = true
 			rt.ConnectEnabled = true
 			rt.TLS.InternalRPC.VerifyIncoming = true
+			rt.TLS.SpecifiedTLSStanza = true
 
 			// server things
 			rt.ServerMode = true
@@ -4509,7 +4512,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		},
 	})
 
-	///////////////////////////////////
+	// /////////////////////////////////
 	// Defaults sanity checks
 
 	run(t, testCase{
@@ -4532,7 +4535,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		},
 	})
 
-	///////////////////////////////////
+	// /////////////////////////////////
 	// Auto Config related tests
 	run(t, testCase{
 		desc: "auto config and auto encrypt error",
@@ -4767,6 +4770,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.DataDir = dataDir
 			rt.TLS.InternalRPC.VerifyOutgoing = true
 			rt.TLS.AutoTLS = true
+			rt.TLS.SpecifiedTLSStanza = true
 		},
 	})
 
@@ -5025,6 +5029,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.ServerMode = true
 			rt.SkipLeaveOnInt = true
 			rt.TLS.InternalRPC.CertFile = "foo"
+			rt.TLS.SpecifiedTLSStanza = true
 			rt.RPCConfig.EnableStreaming = true
 		},
 	})
@@ -5462,6 +5467,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
+			rt.TLS.SpecifiedTLSStanza = true
 
 			rt.TLS.InternalRPC.CAFile = "internal_rpc_ca_file"
 			rt.TLS.InternalRPC.CAPath = "default_ca_path"
@@ -5510,6 +5516,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
+			rt.TLS.SpecifiedTLSStanza = true
 
 			rt.TLS.InternalRPC.VerifyServerHostname = true
 			rt.TLS.InternalRPC.VerifyOutgoing = true
@@ -5537,6 +5544,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
 			rt.TLS.GRPC.UseAutoCert = false
+			rt.TLS.SpecifiedTLSStanza = false // TLS stanza was defined but is empty.
 		},
 	})
 	run(t, testCase{
@@ -5558,6 +5566,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
 			rt.TLS.GRPC.UseAutoCert = false
+			rt.TLS.SpecifiedTLSStanza = false // TLS stanza was defined but is empty.
 		},
 	})
 	run(t, testCase{
@@ -5604,6 +5613,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
 			rt.TLS.GRPC.UseAutoCert = true
+			rt.TLS.SpecifiedTLSStanza = true
 		},
 	})
 	run(t, testCase{
@@ -5632,6 +5642,37 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
 			rt.TLS.GRPC.UseAutoCert = false
+			rt.TLS.SpecifiedTLSStanza = true
+		},
+	})
+	run(t, testCase{
+		desc: "SpecifiedTLSStanza is not set if tls stanza was not defined",
+		args: []string{
+			`-data-dir=` + dataDir,
+		},
+		json: []string{`
+			{
+				"cert_file": "foo",
+				"key_file": "bar"
+			}
+		`},
+		hcl: []string{`
+			cert_file = "foo"
+			key_file = "bar"
+		`},
+		expected: func(rt *RuntimeConfig) {
+			rt.DataDir = dataDir
+			rt.TLS.SpecifiedTLSStanza = false
+			rt.TLS.HTTPS.CertFile = "foo"
+			rt.TLS.HTTPS.KeyFile = "bar"
+			rt.TLS.InternalRPC.CertFile = "foo"
+			rt.TLS.InternalRPC.KeyFile = "bar"
+			rt.TLS.GRPC.CertFile = "foo"
+			rt.TLS.GRPC.KeyFile = "bar"
+		},
+		expectedWarnings: []string{
+			"The 'cert_file' field is deprecated. Use the 'tls.defaults.cert_file' field instead.",
+			"The 'key_file' field is deprecated. Use the 'tls.defaults.key_file' field instead.",
 		},
 	})
 }
@@ -5655,9 +5696,10 @@ func (tc testCase) run(format string, dataDir string) func(t *testing.T) {
 
 		for i, data := range tc.source(format) {
 			opts.sources = append(opts.sources, FileSource{
-				Name:   fmt.Sprintf("src-%d.%s", i, format),
-				Format: format,
-				Data:   data,
+				Name:     fmt.Sprintf("src-%d.%s", i, format),
+				Format:   format,
+				Data:     data,
+				FromUser: true,
 			})
 		}
 
@@ -6441,6 +6483,7 @@ func TestLoad_FullConfig(t *testing.T) {
 			ServerName:              "Oerr9n1G",
 			Domain:                  "7W1xXSqd",
 			EnableAgentTLSForChecks: true,
+			SpecifiedTLSStanza:      true,
 		},
 		TaggedAddresses: map[string]string{
 			"7MYgHrYH": "dALJAhLD",