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

Update --profile flag to override default chain's profile rather than replacing default chain #684

Conversation

dannycjones
Copy link
Contributor

Description of change

A previous PR was open for this change but was very stale: #415.

This change means that a profile can be specified that does not configure any credentials, but may configure other things. Mountpoint does not read any configuration from an AWS Profile other than credentials today but may do in the future.

Before this change, the entire credential chain was replaced with one that would supply profile credentials or error - no fallback. This was not in line with AWS CLI or SDKs.

This should fix #389 since the default chain also allows credential process credentials to be used.

Does this change impact existing behavior?

Yes, it fixes the behavior of the --profile <AWS_PROFILE> flag to be in line with AWS_PROFILE environment variable, as well as --profile <AWS_PROFILE> flag of the AWS CLI.

It also fixes bug #389.

Note: It does not validate that the AWS profile exists, like the AWS CLI does. If the profile does not exist, the parameter will be ignored. Mountpoint already behaves this way today when using the AWS_PROFILE environment variable. Mountpoint previously reporting an error was a side effect of replacing the default chain with a profile provider.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

@dannycjones dannycjones changed the title Replace profile provider with default plus override 2 Update --profile flag to override default chain's profile rather than replacing default chain Dec 29, 2023
@dannycjones
Copy link
Contributor Author

Opened #685 to address crate builds issue.

@dannycjones dannycjones marked this pull request as draft January 2, 2024 12:17
@dannycjones dannycjones temporarily deployed to PR integration tests January 11, 2024 12:16 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 11, 2024 12:16 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 11, 2024 12:16 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 10:23 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 10:23 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 10:23 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 10:23 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 10:54 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 10:54 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 10:54 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 10:54 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 11:20 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 11:21 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 11:21 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests January 22, 2024 11:21 — with GitHub Actions Inactive
@dannycjones
Copy link
Contributor Author

Closing this out since:

We should still consider replacing the use of the profile credential provider in Mountpoint with the default chain and a profile override, however I would not currently prioritize this due to a) this issue not being reported and b) the possibility that we remove Mountpoint's dependency on mountpoint-s3-client in the foreseeable future.

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.

--profile does not correctly use credential_process (Appstream)
2 participants