Merge pull request #2358 from jy19/roadmap385

ECS support JSON key and version for secrets

.gitignore

@@ -22,3 +22,4 @@ _bin/
 /misc/v3-task-endpoint-validator/v3-task-endpoint-validator
 /misc/elastic-inference-validator/elastic-inference-validator
 /bin
+*.iml

agent/asm/asm.go

@@ -15,10 +15,12 @@ package asm
 
 import (
 	"encoding/json"
+	"fmt"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/secretsmanager"
 	"github.com/aws/aws-sdk-go/service/secretsmanager/secretsmanageriface"
+	"github.com/cihub/seelog"
 	"github.com/docker/docker/api/types"
 	"github.com/pkg/errors"
 )
@@ -87,6 +89,33 @@ func extractASMValue(out *secretsmanager.GetSecretValueOutput) (types.AuthConfig
 	return dac, nil
 }
 
+func GetSecretFromASMWithInput(input *secretsmanager.GetSecretValueInput,
+	client secretsmanageriface.SecretsManagerAPI, jsonKey string) (string, error) {
+	out, err := client.GetSecretValue(input)
+	if err != nil {
+		return "", errors.Wrapf(err, "secret %s", *input.SecretId)
+	}
+
+	if jsonKey == "" {
+		return aws.StringValue(out.SecretString), nil
+	}
+
+	secretMap := make(map[string]interface{})
+	jsonErr := json.Unmarshal([]byte(*out.SecretString), &secretMap)
+	if jsonErr != nil {
+		seelog.Warnf("Error when treating retrieved secret value with secret id %s as JSON and calling unmarshal.", *input.SecretId)
+		return "", jsonErr
+	}
+
+	secretValue, ok := secretMap[jsonKey]
+	if !ok {
+		err = errors.New(fmt.Sprintf("retrieved secret from Secrets Manager did not contain json key %s", jsonKey))
+		return "", err
+	}
+
+	return fmt.Sprintf("%v", secretValue), nil
+}
+
 // GetSecretFromASM makes the api call to the AWS Secrets Manager service to
 // retrieve the secret value
 func GetSecretFromASM(secretID string, client secretsmanageriface.SecretsManagerAPI) (string, error) {

agent/asm/asm_test.go

@@ -22,6 +22,17 @@ import (
 	"github.com/aws/aws-sdk-go/service/secretsmanager"
 	"github.com/aws/aws-sdk-go/service/secretsmanager/secretsmanageriface"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	versionID                = "versionId"
+	versionStage             = "versionStage"
+	jsonKey                  = "jsonKey"
+	valueFrom                = "arn:aws:secretsmanager:region:account-id:secret:secretId"
+	secretValue              = "secretValue"
+	jsonSecretValue          = "{\"" + jsonKey + "\": \"" + secretValue + "\",\"some-other-key\": \"secret2\"}"
+	malformedJsonSecretValue = "{\"" + jsonKey + "\": \"" + secretValue
 )
 
 type mockGetSecretValue struct {
@@ -111,11 +122,69 @@ func TestASMGetAuthConfig(t *testing.T) {
 }
 
 func TestGetSecretFromASM(t *testing.T) {
-	asmClient := mockGetSecretValue{
+	asmClient := createASMInterface(secretValue)
+	_, err := GetSecretFromASM("secretName", asmClient)
+	assert.NoError(t, err)
+}
+
+func TestGetSecretFromASMWithJsonKey(t *testing.T) {
+	asmClient := createASMInterface(jsonSecretValue)
+	secretValueInput := createSecretValueInput(toPtr(valueFrom), nil, nil)
+	outSecretValue, _ := GetSecretFromASMWithInput(secretValueInput, asmClient, jsonKey)
+	assert.Equal(t, secretValue, outSecretValue)
+}
+
+func TestGetSecretFromASMWithMalformedJSON(t *testing.T) {
+	asmClient := createASMInterface(malformedJsonSecretValue)
+	secretValueInput := createSecretValueInput(toPtr(valueFrom), nil, nil)
+	outSecretValue, err := GetSecretFromASMWithInput(secretValueInput, asmClient, jsonKey)
+	require.Error(t, err)
+	assert.Equal(t, "", outSecretValue)
+}
+
+func TestGetSecretFromASMWithJSONKeyNotFound(t *testing.T) {
+	asmClient := createASMInterface(jsonSecretValue)
+	secretValueInput := createSecretValueInput(toPtr(valueFrom), nil, nil)
+	nonExistentKey := "nonExistentKey"
+	_, err := GetSecretFromASMWithInput(secretValueInput, asmClient, nonExistentKey)
+	assert.Error(t, err)
+}
+
+func TestGetSecretFromASMWithVersionID(t *testing.T) {
+	asmClient := createASMInterface(secretValue)
+	secretValueInput := createSecretValueInput(toPtr(valueFrom), toPtr(versionID), nil)
+	outSecretValue, err := GetSecretFromASMWithInput(secretValueInput, asmClient, "")
+	require.NoError(t, err)
+	assert.Equal(t, secretValue, outSecretValue)
+}
+
+func TestGetSecretFromASMWithVersionIDAndStage(t *testing.T) {
+	asmClient := createASMInterface(secretValue)
+	secretValueInput := createSecretValueInput(toPtr(valueFrom), toPtr(versionID), toPtr(versionStage))
+	outSecretValue, err := GetSecretFromASMWithInput(secretValueInput, asmClient, "")
+	require.NoError(t, err)
+	assert.Equal(t, secretValue, outSecretValue)
+}
+
+func toPtr(input string) *string {
+	if input == "" {
+		return nil
+	}
+	return &input
+}
+
+func createSecretValueInput(secretID *string, versionID *string, versionStage *string) *secretsmanager.GetSecretValueInput {
+	return &secretsmanager.GetSecretValueInput{
+		SecretId:     secretID,
+		VersionId:    versionID,
+		VersionStage: versionStage,
+	}
+}
+
+func createASMInterface(secretValue string) mockGetSecretValue {
+	return mockGetSecretValue{
 		Resp: secretsmanager.GetSecretValueOutput{
-			SecretString: aws.String("secretValue"),
+			SecretString: aws.String(secretValue),
 		},
 	}
-	_, err := GetSecretFromASM("secretName", asmClient)
-	assert.NoError(t, err)
 }

agent/engine/docker_task_engine_test.go

@@ -2503,7 +2503,7 @@ func TestTaskSecretsEnvironmentVariables(t *testing.T) {
 
 	// metadata required for asm secret resource validation
 	asmSecretName := "myASMSecret"
-	asmSecretValueFrom := "asm/mySecret"
+	asmSecretValueFrom := "arn:aws:secretsmanager:region:account-id:secret:" + asmSecretName
 	asmSecretRetrievedValue := "myASMSecretValue"
 	asmSecretRegion := "us-west-2"
 	asmSecretKey := asmSecretValueFrom + "_" + asmSecretRegion
@@ -2700,7 +2700,7 @@ func TestTaskSecretsEnvironmentVariables(t *testing.T) {
 			}).Return(ssmClientOutput, nil).Times(1)
 
 			mockASMClient.EXPECT().GetSecretValue(gomock.Any()).Do(func(in *secretsmanager.GetSecretValueInput) {
-				assert.Equal(t, aws.StringValue(in.SecretId), asmSecretValueFrom)
+				assert.Equal(t, asmSecretValueFrom, aws.StringValue(in.SecretId))
 			}).Return(asmClientOutput, nil).Times(1)
 
 			require.NoError(t, ssmSecretRes.Create())

agent/engine/engine_unix_integ_test.go

@@ -998,7 +998,7 @@ func TestDockerCfgAuth(t *testing.T) {
 			return nil
 		}
 		data, err := ioutil.ReadFile(path)
-		t.Log("Reading file:%s", path)
+		t.Logf("Reading file:%s", path)
 		if err != nil {
 			return err
 		}

agent/taskresource/asmsecret/asmsecret.go

@@ -30,11 +30,17 @@ import (
 	"github.com/aws/amazon-ecs-agent/agent/credentials"
 	"github.com/aws/amazon-ecs-agent/agent/taskresource"
 	resourcestatus "github.com/aws/amazon-ecs-agent/agent/taskresource/status"
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/arn"
+	"github.com/aws/aws-sdk-go/service/secretsmanager"
 )
 
 const (
 	// ResourceName is the name of the asmsecret resource
-	ResourceName = "asmsecret"
+	ResourceName                       = "asmsecret"
+	arnDelimiter                       = ":"
+	asmARNResourceFormat               = "secret:{secretID}"
+	asmARNResourceWithParametersFormat = "secret:secretID:jsonKey:versionStage:versionID"
 )
 
 // ASMSecretResource represents secrets as a task resource.
@@ -294,8 +300,19 @@ func (secret *ASMSecretResource) retrieveASMSecretValue(apiSecret apicontainer.S
 
 	asmClient := secret.asmClientCreator.NewASMClient(apiSecret.Region, iamCredentials)
 	seelog.Infof("ASM secret resource: retrieving resource for secret %v in region %s for task: [%s]", apiSecret.ValueFrom, apiSecret.Region, secret.taskARN)
-	//for asm secret, ValueFrom can be arn or name
-	secretValue, err := asm.GetSecretFromASM(apiSecret.ValueFrom, asmClient)
+	input, jsonKey, err := getASMParametersFromInput(apiSecret.ValueFrom)
+	if err != nil {
+		errorEvents <- fmt.Errorf("trying to retrieve secret with value %s resulted in error: %v", apiSecret.ValueFrom, err)
+		return
+	}
+
+	if input.SecretId == nil {
+		errorEvents <- fmt.Errorf("could not find a secretsmanager secretID from value %s", apiSecret.ValueFrom)
+		return
+
+	}
+
+	secretValue, err := asm.GetSecretFromASMWithInput(input, asmClient, jsonKey)
 	if err != nil {
 		errorEvents <- fmt.Errorf("fetching secret data from AWS Secrets Manager in region %s: %v", apiSecret.Region, err)
 		return
@@ -309,6 +326,65 @@ func (secret *ASMSecretResource) retrieveASMSecretValue(apiSecret apicontainer.S
 	secret.secretData[secretKey] = secretValue
 }
 
+func pointerOrNil(in string) *string {
+	if in == "" {
+		return nil
+	}
+
+	return aws.String(in)
+}
+
+// Agent follows what Cloudformation does here with using Dynamic References to specify Template Values
+// in the format secret-id:json-key:version-stage:version-id
+// the input will always be a full ARN for ASM
+func getASMParametersFromInput(valueFrom string) (input *secretsmanager.GetSecretValueInput, jsonKey string, err error) {
+	arnObj, err := arn.Parse(valueFrom)
+	if err != nil {
+		seelog.Warnf("Unable to parse ARN %s when trying to retrieve ASM secret", valueFrom)
+		return nil, "", err
+	}
+
+	input = &secretsmanager.GetSecretValueInput{}
+
+	paramValues := strings.Split(arnObj.Resource, arnDelimiter) // arnObj.Resource looks like secret:secretID:...
+	if len(paramValues) == len(strings.Split(asmARNResourceFormat, arnDelimiter)) {
+		input.SecretId = &valueFrom
+		return input, "", nil
+	}
+	if len(paramValues) != len(strings.Split(asmARNResourceWithParametersFormat, arnDelimiter)) {
+		// can't tell what input this is, throw some error
+		err = errors.New("unexpected ARN format with parameters when trying to retrieve ASM secret")
+		return nil, "", err
+	}
+
+	input.SecretId = pointerOrNil(reconstructASMARN(arnObj))
+	jsonKey = paramValues[2]
+	input.VersionStage = pointerOrNil(paramValues[3])
+	input.VersionId = pointerOrNil(paramValues[4])
+
+	return input, jsonKey, nil
+}
+
+// this method is to reconstruct an ASM ARN that has the enhancement parameters
+// attached to it. in order to call secretsmanager:GetSecretValue, the entire ARN
+// (including the 6 character special identifier tacked on by ASM) is required or
+// just the secret name itself is required.
+func reconstructASMARN(arnARN arn.ARN) string {
+	// arn resource should look like secret:secretID:jsonKey:versionStage:versionID
+	secretIDAndParams := strings.Split(arnARN.Resource, arnDelimiter)
+	// reconstruct the secret id without the parameters
+	secretID := fmt.Sprintf("%s%s%s", secretIDAndParams[0], arnDelimiter, secretIDAndParams[1])
+	secretIDARN := arn.ARN{
+		Partition: arnARN.Partition,
+		Service:   arnARN.Service,
+		Region:    arnARN.Region,
+		AccountID: arnARN.AccountID,
+		Resource:  secretID,
+	}.String()
+
+	return secretIDARN
+}
+
 // getRequiredSecrets returns the requiredSecrets field of asmsecret task resource
 func (secret *ASMSecretResource) getRequiredSecrets() map[string]apicontainer.Secret {
 	secret.lock.RLock()