use shared credentials file for session creation #374
Conversation
| var data []byte | ||
| switch { | ||
| case len(secret.Data["credentials"]) > 0: | ||
| data = secret.Data["credentials"] |
There was a problem hiding this comment.
Just for my understanding really, this credentials file, will it have a reference to the token that is being projected into the pod or does that come from somewhere else?
There was a problem hiding this comment.
It will have the path to the token file in it. Will have to be in sync with the projected token location in openshift/machine-api-operator#743.
Currently, this is just to make it possible to use STS cred, but phase 0 will only allow CCO in Manual mode and cluster admin will have to create the cred secrets manually.
sjenning
force-pushed
the
support-new-cred-format
branch
3 times, most recently
from
1f76632
to
c34d008
Compare
|
@JoelSpeed I added code to remove the credentials file after the session is created |
|
AWS quota limit |
|
/retest |
sjenning
force-pushed
the
support-new-cred-format
branch
from
c34d008
to
412d119
Compare
|
@JoelSpeed yes, I did test it manually |
|
Perfect! Thanks @sjenning /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest |
|
/retest |
|
/retest |
|
/test e2e-aws-operator |
1 similar comment
|
/test e2e-aws-operator |
| return nil, machineapiapierrors.InvalidMachineConfiguration("AWS credentials secret %v did not contain key %v", | ||
| secretName, AwsCredsSecretAccessKey) | ||
| sharedCredsFile, err := sharedCredentialsFileFromSecret(&secret) | ||
| if err != nil { |
There was a problem hiding this comment.
Looks to me like if the temp file can be created but not written to this function returns early and the temp file is never deleted.
|
/test unit |
|
/override ci/prow/unit There is a single unit test failure that is blocking the PR from merging. The test failure is a result in a change of error message between go 1.14 and go 1.15. This should have blocked the go 1.15 CI upgrade PR from going through, not sure why it didn't 🤔 I will fix the unit test in another PR |
|
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/unit In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
27 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
@joelddiaz @abhinavdahiya @michaelgugino @JoelSpeed
In order to support various styles of authentication, mostly STSAsumeRoleUsingWebhookIdentity, the operator should support using the AWS SDK's default handling of the shared credential file.
Previously the operator only supported static credentials from specific keys in the AWS secret, but now the operator supports reading credentials key to load the shared credential file and pass it on as-is to AWS SDK to authenticate based on the contents of that file.
To make the code easier, the operator now converts the previous access_key, secret_key case into a shared credential file using those values and then using that like if it was provided.
https://issues.redhat.com/browse/CO-1275