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

aws resource: aws_iam_group_membership and resource.SetImporter #116

Merged
merged 3 commits into from
Jun 19, 2020

Conversation

talset
Copy link
Member

@talset talset commented Jun 12, 2020

provider resource: implement SetImporter to set schema.Resource.Importer when resource is not importable.

aws resource: aws_iam_group_membership

@xescugc
Copy link
Member

xescugc commented Jun 12, 2020

Can you rebase this branch with the latest master? We have the big update on dependencies and I think this will get affected.

@xescugc
Copy link
Member

xescugc commented Jun 17, 2020

CI is still failing and for what I see you did not rebase with master yet.

@talset
Copy link
Member Author

talset commented Jun 17, 2020

CI is still failing and for what I see you did not rebase with master yet.

Didn't had time to check the ci yet (this is why I set the in progress label). But the rebase have been done.
As you can see the latest commit on master is 3e12f8d3b6ae9fcc1e6faa726f54a8ca0da43c5d and are present in my branch (https://github.com/cycloidio/terracognita/commits/fl-iamgroupmemberships)

@@ -1083,6 +1110,10 @@ func iamUsers(ctx context.Context, a *aws, resourceType string, filters *filter.
}

func iamUserGroupMemberships(ctx context.Context, a *aws, resourceType string, filters *filter.Filter) ([]provider.Resource, error) {
// if both aws_iam_group_membership and aws_iam_user_group_membership defined, keep only aws_iam_group_membership
if filters.IsIncluded("aws_iam_group_membership") && (!filters.IsExcluded("aws_iam_group_membership")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This condition is weird ?
We check if the aws_iam_group_membership is included and if it's not excluded (so included) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well this is something that I could check in the IsIncluded function.
But currently this function only check if all or a dedicated resource is included, not if the resource is not in exclude.

So we need to check if a user set include aws_iam_group_membership and exclude aws_iam_group_membership. If both are set, the resource will not be imported

@xescugc
Copy link
Member

xescugc commented Jun 18, 2020

I commented to @talset to write a TODO for that specific case and open an issue to track the actual issue opened on the TF provider. So we can rely on it and remove this logic once the TF provider supports it.

@talset talset force-pushed the fl-iamgroupmemberships branch from 29780b5 to 10233d0 Compare June 19, 2020 08:30
@talset talset force-pushed the fl-iamgroupmemberships branch from 10233d0 to 5d12588 Compare June 19, 2020 09:45
@talset talset merged commit 09f64c4 into master Jun 19, 2020
@talset talset deleted the fl-iamgroupmemberships branch June 19, 2020 10:18
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.

4 participants