Capture the created role arn for snapshotting into the AWS yaml parameter and pass it through the command line #1133
Conversation
gregschohn
requested review from
AndreKurait,
chelma,
lewijacn,
mikaylathompson,
peternied and
sumobrian
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1133 +/- ##
=========================================
Coverage 80.71% 80.71%
Complexity 2947 2947
=========================================
Files 399 399
Lines 14962 14962
Branches 1017 1017
=========================================
Hits 12077 12077
Misses 2274 2274
Partials 611 611
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -23,6 +23,7 @@ | |||
| 'schema': { | |||
There was a problem hiding this comment.
Ideally there would be two tests for this:
- one on the python side (if
roleis in the services.yaml, it should be included in the call to the java snapshot tool, a la this test and the ones nearby) - and one on the CDK side, where we set the
managedServiceSourceSnapshotEnabledand make sure the policy gets created / added to the YAML (kind of like this one.
The python test is a low lift with decent helpful value to effectively document how role should effect the rest of the system. The CDK test is a much higher lift that should have been done in my previous PR, but wasn't. And is potentially going to be replaced in the near-ish future.
So I'd really like the python test, the CDK test is optional.
| @@ -23,6 +23,7 @@ | |||
| 'schema': { | |||
| 'repo_uri': {'type': 'string', 'required': True}, | |||
| 'aws_region': {'type': 'string', 'required': True}, | |||
| 'role': {'type': 'string', 'required': False} | |||
There was a problem hiding this comment.
| createSnapshotOnAOSRole(this, artifactS3Arn, serviceTaskRole.roleArn, | ||
| this.region, props.stage, props.defaultDeployId) | ||
| .roleArn; | ||
| // HOW DO I GET THIS servicesYaml into the parameter AND get the service created? |
There was a problem hiding this comment.
I'm not sure what this question is -- what do you mean by "get the service created"?
…eter and pass it through the command line Signed-off-by: Greg Schohn <greg.schohn@gmail.com>
…vices yaml file. Signed-off-by: Greg Schohn <greg.schohn@gmail.com>
Signed-off-by: Greg Schohn <greg.schohn@gmail.com>
gregschohn
force-pushed
the
PassRoleParameterForS3Snapshots
branch
from
aa76e48 to
00c715d
Compare
Description
Specifying 'managedServiceSourceSnapshotEnabled' in the cdk.context.json will now not just create the role for making a snapshot to s3, but it will add it to the services yaml file and the console will thread that through to the command line for snapshot create so that users don't have to.
Issues Resolved
https://opensearch.atlassian.net/browse/MIGRATIONS-2001
Is this a backport? If so, please add backport PR # and/or commits #
Testing
Manual deployments w/ and without the cdk.context.json value and w/ and w/out the value in migration_services.yaml
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.