berkeley-dsep-infra / hubploy

Toolkit to deploy many z2jh based JupyterHubs
BSD 3-Clause "New" or "Revised" License
17 stars 15 forks source link

Kubeconfig update call requires default aws credentials #45

Open salvis2 opened 4 years ago

salvis2 commented 4 years ago

The cluster_auth_aws() function updates the kubeconfig file as part of authentication. This happens on the following lines.

subprocess.check_call(['aws2', 'eks', 'update-kubeconfig',
                       '--name', cluster, '--region', zone])

For me, I spin up the cluster with a programmatic aws user with minimal permissions. This user is called eksctlbot. While cluster_auth_aws() does copy the credentials for this user and includes the [eksctlbot] identifier in the credentials, the update-kubeconfig call doesn't use the profile flag to use the named profile. It tries to use the "default" profile and I get the error:

Unable to locate credentials. You can configure credentials by running "aws configure".

We should require profile names and pass them to aws calls with the --profile flag.

Even if #44 is fixed, this would still attempt to authenticate kubectl with the "default" profile and would not work for this use case, so I've submitted this as a separate issue.

salvis2 commented 4 years ago

@yuvipanda do you think this is better solved by reading the .cfg files or just having another required field in hubploy.yaml: something like cluster.aws.profile_name?

salvis2 commented 4 years ago

Planning on releasing a PR that will expect the hubploy.yaml file to have the fields images.registry.aws.profile and cluster.aws.profile.

I'll have to:

yuvipanda commented 4 years ago

@salvis2 If we merge #48, we wouldn't need to do anything here, right? Since helm (and kubectl, etc) should read from the env var we set, rather than your Home directory's AWS config

yuvipanda commented 4 years ago

Or rather, is it ok for the credentials we put in secret/ to be the 'default' profile? Given that we expect only one profile to be there ever

salvis2 commented 4 years ago

@salvis2 If we merge #48, we wouldn't need to do anything here, right? Since helm (and kubectl, etc) should read from the env var we set, rather than your Home directory's AWS config

When I was testing #48 , I had problems getting the aws eks update-kubeconfig call to work because the first line of my eks credential file is [eksctlbot]. I bet I could just change it to be [default] and it wouldn't complain.

Or rather, is it ok for the credentials we put in secret/ to be the 'default' profile? Given that we expect only one profile to be there ever

Suppose it doesn't matter either way. Easier to just tell people to put [default] in there than change the code, but we definitely should make that a requirement because copy/pasting credentials from ~/.aws/credentials will give some people the same bugs that I got.

yuvipanda commented 4 years ago

@salvis2 re: copy pasting, that makes sense! We could probably error out if there isn't a default profile?

salvis2 commented 4 years ago

I can make a PR to instruct users to put [default] in as the first line. Do you want to have a specific error to let them know if they put the profile in wrong?