Skip to content

Conversation

@pahud
Copy link
Contributor

@pahud pahud commented Nov 12, 2025

Issue # (if applicable)

Closes #36017.

Reason for this change

OpenSearch Domain construct fails when using cross-account KMS keys for encryption at rest. The construct currently uses KMS Key ID (keyRef.keyId) which doesn't work for cross-account scenarios - OpenSearch requires the full ARN to properly identify and access keys from different accounts.

This blocks users from implementing cross-account encryption patterns, which are common in enterprise multi-account architectures.

Description of changes

Added intelligent KMS key identifier selection logic that chooses between Key ID and Key ARN based on the key's account and region:

  • Cross-account keys: Uses full ARN (enables previously broken scenarios)
  • Cross-region keys: Uses full ARN (additional support)
  • Same-account, same-region keys: Uses Key ID (maintains backward compatibility)

Implementation details:

  • Added selectKmsKeyIdentifier() private method to Domain class
  • Uses Stack.of() and Token.isUnresolved() for account/region comparison
  • Properly handles unresolved tokens (deploy-time values)
  • Modified line 1991 in domain.ts to use the new selection logic

This follows the same pattern used in the DynamoDB construct for encryption key handling.

Describe any new or updated permissions being added

N/A - No new IAM permissions required. This change only affects how the KMS key identifier is passed to CloudFormation.

Description of how you validated changes

  • Unit tests: Added 3 comprehensive test cases:

    1. Same-account KMS key usage (validates backward compatibility - uses Key ID)
    2. Cross-account KMS key usage (validates bug fix - uses ARN)
    3. Cross-region KMS key usage (validates edge case - uses ARN)
  • Integration tests: All 18 OpenSearch integration tests passed with UNCHANGED snapshots, confirming no breaking changes to existing scenarios

  • Test results: 1,709/1,709 unit tests passed (100% pass rate)

  • Backward compatibility: Verified via UNCHANGED integration test snapshots - existing same-account scenarios continue to use Key ID format

Checklist


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

…in encryption

- Add `selectKmsKeyIdentifier` method to dynamically select KMS key identifier
- Support cross-account and cross-region KMS keys by using full key ARN
- Maintain backward compatibility for same-account, same-region keys
- Add comprehensive test cases for different KMS key scenarios
- Enhance encryption at rest configuration with more robust key selection logic
This change improves the flexibility of KMS key configuration for OpenSearch domains by intelligently selecting the appropriate key identifier based on the key's origin and context.
- Add example for supplying custom KMS key for encryption at rest
- Include example of using cross-account KMS key for domain encryption
- Improve documentation clarity for encryption configuration
- Provide code snippets demonstrating key usage scenarios
@aws-cdk-automation aws-cdk-automation requested a review from a team November 12, 2025 00:46
@github-actions github-actions bot added bug This issue is a bug. effort/medium Medium work item – several days of effort p2 labels Nov 12, 2025
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Nov 12, 2025
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.

✅ A exemption request has been requested. Please wait for a maintainer's review.

@pahud pahud changed the title fix(aws-opensearchservice): use KMS Key ARN for cross-account encryption fix(opensearchservice): use KMS Key ARN for cross-account encryption Nov 12, 2025
@pahud
Copy link
Contributor Author

pahud commented Nov 12, 2025

Exemption Request - we have unit tests covered.

@aws-cdk-automation aws-cdk-automation added pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. pr/needs-maintainer-review This PR needs a review from a Core Team Member labels Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This issue is a bug. contribution/core This is a PR that came from AWS. effort/medium Medium work item – several days of effort p2 pr/needs-maintainer-review This PR needs a review from a Core Team Member pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(aws-opensearchservice): Domain construct should use KMS Key ARN instead of KeyId for cross-account reference

2 participants