Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for configuring Cloud Backup Snapshot Export Buckets to require private networking (PrivateLink) and updates associated resource implementation and contract-testing automation.
Changes:
- Introduces
RequirePrivateNetworkingas an optional boolean property in the resource schema, docs, and example template. - Updates the resource handler to use the newer Atlas SDK client and includes
RequirePrivateNetworkingin create/read/list model mapping. - Adds/updates contract-testing helper scripts and wires this resource into the GitHub Actions contract-testing workflow.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/cloud-backup-snapshot-export-bucket/CloudBackupSnapshotExportBucket.json | Example template now includes RequirePrivateNetworking. |
| cfn-resources/cloud-backup-snapshot-export-bucket/test/inputs_1_create.template.json | Test input template updated to include RequirePrivateNetworking. |
| cfn-resources/cloud-backup-snapshot-export-bucket/test/contract-testing/cfn-test-delete.sh | New wrapper script for deleting contract test resources. |
| cfn-resources/cloud-backup-snapshot-export-bucket/test/contract-testing/cfn-test-create.sh | New wrapper script for creating contract test resources. |
| cfn-resources/cloud-backup-snapshot-export-bucket/test/cfn-test-delete-inputs.sh | Refactors deletion logic to read created inputs and clean up Atlas + AWS resources. |
| cfn-resources/cloud-backup-snapshot-export-bucket/test/cfn-test-create-inputs.sh | Refactors creation logic for Atlas access role + AWS IAM role + S3 bucket and generates inputs. |
| cfn-resources/cloud-backup-snapshot-export-bucket/resource-role.yaml | Tightens execution role trust policy conditions and reduces policy actions. |
| cfn-resources/cloud-backup-snapshot-export-bucket/mongodb-atlas-cloudbackupsnapshotexportbucket.json | Schema updated to add RequirePrivateNetworking property. |
| cfn-resources/cloud-backup-snapshot-export-bucket/docs/README.md | Docs updated to document RequirePrivateNetworking and formatting tweaks. |
| cfn-resources/cloud-backup-snapshot-export-bucket/cmd/resource/resource.go | Handler switched to AtlasSDK client and passes/maps RequirePrivateNetworking. |
| cfn-resources/cloud-backup-snapshot-export-bucket/cmd/resource/model.go | Model updated to include RequirePrivateNetworking. |
| cfn-resources/cloud-backup-snapshot-export-bucket/cmd/resource/config.go | Newly generated type configuration stub. |
| cfn-resources/cloud-backup-snapshot-export-bucket/Makefile | Adds contract-testing helper targets and changes debug build output name. |
| .github/workflows/contract-testing.yaml | Adds change detection + contract test job for this resource. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "BucketName": "", | ||
| "Profile": "default" | ||
| "Profile": "default", | ||
| "RequirePrivateNetworking": "false" |
There was a problem hiding this comment.
RequirePrivateNetworking is defined as a boolean in the resource schema, but this template sets it to the string "false". This will produce an invalid inputs file (and can break contract tests / model unmarshalling). Set it to the JSON boolean false (no quotes).
| "RequirePrivateNetworking": "false" | |
| "RequirePrivateNetworking": false |
| region=$AWS_DEFAULT_REGION | ||
| awsRegion=$AWS_DEFAULT_REGION | ||
| if [ -z "$region" ]; then | ||
| region=$(aws configure get region) | ||
| awsRegion=$region | ||
| fi |
There was a problem hiding this comment.
With set -euo pipefail, assigning region=$AWS_DEFAULT_REGION / awsRegion=$AWS_DEFAULT_REGION will exit when AWS_DEFAULT_REGION is unset, so the later fallback to aws configure get region is unreachable. Use ${AWS_DEFAULT_REGION:-} (or similar) so the script can fall back cleanly.
| profile=${MONGODB_ATLAS_PROFILE} | ||
| fi | ||
|
|
||
| projectName="${1:-$PROJECT_NAME}" |
There was a problem hiding this comment.
projectName="${1:-$PROJECT_NAME}" will error under set -u if the script is invoked without a positional arg and PROJECT_NAME is unset. Either require an explicit argument (and print a usage message) or use a safe default expansion like ${PROJECT_NAME:-} before validating it's non-empty.
| projectName="${1:-$PROJECT_NAME}" | |
| projectName="${1:-${PROJECT_NAME:-}}" | |
| if [ -z "$projectName" ]; then | |
| echo "Usage: $0 <project-name> (or set PROJECT_NAME environment variable)" >&2 | |
| exit 1 | |
| fi |
| roleName="" | ||
| fi | ||
|
|
||
| keyRegion=$AWS_DEFAULT_REGION |
There was a problem hiding this comment.
With set -euo pipefail, keyRegion=$AWS_DEFAULT_REGION will exit if AWS_DEFAULT_REGION is unset, so the fallback to aws configure get region won't run. Assign from ${AWS_DEFAULT_REGION:-} (or guard with ${VAR-}) so the script works when only AWS CLI config is present.
| keyRegion=$AWS_DEFAULT_REGION | |
| keyRegion=${AWS_DEFAULT_REGION:-} |
| debug: | ||
| cfn generate | ||
| env GOOS=$(goos) CGO_ENABLED=$(cgo) GOARCH=$(goarch) go build -ldflags="$(ldXflagsD)" -tags="$(tags)" -o bin/bootstrap cmd/main.go | ||
| env GOOS=$(goos) CGO_ENABLED=$(cgo) GOARCH=$(goarch) go build -ldflags="$(ldXflagsD)" -tags="$(tags)" -o bin/debug cmd/main.go |
There was a problem hiding this comment.
The debug target builds to bin/debug, but the CloudFormation RPDK tooling (and repo helper scripts like cfn-submit-helper.sh / cfn-testing-helper.sh) expect the executable at bin/bootstrap even for debug builds. Consider keeping the output path as bin/bootstrap and only changing the ldflags/log level for debug.
| env GOOS=$(goos) CGO_ENABLED=$(cgo) GOARCH=$(goarch) go build -ldflags="$(ldXflagsD)" -tags="$(tags)" -o bin/debug cmd/main.go | |
| env GOOS=$(goos) CGO_ENABLED=$(cgo) GOARCH=$(goarch) go build -ldflags="$(ldXflagsD)" -tags="$(tags)" -o bin/bootstrap cmd/main.go |
| @@ -0,0 +1,19 @@ | |||
| // Code generated by 'cfn generate', changes will be undone by the next invocation. DO NOT EDIT. | |||
| // Updates to this type are made my editing the schema file and executing the 'generate' command. | |||
There was a problem hiding this comment.
Typo in generated file header: "made my editing" should be "made by editing".
| // Updates to this type are made my editing the schema file and executing the 'generate' command. | |
| // Updates to this type are made by editing the schema file and executing the 'generate' command. |
|
This PR has gone 30 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 30 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy! |
Proposed changes
Jira ticket: CLOUDP-#
Please include a summary of the fix/feature/change, including any relevant motivation and context.
Link to any related issue(s):
Type of change:
expected)
Manual QA performed:
Required Checklist:
make fmtand formatted my codeworks in Atlas
Further comments