-
Notifications
You must be signed in to change notification settings - Fork 7
Add IAM control field for embedded IAM in S3 server #6
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
Conversation
This change adds support for the new embedded IAM architecture where IAM API is built into the S3 server (on the same port as S3). Changes: - Add IAM field to FilerServerSpec to control embedded IAM - Update Kubernetes exporter to pass -iam=false when explicitly disabled IAM is enabled by default when S3 is enabled, following the pattern used by MinIO and Ceph RGW.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @chrislusf, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new configuration option to manage the embedded IAM API within the S3 server, aligning with a recent architectural change to embed IAM by default. It provides explicit control for users to disable the embedded IAM if necessary, while ensuring it's active by default when S3 is in use. The changes involve updating the cluster specification and the Kubernetes exporter to reflect this new configuration. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new IAM field in the FilerServerSpec to allow controlling the embedded IAM feature. The change correctly updates the Kubernetes exporter to respect this new setting. The implementation is clean and follows the intended logic of enabling IAM by default and only disabling it when explicitly configured. My feedback includes a minor suggestion to improve code maintainability by removing a redundant comment.
| if filer.IAM != nil && !*filer.IAM { | ||
| container["command"] = append(container["command"].([]string), "-iam=false") | ||
| } | ||
| // Note: IAM API is accessible on the same port as S3 (S3Port) when embedded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
This PR adds support for the new embedded IAM architecture introduced in seaweedfs/seaweedfs#7740.
Background
IAM API is now embedded in the S3 server by default (on the same port as S3), following the pattern used by MinIO and Ceph RGW.
Changes
Cluster Spec (
pkg/cluster/spec/spec.go)IAMfield toFilerServerSpecto control embedded IAMKubernetes Exporter (
pkg/exporters/kubernetes.go)-iam=falseflag when IAM is explicitly disabledUsage
To disable embedded IAM:
Related PRs