Manage IAM instance profiles #39

andrewjhumphrey · 2017-11-02T01:40:31Z

Instance profiles are a bit of a hybrid between IAM and EC2, but if you've moved IAM management out of your cloudformation templates it feels a little weird to leave the instance profiles behind.

This PR adds the ability to manage instance profiles.

The IAM API call that is used to grab users, groups, roles, policies GetAccountAuthorizationDetails doesn't support instance profiles, so there is a bit of copy'pasta code here to use the ListInstanceProfiles API call. Sorry about that. I suspect that this section might be ripe for a little refactoring. My golang is crappy enough that I haven't attempted that.

Demo

$ ../bin/iamy-darwin-amd64 pull -d .
$ cd envato-platform-development-123456789012/iam/instance_profile/
$ ls -1F
EC2ReadOnly.yaml
aws-elasticbeanstalk-ec2-role.yaml
ecsInstanceRole.yaml
test/
$ cat EC2ReadOnly.yaml 
Roles:
- EC2ReadOnly
$ cp EC2ReadOnly.yaml test/testpath.yaml   # Add role to an instance profile
$ echo '{}' > EC2ReadOnly.yaml # Remove roles from an instance profile
$ cp test/testpath.yaml test/newtestpath.yaml # Create a new instance profile (with path)
$ rm HumpyIAMDBTestRole.yaml  # Remove an instance profile
$ cd ../../..
$ ../bin/iamy-darwin-amd64 push -d . --dry-run
Commands to push changes to AWS:

      aws iam remove-role-from-instance-profile --instance-profile-name EC2ReadOnly --role-name EC2ReadOnly
      aws create-instance-profile --instance-profile-name newtestpath --path /test/
      aws iam add-role-to-instance-profile --instance-profile-name newtestpath --role-name EC2ReadOnly
      aws iam add-role-to-instance-profile --instance-profile-name testpath --role-name EC2ReadOnly
      aws iam delete-instance-profile --instance-profile-name HumpyIAMDBTestRole
Dry-run mode not running aws commands

jacobbednarz · 2017-11-02T02:14:11Z

iamy/aws.go

