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

Support optional/configurable IAMEndpoint for Minio Client (#32581) #32581

Merged
merged 11 commits into from
Nov 22, 2024

Conversation

mowoc-ocp
Copy link
Contributor

Targeting issue #32271

This modification allows native Kubernetes + AWS (EKS) authentication with the Minio client, to Amazon S3 using the IRSA role assigned to a Service account by replacing the hard coded reference to the DefaultIAMRoleEndpoint with an optional configurable endpoint.

Internally, Minio's credentials.IAM provider implements a discovery flow for IAM Endpoints if it is not set.

For backwards compatibility:

  • We have added a configuration mechanism for an IamEndpoint to retain the unit test safety in minio_test.go.
  • We believe existing clients will continue to function the same without needing to provide a new config property since the internals of Minio client also often resolve to the http://169.254.169.254 default endpoint that was being hard coded before

To test, we were able to build a docker image from source and, observe it choosing the expected IAM endpoint, and see files uploaded via the client.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 20, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 20, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Nov 20, 2024
@mowoc-ocp mowoc-ocp changed the title Support optional/configurable IAMEndpoint for Minio Client Support optional/configurable IAMEndpoint for Minio Client (#32581) Nov 20, 2024
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Looks good.

by the way, "app.example.ini" and related documents https://gitea.com/gitea/docs/pulls also need to update 🙏

@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 Nov 21, 2024
@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 Nov 21, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 21, 2024
@lunny lunny added this to the 1.23.0 milestone Nov 21, 2024
@wxiaoguang wxiaoguang removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 21, 2024
@wxiaoguang
Copy link
Contributor

by the way, "app.example.ini" and related documents https://gitea.com/gitea/docs/pulls also need to update 🙏

Wait for config example update before merge. If there is any difficulty, maintainers could also help.

@lunny lunny added reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Nov 21, 2024
@mowoc-ocp
Copy link
Contributor Author

@wxiaoguang thank you for the feedback, I'll take care of the documentation PR today!

@github-actions github-actions bot added the docs-update-needed The document needs to be updated synchronously label Nov 21, 2024
@mowoc-ocp
Copy link
Contributor Author

Added the requested updates for app.example.ini and opened https://gitea.com/gitea/docs/pulls/103

Let me know if there is any feedback on documentation structure or anywhere I missed that I also need to update. Thanks!

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 22, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 22, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 22, 2024
@go-gitea go-gitea deleted a comment from GiteaBot Nov 22, 2024
@lunny lunny enabled auto-merge (squash) November 22, 2024 18:32
@GiteaBot
Copy link
Contributor

@mowoc-ocp please fix the merge conflicts. 🍵

@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 22, 2024
@lunny lunny merged commit 713364f into go-gitea:main Nov 22, 2024
26 checks passed
@mowoc-ocp mowoc-ocp deleted the minioIamDiscoverySupport branch November 22, 2024 21:16
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 23, 2024
* giteaofficial/main:
  Support optional/configurable IAMEndpoint for Minio Client (go-gitea#32581) (go-gitea#32581)
  Update the list of watchers and stargazers when clicking watch/unwatch or star/unstar (go-gitea#32570)
  Apply to became a maintainer (go-gitea#32614)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-update-needed The document needs to be updated synchronously lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants