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: make the audit role template_url configurable #1

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

JonZeolla
Copy link

No description provided.

@@ -28,6 +28,7 @@ organization = {
disable_sso_management = false
deploy_audit_role = true
audit_role_name = "security-audit"
audit_role_template_url = "https://s3.amazonaws.com/pht-cloudformation/aws-account-automation/AuditRole-Template.yaml"
Copy link
Author

Choose a reason for hiding this comment

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

Even though it's not required, I figured it would still be a good idea to highlight that it's configurable in the example here; I'd be good removing it and using the default if you prefer.

@@ -112,8 +112,7 @@ organization = {
phone_number = "+14041234567"
}

organization_units = {

organizational_units = {
Copy link
Author

Choose a reason for hiding this comment

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

It seemed to make sense to rename this

@@ -85,7 +83,7 @@ git submodule add https://github.com/primeharbor/pht-account-configurator
git submodule init
git submodule update

EOF
<!--EOF
Copy link
Author

Choose a reason for hiding this comment

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

Since the below seemed to be a note to self, I figured not rendering it would make sense. Up to you

@@ -21,8 +21,6 @@ else
REPO=github.com/primeharbor/org-kickstart//modules/security_services?ref=$VERSION
fi

REPO=/Users/chris/AWS/org-kickstart/modules/security_services
Copy link
Author

@JonZeolla JonZeolla Jul 25, 2024

Choose a reason for hiding this comment

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

I assume this was an unintentional leave-behind from troubleshooting or maybe bootstrapping the project

./scripts/import-org.sh
./scripts/import_org.sh
Copy link
Author

Choose a reason for hiding this comment

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

Found a small typo

2. Comment out the `service_control_policies = {}` block
3. We will re-enable them after the first apply.
4. Delete the `security_services.tf` file on the first run.
1. **IMPORTANT**: Ensure that your `payer_name` and `payer_email` variables align with what you manually used when creating the first AWS account, otherwise
Copy link
Author

Choose a reason for hiding this comment

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

I feel like the importance of this one can't be understated; perhaps break it out to be even more obvious?

Comment on lines -59 to +58
parent_ou_id = "ou-yyyy-yyyyyyyy"
parent_ou_id = "Sandbox"
Copy link
Author

Choose a reason for hiding this comment

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

This (and the related changes below) is another important update, which aligns the docs with improvements made in d13c841 and c35332e

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.

1 participant