aws-iam-authenticator
aws-iam-authenticator copied to clipboard
fix: check credential expiration timestamp when generating tokens
What this PR does / why we need it:
There are two expirations which must be considered when using a signed EKS token:
- 15 minutes after the point in time when the AWS STS request has been signed
- The underlying AWS credentials can expire at which point the token won't be accepted
The second case is particularly common when making frequent requests while using AssumeRole or AssumeRoleWithWebRequest as mentioned in #590 as the default session timeout is 1 hour.
This PR adds an additional check fetching the AWS credential expiration and using that as the returned expiration if it is before the 15 minute token expiration.
Which issue(s) this PR fixes
Fixes #590
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #
Replaces https://github.com/kubernetes-sigs/aws-iam-authenticator/pull/724 /assign @micahhausler
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: alvaroaleman Once this PR has been reviewed and has the lgtm label, please ask for approval from micahhausler. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
I'll check on this, but I think this PR is unnecessary. Most AWS temporary creds' advertised expiration is well before (as much as 50% of the cred lifetime) the actual credential is invalid. This is meant to increase the stability of an application in the event that the credential distribution method is ever impaired. Ex: If you have a 6 hour cred, but the advertised expiration is after 4 hours and normal issuance of new creds is every 4 hours, you can ride out a new credential distribution disruption for up to 2 hours.
I'll check on this, but I think this PR is unnecessary. Most AWS temporary creds' advertised expiration is well before (as much as 50% of the cred lifetime) the actual credential is invalid. This is meant to increase the stability of an application in the event that the credential distribution method is ever impaired. Ex: If you have a 6 hour cred, but the advertised expiration is after 4 hours and normal issuance of new creds is every 4 hours, you can ride out a new credential distribution disruption for up to 2 hours.
This change is definitely necessary, we made this after we saw frequent auth failures which this fixes, which is why we are currently using a fork with this change. You might be right that this is less of a problem with longer-lived creds, but we for example have a max session validity of 1h for compliance reason. Without this change, it happens regularly that the creds used to sign this request expire and the returned token stops working.
I'm ok adding this behavior with a flag (default off) because of the above situation where a cred might be reported as expired, but is still valid, we wouldn't want to create tokens with a reported expiration in the past.
I'll get #741 merged first which includes a test you should be able to extend for the new behavior
Ok, I've merged #741, that test should be extensible for this change, with the addition of a flag
@micahhausler thanks for that, I've updated the PR, please have another look
@micahhausler any chance you could have a look?
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
It would be great if this could get a review /remove-lifecycle stale
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle rotten - Close this PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Reopen this PR with
/reopen - Mark this PR as fresh with
/remove-lifecycle rotten - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/close
@k8s-triage-robot: Closed this PR.
In response to this:
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied- After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied- After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closedYou can:
- Reopen this PR with
/reopen- Mark this PR as fresh with
/remove-lifecycle rotten- Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/close
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-sigs/prow repository.
PR needs rebase.
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-sigs/prow repository.