Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IAMUserPolicyAttachment #196

Merged
merged 4 commits into from
Apr 29, 2020
Merged

Conversation

sahil-lakhwani
Copy link
Contributor

Description of your changes

Adds UserPolicyAttachment resource, which helps to attach IAM managed policy to an IAM user.

This depends on (and is a checkout from) #194 and should not be merged before that

Example: examples/iam/userpolicyattachment.yaml

Checklist

I have:

  • Run make reviewable to ensure this PR is ready for review.
  • Ensured this PR contains a neat, self documenting set of commits.
  • Updated any relevant documentation, examples, or release notes.
  • Updated the dependencies in app.yaml to include any new role permissions.

@sahil-lakhwani sahil-lakhwani mentioned this pull request Apr 17, 2020
4 tasks
Copy link
Member

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments, only reviewed parts that are new on top of #194


// UserName presents the name of the IAM user.
// +optional
UserName string `json:"userName,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be pointer type I believe.

"github.com/crossplane/crossplane-runtime/pkg/resource"
)

// UserNameReferencerForUserPolicyAttachment is an attribute referencer that retrieves Name from a referenced User
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referencer structure will need to be revised per pattern in #198

@sahil-lakhwani sahil-lakhwani changed the title Userpolicy IAMUserPolicyAttachment Apr 24, 2020
Copy link
Member

@negz negz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending updates to #194.

UserPolicyAttachmentKind = reflect.TypeOf(IAMUserPolicyAttachment{}).Name()
UserPolicyAttachmentGroupKind = schema.GroupKind{Group: Group, Kind: UserPolicyAttachmentKind}.String()
UserPolicyAttachmentKindAPIVersion = UserPolicyAttachmentKind + "." + SchemeGroupVersion.String()
UserPolicyAttachmentGroupVersionKind = SchemeGroupVersion.WithKind(UserPolicyAttachmentKind)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be IAMUserPolicyAttachment<X> to match the name of the struct.

Copy link
Member

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but guessing it will need rebase after IAMUser PR is merged

@negz negz marked this pull request as ready for review April 29, 2020 02:06
@negz
Copy link
Member

negz commented Apr 29, 2020

@sahil-lakhwani Could you please rebase this on master now that #194 is merged. Once you do so I'll merge this PR.

Signed-off-by: sahil-lakhwani <sahilakhwani@gmail.com>
…tachment'

Signed-off-by: sahil-lakhwani <sahilakhwani@gmail.com>
Signed-off-by: sahil-lakhwani <sahilakhwani@gmail.com>
Signed-off-by: sahil-lakhwani <sahilakhwani@gmail.com>
@sahil-lakhwani
Copy link
Contributor Author

@sahil-lakhwani Could you please rebase this on master now that #194 is merged. Once you do so I'll merge this PR.

@negz I have update this branch with master

@negz negz merged commit 4375238 into crossplane-contrib:master Apr 29, 2020
@prasek prasek mentioned this pull request Aug 6, 2020
wolffbe pushed a commit to wolffbe/provider-aws that referenced this pull request Feb 12, 2021
namku pushed a commit to namku/provider-aws that referenced this pull request Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants