From 1106250d09944ec24838afc6a17af2081813b30c Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Fri, 31 Jul 2020 18:45:12 +0000 Subject: [PATCH 1/9] Ignore CFN managed buckets by checking for existence of CFN tags CloudFormation managed resources are automatically tagged with: aws:cloudformation:stack-name Ignore policies attached to buckets containing this tag to stop iamy competing with CloudFormation to manage bucket policies. --- iamy/aws.go | 23 ++++++++++++++++------- iamy/aws_test.go | 30 ++++++++++++++++++++++++++++-- iamy/s3.go | 32 ++++++++++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/iamy/aws.go b/iamy/aws.go index 3e0c6a9..5b7badb 100644 --- a/iamy/aws.go +++ b/iamy/aws.go @@ -99,7 +99,7 @@ func (a *AwsFetcher) fetchS3Data() error { if b.policyJson == "" { continue } - if ok, err := a.isSkippableManagedResource(CfnS3Bucket, b.name); ok { + if ok, err := a.isSkippableManagedResource(CfnS3Bucket, b.name, b.tags); ok { log.Printf(err) continue } @@ -209,7 +209,7 @@ func (a *AwsFetcher) marshalRoleDescriptionAsync(roleName string, target *string func (a *AwsFetcher) populateInstanceProfileData(resp *iam.ListInstanceProfilesOutput) error { for _, profileResp := range resp.InstanceProfiles { - if ok, err := a.isSkippableManagedResource(CfnInstanceProfile, *profileResp.InstanceProfileName); ok { + if ok, err := a.isSkippableManagedResource(CfnInstanceProfile, *profileResp.InstanceProfileName, map[string]string{}); ok { log.Printf(err) continue } @@ -229,7 +229,7 @@ func (a *AwsFetcher) populateInstanceProfileData(resp *iam.ListInstanceProfilesO func (a *AwsFetcher) populateIamData(resp *iam.GetAccountAuthorizationDetailsOutput) error { for _, userResp := range resp.UserDetailList { - if ok, err := a.isSkippableManagedResource(CfnIamUser, *userResp.UserName); ok { + if ok, err := a.isSkippableManagedResource(CfnIamUser, *userResp.UserName, map[string]string{}); ok { log.Printf(err) continue } @@ -259,7 +259,7 @@ func (a *AwsFetcher) populateIamData(resp *iam.GetAccountAuthorizationDetailsOut } for _, groupResp := range resp.GroupDetailList { - if ok, err := a.isSkippableManagedResource(CfnIamGroup, *groupResp.GroupName); ok { + if ok, err := a.isSkippableManagedResource(CfnIamGroup, *groupResp.GroupName, map[string]string{}); ok { log.Printf(err) continue } @@ -280,7 +280,7 @@ func (a *AwsFetcher) populateIamData(resp *iam.GetAccountAuthorizationDetailsOut } for _, roleResp := range resp.RoleDetailList { - if ok, err := a.isSkippableManagedResource(CfnIamRole, *roleResp.RoleName); ok { + if ok, err := a.isSkippableManagedResource(CfnIamRole, *roleResp.RoleName, map[string]string{}); ok { log.Printf(err) continue } @@ -310,7 +310,7 @@ func (a *AwsFetcher) populateIamData(resp *iam.GetAccountAuthorizationDetailsOut } for _, policyResp := range resp.Policies { - if ok, err := a.isSkippableManagedResource(CfnIamPolicy, *policyResp.PolicyName); ok { + if ok, err := a.isSkippableManagedResource(CfnIamPolicy, *policyResp.PolicyName, map[string]string{}); ok { log.Printf(err) continue } @@ -396,6 +396,10 @@ func (a *AwsFetcher) getAccount() (*Account, error) { return &acct, nil } +// CFN automatically tags resources with this and other tags: +// https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-resource-tags.html +const cloudformationStackNameTag = "aws:cloudformation:stack-name" + // isSkippableResource takes the resource identifier as a string and // checks it against known resources that we shouldn't need to manage as // it will already be managed by another process (such as Cloudformation @@ -403,7 +407,12 @@ func (a *AwsFetcher) getAccount() (*Account, error) { // // Returns a boolean of whether it can be skipped and a string of the // reasoning why it was skipped. -func (a *AwsFetcher) isSkippableManagedResource(cfnType CfnResourceType, resourceIdentifier string) (bool, string) { + +func (a *AwsFetcher) isSkippableManagedResource(cfnType CfnResourceType, resourceIdentifier string, tags map[string]string) (bool, string) { + if stackName, ok := tags[cloudformationStackNameTag]; ok { + return true, fmt.Sprintf("CloudFormation generated resource %s in stack %s", resourceIdentifier, stackName) + } + if a.cfn.IsManagedResource(cfnType, resourceIdentifier) { return true, fmt.Sprintf("CloudFormation generated resource %s", resourceIdentifier) } diff --git a/iamy/aws_test.go b/iamy/aws_test.go index d103162..a0c701a 100644 --- a/iamy/aws_test.go +++ b/iamy/aws_test.go @@ -22,7 +22,7 @@ func TestIsSkippableManagedResource(t *testing.T) { for _, name := range skippables { t.Run(name, func(t *testing.T) { - skipped, err := f.isSkippableManagedResource(CfnIamRole, name) + skipped, err := f.isSkippableManagedResource(CfnIamRole, name, map[string]string{}) if skipped == false { t.Errorf("expected %s to be skipped but got false", name) } @@ -36,7 +36,7 @@ func TestIsSkippableManagedResource(t *testing.T) { for _, name := range nonSkippables { t.Run(name, func(t *testing.T) { - skipped, err := f.isSkippableManagedResource(CfnIamRole, name) + skipped, err := f.isSkippableManagedResource(CfnIamRole, name, map[string]string{}) if skipped == true { t.Errorf("expected %s to not be skipped but got true", name) } @@ -47,3 +47,29 @@ func TestIsSkippableManagedResource(t *testing.T) { }) } } + +func TestSkippableTaggedResources(t *testing.T) { + f := AwsFetcher{cfn: &cfnClient{}} + skippableTags := map[string]string{"aws:cloudformation:stack-name": "my-stack"} + + skipped, err := f.isSkippableManagedResource(CfnS3Bucket, "my-bucket", skippableTags) + if err == "" { + t.Errorf("expected an error message but it was empty") + } + if skipped == false { + t.Errorf("expected resource to be skipped but got false") + } +} + +func TestNonSkippableTaggedResources(t *testing.T) { + f := AwsFetcher{cfn: &cfnClient{}} + nonSkippableTags := map[string]string{"Name": "blah"} + + skipped, err := f.isSkippableManagedResource(CfnS3Bucket, "my-bucket", nonSkippableTags) + if err != "" { + t.Errorf("expected no error message but got: %s", err) + } + if skipped == true { + t.Errorf("expected resource to not be skipped but got true") + } +} diff --git a/iamy/s3.go b/iamy/s3.go index 5d846d6..b7c026d 100644 --- a/iamy/s3.go +++ b/iamy/s3.go @@ -13,6 +13,7 @@ import ( ) const NoSuchBucketPolicyErrCode = "NoSuchBucketPolicy" +const NoSuchTagSetErrCode = "NoSuchTagSet" func newRegionClientMap(s *session.Session) *regionClientMap { return ®ionClientMap{ @@ -62,6 +63,7 @@ type bucket struct { name string policyJson string exists bool + tags map[string]string } func (c *s3Client) withRegion(region string) s3iface.S3API { @@ -86,6 +88,13 @@ func (c *s3Client) populateBucket(b *bucket) error { } region := s3.NormalizeBucketLocation(normaliseString(r.LocationConstraint)) + + tags, err := c.fetchTags(b.name, region) + if err != nil { + return err + } + b.tags = tags + b.policyJson, err = c.GetBucketPolicyDoc(b.name, region) return err @@ -103,6 +112,7 @@ func (c *s3Client) listAllBuckets() ([]*bucket, error) { for _, rb := range bucketListResp.Buckets { b := bucket{name: *rb.Name} + b.exists = true buckets = append(buckets, &b) wg.Add(1) @@ -115,8 +125,6 @@ func (c *s3Client) listAllBuckets() ([]*bucket, error) { oneOfTheErrorsDuringPopulation = errors.New(fmt.Sprintf("Error while getting details for S3 bucket %s: %s", b.name, err)) } } - } else { - b.exists = true } }() } @@ -153,3 +161,23 @@ func (c *s3Client) GetBucketPolicyDoc(name, region string) (string, error) { return *resp.Policy, nil } + +func (c *s3Client) fetchTags(name, region string) (map[string]string, error) { + tags := make(map[string]string) + clientForRegion := c.withRegion(region) + tagsResponse, err := clientForRegion.GetBucketTagging(&s3.GetBucketTaggingInput{Bucket: aws.String(name)}) + if err != nil { + if awsErr, ok := err.(awserr.Error); ok { + if awsErr.Code() == NoSuchTagSetErrCode { + return tags, nil + } + } + return tags, err + } + for _, tag := range tagsResponse.TagSet { + if tag != nil { + tags[*tag.Key] = *tag.Value + } + } + return tags, nil +} From 909988b265776a940102992189adf4dd15c854aa Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Sat, 1 Aug 2020 09:56:12 +0400 Subject: [PATCH 2/9] Skip IAM users & roles tagged by CFN --- iamy/aws.go | 19 +++++++++++++------ iamy/aws_test.go | 42 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/iamy/aws.go b/iamy/aws.go index 5b7badb..f283f91 100644 --- a/iamy/aws.go +++ b/iamy/aws.go @@ -229,7 +229,12 @@ func (a *AwsFetcher) populateInstanceProfileData(resp *iam.ListInstanceProfilesO func (a *AwsFetcher) populateIamData(resp *iam.GetAccountAuthorizationDetailsOutput) error { for _, userResp := range resp.UserDetailList { - if ok, err := a.isSkippableManagedResource(CfnIamUser, *userResp.UserName, map[string]string{}); ok { + tags := make(map[string]string) + for _, tag := range userResp.Tags { + tags[*tag.Key] = *tag.Value + } + + if ok, err := a.isSkippableManagedResource(CfnIamUser, *userResp.UserName, tags); ok { log.Printf(err) continue } @@ -239,7 +244,6 @@ func (a *AwsFetcher) populateIamData(resp *iam.GetAccountAuthorizationDetailsOut Name: *userResp.UserName, Path: *userResp.Path, }, - Tags: make(map[string]string), } for _, g := range userResp.GroupList { @@ -251,9 +255,7 @@ func (a *AwsFetcher) populateIamData(resp *iam.GetAccountAuthorizationDetailsOut if err := a.populateInlinePolicies(userResp.UserPolicyList, &user.InlinePolicies); err != nil { return err } - for _, t := range userResp.Tags { - user.Tags[*t.Key] = *t.Value - } + user.Tags = tags a.data.Users = append(a.data.Users, &user) } @@ -280,7 +282,12 @@ func (a *AwsFetcher) populateIamData(resp *iam.GetAccountAuthorizationDetailsOut } for _, roleResp := range resp.RoleDetailList { - if ok, err := a.isSkippableManagedResource(CfnIamRole, *roleResp.RoleName, map[string]string{}); ok { + tags := make(map[string]string) + for _, tag := range roleResp.Tags { + tags[*tag.Key] = *tag.Value + } + + if ok, err := a.isSkippableManagedResource(CfnIamRole, *roleResp.RoleName, tags); ok { log.Printf(err) continue } diff --git a/iamy/aws_test.go b/iamy/aws_test.go index a0c701a..c1bfdf9 100644 --- a/iamy/aws_test.go +++ b/iamy/aws_test.go @@ -2,6 +2,8 @@ package iamy import ( "testing" + + "github.com/aws/aws-sdk-go/service/iam" ) func TestIsSkippableManagedResource(t *testing.T) { @@ -48,7 +50,7 @@ func TestIsSkippableManagedResource(t *testing.T) { } } -func TestSkippableTaggedResources(t *testing.T) { +func TestSkippableS3TaggedResources(t *testing.T) { f := AwsFetcher{cfn: &cfnClient{}} skippableTags := map[string]string{"aws:cloudformation:stack-name": "my-stack"} @@ -73,3 +75,41 @@ func TestNonSkippableTaggedResources(t *testing.T) { t.Errorf("expected resource to not be skipped but got true") } } + +func TestSkippableIAMUserResource(t *testing.T) { + f := AwsFetcher{cfn: &cfnClient{}} + key := "aws:cloudformation:stack-name" + val := "my-stack" + userName := "my-user" + path := "/" + userList := []*iam.UserDetail{ + {Tags: []*iam.Tag{{Key: &key, Value: &val}}, UserName: &userName, Path: &path}, + } + + resp := iam.GetAccountAuthorizationDetailsOutput{UserDetailList: userList} + f.populateIamData(&resp) + for _, user := range f.data.Users { + if user.Name == userName { + t.Error("Expected to skip user with CFN tags") + } + } +} + +func TestSkippableIAMRoleResource(t *testing.T) { + f := AwsFetcher{cfn: &cfnClient{}} + key := "aws:cloudformation:stack-name" + val := "my-stack" + roleName := "my-role" + path := "/" + roleList := []*iam.RoleDetail{ + {Tags: []*iam.Tag{{Key: &key, Value: &val}}, RoleName: &roleName, Path: &path}, + } + + resp := iam.GetAccountAuthorizationDetailsOutput{RoleDetailList: roleList} + f.populateIamData(&resp) + for _, role := range f.data.Roles { + if role.Name == roleName { + t.Error("Expected to skip role with CFN tags") + } + } +} From a6c20592a07b7b4af848ffad5e1621b1f18b53e6 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Fri, 4 Sep 2020 12:57:52 +0400 Subject: [PATCH 3/9] Require --skip-cfn-tagged flag to trigger new tag filtering behaviour --- iamy.go | 16 ++++++----- iamy/aws.go | 7 +++-- iamy/aws_test.go | 69 +++++++++++++++++++++++++++++++++++++++++++++--- iamy/iam.go | 2 +- pull.go | 3 ++- 5 files changed, 82 insertions(+), 15 deletions(-) diff --git a/iamy.go b/iamy.go index 722f508..4fc4aa8 100644 --- a/iamy.go +++ b/iamy.go @@ -41,13 +41,14 @@ type Ui struct { func main() { var ( - debug = kingpin.Flag("debug", "Show debugging output").Bool() - pull = kingpin.Command("pull", "Syncs IAM users, groups and policies from the active AWS account to files") - pullDir = pull.Flag("dir", "The directory to dump yaml files to").Default(defaultDir).Short('d').String() - canDelete = pull.Flag("delete", "Delete extraneous files from destination dir").Bool() - lookupCfn = pull.Flag("accurate-cfn", "Fetch all known resource names from cloudformation to get exact filtering").Bool() - push = kingpin.Command("push", "Syncs IAM users, groups and policies from files to the active AWS account") - pushDir = push.Flag("dir", "The directory to load yaml files from").Default(defaultDir).Short('d').ExistingDir() + debug = kingpin.Flag("debug", "Show debugging output").Bool() + pull = kingpin.Command("pull", "Syncs IAM users, groups and policies from the active AWS account to files") + pullDir = pull.Flag("dir", "The directory to dump yaml files to").Default(defaultDir).Short('d').String() + canDelete = pull.Flag("delete", "Delete extraneous files from destination dir").Bool() + lookupCfn = pull.Flag("accurate-cfn", "Fetch all known resource names from cloudformation to get exact filtering").Bool() + skipCfnTagged = pull.Flag("skip-cfn-tagged", "Skips entities or associated entities (buckets for bucket policies) tagged with default cloudformation tags").Bool() + push = kingpin.Command("push", "Syncs IAM users, groups and policies from files to the active AWS account") + pushDir = push.Flag("dir", "The directory to load yaml files from").Default(defaultDir).Short('d').ExistingDir() ) dryRun = kingpin.Flag("dry-run", "Show what would happen, but don't prompt to do it").Bool() @@ -85,6 +86,7 @@ func main() { Dir: *pullDir, CanDelete: *canDelete, HeuristicCfnMatching: !*lookupCfn, + SkipCfnTagged: *skipCfnTagged, }) } } diff --git a/iamy/aws.go b/iamy/aws.go index f283f91..2aa12aa 100644 --- a/iamy/aws.go +++ b/iamy/aws.go @@ -17,6 +17,7 @@ type AwsFetcher struct { // when pushing to AWS SkipFetchingPolicyAndRoleDescriptions bool HeuristicCfnMatching bool + SkipCfnTagged bool Debug *log.Logger @@ -416,8 +417,10 @@ const cloudformationStackNameTag = "aws:cloudformation:stack-name" // reasoning why it was skipped. func (a *AwsFetcher) isSkippableManagedResource(cfnType CfnResourceType, resourceIdentifier string, tags map[string]string) (bool, string) { - if stackName, ok := tags[cloudformationStackNameTag]; ok { - return true, fmt.Sprintf("CloudFormation generated resource %s in stack %s", resourceIdentifier, stackName) + if a.SkipCfnTagged { + if stackName, ok := tags[cloudformationStackNameTag]; ok { + return true, fmt.Sprintf("CloudFormation generated resource %s in stack %s", resourceIdentifier, stackName) + } } if a.cfn.IsManagedResource(cfnType, resourceIdentifier) { diff --git a/iamy/aws_test.go b/iamy/aws_test.go index c1bfdf9..454fd80 100644 --- a/iamy/aws_test.go +++ b/iamy/aws_test.go @@ -51,7 +51,7 @@ func TestIsSkippableManagedResource(t *testing.T) { } func TestSkippableS3TaggedResources(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}} + f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: true} skippableTags := map[string]string{"aws:cloudformation:stack-name": "my-stack"} skipped, err := f.isSkippableManagedResource(CfnS3Bucket, "my-bucket", skippableTags) @@ -63,8 +63,21 @@ func TestSkippableS3TaggedResources(t *testing.T) { } } +func TestSkippableS3TaggedResources_WithSkipFalse(t *testing.T) { + f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: false} + skippableTags := map[string]string{"aws:cloudformation:stack-name": "my-stack"} + + skipped, err := f.isSkippableManagedResource(CfnS3Bucket, "my-bucket", skippableTags) + if err != "" { + t.Errorf("expected no error message but it was " + err) + } + if skipped == true { + t.Errorf("expected resource to not be skipped but got true") + } +} + func TestNonSkippableTaggedResources(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}} + f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: true} nonSkippableTags := map[string]string{"Name": "blah"} skipped, err := f.isSkippableManagedResource(CfnS3Bucket, "my-bucket", nonSkippableTags) @@ -77,7 +90,7 @@ func TestNonSkippableTaggedResources(t *testing.T) { } func TestSkippableIAMUserResource(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}} + f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: true} key := "aws:cloudformation:stack-name" val := "my-stack" userName := "my-user" @@ -95,8 +108,32 @@ func TestSkippableIAMUserResource(t *testing.T) { } } +func TestSkippableIAMUserResource_WithSkipFalse(t *testing.T) { + f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: false} + key := "aws:cloudformation:stack-name" + val := "my-stack" + userName := "my-user" + path := "/" + userList := []*iam.UserDetail{ + {Tags: []*iam.Tag{{Key: &key, Value: &val}}, UserName: &userName, Path: &path}, + } + + resp := iam.GetAccountAuthorizationDetailsOutput{UserDetailList: userList} + f.populateIamData(&resp) + foundUser := false + for _, user := range f.data.Users { + if user.Name == userName { + foundUser = true + } + } + + if !foundUser { + t.Error("Expected to not skip user with CFN tags when SkipCfnTagged: false") + } +} + func TestSkippableIAMRoleResource(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}} + f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: true} key := "aws:cloudformation:stack-name" val := "my-stack" roleName := "my-role" @@ -113,3 +150,27 @@ func TestSkippableIAMRoleResource(t *testing.T) { } } } + +func TestSkippableIAMRoleResource_WithSkipFalse(t *testing.T) { + f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: false, SkipFetchingPolicyAndRoleDescriptions: true} + key := "aws:cloudformation:stack-name" + val := "my-stack" + roleName := "my-role" + path := "/" + str := "{}" + roleList := []*iam.RoleDetail{ + {Tags: []*iam.Tag{{Key: &key, Value: &val}}, RoleName: &roleName, Path: &path, AssumeRolePolicyDocument: &str}, + } + + resp := iam.GetAccountAuthorizationDetailsOutput{RoleDetailList: roleList} + f.populateIamData(&resp) + foundRole := false + for _, role := range f.data.Roles { + if role.Name == roleName { + foundRole = true + } + } + if !foundRole { + t.Error("Expected to not skip role with CFN tags and SkipCfnTagged: false") + } +} diff --git a/iamy/iam.go b/iamy/iam.go index d6489ee..17f869c 100644 --- a/iamy/iam.go +++ b/iamy/iam.go @@ -27,7 +27,7 @@ func (c *iamClient) getPolicyDescription(arn string) (string, error) { func (c *iamClient) getRoleDescription(name string) (string, error) { resp, err := c.GetRole(&iam.GetRoleInput{RoleName: &name}) - if err == nil && resp.Role.Description != nil { + if err == nil && resp.Role != nil && resp.Role.Description != nil { return *resp.Role.Description, nil } return "", err diff --git a/pull.go b/pull.go index bb57eee..6e9063b 100644 --- a/pull.go +++ b/pull.go @@ -10,10 +10,11 @@ type PullCommandInput struct { Dir string CanDelete bool HeuristicCfnMatching bool + SkipCfnTagged bool } func PullCommand(ui Ui, input PullCommandInput) { - aws := iamy.AwsFetcher{Debug: ui.Debug, HeuristicCfnMatching: input.HeuristicCfnMatching} + aws := iamy.AwsFetcher{Debug: ui.Debug, HeuristicCfnMatching: input.HeuristicCfnMatching, SkipCfnTagged: input.SkipCfnTagged} data, err := aws.Fetch() if err != nil { ui.Error.Fatal(fmt.Printf("%s", err)) From b342f22a64d70adbbbe2657a22ef8154bc79b097 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Wed, 9 Sep 2020 13:10:44 +0400 Subject: [PATCH 4/9] Implement --skip-tagged to skip resources containing a given tag --- iamy.go | 12 ++++++++++-- iamy/aws.go | 14 ++++++-------- iamy/aws_test.go | 26 ++++++++++++++------------ pull.go | 4 ++-- 4 files changed, 32 insertions(+), 24 deletions(-) diff --git a/iamy.go b/iamy.go index 4fc4aa8..eecf4f4 100644 --- a/iamy.go +++ b/iamy.go @@ -39,6 +39,10 @@ type Ui struct { Exit func(code int) } +// CFN automatically tags resources with this and other tags: +// https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-resource-tags.html +const cloudformationStackNameTag = "aws:cloudformation:stack-name" + func main() { var ( debug = kingpin.Flag("debug", "Show debugging output").Bool() @@ -46,7 +50,8 @@ func main() { pullDir = pull.Flag("dir", "The directory to dump yaml files to").Default(defaultDir).Short('d').String() canDelete = pull.Flag("delete", "Delete extraneous files from destination dir").Bool() lookupCfn = pull.Flag("accurate-cfn", "Fetch all known resource names from cloudformation to get exact filtering").Bool() - skipCfnTagged = pull.Flag("skip-cfn-tagged", "Skips entities or associated entities (buckets for bucket policies) tagged with default cloudformation tags").Bool() + skipCfnTagged = pull.Flag("skip-cfn-tagged", fmt.Sprintf("Shorthand for --skip-tagged %s", cloudformationStackNameTag)).Bool() + skipTagged = pull.Flag("skip-tagged", "Skips entities or associated entities (buckets for bucket policies) tagged with a given tag").Strings() push = kingpin.Command("push", "Syncs IAM users, groups and policies from files to the active AWS account") pushDir = push.Flag("dir", "The directory to load yaml files from").Default(defaultDir).Short('d').ExistingDir() ) @@ -74,6 +79,9 @@ func main() { } performVersionChecks() + if *skipCfnTagged { + *skipTagged = append(*skipTagged, cloudformationStackNameTag) + } switch cmd { case push.FullCommand(): @@ -86,7 +94,7 @@ func main() { Dir: *pullDir, CanDelete: *canDelete, HeuristicCfnMatching: !*lookupCfn, - SkipCfnTagged: *skipCfnTagged, + SkipTagged: *skipTagged, }) } } diff --git a/iamy/aws.go b/iamy/aws.go index 2aa12aa..84d46c2 100644 --- a/iamy/aws.go +++ b/iamy/aws.go @@ -17,7 +17,7 @@ type AwsFetcher struct { // when pushing to AWS SkipFetchingPolicyAndRoleDescriptions bool HeuristicCfnMatching bool - SkipCfnTagged bool + SkipTagged []string Debug *log.Logger @@ -404,10 +404,6 @@ func (a *AwsFetcher) getAccount() (*Account, error) { return &acct, nil } -// CFN automatically tags resources with this and other tags: -// https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-resource-tags.html -const cloudformationStackNameTag = "aws:cloudformation:stack-name" - // isSkippableResource takes the resource identifier as a string and // checks it against known resources that we shouldn't need to manage as // it will already be managed by another process (such as Cloudformation @@ -417,9 +413,11 @@ const cloudformationStackNameTag = "aws:cloudformation:stack-name" // reasoning why it was skipped. func (a *AwsFetcher) isSkippableManagedResource(cfnType CfnResourceType, resourceIdentifier string, tags map[string]string) (bool, string) { - if a.SkipCfnTagged { - if stackName, ok := tags[cloudformationStackNameTag]; ok { - return true, fmt.Sprintf("CloudFormation generated resource %s in stack %s", resourceIdentifier, stackName) + if len(a.SkipTagged) > 0 { + for _, tag := range a.SkipTagged { + if stackName, ok := tags[tag]; ok { + return true, fmt.Sprintf("Skipping resource %s tagged with %s in stack %s", resourceIdentifier, tag, stackName) + } } } diff --git a/iamy/aws_test.go b/iamy/aws_test.go index 454fd80..dec3965 100644 --- a/iamy/aws_test.go +++ b/iamy/aws_test.go @@ -6,6 +6,8 @@ import ( "github.com/aws/aws-sdk-go/service/iam" ) +const cloudformationStackNameTag = "aws:cloudformation:stack-name" + func TestIsSkippableManagedResource(t *testing.T) { skippables := []string{ "myalias-123/iam/role/aws-service-role/spot.amazonaws.com/AWSServiceRoleForEC2Spot.yaml", @@ -51,7 +53,7 @@ func TestIsSkippableManagedResource(t *testing.T) { } func TestSkippableS3TaggedResources(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: true} + f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{cloudformationStackNameTag}} skippableTags := map[string]string{"aws:cloudformation:stack-name": "my-stack"} skipped, err := f.isSkippableManagedResource(CfnS3Bucket, "my-bucket", skippableTags) @@ -63,8 +65,8 @@ func TestSkippableS3TaggedResources(t *testing.T) { } } -func TestSkippableS3TaggedResources_WithSkipFalse(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: false} +func TestSkippableS3TaggedResources_WithNoSkipTags(t *testing.T) { + f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{}} skippableTags := map[string]string{"aws:cloudformation:stack-name": "my-stack"} skipped, err := f.isSkippableManagedResource(CfnS3Bucket, "my-bucket", skippableTags) @@ -77,7 +79,7 @@ func TestSkippableS3TaggedResources_WithSkipFalse(t *testing.T) { } func TestNonSkippableTaggedResources(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: true} + f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{cloudformationStackNameTag}} nonSkippableTags := map[string]string{"Name": "blah"} skipped, err := f.isSkippableManagedResource(CfnS3Bucket, "my-bucket", nonSkippableTags) @@ -90,7 +92,7 @@ func TestNonSkippableTaggedResources(t *testing.T) { } func TestSkippableIAMUserResource(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: true} + f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{cloudformationStackNameTag}} key := "aws:cloudformation:stack-name" val := "my-stack" userName := "my-user" @@ -108,8 +110,8 @@ func TestSkippableIAMUserResource(t *testing.T) { } } -func TestSkippableIAMUserResource_WithSkipFalse(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: false} +func TestSkippableIAMUserResource_WithNoSkipTags(t *testing.T) { + f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{}} key := "aws:cloudformation:stack-name" val := "my-stack" userName := "my-user" @@ -128,12 +130,12 @@ func TestSkippableIAMUserResource_WithSkipFalse(t *testing.T) { } if !foundUser { - t.Error("Expected to not skip user with CFN tags when SkipCfnTagged: false") + t.Error("Expected to not skip user with CFN tags when SkipTagged: []string{}") } } func TestSkippableIAMRoleResource(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: true} + f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{cloudformationStackNameTag}} key := "aws:cloudformation:stack-name" val := "my-stack" roleName := "my-role" @@ -151,8 +153,8 @@ func TestSkippableIAMRoleResource(t *testing.T) { } } -func TestSkippableIAMRoleResource_WithSkipFalse(t *testing.T) { - f := AwsFetcher{cfn: &cfnClient{}, SkipCfnTagged: false, SkipFetchingPolicyAndRoleDescriptions: true} +func TestSkippableIAMRoleResource_WithNoSkipTags(t *testing.T) { + f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{}, SkipFetchingPolicyAndRoleDescriptions: true} key := "aws:cloudformation:stack-name" val := "my-stack" roleName := "my-role" @@ -171,6 +173,6 @@ func TestSkippableIAMRoleResource_WithSkipFalse(t *testing.T) { } } if !foundRole { - t.Error("Expected to not skip role with CFN tags and SkipCfnTagged: false") + t.Error("Expected to not skip role with CFN tags and SkipTagged: []string{}") } } diff --git a/pull.go b/pull.go index 6e9063b..8f5eeed 100644 --- a/pull.go +++ b/pull.go @@ -10,11 +10,11 @@ type PullCommandInput struct { Dir string CanDelete bool HeuristicCfnMatching bool - SkipCfnTagged bool + SkipTagged []string } func PullCommand(ui Ui, input PullCommandInput) { - aws := iamy.AwsFetcher{Debug: ui.Debug, HeuristicCfnMatching: input.HeuristicCfnMatching, SkipCfnTagged: input.SkipCfnTagged} + aws := iamy.AwsFetcher{Debug: ui.Debug, HeuristicCfnMatching: input.HeuristicCfnMatching, SkipTagged: input.SkipTagged} data, err := aws.Fetch() if err != nil { ui.Error.Fatal(fmt.Printf("%s", err)) From abca9f62d32964dd36de4dcab1dac4efaacb929e Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Wed, 9 Sep 2020 13:11:46 +0400 Subject: [PATCH 5/9] Reword flag help text --- iamy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iamy.go b/iamy.go index eecf4f4..747b605 100644 --- a/iamy.go +++ b/iamy.go @@ -51,7 +51,7 @@ func main() { canDelete = pull.Flag("delete", "Delete extraneous files from destination dir").Bool() lookupCfn = pull.Flag("accurate-cfn", "Fetch all known resource names from cloudformation to get exact filtering").Bool() skipCfnTagged = pull.Flag("skip-cfn-tagged", fmt.Sprintf("Shorthand for --skip-tagged %s", cloudformationStackNameTag)).Bool() - skipTagged = pull.Flag("skip-tagged", "Skips entities or associated entities (buckets for bucket policies) tagged with a given tag").Strings() + skipTagged = pull.Flag("skip-tagged", "Skips IAM entities (or buckets associated with bucket policies) tagged with a given tag").Strings() push = kingpin.Command("push", "Syncs IAM users, groups and policies from files to the active AWS account") pushDir = push.Flag("dir", "The directory to load yaml files from").Default(defaultDir).Short('d').ExistingDir() ) From a250cd039bc74d575cf41954f3af4855ddef9415 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Wed, 9 Sep 2020 13:37:44 +0400 Subject: [PATCH 6/9] Update test to use cloudformationStackNameTag var --- iamy/aws_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/iamy/aws_test.go b/iamy/aws_test.go index dec3965..8aa8a0d 100644 --- a/iamy/aws_test.go +++ b/iamy/aws_test.go @@ -54,7 +54,7 @@ func TestIsSkippableManagedResource(t *testing.T) { func TestSkippableS3TaggedResources(t *testing.T) { f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{cloudformationStackNameTag}} - skippableTags := map[string]string{"aws:cloudformation:stack-name": "my-stack"} + skippableTags := map[string]string{cloudformationStackNameTag: "my-stack"} skipped, err := f.isSkippableManagedResource(CfnS3Bucket, "my-bucket", skippableTags) if err == "" { @@ -67,7 +67,7 @@ func TestSkippableS3TaggedResources(t *testing.T) { func TestSkippableS3TaggedResources_WithNoSkipTags(t *testing.T) { f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{}} - skippableTags := map[string]string{"aws:cloudformation:stack-name": "my-stack"} + skippableTags := map[string]string{cloudformationStackNameTag: "my-stack"} skipped, err := f.isSkippableManagedResource(CfnS3Bucket, "my-bucket", skippableTags) if err != "" { @@ -93,7 +93,7 @@ func TestNonSkippableTaggedResources(t *testing.T) { func TestSkippableIAMUserResource(t *testing.T) { f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{cloudformationStackNameTag}} - key := "aws:cloudformation:stack-name" + key := cloudformationStackNameTag val := "my-stack" userName := "my-user" path := "/" @@ -112,7 +112,7 @@ func TestSkippableIAMUserResource(t *testing.T) { func TestSkippableIAMUserResource_WithNoSkipTags(t *testing.T) { f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{}} - key := "aws:cloudformation:stack-name" + key := cloudformationStackNameTag val := "my-stack" userName := "my-user" path := "/" @@ -136,7 +136,7 @@ func TestSkippableIAMUserResource_WithNoSkipTags(t *testing.T) { func TestSkippableIAMRoleResource(t *testing.T) { f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{cloudformationStackNameTag}} - key := "aws:cloudformation:stack-name" + key := cloudformationStackNameTag val := "my-stack" roleName := "my-role" path := "/" @@ -155,7 +155,7 @@ func TestSkippableIAMRoleResource(t *testing.T) { func TestSkippableIAMRoleResource_WithNoSkipTags(t *testing.T) { f := AwsFetcher{cfn: &cfnClient{}, SkipTagged: []string{}, SkipFetchingPolicyAndRoleDescriptions: true} - key := "aws:cloudformation:stack-name" + key := cloudformationStackNameTag val := "my-stack" roleName := "my-role" path := "/" From b2b2573ff68000645d6599db0599bf156e1b0e52 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Wed, 23 Sep 2020 12:26:21 +0400 Subject: [PATCH 7/9] Remove --skip-cfn-tagged flag --- iamy.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/iamy.go b/iamy.go index 747b605..dde921e 100644 --- a/iamy.go +++ b/iamy.go @@ -45,15 +45,14 @@ const cloudformationStackNameTag = "aws:cloudformation:stack-name" func main() { var ( - debug = kingpin.Flag("debug", "Show debugging output").Bool() - pull = kingpin.Command("pull", "Syncs IAM users, groups and policies from the active AWS account to files") - pullDir = pull.Flag("dir", "The directory to dump yaml files to").Default(defaultDir).Short('d').String() - canDelete = pull.Flag("delete", "Delete extraneous files from destination dir").Bool() - lookupCfn = pull.Flag("accurate-cfn", "Fetch all known resource names from cloudformation to get exact filtering").Bool() - skipCfnTagged = pull.Flag("skip-cfn-tagged", fmt.Sprintf("Shorthand for --skip-tagged %s", cloudformationStackNameTag)).Bool() - skipTagged = pull.Flag("skip-tagged", "Skips IAM entities (or buckets associated with bucket policies) tagged with a given tag").Strings() - push = kingpin.Command("push", "Syncs IAM users, groups and policies from files to the active AWS account") - pushDir = push.Flag("dir", "The directory to load yaml files from").Default(defaultDir).Short('d').ExistingDir() + debug = kingpin.Flag("debug", "Show debugging output").Bool() + pull = kingpin.Command("pull", "Syncs IAM users, groups and policies from the active AWS account to files") + pullDir = pull.Flag("dir", "The directory to dump yaml files to").Default(defaultDir).Short('d').String() + canDelete = pull.Flag("delete", "Delete extraneous files from destination dir").Bool() + lookupCfn = pull.Flag("accurate-cfn", "Fetch all known resource names from cloudformation to get exact filtering").Bool() + skipTagged = pull.Flag("skip-tagged", "Skips IAM entities (or buckets associated with bucket policies) tagged with a given tag").Strings() + push = kingpin.Command("push", "Syncs IAM users, groups and policies from files to the active AWS account") + pushDir = push.Flag("dir", "The directory to load yaml files from").Default(defaultDir).Short('d').ExistingDir() ) dryRun = kingpin.Flag("dry-run", "Show what would happen, but don't prompt to do it").Bool() @@ -79,9 +78,6 @@ func main() { } performVersionChecks() - if *skipCfnTagged { - *skipTagged = append(*skipTagged, cloudformationStackNameTag) - } switch cmd { case push.FullCommand(): From c60f2ce16210c2a03574d1e11f3c15c84b1ed3d9 Mon Sep 17 00:00:00 2001 From: Patrick Robinson Date: Tue, 18 May 2021 13:28:17 +1000 Subject: [PATCH 8/9] Ensure filtering S3 bucket policies is done in pull and push To avoid unexpected push behaviour --- iamy.go | 4 +++- push.go | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/iamy.go b/iamy.go index dde921e..8328d96 100644 --- a/iamy.go +++ b/iamy.go @@ -82,7 +82,9 @@ func main() { switch cmd { case push.FullCommand(): PushCommand(ui, PushCommandInput{ - Dir: *pushDir, + Dir: *pushDir, + HeuristicCfnMatching: !*lookupCfn, + SkipTagged: *skipTagged, }) case pull.FullCommand(): diff --git a/push.go b/push.go index cd68ab5..169a956 100644 --- a/push.go +++ b/push.go @@ -12,7 +12,9 @@ import ( ) type PushCommandInput struct { - Dir string + Dir string + HeuristicCfnMatching bool + SkipTagged []string } func PushCommand(ui Ui, input PushCommandInput) { @@ -22,6 +24,8 @@ func PushCommand(ui Ui, input PushCommandInput) { aws := iamy.AwsFetcher{ SkipFetchingPolicyAndRoleDescriptions: true, Debug: ui.Debug, + HeuristicCfnMatching: input.HeuristicCfnMatching, + SkipTagged: input.SkipTagged, } allDataFromYaml, err := yaml.Load() From 1541fe473bd4e78e76ec87733d2c3a310579cf74 Mon Sep 17 00:00:00 2001 From: Patrick Robinson Date: Tue, 18 May 2021 16:54:16 +1000 Subject: [PATCH 9/9] Revert remove --skip-cfn-tagged flag --- iamy.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/iamy.go b/iamy.go index 8328d96..4804e0b 100644 --- a/iamy.go +++ b/iamy.go @@ -45,14 +45,15 @@ const cloudformationStackNameTag = "aws:cloudformation:stack-name" func main() { var ( - debug = kingpin.Flag("debug", "Show debugging output").Bool() - pull = kingpin.Command("pull", "Syncs IAM users, groups and policies from the active AWS account to files") - pullDir = pull.Flag("dir", "The directory to dump yaml files to").Default(defaultDir).Short('d').String() - canDelete = pull.Flag("delete", "Delete extraneous files from destination dir").Bool() - lookupCfn = pull.Flag("accurate-cfn", "Fetch all known resource names from cloudformation to get exact filtering").Bool() - skipTagged = pull.Flag("skip-tagged", "Skips IAM entities (or buckets associated with bucket policies) tagged with a given tag").Strings() - push = kingpin.Command("push", "Syncs IAM users, groups and policies from files to the active AWS account") - pushDir = push.Flag("dir", "The directory to load yaml files from").Default(defaultDir).Short('d').ExistingDir() + debug = kingpin.Flag("debug", "Show debugging output").Bool() + pull = kingpin.Command("pull", "Syncs IAM users, groups and policies from the active AWS account to files") + pullDir = pull.Flag("dir", "The directory to dump yaml files to").Default(defaultDir).Short('d').String() + canDelete = pull.Flag("delete", "Delete extraneous files from destination dir").Bool() + lookupCfn = pull.Flag("accurate-cfn", "Fetch all known resource names from cloudformation to get exact filtering").Bool() + skipCfnTagged = pull.Flag("skip-cfn-tagged", fmt.Sprintf("Shorthand for --skip-tagged %s", cloudformationStackNameTag)).Bool() + skipTagged = pull.Flag("skip-tagged", "Skips IAM entities (or buckets associated with bucket policies) tagged with a given tag").Strings() + push = kingpin.Command("push", "Syncs IAM users, groups and policies from files to the active AWS account") + pushDir = push.Flag("dir", "The directory to load yaml files from").Default(defaultDir).Short('d').ExistingDir() ) dryRun = kingpin.Flag("dry-run", "Show what would happen, but don't prompt to do it").Bool() @@ -78,6 +79,9 @@ func main() { } performVersionChecks() + if *skipCfnTagged { + *skipTagged = append(*skipTagged, cloudformationStackNameTag) + } switch cmd { case push.FullCommand():