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

fix: load AWS config and assume role #168

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

luisdavim
Copy link
Contributor

@luisdavim luisdavim commented Aug 2, 2023

This fixes vmware-tanzu/velero#3142

Please also have a look at vmware-tanzu/velero#6598

@codecov-commenter
Copy link

Codecov Report

Merging #168 (bd10b15) into main (e95ecde) will increase coverage by 0.38%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #168      +/-   ##
==========================================
+ Coverage   17.48%   17.86%   +0.38%     
==========================================
  Files           6        6              
  Lines         652      638      -14     
==========================================
  Hits          114      114              
+ Misses        532      518      -14     
  Partials        6        6              
Files Changed Coverage Δ
velero-plugin-for-aws/object_store.go 11.25% <0.00%> (+0.58%) ⬆️
velero-plugin-for-aws/volume_snapshotter.go 31.79% <0.00%> (-0.45%) ⬇️

Copy link
Contributor

@reasonerjt reasonerjt left a comment

Choose a reason for hiding this comment

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

I see an error when I patch the plugin with this change and test with IRSA

...
sg="Error getting backup store for this location" backupLocation=velero/default controller=backup-sync error="rpc error: code = Unknown desc = AccessDenied: User: arn:aws:sts::822276436757:assumed-role/eksctl-jt-velero-irsa-addon-iamserviceaccoun-Role1-1T470KOHYLTBK/1691417499559809485 is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam::822276436757:role/eksctl-jt-velero-irsa-addon-iamserviceaccoun-Role1-1T470KOHYLTBK\n\tstatus code: 403, request id: 9e390336-c55f-4dd0-9d34-ec8233945ab6" error.file="/go/src/velero-plugin-for-aws/velero-plugin-for-aws/volume_snapshotter.go:67" error.function=main.getSession logSource="pkg/controller/backup_sync_controller.go:100"

Signed-off-by: Luis Davim <dluis@vmware.com>
@luisdavim
Copy link
Contributor Author

I see an error when I patch the plugin with this change and test with IRSA

...
sg="Error getting backup store for this location" backupLocation=velero/default controller=backup-sync error="rpc error: code = Unknown desc = AccessDenied: User: arn:aws:sts::822276436757:assumed-role/eksctl-jt-velero-irsa-addon-iamserviceaccoun-Role1-1T470KOHYLTBK/1691417499559809485 is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam::822276436757:role/eksctl-jt-velero-irsa-addon-iamserviceaccoun-Role1-1T470KOHYLTBK\n\tstatus code: 403, request id: 9e390336-c55f-4dd0-9d34-ec8233945ab6" error.file="/go/src/velero-plugin-for-aws/velero-plugin-for-aws/volume_snapshotter.go:67" error.function=main.getSession logSource="pkg/controller/backup_sync_controller.go:100"

That looks like the role is being assumed twice, I guess the SDK is handling that for us now and the extra assume role is not needed, I've updated the PR, can you run that test again? Thanks.

@blackpiglet blackpiglet merged commit 62fce4b into vmware-tanzu:main Aug 9, 2023
2 checks passed
@luisdavim luisdavim deleted the fix_aws_creds branch August 9, 2023 12:27
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