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

Initial functionality #1

Merged
merged 26 commits into from
Sep 24, 2024
Merged

Initial functionality #1

merged 26 commits into from
Sep 24, 2024

Conversation

dav3r
Copy link
Member

@dav3r dav3r commented Sep 9, 2024

🗣 Description

This PR contains the first version of Terraform code to perform the initial configuration of a newly-created COOL Cyber Hygiene (CyHy) AWS account.

💭 Motivation and context

This is a necessary first step towards having a CyHy account that is a full-fledged member of the COOL.

NOTE: Per team consensus, I will rename this repo from cool-accounts-cyhy to cool-accounts-cyhy-tf-root after this PR is merged.

🧪 Testing

I applied this Terraform in a development environment and verified that it worked as intended.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated
    to reflect the changes in this PR.
  • All new and existing tests pass.

✅ Post-merge checklist

  • Rename this repo from cool-accounts-cyhy to cool-accounts-cyhy-tf-root

@dav3r dav3r added the improvement This issue or pull request will add new or improve existing functionality label Sep 9, 2024
@dav3r dav3r self-assigned this Sep 9, 2024
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

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

  • I think this account should also have the "disable inactive IAM users" jazz that I recently added to the COOL.
  • Don't forget that when you rename you will have to migrate the state (defined in backend.tf) over to the new key name.

user-group-mod-notification.tf Outdated Show resolved Hide resolved
@dav3r
Copy link
Member Author

dav3r commented Sep 13, 2024

  • I think this account should also have the "disable inactive IAM users" jazz that I recently added to the COOL.

This was done in the following commits:

@dav3r dav3r marked this pull request as ready for review September 13, 2024 19:31
@dav3r dav3r requested a review from a team September 13, 2024 19:31
Copy link
Member

@felddy felddy left a comment

Choose a reason for hiding this comment

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

Looks good. (I actually read it all.)
Strong work! 💪
:shipit:

README.md Outdated Show resolved Hide resolved
Copy link
Member

@felddy felddy left a comment

Choose a reason for hiding this comment

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

Even stronger! 🦾

Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Solid groundwork here 💪💪💪 Just a few questions for your consideration.

locals.tf Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
provisionssmsessionmanager_policy.tf Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
@dav3r dav3r requested a review from jsf9k September 23, 2024 18:13
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

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

You need to remove the unused variables from the README.md as well.

@dav3r dav3r force-pushed the first-commits branch 2 times, most recently from 2b8140d to 9c2d149 Compare September 23, 2024 21:30
dav3r and others added 3 commits September 23, 2024 17:31
This makes our bucket names unique and allows us to remove some local variables that are no longer needed.

Co-authored-by: Nicholas McDonnell <[email protected]>
Co-authored-by: Jeremy Frasier <[email protected]>
@dav3r dav3r merged commit 37221c4 into develop Sep 24, 2024
4 checks passed
@dav3r dav3r deleted the first-commits branch September 24, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement This issue or pull request will add new or improve existing functionality
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants