Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new AWS podIdentity #5061

Merged
merged 15 commits into from
Jan 9, 2024
Merged

Add new AWS podIdentity #5061

merged 15 commits into from
Jan 9, 2024

Conversation

JorTurFer
Copy link
Member

@JorTurFer JorTurFer commented Oct 8, 2023

Special KUDOS to @ThaSami, who has developed almost all the WebIdentity and caching code

This PR Introduced a new authentication option for AWS, relying 100% on TriggerAuthentication CRD for getting all the required information instead of mixing TriggerAuthentication and trigger.metadata section. For this, 2 new fields have been added:

  • roleArn: Similar to already existing clientId, this new field is used for manually providing the AWS roleArn that KEDA has to use.
  • identityOwner: This field is used for specifying if you want to use KEDA's roleArn or the workload roleArn (recovered from the workload service account annotations). This field is an enum with 3 valid values: unset, keda, workload (unset is treaded as keda)
    These fields are mutually exclusive and there is a validation in admission webhooks to check this.

This new authentication allows 3 different working ways:

  • Using KEDA's role (identityOwner: keda): In this scenario, KEDA's role has the required permission for accessing to the resources.
  • Using workload's role via AssumeRole(identityOwner: workload or roleArn: VALUE): In this scenario, KEDA will use its own role to assume workloads role via AssumeRole API. This requires that KEDA's role can assume other role inside AWS AIM. (and it's the assume role who has the access instead of KEDA's role)
  • Using workload's role via Web Identity Role(identityOwner: workload or roleArn: VALUE): In this scenario, KEDA will request the role via WebIdentityRole API. This requires an OIDC federation between KEDA's service account and the role that KEDA has to assume.(and it's the assume role who has the access instead of KEDA's role)

Additionally, this PR adds a cache for storing the aws credentials across all the usages instead of creating a credential instance per scaler. This means that if we are using the same role on several scalers, KEDA will use the same instance/token for all the scalers instead of requesting (and refreshing) a token for each scaler. To know if the aws config is in use in other scalers, we will use a new unique key introduced as scaler.Config level which is based on resourceType,namespace,resourceName,triggerIndex to be unique accross all the resources.
This new caching system has been included only as part of this new authenticaiton and also using AWS_ACCESS_KEY and AWS_ACCESS_SECRET.

Checklist

Fixes #4134
Fixes #5178
Fixes #5297

Relates to:

@github-actions
Copy link

github-actions bot commented Oct 8, 2023

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

While you are waiting, make sure to:

Learn more about:

@blakepettersson
Copy link

blakepettersson commented Nov 28, 2023

To add further confusion, AWS has just released a new alternative to IRSA called Pod Identity 😄

I'm not sure of the relevance it will have, if any, on this PR but would be worth to have a look to see the potential impact of this feature.

@JorTurFer
Copy link
Member Author

JorTurFer commented Dec 30, 2023

/run-e2e aws
Update: You can check the progress here

@JorTurFer
Copy link
Member Author

JorTurFer commented Dec 31, 2023

/run-e2e aws
Update: You can check the progress here

@JorTurFer JorTurFer force-pushed the add-aws branch 2 times, most recently from 66de066 to 02a2bf1 Compare December 31, 2023 12:15
@JorTurFer
Copy link
Member Author

JorTurFer commented Dec 31, 2023

/run-e2e aws
Update: You can check the progress here

@JorTurFer JorTurFer marked this pull request as ready for review December 31, 2023 12:21
@JorTurFer JorTurFer requested a review from a team as a code owner December 31, 2023 12:21
@JorTurFer JorTurFer changed the title WIP - Add new AWS podIdentity Add new AWS podIdentity Dec 31, 2023
Co-authored-by: Sami S <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer
Copy link
Member Author

JorTurFer commented Dec 31, 2023

/run-e2e
Update: You can check the progress here

@JorTurFer
Copy link
Member Author