+			}
+			return true
+		})
+	if populateInstanceProfileErr != nil {


does this need to be here? looks like you've already got a check for this on L136.

I think that's right, the L136 is to escape the iterator function

Yep, it needs to be there.
The function that includes line 136 is called by the paging function a.iam.ListInstanceProfilesPages with each back results. So the return false is effectively the break.

jacobbednarz · 2017-11-02T02:15:39Z

iamy/aws.go

+		if len(profileResp.Roles) > 0 {
+			for _, roleResp := range  profileResp.Roles {
+				role := *(roleResp.RoleName)
+				profile.Roles = append (profile.Roles, role)


we've got an extra bit of whitespace after the append keyword here.

I like whitespace.
But fixed in 043328d

jacobbednarz · 2017-11-02T02:16:07Z

iamy/aws.go

+			Path: *profileResp.Path,
+		}}
+		if len(profileResp.Roles) > 0 {
+			for _, roleResp := range  profileResp.Roles {


double spaces after range?

Yep probably a good idea to run a go fmt over these modified files. I typically add a hook to my editor to run goimports on save @andrewjhumphrey

jacobbednarz · 2017-11-02T02:24:25Z

iamy/models.go

@@ -113,6 +113,15 @@ type Role struct {
 	Policies                 []string        `json:"Policies,omitempty"`
 }

+type InstanceProfile struct {


we probably want to fix the alignment on this one

type InstanceProfile struct { iamService `json:"-"` Roles []string `json:"Roles,omitempty"` }

It's aligned if your tabs are set to 8 :-)

jacobbednarz · 2017-11-02T02:28:37Z

iamy/models.go

@@ -145,6 +154,7 @@ type AccountData struct {
 	Roles          []*Role
 	Policies       []*Policy
 	BucketPolicies []*BucketPolicy
+	InstanceProfiles []*InstanceProfile


we should look to align all of these struct fields

jacobbednarz · 2017-11-02T02:28:53Z

iamy/models.go

@@ -154,6 +164,7 @@ func NewAccountData(account string) *AccountData {
 		Groups:   []*Group{},
 		Roles:    []*Role{},
 		Policies: []*Policy{},
+		InstanceProfiles: []*InstanceProfile{},


we should look to align all of these struct fields

jacobbednarz · 2017-11-02T02:35:21Z

iamy/aws.go

+			Name: *profileResp.InstanceProfileName,
+			Path: *profileResp.Path,
+		}}
+		if len(profileResp.Roles) > 0 {


you don't need to wrap the for loop in a length check since golang handles when the range is empty. see https://play.golang.org/p/UUhRUwypIk for an example.

Ahh, yes. I've cleaned it up in 728fc67 . That was a hail mary debugging attempt when I had:

type InstanceProfile struct { iamService `json:"-"` Roles []string `json:"Roles",omitempty` }

instead of (note the quotes in the omitempty bit)

type InstanceProfile struct { iamService `json:"-"` Roles []string `json:"Roles,omitempty"` }

and It was emitting Roles: null into the YAML instead of ignoring an empty Roles attribute.

mtibben · 2017-11-02T02:46:01Z

This is looking fantastic @andrewjhumphrey, a great addition.

This starts to cut into #35 also. I'm thinking it might be useful to have some kind of mechanism to specify what data you want iamy to manage. Maybe a dotfile where you can specify entities to skip. Any thoughts @pda @lox ?

jacobbednarz · 2017-11-02T03:03:34Z

iamy/awsdiff.go

+                        }
+		} else {
+                        // Create instance profile
+			a.cmds.Add("aws","create-instance-profile","--instance-profile-name",toInstanceProfile.Name,"--path",path(toInstanceProfile.Path))


spacing between these parameters would be 💯

joho · 2017-11-02T03:09:45Z

Yeah I was worried on initial review that all of a sudden we'd be managing more things with iamy than we expected on an update. Not sure if opt in or opt out is the best model. Maybe opt-out is default for everything available to be maanged at a major release, then on minor releases any new account info is opt-in... then flip it next time we bump major.

Maybe I'm overthinking it, just add this and bump to 3.0.

Also I pulled branch locally to run go fmt, still got one little outstanding whitespace thing.

andrewjhumphrey · 2017-11-02T03:14:24Z

I don't see that dryRun indent problem in my tree. Much strange! As that was a change from previous PR I did though (#30) so it's entirely likely I've pooched something.

mtibben · 2017-11-02T03:38:21Z

iamy/models.go

+}
+
+func (ip InstanceProfile) ResourceType() string {
+	return "instance_profile"


I think we'll want to follow the ARN format instance-profile

mtibben · 2017-11-02T03:38:54Z

iamy/yaml.go

@@ -13,7 +13,7 @@ import (
 )

 const pathTemplateBlob = "{{.Account}}/{{.Resource.Service}}/{{.Resource.ResourceType}}{{.Resource.ResourcePath}}{{.Resource.ResourceName}}.yaml"
-const pathRegexBlob = `^(?P<account>[^/]+)/(?P<entity>(iam/user|iam/group|iam/policy|iam/role|s3))(?P<resourcepath>.*/)(?P<resourcename>[^/]+)\.yaml$`
+const pathRegexBlob = `^(?P<account>[^/]+)/(?P<entity>(iam/instance_profile|iam/user|iam/group|iam/policy|iam/role|s3))(?P<resourcepath>.*/)(?P<resourcename>[^/]+)\.yaml$`


instance-profile as per ARN also here

mtibben · 2017-11-02T03:39:01Z

iamy/yaml.go

@@ -106,6 +106,10 @@ func (a *YamlLoadDumper) Load() ([]AccountData, error) {
 				p := Policy{iamService: nameAndPath}
 				err = a.unmarshalYamlFile(fp, &p)
 				accounts[accountid].addPolicy(&p)
+			case "iam/instance_profile":


instance-profile as per ARN also here

Fixed in c351b24

pda · 2017-11-02T22:57:40Z

I'm thinking it might be useful to have some kind of mechanism to specify what data you want iamy to manage. Maybe a dotfile where you can specify entities to skip.

Yeah some mechanism to opt out of some types (IAM / S3 / EC2 / etc…) could be good. I can't think of a scenario where I'd use it though; I'm happy to track everything it can find. If it's via a config file, I'd strongly prefer a non-dot-file. I hate it when important files are sometimes invisible / excluded from searches / etc.

Somewhat related to opting out of certain types is the auto-ignore of CloudFormation-managed resources based on a dubious pattern match on the resource name. Perhaps that should be opt-in / opt-out or the filter should be configurable?

mtibben · 2017-11-14T03:00:42Z

Thanks @andrewjhumphrey, also I've made you a contributor with write access as it looks like you're an active user

Manage IAM instance profiles

fdfaa81

jacobbednarz reviewed Nov 2, 2017

View reviewed changes

Fix whitespace problems

043328d

jacobbednarz reviewed Nov 2, 2017

View reviewed changes

Remove unnecessary guard test, golang range does the right thing

728fc67

andrewjhumphrey added 3 commits November 2, 2017 11:14

More whitespace fixes

2d2144a

Found it! Fixed

4b08e46

go fmt? Who knew! *blush*

a0ccc80

mtibben reviewed Nov 2, 2017

View reviewed changes

Use ARN standard of - instead of _

c351b24

mtibben merged commit f6d5b26 into 99designs:master Nov 14, 2017

simpsora mentioned this pull request Sep 8, 2020

Skip resources tagged with aws:cloudformation:stack-name #73

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Manage IAM instance profiles #39

Manage IAM instance profiles #39

andrewjhumphrey commented Nov 2, 2017 •

edited

Loading

jacobbednarz Nov 2, 2017

mtibben Nov 2, 2017

andrewjhumphrey Nov 2, 2017

jacobbednarz Nov 2, 2017

andrewjhumphrey Nov 2, 2017

jacobbednarz Nov 2, 2017

mtibben Nov 2, 2017

andrewjhumphrey Nov 2, 2017

jacobbednarz Nov 2, 2017

andrewjhumphrey Nov 2, 2017

jacobbednarz Nov 2, 2017

andrewjhumphrey Nov 2, 2017

jacobbednarz Nov 2, 2017

andrewjhumphrey Nov 2, 2017

jacobbednarz Nov 2, 2017

andrewjhumphrey Nov 2, 2017

mtibben commented Nov 2, 2017

jacobbednarz Nov 2, 2017

joho commented Nov 2, 2017

andrewjhumphrey commented Nov 2, 2017

mtibben Nov 2, 2017

andrewjhumphrey Nov 2, 2017

mtibben Nov 2, 2017

andrewjhumphrey Nov 2, 2017

mtibben Nov 2, 2017

andrewjhumphrey Nov 2, 2017

pda commented Nov 2, 2017

mtibben commented Nov 14, 2017

Manage IAM instance profiles #39

Manage IAM instance profiles #39

Conversation

andrewjhumphrey commented Nov 2, 2017 • edited Loading

Demo

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtibben commented Nov 2, 2017

Choose a reason for hiding this comment

joho commented Nov 2, 2017

andrewjhumphrey commented Nov 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pda commented Nov 2, 2017

mtibben commented Nov 14, 2017

andrewjhumphrey commented Nov 2, 2017 •

edited

Loading