Fix the docker auth configuration overrides problem in #732

agent/config/config_unix_test.go

@@ -15,13 +15,16 @@
 package config
 
 import (
+	"fmt"
+	"io/ioutil"
 	"os"
 	"testing"
 	"time"
 
 	"github.com/aws/amazon-ecs-agent/agent/ec2"
 	"github.com/aws/amazon-ecs-agent/agent/engine/dockerclient"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestConfigDefault(t *testing.T) {
@@ -62,3 +65,86 @@ func TestConfigDefault(t *testing.T) {
 	assert.Equal(t, DefaultImageCleanupTimeInterval, cfg.ImageCleanupInterval, "ImageCleanupInterval default is set incorrectly")
 	assert.Equal(t, DefaultNumImagesToDeletePerCycle, cfg.NumImagesToDeletePerCycle, "NumImagesToDeletePerCycle default is set incorrectly")
 }
+
+// TestConfigFromFile tests the configuration can be read from file
+func TestConfigFromFile(t *testing.T) {
+	cluster := "TestCluster"
+	dockerAuthType := "dockercfg"
+	dockerAuth := `{
+  "https://index.docker.io/v1/":{
+    "auth":"admin",
+    "email":"email"
+  }
+}`
+	configContent := fmt.Sprintf(`{
+  "Cluster": "%s",
+  "EngineAuthType": "%s",
+  "EngineAuthData": %s,
+  "DataDir": "/var/run/ecs_agent",
+  "TaskIAMRoleEnabled": true,
+  "InstanceAttributes": {
+    "attribute1": "value1"
+  }
+}`, cluster, dockerAuthType, dockerAuth)
+
+	configFile := setupDockerAuthConfiguration(t, configContent)
+	defer os.Remove(configFile)
+
+	os.Setenv("ECS_AGENT_CONFIG_FILE_PATH", configFile)
+	defer os.Unsetenv("ECS_AGENT_CONFIG_FILE_PATH")
+
+	config, err := fileConfig()
+	assert.NoError(t, err, "reading configuration from file failed")
+
+	assert.Equal(t, cluster, config.Cluster, "cluster name not as expected from file")
+	assert.Equal(t, dockerAuthType, config.EngineAuthType, "docker auth type not as expected from file")
+	assert.Equal(t, dockerAuth, string(config.EngineAuthData.Contents()), "docker auth data not as expected from file")
+}
+
+// TestDockerAuthMergeFromFile tests docker auth read from file correctly after merge
+func TestDockerAuthMergeFromFile(t *testing.T) {
+	cluster := "myCluster"
+	dockerAuthType := "dockercfg"
+	dockerAuth := `{
+  "https://index.docker.io/v1/":{
+    "auth":"admin",
+    "email":"email"
+  }
+}`
+	configContent := fmt.Sprintf(`{
+  "Cluster": "TestCluster",
+  "EngineAuthType": "%s",
+  "EngineAuthData": %s,
+  "DataDir": "/var/run/ecs_agent",
+  "TaskIAMRoleEnabled": true,
+  "InstanceAttributes": {
+    "attribute1": "value1"
+  }
+}`, dockerAuthType, dockerAuth)
+
+	configFile := setupDockerAuthConfiguration(t, configContent)
+	defer os.Remove(configFile)
+
+	os.Setenv("ECS_CLUSTER", cluster)
+	os.Setenv("ECS_AGENT_CONFIG_FILE_PATH", configFile)
+	defer os.Unsetenv("ECS_CLUSTER")
+	defer os.Unsetenv("ECS_AGENT_CONFIG_FILE_PATH")
+
+	config, err := NewConfig(ec2.NewBlackholeEC2MetadataClient())
+	assert.NoError(t, err, "create configuration failed")
+
+	assert.Equal(t, cluster, config.Cluster, "cluster name not as expected from environment variable")
+	assert.Equal(t, dockerAuthType, config.EngineAuthType, "docker auth type not as expected from file")
+	assert.Equal(t, dockerAuth, string(config.EngineAuthData.Contents()), "docker auth data not as expected from file")
+}
+
+// setupDockerAuthConfiguration create a temp file store the configuration
+func setupDockerAuthConfiguration(t *testing.T, configContent string) string {
+	configFile, err := ioutil.TempFile("", "ecs-test")
+	require.NoError(t, err, "creating temp file for configuration failed")
+
+	_, err = configFile.Write([]byte(configContent))
+	require.NoError(t, err, "writing configuration to file failed")
+
+	return configFile.Name()
+}

agent/config/types.go

@@ -154,9 +154,12 @@ type SensitiveRawMessage struct {
 	contents json.RawMessage
 }
 
-// NewSensitiveRawMessage returns a new encapsulated json.RawMessage that
-// cannot be accidentally logged via .String/.GoString/%v/%#v
+// NewSensitiveRawMessage returns a new encapsulated json.RawMessage or nil if
+// the data is empty. It cannot be accidentally logged via .String/.GoString/%v/%#v
 func NewSensitiveRawMessage(data json.RawMessage) *SensitiveRawMessage {
+	if len(data) == 0 {
+		return nil
+	}
 	return &SensitiveRawMessage{contents: data}
 }
 

agent/config/types_test.go

@@ -17,6 +17,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestSensitiveRawMessageImplements(t *testing.T) {
@@ -29,15 +31,20 @@ func TestSensitiveRawMessageImplements(t *testing.T) {
 func TestSensitiveRawMessage(t *testing.T) {
 	sensitive := NewSensitiveRawMessage(json.RawMessage("secret"))
 
-	for i, str := range []string{
+	for _, str := range []string{
 		sensitive.String(),
 		sensitive.GoString(),
 		fmt.Sprintf("%v", sensitive),
 		fmt.Sprintf("%#v", sensitive),
 		fmt.Sprint(sensitive),
 	} {
-		if str != "[redacted]" {
-			t.Errorf("#%v: expected redacted, got %s", i, str)
-		}
+		assert.Equal(t, "[redacted]", str, "expected redacted")
 	}
 }
+
+// TestEmptySensitiveRawMessage tests the message content is empty
+func TestEmptySensitiveRawMessage(t *testing.T) {
+	sensitive := NewSensitiveRawMessage(json.RawMessage(""))
+
+	assert.Nil(t, sensitive, "empty message should return nil")
+}

agent/engine/docker_container_engine.go

@@ -16,6 +16,7 @@ package engine
 import (
 	"archive/tar"
 	"bufio"
+	"encoding/json"
 	"io"
 	"strings"
 	"sync"
@@ -146,9 +147,13 @@ func NewDockerGoClient(clientFactory dockerclient.Factory, cfg *config.Config) (
 		return nil, err
 	}
 
+	var dockerAuthData json.RawMessage
+	if cfg.EngineAuthData != nil {
+		dockerAuthData = cfg.EngineAuthData.Contents()
+	}
 	return &dockerGoClient{
 		clientFactory:    clientFactory,
-		auth:             dockerauth.NewDockerAuthProvider(cfg.EngineAuthType, cfg.EngineAuthData.Contents()),
+		auth:             dockerauth.NewDockerAuthProvider(cfg.EngineAuthType, dockerAuthData),
 		ecrClientFactory: ecr.NewECRFactory(cfg.AcceptInsecureCert),
 		config:           cfg,
 	}, nil