Added AWS IAMGroup with UserMembership, and PolicyAttachment #249

rahulchheda · 2020-06-12T07:05:47Z

- Added CRUD operations for various resources
- Added test files for most of files (few exceptions are left, working on then actively)
- Added examples for each resource
- Added controllers for each resources

Signed-off-by: Rahul M Chheda rchheda@infracloud.io

Description of your changes

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 · 2020-06-22T10:51:44Z

apis/identity/v1alpha1/iamgroup_types.go

+type IAMGroupSpec struct {
+	runtimev1alpha1.ResourceSpec `json:",inline"`
+	// +optional
+	ForProvider IAMGroupParameters `json:"forProvider"`


This field is a required field, so should not be annotated with optional

sahil-lakhwani · 2020-06-22T10:55:16Z

apis/identity/v1alpha1/iamgrouppolicyattachment_types.go

+
+// An IAMGroupPolicyAttachment is a managed resource that represents an AWS IAM
+// Group policy attachment.
+// +kubebuilder:printcolumn:name="USERNAME",type="string",JSONPath=".spec.forProvider.groupName"


The column name should be GROUPNAME I assume.

sahil-lakhwani · 2020-06-22T11:25:21Z

apis/identity/v1alpha1/iamusergroupattachment_types.go

+// +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp"
+// +kubebuilder:subresource:status
+// +kubebuilder:resource:scope=Cluster
+type IAMUserGroupAttachment struct {


Would IAMGroupUserMembership be a good name?

Yup, seems good and pushed the changes

sahil-lakhwani · 2020-06-22T11:27:22Z

pkg/clients/iam/iamgroup.go

+}
+
+// GenerateIAMGroup assigns the in IAMGroupParameters to group.
+func GenerateIAMGroup(in v1alpha1.IAMGroupParameters, role *iam.Group) error {


Is this function being used anywhere?

sahil-lakhwani · 2020-06-22T11:35:57Z

pkg/clients/iam/iamgroup_test.go

@@ -0,0 +1 @@
+package iam


I think we can remove this file.

sahil-lakhwani · 2020-06-22T11:36:48Z

pkg/clients/iam/iamusergroupattachment_test.go

@@ -0,0 +1 @@
+package iam


We can remove this file maybe.

sahil-lakhwani · 2020-06-22T11:52:26Z

pkg/controller/identity/iamusergroupattachment/controller.go

+func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.ExternalUpdate, error) {
+	// Updating any field will create a new User-Policy attachment in AWS, which will be
+	// irrelevant/out-of-sync to the original defined attachment.
+	// It is encouraged to instead create a new IAMUserPolicyAttachment resource.


Suggested change

// It is encouraged to instead create a new IAMUserPolicyAttachment resource.

// It is encouraged to instead create a new IAMUserGroupAttachment resource.

sahil-lakhwani · 2020-06-22T11:52:50Z

pkg/controller/identity/iamusergroupattachment/controller.go

+}
+
+func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.ExternalUpdate, error) {
+	// Updating any field will create a new User-Policy attachment in AWS, which will be


Suggested change

// Updating any field will create a new User-Policy attachment in AWS, which will be

// Updating any field will create a new User-Group attachment in AWS, which will be

sahil-lakhwani · 2020-06-28T16:22:31Z

apis/identity/v1alpha1/iamgrouppolicyattachment_types.go

+	// PolicyARN is the Amazon Resource Name (ARN) of the IAM policy you want to
+	// attach.
+	// +immutable
+	PolicyARN string `json:"policyArn"`


I think a reference to the IAMPolicy resource can also be added here.

sahil-lakhwani · 2020-06-28T16:27:58Z

apis/identity/v1alpha1/iamgroupusermembership_types.go

+	// GroupName is the Amazon IAM Group Name (IAMGroup) of the IAM group you want to
+	// add User to.
+	// +immutable
+	GroupName string `json:"groupName"`


Also here, we can add a reference to the IAMGroup resource.

negz · 2020-07-01T20:01:59Z

Hi @rahulchheda and @sahil-lakhwani,

Can you please comment here when you've finished your internal review and feel this PR is ready for @muvaf to review.

Thanks,
Nic

negz · 2020-07-01T20:03:04Z

Also - it seems like this PR may need to be rebased on master. I see a lot of unrelated commits in the history.

rahulchheda · 2020-07-02T09:33:11Z

Apologies @negz and @muvaf , fixed the rebase issue on this PR. I think this PR is ready for review. I guess internal reviews have been done, or @sahil-lakhwani , would you like to take one more pass?

sahil-lakhwani · 2020-07-02T23:48:13Z

apis/identity/v1alpha1/iamgrouppolicyattachment_types.go

+	// PolicyARN is the Amazon Resource Name (ARN) of the IAM policy you want to
+	// attach.
+	// +immutable
+	PolicyARN string `json:"policyArn,omitempty"`


I think this should be annotated with +optional and should be of pointer type.

sahil-lakhwani · 2020-07-02T23:49:02Z

apis/identity/v1alpha1/iamgrouppolicyattachment_types.go

+
+	// GroupName presents the name of the IAMGroup.
+	// +immutable
+	GroupName string `json:"groupName,omitempty"`


same here, the filed should be annotated with +optional and be of pointer type.

sahil-lakhwani · 2020-07-02T23:52:32Z

apis/identity/v1alpha1/iamgroupusermembership_types.go

+	// GroupName is the Amazon IAM Group Name (IAMGroup) of the IAM group you want to
+	// add User to.
+	// +immutable
+	GroupName string `json:"groupName,omitempty"`


Same here as well, GroupName should be pointer type.

negz · 2020-07-03T01:33:25Z

Thanks @rahulchheda and @sahil-lakhwani! Looks like there's a little more review to go. I'll wait until @sahil-lakhwani has approved this PR before we have a maintainer review it.

sahil-lakhwani · 2020-07-06T10:11:34Z

@negz The PR looks good to me after the latest commit. I think it's ready for you to review.

muvaf

Thank you @rahulchheda and @sahil-lakhwani , really impressed!

It looks pretty good already. I have left a few small comments. It should be ready to merge after they are addressed.

pkg/controller/identity/iamgroup/controller.go

pkg/controller/identity/iamgroupusermembership/controller.go

pkg/controller/identity/iamgrouppolicyattachment/controller.go

pkg/controller/identity/iamgroupusermembership/controller.go

pkg/controller/identity/iamgrouppolicyattachment/controller.go

rahulchheda · 2020-07-10T08:27:49Z

@muvaf , I'm done with the changes, could you take a look? If it's good, remind me to squash my commits.

muvaf

LGTM after fixing two small things for test to pass and the commit history. Thanks!

pkg/controller/identity/iamgrouppolicyattachment/controller.go

pkg/controller/identity/iamgroupusermembership/controller.go

Signed-off-by: Rahul M Chheda <rchheda@infracloud.io>

Added AWS IAMGroup with UserMembership, and PolicyAttachment

DynamoDB: add aws_dynamodb_tag resource

negz requested a review from muvaf June 15, 2020 17:09

negz assigned muvaf Jun 15, 2020

sahil-lakhwani suggested changes Jun 22, 2020

View reviewed changes

rahulchheda changed the title ~~Added AWS IAMGroup with UserAttachment, and PolicyAttachment~~ Added AWS IAMGroup with UserMembership, and PolicyAttachment Jun 23, 2020

rahulchheda force-pushed the iamGroup branch from 311c271 to d21e8c0 Compare June 23, 2020 10:41

muvaf added this to In progress in v0.13 via automation Jun 23, 2020

sahil-lakhwani reviewed Jun 28, 2020

View reviewed changes

rahulchheda force-pushed the iamGroup branch from b48807b to 60fe291 Compare July 2, 2020 09:19

sahil-lakhwani reviewed Jul 2, 2020

View reviewed changes

muvaf reviewed Jul 9, 2020

View reviewed changes

muvaf approved these changes Jul 10, 2020

View reviewed changes

pkg/controller/identity/iamgrouppolicyattachment/controller.go Outdated Show resolved Hide resolved

pkg/controller/identity/iamgroupusermembership/controller.go Outdated Show resolved Hide resolved

v0.13 automation moved this from In progress to PR open Jul 10, 2020

Added AWS IAMGroup with UserMembership, and PolicyAttachment

43d5c1a

Signed-off-by: Rahul M Chheda <rchheda@infracloud.io>

rahulchheda force-pushed the iamGroup branch from c04170e to 43d5c1a Compare July 10, 2020 13:54

muvaf merged commit 176a1f9 into crossplane-contrib:master Jul 10, 2020

v0.13 automation moved this from PR open to Done Jul 10, 2020

This was referenced Aug 6, 2020

Expanding IAM support #151

Closed

Add IAMGroup as a custom resource #233

Closed

infinitecompute mentioned this pull request Aug 6, 2020

Additional AWS managed resources #149

Closed

26 tasks

wolffbe pushed a commit to wolffbe/provider-aws that referenced this pull request Feb 12, 2021

Merge pull request crossplane-contrib#249 from rahulchheda/iamGroup

58b8afc

Added AWS IAMGroup with UserMembership, and PolicyAttachment

namku pushed a commit to namku/provider-aws that referenced this pull request Mar 9, 2021

Merge pull request crossplane-contrib#249 from rahulchheda/iamGroup

a5f3a55

Added AWS IAMGroup with UserMembership, and PolicyAttachment

negz removed this from Done in v0.13 May 26, 2022

tektondeploy pushed a commit to gtn3010/provider-aws that referenced this pull request Mar 12, 2024

Merge pull request crossplane-contrib#249 from MyzaTaras/issue-182

0aee681

DynamoDB: add aws_dynamodb_tag resource

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added AWS IAMGroup with UserMembership, and PolicyAttachment #249

Added AWS IAMGroup with UserMembership, and PolicyAttachment #249

rahulchheda commented Jun 12, 2020 •

edited

Loading

sahil-lakhwani Jun 22, 2020

sahil-lakhwani Jun 22, 2020

sahil-lakhwani Jun 22, 2020

rahulchheda Jun 23, 2020

sahil-lakhwani Jun 22, 2020

sahil-lakhwani Jun 22, 2020

sahil-lakhwani Jun 22, 2020

sahil-lakhwani Jun 22, 2020

sahil-lakhwani Jun 22, 2020

sahil-lakhwani Jun 28, 2020

sahil-lakhwani Jun 28, 2020

negz commented Jul 1, 2020

negz commented Jul 1, 2020

rahulchheda commented Jul 2, 2020

sahil-lakhwani Jul 2, 2020

sahil-lakhwani Jul 2, 2020

sahil-lakhwani Jul 2, 2020

negz commented Jul 3, 2020

sahil-lakhwani commented Jul 6, 2020

muvaf left a comment

rahulchheda commented Jul 10, 2020

muvaf left a comment

	// It is encouraged to instead create a new IAMUserPolicyAttachment resource.
	// It is encouraged to instead create a new IAMUserGroupAttachment resource.

	// Updating any field will create a new User-Policy attachment in AWS, which will be
	// Updating any field will create a new User-Group attachment in AWS, which will be

Added AWS IAMGroup with UserMembership, and PolicyAttachment #249

Added AWS IAMGroup with UserMembership, and PolicyAttachment #249

Conversation

rahulchheda commented Jun 12, 2020 • edited Loading

Description of your changes

Checklist

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

negz commented Jul 1, 2020

negz commented Jul 1, 2020

rahulchheda commented Jul 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

negz commented Jul 3, 2020

sahil-lakhwani commented Jul 6, 2020

muvaf left a comment

Choose a reason for hiding this comment

rahulchheda commented Jul 10, 2020

muvaf left a comment

Choose a reason for hiding this comment

rahulchheda commented Jun 12, 2020 •

edited

Loading