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

Do not set AWS_PROFILE env in generated kubeconfig #1082

Merged
merged 3 commits into from
Mar 22, 2024

Conversation

rquitales
Copy link
Member

@rquitales rquitales commented Mar 20, 2024

Proposed changes

AWS_PROFILE values can vary between users despite variations providing the same level of access to the AWS resources. Always setting the profile name in the kubeconfig will mean that other users of the Pulumi program will need to ensure that their profile names also match, which isn't ideal.

This change would enable users to switch profiles, without needing to regenerate a Kubeconfig as the underlying eks get-token command will use the ambient AWS_PROFILE env var to determine the best profile.

I have manually validated that this change works locally by explicitly specifying a custom profile on the first pulumi up, changing the profile to another name and creating an on cluster deployment to see if the generated kubeconfig can handle the profile name change.

Related issues (optional)

Fixes: #868

Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

1 similar comment
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

AWS_PROFILE values can vary between users despite variations providing
the same level of access to the AWS resources. Always setting the
profile name in the kubeconfig will mean that other users of the Pulumi
program will need to ensure that their profile names also match, which
isn't ideal.
@rquitales rquitales force-pushed the rquitales/do-not-default-aws-profile branch from b24ffd9 to 11a3de7 Compare March 21, 2024 00:03
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@rquitales rquitales force-pushed the rquitales/do-not-default-aws-profile branch from 11a3de7 to aaa5920 Compare March 21, 2024 15:49
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@rquitales rquitales force-pushed the rquitales/do-not-default-aws-profile branch from aaa5920 to 4340386 Compare March 21, 2024 16:42
@rquitales rquitales marked this pull request as ready for review March 21, 2024 16:43
@rquitales rquitales requested review from thomas11 and a team March 21, 2024 16:43
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@rquitales rquitales force-pushed the rquitales/do-not-default-aws-profile branch from 4340386 to fdaabbf Compare March 21, 2024 18:06
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Copy link
Contributor

@thomas11 thomas11 left a comment

Choose a reason for hiding this comment

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

Sadly, the upgrade tests are failing again due to a new AMI id :(

examples/utils/utils.go Show resolved Hide resolved
Recording fresh test snapshots for `TestExamplesUpgrade`.
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@rquitales rquitales merged commit fc90daf into master Mar 22, 2024
40 checks passed
@rquitales rquitales deleted the rquitales/do-not-default-aws-profile branch March 22, 2024 21:27
@fernandocarletti
Copy link

fernandocarletti commented May 14, 2024

Hello! Just a suggestion for the future. This is a breaking change, since it does not offer backward compatibility with the old behavior (I know this the hard way :), it sounds this should be released as 3.0 instead as 2.3, since minor versions should keep backward compatibility.

I'm assuming Pulumi does use semver, based on this, which made me overlook it before merging the dependabot PR (and yes, I know I should have checked the changelog more careful, there's a caution note there, that's why it is a friendly suggestion and not a rant hahaha).

@gunzy83
Copy link

gunzy83 commented Jun 21, 2024

This absolutely is a breaking change. Took me a while to track down how to deal with this on upgrade (given I am already applying transformations to overcome deficiencies).

To me it makes little sense that you can create the cluster with an AWS profile like this:

providerCredentialOpts: {
  profileName: awsProfile,
},

The profile is saved into the Kubeconfig for the initial deploy of the cluster resources, but then immediately creates a provider (${name}-provider) inside the Cluster resource without the profile included in its Kubeconfig (which then throws Unable to locate credentials. You can configure credentials by running "aws configure". even if it is not used in the program). I would assume in the same stack you would still need the profile used to create the cluster which would need to remain consistent between machines?

Even though we are using an unconventional way of accessing AWS config (using AWS_CONFIG_FILE to a project local dir with local and CI config files in them), I find it baffling that an org trying to deal with things at any scale would not at least opt for consistent profile names for environments to reduce confusion and doesn't work on my machine problems. Anyway...

@rquitales
Copy link
Member Author

Thanks for your feedback on this change. We've previously received comments on the previous behavior from a number of customers and how it affects their user journeys in a multi-user setup. This coupled with the discussions in #868 and pulumi/pulumi-aws#1906 (comment) led us to revisit the behavior of the generated kubeconfig and its behavior.
I do apologize for any pain this may have caused and I'll be sure to surface this with the rest of the team such that impacts like this are minimized in the future.

flostadler pushed a commit that referenced this pull request Sep 4, 2024
### Proposed changes

AWS_PROFILE values can vary between users despite variations providing
the same level of access to the AWS resources. Always setting the
profile name in the kubeconfig will mean that other users of the Pulumi
program will need to ensure that their profile names also match, which
isn't ideal.

This change would enable users to switch profiles, without needing to
regenerate a Kubeconfig as the underlying `eks get-token` command will
use the ambient `AWS_PROFILE` env var to determine the best profile.

I have manually validated that this change works locally by explicitly
specifying a custom profile on the first `pulumi up`, changing the
profile to another name and creating an on cluster deployment to see if
the generated kubeconfig can handle the profile name change.

### Related issues (optional)

Fixes: #868

---------

Co-authored-by: Pulumi Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't set AWS_PROFILE automatically if specified
5 participants