Skip to content

Commit

Permalink
Merge pull request #718 from PettitWesley/tag-regcreds
Browse files Browse the repository at this point in the history 
Resource tagging in the registry-creds up command
  • Loading branch information
@PettitWesley
PettitWesley authored Mar 1, 2019
2 parents 5407919 + 6aec326 commit 0f138cc
Show file tree
Hide file tree
Showing 7 changed files with 235 additions and 14 deletions.
19 changes: 16 additions & 3 deletions ecs-cli/modules/cli/regcreds/create_task_execution_role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,15 @@ type executionRoleParams struct {
CredEntries map[string]regcredio.CredsOutputEntry
RoleName string
Region string
Tags map[string]*string
}

// returns the time of IAM policy creation so that other resources (i.e., output file) can be dated to match
func createTaskExecutionRole(params executionRoleParams, iamClient iamClient.Client, kmsClient kmsClient.Client) (*time.Time, error) {
log.Infof("Creating resources for task execution role %s...", params.RoleName)

// create role
roleName, err := createOrFindRole(params.RoleName, iamClient)
roleName, err := createOrFindRole(params.RoleName, iamClient, convertToIAMTags(params.Tags))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -88,8 +89,8 @@ func createRegistryCredentialsPolicy(roleName, policyDoc string, createTime time
return policyResult.Policy, nil
}

func createOrFindRole(roleName string, client iamClient.Client) (string, error) {
roleResult, err := client.CreateOrFindRole(roleName, roleDescriptionString, assumeRolePolicyDocString)
func createOrFindRole(roleName string, client iamClient.Client, tags []*iam.Tag) (string, error) {
roleResult, err := client.CreateOrFindRole(roleName, roleDescriptionString, assumeRolePolicyDocString, tags)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -119,3 +120,15 @@ func attachRolePolicies(secretPolicyARN, roleName, region string, client iamClie

return nil
}

func convertToIAMTags(tags map[string]*string) []*iam.Tag {
var iamTags []*iam.Tag
for key, value := range tags {
iamTags = append(iamTags, &iam.Tag{
Key: aws.String(key),
Value: value,
})
}

return iamTags
}
76 changes: 71 additions & 5 deletions ecs-cli/modules/cli/regcreds/create_task_execution_role_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestCreateTaskExecutionRole(t *testing.T) {

mocks := setupTestController(t)
gomock.InOrder(
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString).Return(*testRoleArn, nil),
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString, nil).Return(*testRoleArn, nil),
mocks.MockIAM.EXPECT().CreateRole(gomock.Any()).Return(&iam.CreateRoleOutput{Role: &iam.Role{Arn: testRoleArn}}, nil),
)
gomock.InOrder(
Expand Down Expand Up @@ -76,7 +76,7 @@ func TestCreateTaskExecutionRole_NoKMSKey(t *testing.T) {

mocks := setupTestController(t)
gomock.InOrder(
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString).Return(*testRoleArn, nil),
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString, nil).Return(*testRoleArn, nil),
mocks.MockIAM.EXPECT().CreateRole(gomock.Any()).Return(&iam.CreateRoleOutput{Role: &iam.Role{Arn: testRoleArn}}, nil),
)
gomock.InOrder(
Expand Down Expand Up @@ -110,7 +110,7 @@ func TestCreateTaskExecutionRole_RoleExists(t *testing.T) {
mocks := setupTestController(t)
gomock.InOrder(
// CreateOrFindRole should return nil if given role already exists
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString).Return("", nil),
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString, nil).Return("", nil),
mocks.MockIAM.EXPECT().CreateRole(gomock.Any()).Return(nil, roleExistsError),
)
gomock.InOrder(
Expand Down Expand Up @@ -140,7 +140,7 @@ func TestCreateTaskExecutionRole_ErrorOnCreateRoleFails(t *testing.T) {

mocks := setupTestController(t)
gomock.InOrder(
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString).Return("", errors.New("something went wrong")),
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString, nil).Return("", errors.New("something went wrong")),
mocks.MockIAM.EXPECT().CreateRole(gomock.Any()).Return(nil, errors.New("something went wrong")),
)

Expand All @@ -166,7 +166,7 @@ func TestCreateTaskExecutionRole_ErrorOnCreatePolicyFails(t *testing.T) {

mocks := setupTestController(t)
gomock.InOrder(
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString).Return(*testRoleArn, nil),
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString, nil).Return(*testRoleArn, nil),
mocks.MockIAM.EXPECT().CreateRole(gomock.Any()).Return(&iam.CreateRoleOutput{Role: &iam.Role{Arn: testRoleArn}}, nil),
)
gomock.InOrder(
Expand All @@ -182,3 +182,69 @@ func TestCreateTaskExecutionRole_ErrorOnCreatePolicyFails(t *testing.T) {
_, err := createTaskExecutionRole(testParams, mocks.MockIAM, mocks.MockKMS)
assert.Error(t, err, "Expected error when CreatePolicy fails")
}

func TestCreateTaskExecutionRoleWithTags(t *testing.T) {
testRegistry := "myreg.test.io"
testRegCredARN := "arn:aws:secret/some-test-arn"
testRegKMSKey := "arn:aws:kms:key/67yt-756yth"

testCreds := map[string]regcredio.CredsOutputEntry{
testRegistry: regcredio.BuildOutputEntry(testRegCredARN, testRegKMSKey, []string{"test"}),
}

testRoleName := "myNginxProjectRole"

testPolicyArn := aws.String("arn:aws:iam::policy/" + testRoleName + "-policy")
testRoleArn := aws.String("arn:aws:iam::role/" + testRoleName)

testParams := executionRoleParams{
CredEntries: testCreds,
RoleName: testRoleName,
Region: "us-west-2",
Tags: map[string]*string{
"Hey": aws.String("Jude"),
"Come": aws.String("Together"),
"Hello": aws.String("Goodbye"),
"Abbey": aws.String("Road"),
},
}

expectedTags := []*iam.Tag{
&iam.Tag{
Key: aws.String("Hey"),
Value: aws.String("Jude"),
},
&iam.Tag{
Key: aws.String("Come"),
Value: aws.String("Together"),
},
&iam.Tag{
Key: aws.String("Hello"),
Value: aws.String("Goodbye"),
},
&iam.Tag{
Key: aws.String("Abbey"),
Value: aws.String("Road"),
},
}

mocks := setupTestController(t)
gomock.InOrder(
mocks.MockIAM.EXPECT().CreateOrFindRole(testRoleName, roleDescriptionString, assumeRolePolicyDocString, gomock.Any()).Do(func(w, x, y, z interface{}) {
tags := z.([]*iam.Tag)
assert.ElementsMatch(t, tags, expectedTags, "Expected Tags to match")
}).Return(*testRoleArn, nil),
mocks.MockIAM.EXPECT().CreateRole(gomock.Any()).Return(&iam.CreateRoleOutput{Role: &iam.Role{Arn: testRoleArn}}, nil),
)
gomock.InOrder(
// If KMSKeyID present, first thing to happen should be verifying its ARN
mocks.MockKMS.EXPECT().GetValidKeyARN(testRegKMSKey).Return(testRegKMSKey, nil),
mocks.MockIAM.EXPECT().CreatePolicy(gomock.Any()).Return(&iam.CreatePolicyOutput{Policy: &iam.Policy{Arn: testPolicyArn}}, nil),
mocks.MockIAM.EXPECT().AttachRolePolicy(getExecutionRolePolicyARN("us-west-2"), testRoleName).Return(nil, nil),
mocks.MockIAM.EXPECT().AttachRolePolicy(*testPolicyArn, testRoleName).Return(nil, nil),
)

policyCreateTime, err := createTaskExecutionRole(testParams, mocks.MockIAM, mocks.MockKMS)
assert.NoError(t, err, "Unexpected error when creating task execution role")
assert.NotNil(t, policyCreateTime, "Expected policy create time to be non-nil")
}
43 changes: 43 additions & 0 deletions ecs-cli/modules/cli/regcreds/regcreds_app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@ import (
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/clients/aws/iam"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/clients/aws/kms"
secretsClient "github.com/aws/amazon-ecs-cli/ecs-cli/modules/clients/aws/secretsmanager"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/clients/aws/tagging"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/commands/flags"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/config"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/utils"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/utils/regcredio"
"github.com/aws/aws-sdk-go/aws"
taggingSDK "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi"
"github.com/aws/aws-sdk-go/service/secretsmanager"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -85,6 +88,14 @@ func Up(c *cli.Context) {
log.Fatal("Error executing 'up': ", err)
}

var tags map[string]*string
if tagVal := c.String(flags.ResourceTagsFlag); tagVal != "" {
tags, err = utils.GetTagsMap(tagVal)
if err != nil {
log.Fatal("Error executing 'up': ", err)
}
}

var policyCreateTime *time.Time
if !skipRole {
region := commandConfig.Session.Config.Region
Expand All @@ -93,6 +104,7 @@ func Up(c *cli.Context) {
CredEntries: credentialOutput,
RoleName: roleName,
Region: *region,
Tags: tags,
}

policyCreateTime, err = createTaskExecutionRole(roleParams, iamClient, kmsClient)
Expand All @@ -103,6 +115,14 @@ func Up(c *cli.Context) {
log.Info("Skipping role creation.")
}

if len(tags) > 0 {
taggingClient := tagging.NewTaggingClient(commandConfig)
err = tagRegistryCredentials(credentialOutput, tags, taggingClient)
if err != nil {
log.Fatal("Failed to tag resources: ", err)
}
}

// produce output file
if !skipOutput {
regcredio.GenerateCredsOutput(credentialOutput, roleName, outputDir, policyCreateTime)
Expand Down Expand Up @@ -306,3 +326,26 @@ func validateOutputOptions(outputDir string, skipOutput bool) error {
}
return nil
}

func tagRegistryCredentials(creds map[string]regcredio.CredsOutputEntry, tags map[string]*string, taggingClient tagging.Client) error {
var arns []*string

for _, credInfo := range creds {
arns = append(arns, aws.String(credInfo.CredentialARN))
}

input := &taggingSDK.TagResourcesInput{
ResourceARNList: arns,
Tags: tags,
}
output, err := taggingClient.TagResources(input)
if err != nil {
return err
}

for resource, info := range output.FailedResourcesMap {
return fmt.Errorf("Failed to tag resource %s; error=%s", resource, *info.ErrorMessage)
}

return nil
}
92 changes: 92 additions & 0 deletions ecs-cli/modules/cli/regcreds/regcreds_app_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import (
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/clients/aws/iam/mock"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/clients/aws/kms/mock"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/clients/aws/secretsmanager/mock"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/clients/aws/tagging/mock"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/utils/regcredio"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/kms"
taggingSDK "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi"
secretsmanager "github.com/aws/aws-sdk-go/service/secretsmanager"
"github.com/golang/mock/gomock"
"github.com/pkg/errors"
Expand Down Expand Up @@ -216,6 +218,96 @@ func TestGetOrCreateRegistryCredentials_ErrorOnUpdate(t *testing.T) {
assert.Error(t, err)
}

func TestTagRegistryCredentials(t *testing.T) {
creds := map[string]regcredio.CredsOutputEntry{
"the-who-registry.com": regcredio.CredsOutputEntry{
CredentialARN: "arn:aws:secretsmanager:eu-west-1:111111111111:secret:path/whoareyou-1978",
},
}

tags := map[string]*string{
"Baba": aws.String("O'riley"),
"Eminence": aws.String("Front"),
"My": aws.String("Generation"),
}

ctrl := gomock.NewController(t)

mockTagging := mock_tagging.NewMockClient(ctrl)

gomock.InOrder(
mockTagging.EXPECT().TagResources(gomock.Any()).Do(func(x interface{}) {
input := x.(*taggingSDK.TagResourcesInput)
assert.Equal(t, tags, input.Tags, "Expected tags to match")
}).Return(&taggingSDK.TagResourcesOutput{}, nil),
)

err := tagRegistryCredentials(creds, tags, mockTagging)
assert.NoError(t, err, "Unexpected error calling tagRegistryCredentials")
}

func TestTagRegistryCredentialsError(t *testing.T) {
creds := map[string]regcredio.CredsOutputEntry{
"the-who-registry.com": regcredio.CredsOutputEntry{
CredentialARN: "arn:aws:secretsmanager:eu-west-1:111111111111:secret:path/whoareyou-1978",
},
}

tags := map[string]*string{
"Baba": aws.String("O'riley"),
"Eminence": aws.String("Front"),
"My": aws.String("Generation"),
}

ctrl := gomock.NewController(t)

mockTagging := mock_tagging.NewMockClient(ctrl)

gomock.InOrder(
mockTagging.EXPECT().TagResources(gomock.Any()).Do(func(x interface{}) {
input := x.(*taggingSDK.TagResourcesInput)
assert.Equal(t, tags, input.Tags, "Expected tags to match")
}).Return(nil, fmt.Errorf("Some API error")),
)

err := tagRegistryCredentials(creds, tags, mockTagging)
assert.Error(t, err, "Expected error calling tagRegistryCredentials")
}

func TestTagRegistryCredentialsFailedResources(t *testing.T) {
creds := map[string]regcredio.CredsOutputEntry{
"the-who-registry.com": regcredio.CredsOutputEntry{
CredentialARN: "arn:aws:secretsmanager:eu-west-1:111111111111:secret:path/whoareyou-1978",
},
}

tags := map[string]*string{
"Baba": aws.String("O'riley"),
"Eminence": aws.String("Front"),
"My": aws.String("Generation"),
}

ctrl := gomock.NewController(t)

mockTagging := mock_tagging.NewMockClient(ctrl)

gomock.InOrder(
mockTagging.EXPECT().TagResources(gomock.Any()).Do(func(x interface{}) {
input := x.(*taggingSDK.TagResourcesInput)
assert.Equal(t, tags, input.Tags, "Expected tags to match")
}).Return(&taggingSDK.TagResourcesOutput{
FailedResourcesMap: map[string]*taggingSDK.FailureInfo{
"arn:aws:secretsmanager:eu-west-1:111111111111:secret:path/whoareyou-1978": &taggingSDK.FailureInfo{
ErrorMessage: aws.String("Auth Error: who are you"),
},
},
}, nil),
)

err := tagRegistryCredentials(creds, tags, mockTagging)
assert.Error(t, err, "Expected error calling tagRegistryCredentials")
}

func TestValidateCredsInput_ErrorEmptyCreds(t *testing.T) {
emptyCredMap := make(map[string]regcredio.RegistryCredEntry)
emptyCredsInput := regcredio.ECSRegCredsInput{
Expand Down
7 changes: 5 additions & 2 deletions ecs-cli/modules/clients/aws/iam/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type Client interface {
AttachRolePolicy(policyArn, roleName string) (*iam.AttachRolePolicyOutput, error)
CreateRole(iam.CreateRoleInput) (*iam.CreateRoleOutput, error)
CreatePolicy(iam.CreatePolicyInput) (*iam.CreatePolicyOutput, error)
CreateOrFindRole(string, string, string) (string, error)
CreateOrFindRole(string, string, string, []*iam.Tag) (string, error)
}

type iamClient struct {
Expand Down Expand Up @@ -81,12 +81,15 @@ func (c *iamClient) CreatePolicy(input iam.CreatePolicyInput) (*iam.CreatePolicy
}

// CreateOrFindRole returns a new role ARN or an empty string if role already exists
func (c *iamClient) CreateOrFindRole(roleName, roleDescription, assumeRolePolicyDoc string) (string, error) {
func (c *iamClient) CreateOrFindRole(roleName, roleDescription, assumeRolePolicyDoc string, tags []*iam.Tag) (string, error) {
createRoleRequest := iam.CreateRoleInput{
AssumeRolePolicyDocument: aws.String(assumeRolePolicyDoc),
Description: aws.String(roleDescription),
RoleName: aws.String(roleName),
}
if len(tags) > 0 {
createRoleRequest.Tags = tags
}
roleResult, err := c.CreateRole(createRoleRequest)
// if err is b/c role already exists, OK to continue
if err != nil && !utils.EntityAlreadyExists(err) {
Expand Down
8 changes: 4 additions & 4 deletions ecs-cli/modules/clients/aws/iam/mock/client.go
View file

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

Loading

0 comments on commit 0f138cc

Please sign in to comment.