feat(aws): Add support for externalId in AssumeRole for TriggerAuthentication#7388
feat(aws): Add support for externalId in AssumeRole for TriggerAuthentication#7388Sahar23391 wants to merge 4 commits intokedacore:mainfrom
Conversation
|
Thank you for your contribution! 🙏 Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected. While you are waiting, make sure to:
Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient. Learn more about our contribution guide. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
1b9cda6 to
b9a0edc
Compare
…tication This change adds support for passing an external ID when KEDA assumes an AWS role via the TriggerAuthentication resource. The external ID can be specified using the annotation: keda.sh/aws-role-external-id: <your-external-id> The external ID is then passed to AWS STS during the AssumeRole API call, which is required when assuming roles that have an external ID condition in their trust policy. Changes: - Add AwsRoleExternalIDAnnotation constant - Add AwsRoleExternalID field to AuthorizationMetadata - Update getTriggerAuthSpec to return annotations - Update resolveAuthRef to extract external ID from annotations - Update GetAwsAuthorization to parse external ID from authParams - Update cache key generation to include external ID - Update retrievePodIdentityCredentials to pass ExternalID when assuming role - Update GetAwsConfig to pass ExternalID (deprecated path) Signed-off-by: sahar23391 <sahar@rocksteady.io>
b9a0edc to
b849499
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for specifying an external ID when KEDA assumes AWS IAM roles via TriggerAuthentication resources. The external ID is a security best practice for cross-account role assumption that helps prevent the "confused deputy" problem. Users can now specify the external ID using the annotation keda.sh/aws-role-external-id on their TriggerAuthentication or ClusterTriggerAuthentication resources.
Changes:
- Added new annotation constant
AwsRoleExternalIDAnnotationfor specifying external IDs in TriggerAuthentication - Updated authentication resolution logic to extract and pass external IDs from annotations to AWS authorization metadata
- Modified AWS credential caching to include external ID in cache keys, ensuring different external IDs create separate cache entries
- Updated AssumeRole API calls to include the external ID parameter when provided
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apis/keda/v1alpha1/triggerauthentication_types.go | Added AwsRoleExternalIDAnnotation constant to define the annotation key for specifying external IDs |
| pkg/scaling/resolver/scale_resolvers.go | Modified getTriggerAuthSpec to return annotations and resolveAuthRef to extract external ID from annotations into authParams |
| pkg/scalers/aws/aws_authorization.go | Added AwsRoleExternalID field to AuthorizationMetadata struct to store the external ID |
| pkg/scalers/aws/aws_common.go | Updated GetAwsAuthorization to parse external ID from authParams and GetAwsConfig to pass external ID to AssumeRole provider |
| pkg/scalers/aws/aws_config_cache.go | Modified cache key generation to include external ID and updated retrievePodIdentityCredentials to pass external ID to AssumeRole API |
| CHANGELOG.md | Added entry documenting the new external ID support feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if roleExternalID != "" { | ||
| options.ExternalID = &roleExternalID | ||
| } |
There was a problem hiding this comment.
The external ID is being applied to the AssumeRole credential provider (lines 170-172). However, this function also uses a WebIdentityRole credential provider as the primary method (before this code, at lines 152-162). If web identity token authentication is being used, the external ID will be ignored. Consider whether the external ID should also be passed to the WebIdentityRoleProvider for consistency, especially if users are using OIDC federation with cross-account access that requires an external ID. The AWS SDK's WebIdentityRoleOptions also supports ExternalID.
There was a problem hiding this comment.
If something is resolved, please also provide an explanation as to why something does or does not need to be adjusted.
| - **General**: Correct error message when awsSecretAccessKey is missing in credential-based authentication ([#7265](https://github.com/kedacore/keda/pull/7265)) | ||
| - **General**: Raw metrics stream - include trigger activity status in response ([#7369](https://github.com/kedacore/keda/issues/7369)) | ||
| - **AWS CloudWatch Scaler**: Add cross-account observability support ([#7189](https://github.com/kedacore/keda/issues/7189)) | ||
| - **AWS Scalers**: Add support for `externalId` in AssumeRole via TriggerAuthentication annotation `keda.sh/aws-role-external-id` ([#XXX](https://github.com/kedacore/keda/issues/XXX)) |
There was a problem hiding this comment.
The CHANGELOG entry is missing the issue number. Please replace ([#XXX](https://github.com/kedacore/keda/issues/XXX)) with the actual issue or pull request number that this change addresses.
| - **AWS Scalers**: Add support for `externalId` in AssumeRole via TriggerAuthentication annotation `keda.sh/aws-role-external-id` ([#XXX](https://github.com/kedacore/keda/issues/XXX)) | |
| - **AWS Scalers**: Add support for `externalId` in AssumeRole via TriggerAuthentication annotation `keda.sh/aws-role-external-id` (TBD) |
| if val, ok := authParams["awsRoleExternalId"]; ok && val != "" { | ||
| meta.AwsRoleExternalID = val | ||
| } |
There was a problem hiding this comment.
No tests have been added to verify the external ID functionality. The codebase has existing test coverage for AWS authorization (e.g., in pkg/scalers/aws/aws_config_cache_test.go and pkg/scalers/kafka_scaler_test.go). Consider adding tests to verify:
- Cache key generation includes the external ID (to ensure different external IDs create different cache entries)
- GetAwsAuthorization correctly parses the awsRoleExternalId from authParams
- The external ID is properly passed to the AssumeRole API call
- The annotation extraction in resolveAuthRef works correctly
There was a problem hiding this comment.
If something is resolved, please also provide an explanation as to why something does or does not need to be adjusted.
Document the keda.sh/aws-role-external-id annotation for cross-account AssumeRole with external ID support. Relates to kedacore/keda#7388 Signed-off-by: sahar23391 <sahar@rocksteady.io>
| podIdentity = *triggerAuthSpec.PodIdentity | ||
| } | ||
| // Extract AWS role external ID from annotations if present | ||
| if externalID, ok := triggerAuthAnnotations[kedav1alpha1.AwsRoleExternalIDAnnotation]; ok && externalID != "" { |
There was a problem hiding this comment.
I'd not use annotation for this but a field in the spec. It's more aligned with current configurations. WDYT @kedacore/keda-core-maintainers @kedacore/keda-core-contributors
Description
This PR adds support for passing an external ID when KEDA assumes an AWS role via the TriggerAuthentication resource.
The external ID can be specified using the annotation:
The external ID is then passed to AWS STS during the AssumeRole API call, which is required when assuming roles that have an external ID condition in their trust policy. This is a common security pattern for cross-account access.
Changes
AwsRoleExternalIdAnnotationconstant (keda.sh/aws-role-external-id)AwsRoleExternalIdfield toAuthorizationMetadatagetTriggerAuthSpecto return annotationsresolveAuthRefto extract external ID from annotationsGetAwsAuthorizationto parse external ID from authParamsretrievePodIdentityCredentialsto pass ExternalID when assuming roleGetAwsConfigto pass ExternalID (deprecated code path)Checklist
make generate-scalers-schemahas been run to update any outdated generated files.Fixes #
Relates to #