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: Move parameters from secrets manager to ssm #455

Merged
merged 5 commits into from
May 17, 2023

Conversation

dlpzx
Copy link
Contributor

@dlpzx dlpzx commented May 15, 2023

Feature or Bugfix

  • Refactoring

Detail

Replaced externalId and PivotRoleName secrets in SecretsManager by SSM parameters:

  • modified parameter_stack and secrets_stack to create the parameters and remove the secrets.
  • the new externalId parameter uses a pre-existing secret if it exists, otherwise it generates a random sequence of numbers and letters
  • modified the sts handler to retrieve the pivotRoleName and externalId from SSM

Relates

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@dlpzx dlpzx marked this pull request as ready for review May 16, 2023 07:04
@dlpzx
Copy link
Contributor Author

dlpzx commented May 16, 2023

For this feature I have tested 2 scenarios:

  • Scenario 1: deployment from scratch -> when using this branch directly to deploy data.all to a clean account, a random externalId is generated and a parameter with this value is stored in SSM Parameter Store.
  • Scenario 2: backwards compatibility for pre-existing secret in Secrets Manager -> when upgrading to this branch, the pre-existing secret value is used to assign a value to the new parameter in SSM Parameter Store and then the secret is deleted.

For both I have performed several actions in the deployed UI:

  • - List objects in Catalog --> ensure correct configuration of OpenSearch and Lambda
  • - List objects in any other view --> ensure correct configuration of GraphQL API
  • - Link environment and Create Dataset --> ensure correct configuration of cdkproxy
  • - Crawl and sync tables --> ensure correct configuration of short async tasks (IMPORTANT they use the pivotRole and externalId)
  • - Table sharing --> ensure correct configuration of sharing task (IMPORTANT they use the pivotRole and externalId)

@dlpzx dlpzx requested review from noah-paige and nikpodsh May 17, 2023 06:20
@dlpzx dlpzx closed this May 17, 2023
@dlpzx dlpzx deleted the move-parameters-from-secrets-manager-to-ssm branch May 17, 2023 06:39
@dlpzx dlpzx restored the move-parameters-from-secrets-manager-to-ssm branch May 17, 2023 06:44
@dlpzx dlpzx reopened this May 17, 2023
@dlpzx dlpzx changed the base branch from main to v1m6m0 May 17, 2023 12:28
@dlpzx dlpzx merged commit 99c28cd into v1m6m0 May 17, 2023
@dlpzx dlpzx changed the title Move parameters from secrets manager to ssm feat: Move parameters from secrets manager to ssm May 24, 2023
@dlpzx dlpzx mentioned this pull request Jul 11, 2023
@dlpzx dlpzx deleted the move-parameters-from-secrets-manager-to-ssm branch July 14, 2023 10:19
dlpzx added a commit that referenced this pull request Jul 19, 2023
### Feature or Bugfix
Release PR with the following list of features. Refer to each PR for the
details

### Detail
- #498 
- #482 
- #543
- #524 (which also solves #531)
- #532 
- #535 
- #497 
- #515
- #529 
- #562 
- #455 
- #572 
- #567 
- #573 
- #579 
- #578 
- #582 

### Breaking changes - release notes
- ⚠️ IMPORTANT: upgrade to a version >V1.5.0 before upgrading to V1.6 to
avoid deletion of resources in custom resource deletion
- ⚠️ IMPORTANT: requires an update of environments and then datasets
after upgrading. Either using cdk.json parameter
`enable_update_dataall_stacks_in_cicd_pipeline`, waiting for overnight
update stack task, or manually updating first environments and then
datasets
- CloudFront distribution replace for #529 
- Additional EC2 permissions in CDK Synth CodeBuild stage for #543 -->
this can be avoided by upgrading to v1.5.6 before upgrading to v1.6.0
- local development affected by more restrictive pivotRole trust policy


### Relates
V1.6.0 release

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Co-authored-by: Gezim Musliaj <102723839+gmuslia@users.noreply.github.com>
Co-authored-by: Noah Paige <69586985+noah-paige@users.noreply.github.com>
Co-authored-by: nikpodsh <124577300+nikpodsh@users.noreply.github.com>
Co-authored-by: chamcca <40579012+chamcca@users.noreply.github.com>
Co-authored-by: Nikita Podshivalov <nikpodsh@amazon.com>
Co-authored-by: dbalintx <132444646+dbalintx@users.noreply.github.com>
Co-authored-by: mourya-33 <134511711+mourya-33@users.noreply.github.com>
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.

Encrypted Secrets in secret manager should be rotated atleast annually and encrypted with a KMS key
2 participants