Merge pull request #2420 from yunhee-l/dev

Merge branch ' env_files' into dev

agent/acs/model/api/api-2.json

@@ -211,6 +211,7 @@
         "cpu":{"shape":"Integer"},
         "entryPoint":{"shape":"StringList"},
         "environment":{"shape":"EnvironmentVariables"},
+        "environmentFiles":{"shape":"EnvironmentFiles"},
         "essential":{"shape":"Boolean"},
         "image":{"shape":"String"},
         "links":{"shape":"StringList"},
@@ -364,6 +365,23 @@
         "base64"
       ]
     },
+    "EnvironmentFile":{
+      "type":"structure",
+      "members":{
+        "value":{"shape":"String"},
+        "type":{"shape":"EnvironmentFileType"}
+      }
+    },
+    "EnvironmentFiles":{
+      "type":"list",
+      "member":{"shape":"EnvironmentFile"}
+    },
+    "EnvironmentFileType": {
+      "type":"string",
+      "enum":[
+        "s3"
+      ]
+    },
     "EnvironmentVariables":{
       "type":"map",
       "key":{"shape":"String"},

agent/api/container/container.go

@@ -146,6 +146,8 @@ type Container struct {
 	EntryPoint *[]string
 	// Environment is the environment variable set in the container
 	Environment map[string]string `json:"environment"`
+	// EnvironmentFiles is the list of environmentFile used to populate environment variables
+	EnvironmentFiles []EnvironmentFile `json:"environmentFiles"`
 	// Overrides contains the configuration to override of a container
 	Overrides ContainerOverrides `json:"overrides"`
 	// DockerConfig is the configuration used to create the container
@@ -281,6 +283,11 @@ type DockerContainer struct {
 	Container *Container
 }
 
+type EnvironmentFile struct {
+	Value string `json:"value"`
+	Type  string `json:"type"`
+}
+
 // MountPoint describes the in-container location of a Volume and references
 // that Volume by name.
 type MountPoint struct {
@@ -917,6 +924,19 @@ func (c *Container) ShouldCreateWithASMSecret() bool {
 	return false
 }
 
+// ShouldCreateWithEnvFiles returns true if this container needs to
+// retrieve environment variable files
+func (c *Container) ShouldCreateWithEnvFiles() bool {
+	c.lock.RLock()
+	defer c.lock.RUnlock()
+
+	if c.EnvironmentFiles == nil {
+		return false
+	}
+
+	return len(c.EnvironmentFiles) != 0
+}
+
 // MergeEnvironmentVariables appends additional envVarName:envVarValue pairs to
 // the the container's environment values structure
 func (c *Container) MergeEnvironmentVariables(envVars map[string]string) {
@@ -932,6 +952,33 @@ func (c *Container) MergeEnvironmentVariables(envVars map[string]string) {
 	}
 }
 
+// MergeEnvironmentVariablesFromEnvfiles appends environment variable pairs from
+// the retrieved envfiles to the container's environment values list
+// envvars from envfiles will have lower precedence than existing envvars
+func (c *Container) MergeEnvironmentVariablesFromEnvfiles(envVarsList []map[string]string) error {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	// create map if does not exist
+	if c.Environment == nil {
+		c.Environment = make(map[string]string)
+	}
+
+	// envVarsList is a list of map, where each map is from an envfile
+	// iterate over this sequentially because the original order of the
+	// environment files give precedence to the environment variables
+	for _, envVars := range envVarsList {
+		for k, v := range envVars {
+			// existing environment variables have precedence over variables from envfile
+			// only set the env var if key does not already exist
+			if _, ok := c.Environment[k]; !ok {
+				c.Environment[k] = v
+			}
+		}
+	}
+	return nil
+}
+
 // HasSecret returns whether a container has secret based on a certain condition.
 func (c *Container) HasSecret(f func(s Secret) bool) bool {
 	c.lock.RLock()
@@ -1065,3 +1112,11 @@ func (c *Container) GetFirelensConfig() *FirelensConfig {
 
 	return c.FirelensConfig
 }
+
+// GetEnvironmentFiles returns the container's environment files.
+func (c *Container) GetEnvironmentFiles() []EnvironmentFile {
+	c.lock.RLock()
+	defer c.lock.RUnlock()
+
+	return c.EnvironmentFiles
+}

agent/api/container/container_test.go

@@ -666,3 +666,87 @@ func TestGetNetworkModeFromHostConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestShouldCreateWithEnvfiles(t *testing.T) {
+	cases := []struct {
+		in  Container
+		out bool
+	}{
+		{
+			Container{
+				Name:  "containerName",
+				Image: "image:tag",
+				EnvironmentFiles: []EnvironmentFile{
+					EnvironmentFile{
+						Value: "s3://bucket/envfile",
+						Type:  "s3",
+					},
+				},
+			}, true},
+		{
+			Container{
+				Name:             "containerName",
+				Image:            "image:tag",
+				EnvironmentFiles: nil,
+			}, false},
+	}
+
+	for _, test := range cases {
+		container := test.in
+		assert.Equal(t, test.out, container.ShouldCreateWithEnvFiles())
+	}
+}
+
+func TestMergeEnvironmentVariablesFromEnvfiles(t *testing.T) {
+	cases := []struct {
+		Name                   string
+		InContainerEnvironment map[string]string
+		InEnvVarList           []map[string]string
+		OutEnvVarMap           map[string]string
+	}{
+		{
+			Name:                   "merge one item",
+			InContainerEnvironment: map[string]string{"key1": "value1"},
+			InEnvVarList:           []map[string]string{{"key2": "value2"}},
+			OutEnvVarMap: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+		},
+		{
+			Name:                   "merge single item to nil env var map",
+			InContainerEnvironment: nil,
+			InEnvVarList:           []map[string]string{{"key": "value"}},
+			OutEnvVarMap:           map[string]string{"key": "value"},
+		},
+		{
+			Name:                   "merge one item key already exists",
+			InContainerEnvironment: map[string]string{"key1": "value1"},
+			InEnvVarList:           []map[string]string{{"key1": "value2"}},
+			OutEnvVarMap:           map[string]string{"key1": "value1"},
+		},
+		{
+			Name:                   "merge two items with same key",
+			InContainerEnvironment: map[string]string{"key1": "value1"},
+			InEnvVarList: []map[string]string{
+				{"key2": "value2"},
+				{"key2": "value3"},
+			},
+			OutEnvVarMap: map[string]string{
+				"key1": "value1",
+				"key2": "value2",
+			},
+		},
+	}
+
+	for _, test := range cases {
+		t.Run(test.Name, func(t *testing.T) {
+			container := Container{
+				Environment: test.InContainerEnvironment,
+			}
+
+			container.MergeEnvironmentVariablesFromEnvfiles(test.InEnvVarList)
+			assert.True(t, reflect.DeepEqual(test.OutEnvVarMap, container.Environment))
+		})
+	}
+}

agent/api/task/task.go

@@ -45,6 +45,7 @@ import (
 	"github.com/aws/amazon-ecs-agent/agent/taskresource"
 	"github.com/aws/amazon-ecs-agent/agent/taskresource/asmauth"
 	"github.com/aws/amazon-ecs-agent/agent/taskresource/asmsecret"
+	"github.com/aws/amazon-ecs-agent/agent/taskresource/envFiles"
 	"github.com/aws/amazon-ecs-agent/agent/taskresource/firelens"
 	"github.com/aws/amazon-ecs-agent/agent/taskresource/ssmsecret"
 	resourcestatus "github.com/aws/amazon-ecs-agent/agent/taskresource/status"
@@ -371,6 +372,11 @@ func (task *Task) PostUnmarshalTask(cfg *config.Config,
 		}
 	}
 
+	if err := task.initializeEnvfilesResource(cfg, credentialsManager); err != nil {
+		seelog.Errorf("Task [%s]: could not initialize environment files resource: %v", task.Arn, err)
+		return apierrors.NewResourceInitError(task.Arn, err)
+	}
+
 	return nil
 }
 
@@ -2576,3 +2582,74 @@ func (task *Task) GetContainerIndex(containerName string) int {
 	}
 	return -1
 }
+
+func (task *Task) requireEnvfiles() bool {
+	for _, container := range task.Containers {
+		if container.ShouldCreateWithEnvFiles() {
+			return true
+		}
+	}
+	return false
+}
+
+func (task *Task) initializeEnvfilesResource(config *config.Config, credentialsManager credentials.Manager) error {
+
+	for _, container := range task.Containers {
+		if container.ShouldCreateWithEnvFiles() {
+			envfileResource, err := envFiles.NewEnvironmentFileResource(config.Cluster, task.Arn, config.AWSRegion, config.DataDir,
+				container.Name, container.EnvironmentFiles, credentialsManager, task.ExecutionCredentialsID)
+			if err != nil {
+				return errors.Wrapf(err, "unable to initialize envfiles resource for container %s", container.Name)
+			}
+			task.AddResource(envFiles.ResourceName, envfileResource)
+			container.BuildResourceDependency(envfileResource.GetName(), resourcestatus.ResourceCreated, apicontainerstatus.ContainerCreated)
+		}
+	}
+
+	return nil
+}
+
+func (task *Task) getEnvfilesResource(containerName string) (taskresource.TaskResource, bool) {
+	task.lock.RLock()
+	defer task.lock.RUnlock()
+
+	resources, ok := task.ResourcesMapUnsafe[envFiles.ResourceName]
+	if !ok {
+		return nil, false
+	}
+
+	for _, resource := range resources {
+		envfileResource := resource.(*envFiles.EnvironmentFileResource)
+		if envfileResource.GetContainerName() == containerName {
+			return envfileResource, true
+		}
+	}
+
+	// was not able to retrieve envfile resource for specified container name
+	return nil, false
+}
+
+// MergeEnvVarsFromEnvfiles should be called when creating a container -
+// this method reads the environment variables specified in the environment files
+// that was downloaded to disk and merges it with existing environment variables
+func (task *Task) MergeEnvVarsFromEnvfiles(container *apicontainer.Container) *apierrors.ResourceInitError {
+	var envfileResource *envFiles.EnvironmentFileResource
+	resource, ok := task.getEnvfilesResource(container.Name)
+	if !ok {
+		err := errors.New(fmt.Sprintf("task environment files: unable to retrieve environment files resource for container %s", container.Name))
+		return apierrors.NewResourceInitError(task.Arn, err)
+	}
+	envfileResource = resource.(*envFiles.EnvironmentFileResource)
+
+	envVarsList, err := envfileResource.ReadEnvVarsFromEnvfiles()
+	if err != nil {
+		return apierrors.NewResourceInitError(task.Arn, err)
+	}
+
+	err = container.MergeEnvironmentVariablesFromEnvfiles(envVarsList)
+	if err != nil {
+		return apierrors.NewResourceInitError(task.Arn, err)
+	}
+
+	return nil
+}