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

feat: Added iam-read-only-policy module #174

Merged
merged 12 commits into from
Jan 3, 2022

Conversation

Andrey9kin
Copy link
Contributor

Description

Added new module to create customizable read only policy

Motivation and Context

  • As stated in the readme: Creates IAM read-only policy for specified services. Default AWS read-only policies (arn:aws:iam::aws:policy/job-function/ViewOnlyAccess, arn:aws:iam::aws:policy/ReadOnlyAccess), being a one-size-fits-all type of policies, have a lot of things missing as well as something that you might not need. Also, AWS default policies are known for having security issues
    Thus this module is an attempt to build a better base for a customizable usable read-only policy.
  • Module requires a list of allowed services which forces you to compile such a list which will be very useful if you are to create an SCP to forbid usage of not allowed services.
  • Most of our customers asked for something like that so their developers can login to prod with proper read only access and we had a number of requests to have such module available as open source
  • We could open source it under fivexl org but here feels like a better place. Hopefully more people will be able to take advantage of it and perhaps contribute back. So win-win all around

Breaking Changes

A new thing. Does not break anything

How Has This Been Tested?

  • I have tested and validated these changes using one or more of the provided examples/* projects
  • Battle-tested in a several projects

Signed-off-by: Andrey Devyatkin <andrey.devyatkin@fivexl.io>
@Andrey9kin
Copy link
Contributor Author

@antonbabenko could you please take a look when you have time or ask someone to do so. Thanks!

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Please add relevant updates in the main README in the "Usage" and "Examples" sections.

)
}

module "iam_policy" {
Copy link
Member

Choose a reason for hiding this comment

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

Add another example of module call with create_policy = false and update variables defaults accordingly.

examples/iam-read-only-policy/main.tf Show resolved Hide resolved
@@ -0,0 +1,7 @@
terraform {
required_version = ">= 0.15.0"
Copy link
Member

Choose a reason for hiding this comment

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

Please set >= 0.12.6 as on all other modules. We will bump to >= 0.13.1 in all modules in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

examples/iam-read-only-policy/versions.tf Show resolved Hide resolved
modules/iam-read-only-policy/README.md Outdated Show resolved Hide resolved
modules/iam-read-only-policy/variables.tf Outdated Show resolved Hide resolved
}

locals {
console_services = ["resource-groups", "tag", "health"]
Copy link
Member

Choose a reason for hiding this comment

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

Let's move these from locals to separate variable defaults to allow customizations (if necessary).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

modules/iam-read-only-policy/variables.tf Outdated Show resolved Hide resolved
@@ -0,0 +1,7 @@
terraform {
required_version = ">= 0.15.0"
Copy link
Member

Choose a reason for hiding this comment

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

Same version requirements here as in the example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

examples/iam-read-only-policy/README.md Outdated Show resolved Hide resolved
@RafPe
Copy link
Contributor

RafPe commented Dec 28, 2021

@Andrey9kin any update on completing this resource ? I can see potential on using it.

@Andrey9kin
Copy link
Contributor Author

@RafPe will be looking into it tonight! Got few more people asking for it as well

Andrey9kin and others added 7 commits December 29, 2021 21:53
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
Signed-off-by: Andrey Devyatkin <andrey.devyatkin@fivexl.io>
Signed-off-by: Andrey Devyatkin <andrey.devyatkin@fivexl.io>
@Andrey9kin
Copy link
Contributor Author

@antonbabenko sorry for keeping it hanging but end of the year was kind of crazy. Thanks for the great review comments. When you are back from holidays please take a look. Thanks!

Signed-off-by: Andrey Devyatkin <andrey.devyatkin@fivexl.io>
}

data "aws_iam_policy_document" "allowed_services" {

Copy link
Member

Choose a reason for hiding this comment

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

Normally, in other terraform-aws-modules, nothing is being created if create_* = false is defined. This way is compatible with situations when users want to control the creation of all or nothing (e.g. still handy with terragrunt).

I see what you mean in this example. Let's leave it as you have done already. I think it makes sense here.

modules/iam-read-only-policy/versions.tf Outdated Show resolved Hide resolved
examples/iam-read-only-policy/README.md Outdated Show resolved Hide resolved
@antonbabenko antonbabenko changed the title feat: new module iam-read-only-policy feat: Added iam-read-only-policy module Jan 3, 2022
@antonbabenko antonbabenko merged commit 392756f into terraform-aws-modules:master Jan 3, 2022
antonbabenko pushed a commit that referenced this pull request Jan 3, 2022
# [4.8.0](v4.7.0...v4.8.0) (2022-01-03)

### Bug Fixes

* update CI/CD process to enable auto-release workflow ([#175](#175)) ([9278e6f](9278e6f))

### Features

* Added iam-read-only-policy module ([#174](#174)) ([392756f](392756f))
@antonbabenko
Copy link
Member

This PR is included in version 4.8.0 🎉

@github-actions
Copy link

github-actions bot commented Nov 9, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants