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

Cache aws credentials #193

Merged

Conversation

llamahunter
Copy link
Contributor

Fixes #183

Possibly fixes or addresses the following in an alternate way: #151, #99, #63, #140

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 11, 2019
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 11, 2019
profile = session.DefaultSharedConfigProfile
}
// create a cacheing Provider wrapper around the Credentials
if cacheProvider, err := NewFileCacheProvider(clusterID, profile, roleARN, sess.Config.Credentials); err == nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If caching is enabled, wrap the existing Credentials (and its contained Provider) in a new caching provider.

pkg/token/filecache.go Outdated Show resolved Hide resolved
pkg/token/filecache.go Outdated Show resolved Hide resolved
pkg/token/filecache.go Outdated Show resolved Hide resolved
pkg/token/filecache.go Outdated Show resolved Hide resolved
@llamahunter
Copy link
Contributor Author

/assign mattlandis

@llamahunter
Copy link
Contributor Author

Hmm... I think I might be using confusing terminology. This is really caching the AWS credentials, not the kubernetes access token. Would you like me to fix up the identifiers and comments to be clearer about what's going on? Also, probably should reference aws/aws-sdk-go#1329 somewhere in the code, so that people understand why this needs to be done, as the aws-sdk-go people say it's not their responsibility to cache credentials, and any app credential caching should not share a cache with the awscli for compatibility reasons.

@llamahunter
Copy link
Contributor Author

@nckturner @mattmoyer any thoughts on this? Any rework needed? Can it be merged & released?

@nckturner
Copy link
Contributor

/assign

@llamahunter
Copy link
Contributor Author

Still waiting for feedback on this. Does it need rework? Can it be merged?

@llamahunter
Copy link
Contributor Author

Hey... it's been about a month now.... please comment.

@CharlieC3
Copy link

Any activity on this review process? Would love to see this merged.

@llamahunter
Copy link
Contributor Author

I can only ask so many times.... @CharlieC3 do you have any feedback on it?

@CharlieC3
Copy link

I'm no official reviewer of this repo, but...

I'm assuming you were able to build/test this on your end @llamahunter, but I was not due to dependency issues. Given, I only tried for maybe 15 minutes :) I'll give it another go this week to see if I can test it on my end, otherwise the source code looks good at a quick glance, and the PR comments you left are helpful.

@llamahunter
Copy link
Contributor Author

Hmm... I would hope the branch would be able to build. Go is pretty good about packaging all the dependent foo together. The Travis CI builds succeeded, so I think build problems are probably a local config issue on your side.

It seems to work pretty well for me. I have a number of different IAM roles I can adopt to switch to different permissions levels within the cluster. Here's a section of my .kube/config file

users:
- name: arn:aws:eks:us-east-1:########:cluster/ekstest-admin
  user:
    exec:
      apiVersion: client.authentication.k8s.io/v1alpha1
      args:
      - token
      - -i
      - ekstest
      command: aws-iam-authenticator
      env:
      - name: AWS_PROFILE
        value: default
- name: arn:aws:eks:us-east-1:########:cluster/ekstest-dev
  user:
    exec:
      apiVersion: client.authentication.k8s.io/v1alpha1
      args:
      - token
      - -i
      - ekstest
      - --cache
      command: aws-iam-authenticator
      env:
      - name: AWS_PROFILE
        value: dev
- name: arn:aws:eks:us-east-1:########:cluster/ekstest-ops
  user:
    exec:
      apiVersion: client.authentication.k8s.io/v1alpha1
      args:
      - token
      - -i
      - ekstest
      - --cache
      command: aws-iam-authenticator
      env:
      - name: AWS_PROFILE
        value: ops

and my corresponding .aws/config

[default]
region = us-east-1

[dev]
region = us-east-1
credential_process = okta-credential_process arn:aws:iam::########:role/aws-dev

[ops]
region = us-east-1
credential_process = okta-credential_process arn:aws:iam::########:role/aws-ops

My default profile has a corresponding entry in .aws/credentials

[default]
aws_access_key_id = XXXXXXXX
aws_secret_access_key = XXXXXXXX

and so I don't have the --cached argument for that user in my .kube/config. The other two users use okta-credential-process to get SSO credentials

pkg/token/filecache.go Outdated Show resolved Hide resolved
Copy link
Contributor

@nckturner nckturner left a comment

Choose a reason for hiding this comment

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

We do need something similar to this PR to relieve the pain of MFA caching. Apologies on the time to review.

pkg/token/filecache.go Outdated Show resolved Hide resolved
cmd/aws-iam-authenticator/token.go Outdated Show resolved Hide resolved
cmd/aws-iam-authenticator/token.go Outdated Show resolved Hide resolved
pkg/token/filecache.go Outdated Show resolved Hide resolved
pkg/token/filecache.go Outdated Show resolved Hide resolved
pkg/token/filecache.go Outdated Show resolved Hide resolved
pkg/token/filecache.go Outdated Show resolved Hide resolved
@llamahunter
Copy link
Contributor Author

Thx for the feedback. Will review above recommendations and push new code to the PR in about a week. Currently out of the country traveling w/o computer.

