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

feat: add support for a credentials chain for minio access #31051

Merged
merged 4 commits into from
May 27, 2024

Conversation

bohde
Copy link
Contributor

@bohde bohde commented May 22, 2024

We wanted to be able to use the IAM role provided by the EC2 instance metadata in order to access S3 via the Minio configuration. To do this, a new credentials chain is added that will check the following locations for credentials when an access key is not provided. In priority order, they are:

  1. MINIO_ prefixed environment variables
  2. AWS_ prefixed environment variables
  3. a minio credentials file
  4. an aws credentials file
  5. EC2 instance metadata

If a static access key id and secret are not provided, instead
fallback to a pulling credentials from other places, in priority
order:

1. MINIO_ prefixed environment variables
2. AWS_ prefixed environment variables
3. a minio credentials file
4. an aws credentials file
5. EC2 instance metadata

This enables using temporary credentials that are auto-refreshed upon expiration.
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 22, 2024
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 22, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label May 22, 2024
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 22, 2024
@techknowlogick
Copy link
Member

Thanks! Would you be able to add this to the documentation (app.ini example and config cheatsheet) as well? (the body of the PR is more than suitable to be used)

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 23, 2024
@bohde
Copy link
Contributor Author

bohde commented May 23, 2024

I updated the en-us config cheatsheet and app.ini example to document the changes in minio credentials logic. I don't know how to translate this for the zh-cn config cheatsheet. Is there a process for translating these?

Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

Regarding the zh-CN:
Nope, unfortunately not yet.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 23, 2024
@lunny
Copy link
Member

lunny commented May 24, 2024

Can we have a test?

@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 24, 2024
@bohde bohde force-pushed the rb/minio-credentials-chain branch from 1cbf1ae to 79a01e9 Compare May 24, 2024 17:23
@bohde bohde force-pushed the rb/minio-credentials-chain branch from 79a01e9 to 1a4127f Compare May 24, 2024 17:29
@bohde
Copy link
Contributor Author

bohde commented May 24, 2024

I added a test that verifies correct credentials are pulled from all of the possible credential providers in the chain.

@lunny lunny added the type/enhancement An improvement of existing functionality label May 25, 2024
@lunny lunny added this to the 1.23.0 milestone May 25, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 27, 2024
@lunny lunny enabled auto-merge (squash) May 27, 2024 07:57
@lunny lunny disabled auto-merge May 27, 2024 12:55
@lunny lunny merged commit c0880e7 into go-gitea:main May 27, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/docs modifies/go Pull requests that update Go code size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants