Fix docker auth configuration overrides incorrectly

DockerAuth returned as an empty object(which is not nil) when it is not
set from environment variable, which won't be overrides from contents
read from file.

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