Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge branch ' env_files' into dev #2420

Merged
merged 31 commits into from
Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
eaabce2
add environment file to api
jy19 Feb 25, 2020
95b4152
Merge pull request #2375 from jy19/env_files
jy19 Feb 27, 2020
aba9cd0
untested code of envfile as task resource + downloading stuff from s3…
jy19 Mar 3, 2020
40ddd41
Merge pull request #2383 from jy19/env_files
jy19 Mar 9, 2020
6202a5c
Add script for verifying agent s3 artifacts during release process (#…
sparrc Feb 27, 2020
cdb98da
Fix flakey TaskManifest* tests
shubham2892 Feb 27, 2020
88bb087
Fixing data race warning in integ tests
yhlee-aws Mar 2, 2020
e3d5ba6
Stats engine: only skip container on cpu/mem errors (#2386)
sparrc Mar 9, 2020
231cecb
retrieve envfiles from s3 concurrently using go routines
jy19 Mar 10, 2020
79ea0b0
Envfile cleanup logic and unit tests, including oswrapper changes
yhlee-aws Mar 11, 2020
aacd728
retrieve envfiles from s3 concurrently using go routines
jy19 Mar 10, 2020
8b5769a
Merge pull request #2392 from jy19/env_files
jy19 Mar 12, 2020
bacd380
State file version bump and unit test
yhlee-aws Mar 12, 2020
e72c799
read environment variables from downloaded envfiles
jy19 Mar 16, 2020
c8ab428
Merge branch 'env_files' of https://github.com/aws/amazon-ecs-agent i…
jy19 Mar 17, 2020
c32a89f
Merge pull request #2398 from jy19/env_files
jy19 Mar 21, 2020
5d4b459
Merge branch 'dev' into env_files
jy19 Mar 23, 2020
8b025ca
Merge branch 'dev' into env_files
jy19 Mar 23, 2020
31da04b
add logging if invalid line in envfile
jy19 Mar 23, 2020
a7b9b73
Merge pull request #2405 from jy19/env_files
jy19 Mar 25, 2020
f62e038
envfiles: create directory where the envfile will be written
jy19 Mar 25, 2020
986b8ac
Merge pull request #2407 from jy19/env_files
jy19 Mar 26, 2020
981f4e4
envfile: initialize bufio and add location to statefile properly
jy19 Mar 26, 2020
581753d
minor unit test fixes
yhlee-aws Mar 26, 2020
a3897c2
envfile: keep order of envfiles when reading from disk
jy19 Mar 26, 2020
82820b2
adding s3EnvFiles capability
yhlee-aws Mar 28, 2020
94b3cce
rename attribute for s3 envfile for extensibility
yhlee-aws Mar 30, 2020
8c43da5
envfiles: remove sync call since we don't actually need to flush data…
jy19 Mar 30, 2020
d881a3a
Merge pull request #2417 from jy19/env_files
jy19 Mar 31, 2020
f2c2cbc
Merge branch 'dev' into env_files
yhlee-aws Mar 31, 2020
8d8ff43
Merge pull request #2419 from yunhee-l/env_files
yhlee-aws Apr 1, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions agent/acs/model/api/api-2.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down Expand Up @@ -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"},
Expand Down
20 changes: 20 additions & 0 deletions agent/acs/model/ecsacs/api.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 55 additions & 0 deletions agent/api/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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()
Expand Down Expand Up @@ -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
}
84 changes: 84 additions & 0 deletions agent/api/container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
}
}
77 changes: 77 additions & 0 deletions agent/api/task/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Loading