-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 issue 5875 #6283
Fix issue 5875 #6283
Conversation
Codecov Report
@@ Coverage Diff @@
## main #6283 +/- ##
==========================================
- Coverage 41.16% 41.14% -0.03%
==========================================
Files 252 252
Lines 23530 23529 -1
==========================================
- Hits 9686 9680 -6
- Misses 13083 13088 +5
Partials 761 761
|
result[udmrepo.StoreOptionS3SecretKey] = credValue.SecretAccessKey | ||
result[udmrepo.StoreOptionS3Token] = credValue.SessionToken | ||
} else if err == repoconfig.AWSCredFileMissingError { | ||
log.Warn("AWS credential file is not present, so no credentials are provided") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this happens, then there is no credential information in the result, but the IAM way also mounts the credential information into the Velero pod.
Is there a place where the Kopia Library can read credential information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Also curious how kopia handles different auth mechanisms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kopia leverage on minio client to get credentials through IAM, the later queries the related environment variable AWS_ROLE_ARN and AWS_WEB_IDENTITY_TOKEN_FILE and then make the http call to get the credentials.
Kopia does few things like letting it go without checking static ak/sk and setting the http client for minio.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What Velero needs is letting it go without checking static ak/sk as well.
For sure, the IAM role SA should be mounted to Velero pods, this has been done by another PR #5802
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moreover, it looks weird to let it go without checking anything when getting credentials, so here I have modified the code to check the existence of the env variable AWS_ROLE_ARN, only when its value is not empty, we let it go with empty ak/sk
f4e0b78
to
d500965
Compare
Signed-off-by: Lyndon-Li <[email protected]>
Fix issue #5875. Since Kopia has supported IAM, Velero should not require static credentials all the time