m3db · andrewmains12 · Oct 24, 2019 · Oct 22, 2019
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 ## Features
 - **Config**: support env var expansion using go.uber.org/config. 
+- **Config**: Deprecate listenaddress expansion in favor of go.uber.org/config 
+env var expansion ([#2017](https://github.com/m3db/m3/pull/2017/files))
 
 # 0.14.1
 

diff --git a/scripts/development/m3_stack/m3coordinator.yml b/scripts/development/m3_stack/m3coordinator.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/aggregator/m3coordinator.yml b/scripts/docker-integration-tests/aggregator/m3coordinator.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7202"
 
 logging:

diff --git a/scripts/docker-integration-tests/carbon/m3coordinator.yml b/scripts/docker-integration-tests/carbon/m3coordinator.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/cold_writes_simple/m3coordinator.yml b/scripts/docker-integration-tests/cold_writes_simple/m3coordinator.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/dedicated_etcd_embedded_coordinator/m3dbnode.yml b/scripts/docker-integration-tests/dedicated_etcd_embedded_coordinator/m3dbnode.yml
@@ -1,6 +1,5 @@
 coordinator:
   listenAddress:
-    type: "config"
     value: "0.0.0.0:7201"
 
   logging:

diff --git a/scripts/docker-integration-tests/multi_cluster_write/m3coordinator-cluster-a.yml b/scripts/docker-integration-tests/multi_cluster_write/m3coordinator-cluster-a.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/multi_cluster_write/m3coordinator-cluster-b.yml b/scripts/docker-integration-tests/multi_cluster_write/m3coordinator-cluster-b.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/prometheus/m3coordinator.yml b/scripts/docker-integration-tests/prometheus/m3coordinator.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/prometheus_replication/m3coordinator01.yml b/scripts/docker-integration-tests/prometheus_replication/m3coordinator01.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/prometheus_replication/m3coordinator02.yml b/scripts/docker-integration-tests/prometheus_replication/m3coordinator02.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/query_fanout/m3coordinator-cluster-a.yml b/scripts/docker-integration-tests/query_fanout/m3coordinator-cluster-a.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/query_fanout/m3coordinator-cluster-b.yml b/scripts/docker-integration-tests/query_fanout/m3coordinator-cluster-b.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/query_fanout/m3coordinator-cluster-c.yml b/scripts/docker-integration-tests/query_fanout/m3coordinator-cluster-c.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/repair/m3coordinator.yml b/scripts/docker-integration-tests/repair/m3coordinator.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/repair_and_replication/m3coordinator-cluster-a.yml b/scripts/docker-integration-tests/repair_and_replication/m3coordinator-cluster-a.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/repair_and_replication/m3coordinator-cluster-b.yml b/scripts/docker-integration-tests/repair_and_replication/m3coordinator-cluster-b.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/replication/m3coordinator-cluster-a.yml b/scripts/docker-integration-tests/replication/m3coordinator-cluster-a.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/replication/m3coordinator-cluster-b.yml b/scripts/docker-integration-tests/replication/m3coordinator-cluster-b.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/scripts/docker-integration-tests/simple_v2_batch_apis/m3coordinator.yml b/scripts/docker-integration-tests/simple_v2_batch_apis/m3coordinator.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/src/cmd/services/m3query/config/testdata/config.yml b/src/cmd/services/m3query/config/testdata/config.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/src/cmd/services/m3query/config/testdata/config_test.yml b/src/cmd/services/m3query/config/testdata/config_test.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/src/dbnode/config/m3dbnode-all-config.yml b/src/dbnode/config/m3dbnode-all-config.yml
@@ -2,7 +2,6 @@
 coordinator:
   # Address for M3Coordinator to listen for traffic.
   listenAddress:
-    type: "config"
     value: "0.0.0.0:7201"
 
   # All configured M3DB namespaces must be listed in this config if running an

diff --git a/src/dbnode/config/m3dbnode-cluster-template.yml b/src/dbnode/config/m3dbnode-cluster-template.yml
@@ -1,6 +1,5 @@
 coordinator:
   listenAddress:
-    type: "config"
     value: "0.0.0.0:7201"
 
   local:

diff --git a/src/dbnode/config/m3dbnode-local-etcd-proto.yml b/src/dbnode/config/m3dbnode-local-etcd-proto.yml
@@ -1,6 +1,5 @@
 coordinator:
   listenAddress:
-    type: "config"
     value: "0.0.0.0:7201"
 
   local:

diff --git a/src/dbnode/config/m3dbnode-local-etcd.yml b/src/dbnode/config/m3dbnode-local-etcd.yml
@@ -1,6 +1,5 @@
 coordinator:
   listenAddress:
-    type: "config"
     value: "0.0.0.0:7201"
 
   local:

diff --git a/src/query/config/m3coordinator-cluster-template.yml b/src/query/config/m3coordinator-cluster-template.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/src/query/config/m3coordinator-local-etcd.yml b/src/query/config/m3coordinator-local-etcd.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/src/query/config/m3query-dev-etcd.yml b/src/query/config/m3query-dev-etcd.yml
@@ -2,7 +2,6 @@
 # resources (threads primarily).
 
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/src/query/config/m3query-local-etcd.yml b/src/query/config/m3query-local-etcd.yml
@@ -1,5 +1,4 @@
 listenAddress:
-  type: "config"
   value: "0.0.0.0:7201"
 
 logging:

diff --git a/src/x/config/listenaddress/listenaddress.go b/src/x/config/listenaddress/listenaddress.go
@@ -20,6 +20,21 @@
 
 // Package listenaddress provides a configuration struct for resolving
 // a listen address from YAML.
+//
+// Deprecation notice:
+// The environment resolution behavior of this
+// class has largely been superseded
+// by config environment interpolation in go.uber.org/config (see config/README.md for
+// details). Instead of:
+//
+//     listenAddress:
+//       type: environment
+//       envVarListenPort: MY_PORT_VAR
+//
+//  one can do:
+//
+//     listenAddress:
+//       value: 0.0.0.0:${MY_PORT_VAR}
 package listenaddress
 
 import (
@@ -45,22 +60,30 @@ const (
 
 // Configuration is the configuration for resolving a listen address.
 type Configuration struct {
-	// ListenAddressType is the port type for the port
-	ListenAddressType Resolver `yaml:"type" validate:"nonzero"`
+	// DeprecatedListenAddressType is the port type for the port
+	// DEPRECATED: use config interpolation with `value` (config/README.md)
+	DeprecatedListenAddressType Resolver `yaml:"type"`
 
 	// Value is the config specified listen address if using config port type.
 	Value *string `yaml:"value"`
 
 	// EnvVarListenPort specifies the environment variable name for the listen address port.
-	EnvVarListenPort *string `yaml:"envVarListenPort"`
+	// DEPRECATED: use config interpolation with `value` (config/README.md)
+	DeprecatedEnvVarListenPort *string `yaml:"envVarListenPort"`
 
-	// EnvVarListenHost specifies the environment variable name for the listen address hostname.
-	EnvVarListenHost *string `yaml:"envVarListenHost"`
+	// DeprecatedEnvVarListenHost specifies the environment variable name for the listen address hostname.
+	// DEPRECATED: use config interpolation with `value` (config/README.md)
+	DeprecatedEnvVarListenHost *string `yaml:"envVarListenHost"`
 }
 
 // Resolve returns the resolved listen address given the configuration.
 func (c Configuration) Resolve() (string, error) {
-	listenAddrType := c.ListenAddressType
+	listenAddrType := c.DeprecatedListenAddressType
+
+	if listenAddrType == "" {
+		// Default to ConfigResolver
+		listenAddrType = ConfigResolver
+	}
 
 	var listenAddress string
 	switch listenAddrType {
@@ -74,23 +97,23 @@ func (c Configuration) Resolve() (string, error) {
 
 	case EnvironmentResolver:
 		// environment variable for port is required
-		if c.EnvVarListenPort == nil {
+		if c.DeprecatedEnvVarListenPort == nil {
 			err := fmt.Errorf("missing port env var name using: resolver=%s",
 				string(listenAddrType))
 			return "", err
 		}
-		portStr := os.Getenv(*c.EnvVarListenPort)
+		portStr := os.Getenv(*c.DeprecatedEnvVarListenPort)
 		port, err := strconv.Atoi(portStr)
 		if err != nil {
 			err := fmt.Errorf("invalid port env var value using: resolver=%s, name=%s",
-				string(listenAddrType), *c.EnvVarListenPort)
+				string(listenAddrType), *c.DeprecatedEnvVarListenPort)
 			return "", err
 		}
 		// if environment variable for hostname is not set, use the default
-		if c.EnvVarListenHost == nil {
+		if c.DeprecatedEnvVarListenHost == nil {
 			listenAddress = fmt.Sprintf("%s:%d", defaultHostname, port)
 		} else {
-			envHost := os.Getenv(*c.EnvVarListenHost)
+			envHost := os.Getenv(*c.DeprecatedEnvVarListenHost)
 			listenAddress = fmt.Sprintf("%s:%d", envHost, port)
 		}
 

diff --git a/src/x/config/listenaddress/listenaddress_test.go b/src/x/config/listenaddress/listenaddress_test.go
@@ -38,8 +38,19 @@ var (
 
 func TestListenAddressResolver(t *testing.T) {
 	cfg := Configuration{
-		ListenAddressType: ConfigResolver,
-		Value:             &defaultListen,
+		DeprecatedListenAddressType: ConfigResolver,
+		Value:                       &defaultListen,
+	}
+
+	value, err := cfg.Resolve()
+	require.NoError(t, err)
+
+	assert.Equal(t, defaultListen, value)
+}
+
+func TestListenAddressResolverDefaultsToConfigResolver(t *testing.T) {
+	cfg := Configuration{
+		Value: &defaultListen,
 	}
 
 	value, err := cfg.Resolve()
@@ -50,7 +61,7 @@ func TestListenAddressResolver(t *testing.T) {
 
 func TestConfigResolverErrorWhenMissing(t *testing.T) {
 	cfg := Configuration{
-		ListenAddressType: ConfigResolver,
+		DeprecatedListenAddressType: ConfigResolver,
 	}
 
 	_, err := cfg.Resolve()
@@ -63,8 +74,8 @@ func TestEnvironmentVariableResolverWithDefault(t *testing.T) {
 	require.NoError(t, os.Setenv(envListenPort, envPort))
 
 	cfg := Configuration{
-		ListenAddressType: EnvironmentResolver,
-		EnvVarListenPort:  &envListenPort,
+		DeprecatedListenAddressType: EnvironmentResolver,
+		DeprecatedEnvVarListenPort:  &envListenPort,
 	}
 
 	value, err := cfg.Resolve()
@@ -81,9 +92,9 @@ func TestEnvironmentVariableResolver(t *testing.T) {
 	require.NoError(t, os.Setenv(envListenHost, envHost))
 
 	cfg := Configuration{
-		ListenAddressType: EnvironmentResolver,
-		EnvVarListenPort:  &envListenPort,
-		EnvVarListenHost:  &envListenHost,
+		DeprecatedListenAddressType: EnvironmentResolver,
+		DeprecatedEnvVarListenPort:  &envListenPort,
+		DeprecatedEnvVarListenHost:  &envListenHost,
 	}
 
 	value, err := cfg.Resolve()
@@ -99,8 +110,8 @@ func TestInvalidEnvironmentVariableResolver(t *testing.T) {
 	require.NoError(t, os.Setenv(varName, expected))
 
 	cfg := Configuration{
-		ListenAddressType: EnvironmentResolver,
-		EnvVarListenPort:  &varName,
+		DeprecatedListenAddressType: EnvironmentResolver,
+		DeprecatedEnvVarListenPort:  &varName,
 	}
 
 	_, err := cfg.Resolve()
@@ -109,7 +120,7 @@ func TestInvalidEnvironmentVariableResolver(t *testing.T) {
 
 func TestEnvironmentResolverErrorWhenNameMissing(t *testing.T) {
 	cfg := Configuration{
-		ListenAddressType: EnvironmentResolver,
+		DeprecatedListenAddressType: EnvironmentResolver,
 	}
 
 	_, err := cfg.Resolve()
@@ -120,8 +131,8 @@ func TestEnvironmentResolverErrorWhenValueMissing(t *testing.T) {
 	varName := "OTHER_LISTEN_ENV_PORT"
 
 	cfg := Configuration{
-		ListenAddressType: EnvironmentResolver,
-		EnvVarListenPort:  &varName,
+		DeprecatedListenAddressType: EnvironmentResolver,
+		DeprecatedEnvVarListenPort:  &varName,
 	}
 
 	_, err := cfg.Resolve()
@@ -130,7 +141,7 @@ func TestEnvironmentResolverErrorWhenValueMissing(t *testing.T) {
 
 func TestUnknownResolverError(t *testing.T) {
 	cfg := Configuration{
-		ListenAddressType: "some-unknown-resolver",
+		DeprecatedListenAddressType: "some-unknown-resolver",
 	}
 
 	_, err := cfg.Resolve()