Skip to content

Conversation

@infraredgirl
Copy link
Contributor

@infraredgirl infraredgirl commented Jan 13, 2021

@netlify
Copy link

netlify bot commented Jan 13, 2021

Deploy preview for keen-clarke-470db9 ready!

Built with commit 6e2d196

https://deploy-preview-392--keen-clarke-470db9.netlify.app

@ina-stoyanova
Copy link
Contributor

Great start @infraredgirl 🎉

I've added some bits to the sections you mentioned me in for the iam_access_analyzer - these probs need reviewing if they make sense, but I hope is a good start.

Copy link
Contributor Author

@infraredgirl infraredgirl left a comment

Choose a reason for hiding this comment

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

Awesome, thanks @ina-stoyanova. Looks great, I added a couple of comments too.

@marinalimeira
Copy link
Contributor

Just pushed the 5.1 related documentation at 8dd1d15

Co-authored-by: Ina Stoyanova <ina@gruntwork.io>
@rhoboat
Copy link
Contributor

rhoboat commented Jan 19, 2021

This section is really long now: https://github.com/gruntwork-io/gruntwork-io.github.io/blob/ca410b41956807bc9f2cb1b71c9fbe2f839eb1ef/_posts/2019-10-17-how-to-achieve-cis-benchmark-compliance.adoc#deployment-walkthrough I wonder if it would help to have a mini-TOC right here, under this header with all the subsections linked?

Also wanted to say, this is looking great!

@robmorgan
Copy link
Contributor

This section is really long now: https://github.com/gruntwork-io/gruntwork-io.github.io/blob/ca410b41956807bc9f2cb1b71c9fbe2f839eb1ef/_posts/2019-10-17-how-to-achieve-cis-benchmark-compliance.adoc#deployment-walkthrough I wonder if it would help to have a mini-TOC right here, under this header with all the subsections linked?

Also wanted to say, this is looking great!

@rhoboat Similar to my other point, do you think the left side-bar negates the need for an additional ToC?

@robmorgan
Copy link
Contributor

I've added a few new comments. Aside from the feedback, it looks like the traceability matrix is the last thing to complete and then we're done.

infraredgirl and others added 3 commits January 25, 2021 12:42
@infraredgirl
Copy link
Contributor Author

I believe we are ready to open this up for approvals!

@infraredgirl infraredgirl changed the title [WIP] Update the CIS AWS guide for version 1.3.0 Update the CIS AWS guide for version 1.3.0 Jan 25, 2021
link:https://github.com/gruntwork-io/terraform-aws-cis-service-catalog/tree/master/modules/vpc-app-network-acls[`vpc-app-network-acls`
module] and your NACL Rules for the VPC-Mgmt (which is being deprecated) using the
link:https://github.com/gruntwork-io/terraform-aws-cis-service-catalog/tree/master/modules/vpc-mgmt-network-acls[`vpc-mgmt-network-acls`
module].
Copy link
Member

Choose a reason for hiding this comment

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

Code examples plz!

Copy link
Contributor

Choose a reason for hiding this comment

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

@brikis98 @marinalimeira This one is a bit tricky... For the code examples, we need to:

  1. Possibly link to this section: https://www.gruntwork.io/guides/compliance/how-to-achieve-cis-benchmark-compliance/#wrapper_modules
  2. Explain how we extend the vpc-app and vpc-mgmt modules by creating new ones in infrastructure-modules.
  3. Explain how in the new vpc-app and vpc-mgmt modules we need to invoke the vpc-mgmt-network-acls module.
  4. Explain how to create a terragrunt.hcl file that invokes our new VPC modules in infrastructure-modules.

Is there an easier way to do this? Could we just do the last step?

Copy link
Member

Choose a reason for hiding this comment

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

I think the real solution is to add a VPC service to the CIS Service Catalog, which would include vpc-app and vpc-app-network-acls, and deploy that with one terragrunt.hcl. We have the ticket for this already: https://gruntwork.atlassian.net/browse/IAC-1643... But it's not part of the CIS v1.3 project.

So I guess it's a trade-off between:

  1. Updating this guide with a bunch of steps that will take some time to put together and will be deprecated soon.
  2. Doing the VPC service work now, and updating the guide to use that. But note that the VPC service would have to extend the existing VPC service in terraform-aws-service-catalog, and not all our CIS customers have access to that yet, so that would be yet another yak to shave.
  3. Doing some minimal version of (1). That is, we say vaguely something like, "You should use the vpc-app-network-acls module to meet the new CIS v1.3 requirements. Check out our VPC guide for how to deploy a VPC with the vpc-app module. Once that's deployed, add the vpc-app-network-acls module as follows: . Deploy all this using Terragrunt: ."

It's not ideal, but (3) is probably the way to go, and we'll replace it fairly soon when the CIS Service Catalog work is done.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if I am following... What is the difference between (2) and the cis-infra-live-acme + cis-infra-modules?

Copy link
Member

Choose a reason for hiding this comment

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

The old way:

  1. Customer has an infra-modules repo with a bunch of Terraform code in it.
  2. Customers' infra-live repo sets the source URL to their own infra-modules repo.

The new way:

  1. Gruntwork has an service-catalog repo with a bunch of Terraform code in it.
  2. Customers' infra-live repo sets the source URL to Gruntwork's service-catalog repo.

The CIS Service Catalog project is moving us to the new way for CIS... But we're not there yet. So we're trying to decide if we want to do the work of showing customers the "old way" for this VPC NACL stuff, only to throw it away a little later (option 1)... Or if we can go straight to the new way (option 2). Or some minimal version of the old way, as a placeholder until we finish the Service Catalog stuff (option 3).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd vouch with respect for time and simplicity, that we go with the old way. And then as part of CIS Service Catalog work, we come back here to update this.

Copy link
Contributor

@marinalimeira marinalimeira Feb 2, 2021

Choose a reason for hiding this comment

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

I went to the Updating this guide with a bunch of steps that will take some time to put together and will be deprecated soon. direction!

infraredgirl and others added 5 commits January 26, 2021 16:22
Co-authored-by: Yevgeniy Brikman <brikis98@users.noreply.github.com>
- Use NOTE block to point to the upgrade guide
- Link to appropriate deployment walkthrough sections instead of private repos
- Add link to the s3-bucket service as usage example of private-s3-bucket module
brikis98
brikis98 previously approved these changes Feb 3, 2021
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

OK, some minor wording / typo issues to fix, and this is good to go!

To enable IAM Access Analyzer in your AWS Account or Organization, you need to do it separately for every region.

To enable this for your AWS account:
* use the examples provided as part of `learning-and-testing/landingzone`:
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Actually, this whole sentence should be removed, as CIS customers should NOT be using the acccount-baseline modules, as those don't take into account CIS requirements. Instead, they should solely use the iam-access-analyzer module as shown below.

Copy link
Contributor

Choose a reason for hiding this comment

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

ahaa! noted!

Copy link
Contributor

Choose a reason for hiding this comment

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

removed :) good shout!

Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Ship it!

@ina-stoyanova
Copy link
Contributor

Thanks Jim!

@ina-stoyanova ina-stoyanova merged commit 43975fd into master Feb 3, 2021
@ina-stoyanova ina-stoyanova deleted the cis-guide branch February 3, 2021 13:29
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.

7 participants