Skip to content

Conversation

tangobango5
Copy link

@tangobango5 tangobango5 commented Jul 18, 2025

This commit adds support for external ID in AWS SQS scaler to enable secure cross-account access scenarios. External ID is now parsed from TriggerAuthentication and passed to STS AssumeRole operations.

Changes:

  • Add AwsExternalID field to AuthorizationMetadata struct
  • Update GetAwsAuthorization to parse external ID from auth parameters
  • Modify cache key generation to include external ID
  • Update AssumeRole providers to use external ID when available
  • Add comprehensive test coverage for external ID scenarios

The external ID is only used with AssumeRole operations and maintains backward compatibility with existing configurations.

Provide a description of what has been changed

Checklist

Fixes # #6921

Relates to #

@tangobango5 tangobango5 force-pushed the feature/aws-sqs-external-id-support branch 3 times, most recently from 079202f to 7b1e677 Compare July 21, 2025 09:24
@tangobango5 tangobango5 force-pushed the feature/aws-sqs-external-id-support branch from 7ddacbe to 2d88723 Compare July 21, 2025 09:47
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

Awesome improvement! ❤️
As this is part of pod identity (IRSA role assumption), WDYT if we set it as a new parameter in trigger authentication? This would make the new feature available for any AWS Scaler at once.

Please, also open a PR to docs to document the new parameter

@zroubalik
Copy link
Member

Awesome improvement! ❤️ As this is part of pod identity (IRSA role assumption), WDYT if we set it as a new parameter in trigger authentication? This would make the new feature available for any AWS Scaler at once.

Could you please elaborate here?

@JorTurFer
Copy link
Member

Awesome improvement! ❤️ As this is part of pod identity (IRSA role assumption), WDYT if we set it as a new parameter in trigger authentication? This would make the new feature available for any AWS Scaler at once.

Could you please elaborate here?

sure, AwsExternalID is part of the auth SDK and not from SQS SDK. The OP has added the parameter to the SQS trigger but it's passed directly to the auth SDK. The auth code is shared for all the AWS Scalers, so I guess that it's a potential improvement for all the AWS Scalers and not only for SQS. If any other user needs AwsExternalID for other AWS Scaler, we will need to add the parameter there and pass to the shared auth code.
My suggestion is to remove the parameter from the SQS trigger and moving it directly to the TriggerAuthentication to make in shared for any AWS scaler and also to place it closer to the auth code and not in the SQS code to pass it till auth code. The last but not least, this change ONLY applies if pod identity is used (the assumeRole code is podIdentity code), so I see it configured along podIdentity configuration and not as part of the scaler

@zroubalik
Copy link
Member

Awesome improvement! ❤️ As this is part of pod identity (IRSA role assumption), WDYT if we set it as a new parameter in trigger authentication? This would make the new feature available for any AWS Scaler at once.

Could you please elaborate here?

sure, AwsExternalID is part of the auth SDK and not from SQS SDK. The OP has added the parameter to the SQS trigger but it's passed directly to the auth SDK. The auth code is shared for all the AWS Scalers, so I guess that it's a potential improvement for all the AWS Scalers and not only for SQS. If any other user needs AwsExternalID for other AWS Scaler, we will need to add the parameter there and pass to the shared auth code. My suggestion is to remove the parameter from the SQS trigger and moving it directly to the TriggerAuthentication to make in shared for any AWS scaler and also to place it closer to the auth code and not in the SQS code to pass it till auth code. The last but not least, this change ONLY applies if pod identity is used (the assumeRole code is podIdentity code), so I see it configured along podIdentity configuration and not as part of the scaler

Thanks a lot, this make sense!

Fully agree, let's proceed this direction, @tangobango5 FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants