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.

Sign up for GitHub

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

Jump to bottom

Adding policies #11

Merged
merged 50 commits into from
Mar 24, 2022
Merged

Adding policies #11

merged 50 commits into from
Mar 24, 2022

Conversation

jamengual
Copy link
Contributor

what

  • Adding Aditional SCPs to the catalog
  • Adding Parameters
  • Updating example

why

  • Adding AWS recommended SCPs plus some additional SCPs to improve security compliance

references

@jamengual jamengual requested review from a team as code owners February 26, 2021 19:53
@jamengual
Copy link
Contributor Author

/test all

@jamengual
Copy link
Contributor Author

/test all

osterman
osterman previously approved these changes Feb 26, 2021
View reviewed changes
@jamengual
Copy link
Contributor Author

/test all

@jamengual
Copy link
Contributor Author

/test all

aknysh
aknysh reviewed Feb 26, 2021
View reviewed changes
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

please see comments

jamengual and others added 2 commits February 26, 2021 12:46
@jamengual @aknysh
Update catalog/sagemaker-policies.yaml
46bacd1 
Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
@jamengual
Fixing SCPs
974569b
@jamengual
Copy link
Contributor Author

/test all

@jamengual
Copy link
Contributor Author

/test all

@jamengual
Copy link
Contributor Author

/test all

@mergify
Copy link

mergify bot commented Aug 24, 2021

This pull request is now in conflict. Could you fix it @jamengual? 🙏

@jamengual
Copy link
Contributor Author

/test all

@jamengual jamengual requested a review from aknysh March 23, 2022 17:12
aknysh
aknysh reviewed Mar 23, 2022
View reviewed changes
catalog/cloudtrail-policies.yaml Outdated
- "cloudtrail:PutEventSelectors"
- "cloudtrail:StopLogging"
- "cloudtrail:UpdateTrail"

Copy link
Member

Choose a reason for hiding this comment

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

need

resources:
    - "*"

also, can you maybe combine these two policies?
or clean them up since they gave overlapping actions

aknysh
aknysh previously requested changes Mar 23, 2022
View reviewed changes
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

@jamengual thank you, please see comments

@mergify mergify bot dismissed aknysh’s stale review March 23, 2022 18:58

This Pull Request has been updated, so we're dismissing all reviews.

@aknysh
Copy link
Member

aknysh commented Mar 23, 2022

/test all

aknysh
aknysh reviewed Mar 23, 2022
View reviewed changes
catalog/lambda-policies.yaml
@@ -0,0 +1,12 @@
- sid: "DenyLambdaWithoutVpc"
Copy link
Member

Choose a reason for hiding this comment

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

@jamengual is this policy correct?

  1. It says "Deny" but the value is true - should it be "false"?
  2. Other policies use "Bool" in the test, not "Null"
condition:
    - test: "Bool"
      variable: "rds:StorageEncrypted"
      values:
        - false

aknysh
aknysh reviewed Mar 23, 2022
View reviewed changes
catalog/s3-policies.yaml Outdated
%{ for r in split(",", s3_regions_lockdown) }
- ${trimspace(r)}
%{ endfor }
- test: "Null"
Copy link
Member

Choose a reason for hiding this comment

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

should it be "Bool"?

aknysh
aknysh reviewed Mar 23, 2022
View reviewed changes
catalog/vpc-policies.yaml Outdated
- "ec2:CreateVpcPeeringConnection"
- "ec2:AcceptVpcPeeringConnection"
- "globalaccelerator:Create*"
- "globalaccelerator:Update*"
Copy link
Member

Choose a reason for hiding this comment

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

why it's related to creating/updating of globalaccelerator?
Global Accelerator itself does not need a VPC

aknysh
aknysh reviewed Mar 23, 2022
View reviewed changes
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

@jamengual please see comments

@jamengual
Copy link
Contributor Author

/test all

@aknysh aknysh merged commit f9fa883 into master Mar 24, 2022
@mergify mergify bot deleted the adding_policies branch March 24, 2022 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@osterman osterman osterman left review comments

@maartenvanderhoef maartenvanderhoef maartenvanderhoef left review comments

@aknysh aknysh aknysh approved these changes

@mcalhoun mcalhoun mcalhoun left review comments

@joe-niland joe-niland Awaiting requested review from joe-niland

@woz5999 woz5999 Awaiting requested review from woz5999

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jamengual @aknysh @osterman @maartenvanderhoef @mcalhoun @cloudpossebot