JorTurFer commented Dec 31, 2023

/run-e2e aws
Update: You can check the progress here

Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer
Copy link
Member Author

JorTurFer commented Jan 1, 2024

/run-e2e aws
Update: You can check the progress here

@JorTurFer
Copy link
Member Author

PTAL @zroubalik , @tomkerkhove @blakepettersson @ThaSami
I've been working on this feature and it's ready to review. I've addressed all the comments here and also in the issue (or at least I hope so)

Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer
Copy link
Member Author

JorTurFer commented Jan 1, 2024

/run-e2e aws
Update: You can check the progress here

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

Great job, I have a few comments in general about documentation.

Will do another review once kedacore/keda-docs#1251 (comment) is resolved

pkg/scalers/scaler.go Outdated Show resolved Hide resolved
pkg/scalers/aws/aws_authorization.go Outdated Show resolved Hide resolved
pkg/scalers/aws/aws_authorization.go Show resolved Hide resolved
pkg/scalers/aws/aws_common.go Show resolved Hide resolved
pkg/scalers/aws/aws_config_cache.go Show resolved Hide resolved
pkg/scalers/aws/aws_config_cache.go Show resolved Hide resolved
pkg/scalers/aws/aws_common.go Show resolved Hide resolved
pkg/scalers/aws/aws_common.go Show resolved Hide resolved
pkg/scalers/aws/aws_common.go Show resolved Hide resolved
pkg/scalers/aws/aws_common.go Show resolved Hide resolved
JorTurFer and others added 7 commits January 3, 2024 23:19
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado Ferrero <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer
Copy link
Member Author

JorTurFer commented Jan 3, 2024

/run-e2e aws
Update: You can check the progress here

Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer
Copy link
Member Author

JorTurFer commented Jan 3, 2024

/run-e2e aws
Update: You can check the progress here

Copy link
Member

@wozniakjan wozniakjan left a comment

Choose a reason for hiding this comment

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

great work!

few minor nits for your consideration provided below

pkg/scalers/aws/aws_config_cache.go Outdated Show resolved Hide resolved
JorTurFer and others added 2 commits January 7, 2024 18:54
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado Ferrero <[email protected]>
@JorTurFer
Copy link
Member Author

JorTurFer commented Jan 7, 2024

/run-e2e aws
Update: You can check the progress here

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

LGTM, great work everybody, @blakepettersson @ThaSami @JorTurFer !

@JorTurFer JorTurFer merged commit c132117 into kedacore:main Jan 9, 2024
19 checks passed
@JorTurFer JorTurFer deleted the add-aws branch January 9, 2024 16:16
toniiiik pushed a commit to toniiiik/keda that referenced this pull request Jan 15, 2024
* Add new AWS podIdentity

Co-authored-by: Sami S <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>

* use new resources

Signed-off-by: Jorge Turrado <[email protected]>

* fix logs

Signed-off-by: Jorge Turrado <[email protected]>

* rename scalerIndex to triggerIndex and ScalerUniqueKey to TriggerUniqueKey

Signed-off-by: Jorge Turrado <[email protected]>

* add missing headers

Signed-off-by: Jorge Turrado <[email protected]>

* propagate ctx

Signed-off-by: Jorge Turrado <[email protected]>

* Add documentation commments

Signed-off-by: Jorge Turrado <[email protected]>

* Add explanation to new files

Signed-off-by: Jorge Turrado <[email protected]>

* Add semgrep exclusion

Signed-off-by: Jorge Turrado <[email protected]>

* go mod updates

Signed-off-by: Jorge Turrado <[email protected]>

* update comment

Signed-off-by: Jorge Turrado <[email protected]>

* update clientset

Signed-off-by: Jorge Turrado <[email protected]>

* apply feedback

Signed-off-by: Jorge Turrado <[email protected]>

---------

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado Ferrero <[email protected]>
Co-authored-by: Sami S <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants