Skip to content

Commit

Permalink
Merge branch 'ecs-secrets-phase2' into dev
Browse files Browse the repository at this point in the history
  • Loading branch information
yumex93 committed Dec 4, 2018
2 parents b53bf01 + dede076 commit e7f4e08
Show file tree
Hide file tree
Showing 25 changed files with 2,137 additions and 169 deletions.
5 changes: 4 additions & 1 deletion agent/acs/model/api/api-2.json
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,10 @@
},
"SecretProvider":{
"type":"string",
"enum":["ssm"]
"enum":[
"ssm",
"asm"
]
},
"SecretType":{
"type":"string",
Expand Down
48 changes: 44 additions & 4 deletions agent/api/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ const (

// SecretProviderSSM is to show secret provider being SSM
SecretProviderSSM = "ssm"

// SecretProviderASM is to show secret provider being ASM
SecretProviderASM = "asm"

// SecretTypeEnv is to show secret type being ENVIRONMENT_VARIABLE
SecretTypeEnv = "ENVIRONMENT_VARIABLE"
)

// DockerConfig represents additional metadata about a container to run. It's
Expand Down Expand Up @@ -257,9 +263,9 @@ type Secret struct {
Provider string `json:"provider"`
}

// GetSSMSecretResourceCacheKey returns the key required to access the secret
// GetSecretResourceCacheKey returns the key required to access the secret
// from the ssmsecret resource
func (s *Secret) GetSSMSecretResourceCacheKey() string {
func (s *Secret) GetSecretResourceCacheKey() string {
return s.ValueFrom + "_" + s.Region
}

Expand Down Expand Up @@ -752,7 +758,7 @@ func (c *Container) ShouldCreateWithSSMSecret() bool {
c.lock.RLock()
defer c.lock.RUnlock()

//Secrets field will be nil if there is no secrets for container
// Secrets field will be nil if there is no secrets for container
if c.Secrets == nil {
return false
}
Expand All @@ -765,6 +771,25 @@ func (c *Container) ShouldCreateWithSSMSecret() bool {
return false
}

// ShouldCreateWithASMSecret returns true if this container needs to get secret
// value from AWS Secrets Manager
func (c *Container) ShouldCreateWithASMSecret() bool {
c.lock.RLock()
defer c.lock.RUnlock()

// Secrets field will be nil if there is no secrets for container
if c.Secrets == nil {
return false
}

for _, secret := range c.Secrets {
if secret.Provider == SecretProviderASM {
return true
}
}
return false
}

// MergeEnvironmentVariables appends additional envVarName:envVarValue pairs to
// the the container's enviornment values structure
func (c *Container) MergeEnvironmentVariables(envVars map[string]string) {
Expand All @@ -775,8 +800,23 @@ func (c *Container) MergeEnvironmentVariables(envVars map[string]string) {
if c.Environment == nil {
c.Environment = make(map[string]string)
}

for k, v := range envVars {
c.Environment[k] = v
}
}

func (c *Container) HasSecretAsEnv() bool {
c.lock.RLock()
defer c.lock.RUnlock()

// Secrets field will be nil if there is no secrets for container
if c.Secrets == nil {
return false
}
for _, secret := range c.Secrets {
if secret.Type == SecretTypeEnv {
return true
}
}
return false
}
79 changes: 79 additions & 0 deletions agent/api/container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,3 +374,82 @@ func TestMergeEnvironmentVariables(t *testing.T) {
})
}
}

func TestShouldCreateWithASMSecret(t *testing.T) {
cases := []struct {
in Container
out bool
}{
{Container{
Name: "myName",
Image: "image:tag",
Secrets: []Secret{
Secret{
Provider: "asm",
Name: "secret",
ValueFrom: "/test/secretName",
}},
}, true},
{Container{
Name: "myName",
Image: "image:tag",
Secrets: nil,
}, false},
{Container{
Name: "myName",
Image: "image:tag",
Secrets: []Secret{
Secret{
Provider: "ssm",
Name: "secret",
ValueFrom: "/test/secretName",
}},
}, false},
}

for _, test := range cases {
container := test.in
assert.Equal(t, test.out, container.ShouldCreateWithASMSecret())
}
}

func TestHasSecretAsEnv(t *testing.T) {
cases := []struct {
in Container
out bool
}{
{Container{
Name: "myName",
Image: "image:tag",
Secrets: []Secret{
Secret{
Provider: "asm",
Name: "secret",
Type: "ENVIRONMENT_VARIABLE",
ValueFrom: "/test/secretName",
}},
}, true},
{Container{
Name: "myName",
Image: "image:tag",
Secrets: nil,
}, false},
{Container{
Name: "myName",
Image: "image:tag",
Secrets: []Secret{
Secret{
Provider: "asm",
Name: "secret",
Type: "MOUNT_POINT",
ValueFrom: "/test/secretName",
}},
}, false},
}

for _, test := range cases {
container := test.in
assert.Equal(t, test.out, container.HasSecretAsEnv())
}

}
102 changes: 91 additions & 11 deletions agent/api/task/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"github.com/aws/amazon-ecs-agent/agent/ecscni"
"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/ssmsecret"
resourcestatus "github.com/aws/amazon-ecs-agent/agent/taskresource/status"
resourcetype "github.com/aws/amazon-ecs-agent/agent/taskresource/types"
Expand Down Expand Up @@ -247,6 +248,10 @@ func (task *Task) PostUnmarshalTask(cfg *config.Config,
task.initializeSSMSecretResource(credentialsManager, resourceFields)
}

if task.requiresASMSecret() {
task.initializeASMSecretResource(credentialsManager, resourceFields)
}

err := task.initializeDockerLocalVolumes(dockerClient, ctx)
if err != nil {
return apierrors.NewResourceInitError(task.Arn, err)
Expand Down Expand Up @@ -572,6 +577,51 @@ func (task *Task) getAllSSMSecretRequirements() map[string][]apicontainer.Secret
return reqs
}

// requiresASMSecret returns true if at least one container in the task
// needs to retrieve secret from AWS Secrets Manager
func (task *Task) requiresASMSecret() bool {
for _, container := range task.Containers {
if container.ShouldCreateWithASMSecret() {
return true
}
}
return false
}

// initializeASMSecretResource builds the resource dependency map for the asmsecret resource
func (task *Task) initializeASMSecretResource(credentialsManager credentials.Manager,
resourceFields *taskresource.ResourceFields) {
asmSecretResource := asmsecret.NewASMSecretResource(task.Arn, task.getAllASMSecretRequirements(),
task.ExecutionCredentialsID, credentialsManager, resourceFields.ASMClientCreator)
task.AddResource(asmsecret.ResourceName, asmSecretResource)

// for every container that needs asm secret vending as envvar, it needs to wait all secrets got retrieved
for _, container := range task.Containers {
if container.ShouldCreateWithASMSecret() {
container.BuildResourceDependency(asmSecretResource.GetName(),
resourcestatus.ResourceStatus(asmsecret.ASMSecretCreated),
apicontainerstatus.ContainerCreated)
}
}
}

// getAllASMSecretRequirements stores secrets in a task in a map
func (task *Task) getAllASMSecretRequirements() map[string]apicontainer.Secret {
reqs := make(map[string]apicontainer.Secret)

for _, container := range task.Containers {
for _, secret := range container.Secrets {
if secret.Provider == apicontainer.SecretProviderASM {
secretKey := secret.GetSecretResourceCacheKey()
if _, ok := reqs[secretKey]; !ok {
reqs[secretKey] = secret
}
}
}
}
return reqs
}

// BuildCNIConfig constructs the cni configuration from eni
func (task *Task) BuildCNIConfig() (*ecscni.Config, error) {
if !task.isNetworkModeVPC() {
Expand Down Expand Up @@ -1648,20 +1698,49 @@ func (task *Task) getASMAuthResource() ([]taskresource.TaskResource, bool) {
return res, ok
}

// PopulateSSMSecrets appends the container's env var map with ssm parameters
func (task *Task) PopulateSSMSecrets(container *apicontainer.Container) *apierrors.DockerClientConfigError {
resource, ok := task.getSSMSecretsResource()
if !ok {
return &apierrors.DockerClientConfigError{"task secret data: unable to fetch SSM Secrets resource"}
// getSSMSecretsResource retrieves ssmsecret resource from resource map
func (task *Task) getSSMSecretsResource() ([]taskresource.TaskResource, bool) {
task.lock.RLock()
defer task.lock.RUnlock()

res, ok := task.ResourcesMapUnsafe[ssmsecret.ResourceName]
return res, ok
}

// PopulateSecretsAsEnv appends the container's env var map with secrets
func (task *Task) PopulateSecretsAsEnv(container *apicontainer.Container) *apierrors.DockerClientConfigError {
var ssmRes *ssmsecret.SSMSecretResource
var asmRes *asmsecret.ASMSecretResource

if container.ShouldCreateWithSSMSecret() {
resource, ok := task.getSSMSecretsResource()
if !ok {
return &apierrors.DockerClientConfigError{"task secret data: unable to fetch SSM Secrets resource"}
}
ssmRes = resource[0].(*ssmsecret.SSMSecretResource)
}

if container.ShouldCreateWithASMSecret() {
resource, ok := task.getASMSecretsResource()
if !ok {
return &apierrors.DockerClientConfigError{"task secret data: unable to fetch ASM Secrets resource"}
}
asmRes = resource[0].(*asmsecret.ASMSecretResource)
}

ssmResource := resource[0].(*ssmsecret.SSMSecretResource)
envVars := make(map[string]string)

for _, secret := range container.Secrets {
if secret.Provider == apicontainer.SecretProviderSSM {
k := secret.GetSSMSecretResourceCacheKey()
if secretValue, ok := ssmResource.GetCachedSecretValue(k); ok {
if secret.Provider == apicontainer.SecretProviderSSM && secret.Type == apicontainer.SecretTypeEnv {
k := secret.GetSecretResourceCacheKey()
if secretValue, ok := ssmRes.GetCachedSecretValue(k); ok {
envVars[secret.Name] = secretValue
}
}

if secret.Provider == apicontainer.SecretProviderASM && secret.Type == apicontainer.SecretTypeEnv {
k := secret.GetSecretResourceCacheKey()
if secretValue, ok := asmRes.GetCachedSecretValue(k); ok {
envVars[secret.Name] = secretValue
}
}
Expand All @@ -1671,11 +1750,12 @@ func (task *Task) PopulateSSMSecrets(container *apicontainer.Container) *apierro
return nil
}

func (task *Task) getSSMSecretsResource() ([]taskresource.TaskResource, bool) {
// getASMSecretsResource retrieves asmsecret resource from resource map
func (task *Task) getASMSecretsResource() ([]taskresource.TaskResource, bool) {
task.lock.RLock()
defer task.lock.RUnlock()

res, ok := task.ResourcesMapUnsafe[ssmsecret.ResourceName]
res, ok := task.ResourcesMapUnsafe[asmsecret.ResourceName]
return res, ok
}

Expand Down
12 changes: 6 additions & 6 deletions agent/api/task/task_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestAddNetworkResourceProvisioningDependencyWithENI(t *testing.T) {
ENI: &apieni.ENI{},
Containers: []*apicontainer.Container{
{
Name: "c1",
Name: "c1",
TransitionDependenciesMap: make(map[apicontainerstatus.ContainerStatus]apicontainer.TransitionDependencySet),
},
},
Expand Down Expand Up @@ -392,7 +392,7 @@ func TestInitCgroupResourceSpecHappyPath(t *testing.T) {
Memory: taskMemoryLimit,
Containers: []*apicontainer.Container{
{
Name: "c1",
Name: "c1",
TransitionDependenciesMap: make(map[apicontainerstatus.ContainerStatus]apicontainer.TransitionDependencySet),
},
},
Expand Down Expand Up @@ -420,7 +420,7 @@ func TestInitCgroupResourceSpecInvalidARN(t *testing.T) {
Version: "1",
Containers: []*apicontainer.Container{
{
Name: "c1",
Name: "c1",
TransitionDependenciesMap: make(map[apicontainerstatus.ContainerStatus]apicontainer.TransitionDependencySet),
},
},
Expand All @@ -440,8 +440,8 @@ func TestInitCgroupResourceSpecInvalidMem(t *testing.T) {
Memory: taskMemoryLimit,
Containers: []*apicontainer.Container{
{
Name: "C1",
Memory: uint(2048), // container memory > task memory
Name: "C1",
Memory: uint(2048), // container memory > task memory
TransitionDependenciesMap: make(map[apicontainerstatus.ContainerStatus]apicontainer.TransitionDependencySet),
},
},
Expand All @@ -460,7 +460,7 @@ func TestPostUnmarshalWithCPULimitsFail(t *testing.T) {
Version: "1",
Containers: []*apicontainer.Container{
{
Name: "c1",
Name: "c1",
TransitionDependenciesMap: make(map[apicontainerstatus.ContainerStatus]apicontainer.TransitionDependencySet),
},
},
Expand Down
Loading

0 comments on commit e7f4e08

Please sign in to comment.