feat: support ExternalId in AWS AssumeRole for cross-account isolation#7665
feat: support ExternalId in AWS AssumeRole for cross-account isolation#7665shivkumr wants to merge 8 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. |
|
@JorTurFer @rickbrouwer @tangobango5 This adds ExternalId support to AWS AssumeRole for cross-account confused deputy protection. Fixes #7662 and #6921. I noticed the existing PR #6916 by @tangobango5 for the same feature. A few differences in this PR:
Happy to collaborate with @tangobango5 or defer to maintainers on which approach to move forward with. |
c7fdca5 to
7e2a550
Compare
Add ExternalId support to AWS STS AssumeRole calls, enabling secure cross-account access with confused deputy protection. Changes: - Add AwsExternalId field to AuthorizationMetadata struct - Read awsExternalId from authParams in all identity paths: PodIdentityProviderAws, identityOwner=operator, identityOwner=pod - Pass ExternalId to AssumeRoleOptions in GetAwsConfig and retrievePodIdentityCredentials - Include ExternalId in config cache key to prevent collisions - Fix early return in GetAwsConfig that skipped AssumeRole for operator identity when a role ARN was provided Fixes kedacore#7662 Signed-off-by: shivkumr <shivkumr@github.com> Signed-off-by: shivkumr <shivkumr@amazon.com>
…g sort order Signed-off-by: shivkumr <shivkumr@amazon.com>
3a27f73 to
b47e7a6
Compare
|
/run-e2e aws_identity_external_id_test |
Signed-off-by: shivkumr <shivkumr@amazon.com>
- Add max 30 iterations to cleanupMessages to prevent infinite loop - Set VisibilityTimeout=1 so deleted messages don't reappear - Remove DelaySeconds from queue creation and message sending so messages are immediately visible to KEDA Signed-off-by: shivkumr <shivkumr@amazon.com>
1 second was too short — delete call could race against the timeout. 10 seconds gives enough time for the delete to complete while still being short enough to not block the cleanup loop. Signed-off-by: shivkumr <shivkumr@amazon.com>
|
@rickbrouwer The e2e test was hanging due to three issues in the test code (not the feature code):
Could you cancel the current run and re-trigger? Thanks! |
|
/run-e2e aws_identity_external_id_test |
|
Looks clean overall, nice work! A few questions: Could the reading of Further, would it make sense to add a validation/warning when And should the test function names also be renamed from About the unit test, could you add a unit test specifically targeting the During the review I think I also spotted what looks like a pre-existing bug in And lastly, do you plan to open a corresponding PR on https://github.com/kedacore/keda-docs to document the new |
- Nest awsExternalId under awsRoleArn check in all identity paths for consistency - Add warning log when awsExternalId is set without awsRoleArn - Rename test functions ExternalId -> ExternalID per Go conventions - Add unit tests for GetAwsConfig operator + role ARN path - Fix pre-existing bug in RemoveCachedEntry: use hashed cache key instead of raw AwsRoleArn when updating entry with remaining usages Signed-off-by: shivkumr <shivkumr@amazon.com>
|
@rickbrouwer Thanks for the thorough review! Addressed all points in 0e486f2:
For docs update PR: kedacore/keda-docs#1754 |
…path AssumeRoleWithWebIdentity does not accept ExternalId — the OIDC token authenticates the role assumption. ExternalID only applies to the AssumeRole fallback. Signed-off-by: shivkumr <shivkumr@amazon.com>
4a4360d to
0418952
Compare
|
Great! Looks good! |
|
/run-e2e aws_identity_external_id_test |
Signed-off-by: shivkumr <shivkumr@amazon.com>
|
/run-e2e aws* |
|
Before this undergoes a second approval, the related PRs must be carefully reviewed. All three are assessed to determine what can and cannot proceed (if there is a conflict or if they have the same goal). |
|
This will be added via #6916 |
…1754) * docs: document awsExternalId parameter for cross-account AssumeRole Add documentation for the new awsExternalId parameter in TriggerAuthentication, which enables confused deputy protection for multi-tenant environments where a shared KEDA operator assumes roles in different AWS accounts. Relates to kedacore/keda#7665 Signed-off-by: shivkumr <shivkumr@amazon.com> * Apply suggestions from code review Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> * Apply suggestion from @JorTurFer Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> --------- Signed-off-by: shivkumr <shivkumr@amazon.com> Signed-off-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Add
awsExternalIdsupport to AWS TriggerAuthentication, enablingsts:AssumeRolewith ExternalId for cross-account confused deputy protection.Problem
In a shared Kubernetes cluster managed by a platform team, multiple application teams from different AWS accounts deploy their workloads. Each team uses KEDA to autoscale based on AWS resources (e.g., SQS queues) in their own accounts. The KEDA operator runs as a single cluster-wide service with one IAM identity (via IRSA or Pod Identity).
To access cross-account resources, KEDA assumes IAM roles in each team's account. However, without
ExternalIdsupport, there is no way to enforce the confused deputy protection — any tenant who discovers another tenant's role ARN could potentially leverage KEDA's shared identity to access their resources. IAM trust policies that requirests:ExternalIdas a condition are the standard mitigation, but KEDA currently has no mechanism to pass an ExternalId duringAssumeRole.What this enables
Each tenant's IAM role can now require a unique ExternalId in its trust policy. The ExternalId is stored in a namespace-scoped Kubernetes Secret (protected by RBAC) and passed to KEDA via TriggerAuthentication. KEDA includes it in the
AssumeRolecall, and IAM rejects any attempt without the correct ExternalId — providing tenant isolation at the IAM layer.Key changes
GetAwsAuthorization(): readawsExternalIdfromauthParamsin all three identity paths — including theidentityOwner: operatorpath, which previously only setPodIdentityOwner = falsewithout reading any auth parametersGetAwsConfig(): allow the operator identity path to performAssumeRolewhen a role ARN is provided, by refining the early return condition fromif !PodIdentityOwnertoif !PodIdentityOwner && AwsRoleArn == ""Changes
pkg/scalers/aws/aws_authorization.goAwsExternalIdfield toAuthorizationMetadatastructpkg/scalers/aws/aws_common.goawsExternalIdfromauthParamsin all 3 identity paths (pod identity, operator, pod); refined early return inGetAwsConfigto support operator + role ARNpkg/scalers/aws/aws_config_cache.goExternalIdin cache key; pass it toAssumeRoleOptionsinretrievePodIdentityCredentialspkg/scalers/aws/aws_common_test.goUsage
Users pass
awsExternalIdviaTriggerAuthentication.secretTargetRef, same pattern asawsRoleArn:Backward-compatible: if no
awsExternalIdis provided, behavior is identical to before.Checklist
make generate-scalers-schemahas been run to update any outdated generated filesFixes #7662
Fixes #6921
Docs: kedacore/keda-docs#1754