Create a disk cache for credentials, based on cluster, profile,
and roleARN, and use cached credentials until they expire.
Requires that the credential supports Expirer interface (i.e.
works for SSO credenital_process credentials, but not for static
unexpiring credentials).

Caching based on profile/roleARN  permits easy switching of kubectl
config between different user contexts by configuring different
profiles using AWS_PROFILE env variable in the exec block of .kube/config
@llamahunter
Copy link
Contributor Author

Addressed the review comments and rebased against master.

@nckturner
Copy link
Contributor

Appreciate the changes, taking a look.

@nckturner
Copy link
Contributor

Tested this PR after the changes and it works great. Let's get it merged! Specifically tested:

  • MFA caching
  • Switching between two roles with MFA
  • Expired credentials

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 5, 2019
@nckturner
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: llamahunter, nckturner

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 5, 2019
@k8s-ci-robot k8s-ci-robot merged commit a584af6 into kubernetes-sigs:master Apr 5, 2019
@robwittman
Copy link

🙌 Any timeline as to when we can see this in a release?

@nckturner
Copy link
Contributor

@robwittman working on it! Hopefully this week.

@joegoggins
Copy link

Looks cool. Thanks for tackling this. Look forward to trying it in a release. I'd like to consider swapping out our existing MFA caching solution to replace it with this. I tried to derive answers to some technical questions about this by reviewing the code changes but wasn't able to. If these get addressed in the release notes, please ignore and just ping me with a link to that when it ships.

some assumed questions / answers

How do you enable caching?

I assume aws-iam-authenticator --cached

Where does it store it's cached credentials on disk?

I assume somewhere in ~/.kube/config

What does it cache?

I assume something like this is cached on disk and that it lasts for 15 minutes before re-prompting for MFA. Yea?

{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1alpha1","spec":{},"status":{"token":"k8s-aws-v1.REDACTED...."}}`

@llamahunter
Copy link
Contributor Author

See aws-iam-authenticator token --help. Note that this caching is for the SSO credentials used to retrieve the token, not the token itself. Someone else is working on token caching, I believe, but SSO credential fetching takes a long time (7-8 seconds), so caching the credentials is very useful.

Credentials are stored in ~/.kube/cache/aws-iam-authenticator/credentials.yaml unless you override the location with the AWS_IAM_AUTHENTICATOR_CACHE_FILE environment variable

It caches the access_key_id, secret_access_key, and session_token for as long as you have configured your credentials to be cacheable. I have mine set in IAM for 12 hours (the max).

@truncs
Copy link

truncs commented May 2, 2019

@llamahunter Thanks for all the hard work on this! Much appreciated!
On the session duration, even after setting the max duration time in the IAM, the session durations that we get back are for 15 mins. Thoughts on why this might be the case? Happy to open a separate issue for this.

@llamahunter
Copy link
Contributor Author

What is your SSO system? If using Okta, you must set the time in the ~/.okta/config.properties to set the OKTA_STS_DURATION. YMMV if using a different SSO system.

@CharlieC3
Copy link

@truncs I don't believe the duration is configurable due to a lack of configuration options in the AWS API.
https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-authenticating-requests.html
#63
#75

@llamahunter
Copy link
Contributor Author

Longer durations up to 12 hrs for federated access using roles for about a year now. https://aws.amazon.com/about-aws/whats-new/2018/03/longer-role-sessions/

@CharlieC3
Copy link

@llamahunter According to the links in my previous comment, and comments from the other PRs, it seems like aws-iam-authenticator may be using the sig-v4-authenticating-requests method. And if it does, that would still be subject to the duration set by the sig-v4-authenticating-requests method, which is hard-set to 15 minutes.

So it's possible both duration timeouts are getting used here, but the shorter one is not configurable and is consequently the one we're being affected by.

Keep in mind I'm not familiar with this project's source code, but the description of the 15 minute timeout duration for the aws-iam-authenticator fits suspiciously well into the 15 minute auth timeout described in this page.
https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-authenticating-requests.html

@llamahunter
Copy link
Contributor Author

I think you're talking about token timeouts. This is about caching credentials, not tokens. I think there's another issue addressing the caching of tokens, for which there is a built in limit by AWS of 10-15 minutes.

@llamahunter
Copy link
Contributor Author

See the list of 'possibly addresses in alternate way' issues listed at the start of this pull request.

@CharlieC3
Copy link

@llamahunter You might be right, I was under the impression they were affected by the same issue described in this comment.
#63 (comment)

@llamahunter
Copy link
Contributor Author

Yes, per their comment, they appear to be using Okta SSO 2FA stuff, like we do. If they set the IAM federated role connected to their Okta sign on to expire in 12 hours, and enable caching in aws-iam-authenticator, I suspect their lives will be a lot more pleasant. I believe aws-iam-authenticator still fetches the token every time, tho, which, in my experience, is a short time delay (< 1 second). It would be nice if the token were cached for the full 15 minutes to skip kubectl talking to anything other than the EKS k8s control plane endpoint on every invocation.

@truncs
Copy link

truncs commented May 2, 2019

@llamahunter we use 2FA functionality provided by AWS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache credentials from credentials_process
9 participants