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.

Sign up for GitHub

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

Jump to bottom

chore: remove rbac related definition #1197

Merged
merged 2 commits into from
Jan 22, 2025
Merged

chore: remove rbac related definition #1197

merged 2 commits into from
Jan 22, 2025

Conversation

cjc7373
Copy link
Contributor

@cjc7373 cjc7373 commented Nov 14, 2024 •
edited
Loading

also add patroni's policy rules

todo: rabbitmq seems to need some rbac policies

@cjc7373 cjc7373 mentioned this pull request Nov 14, 2024
Merged
zjx20
zjx20 previously approved these changes Nov 14, 2024
View reviewed changes
Copy link
Contributor

@zjx20 zjx20 left a comment

Choose a reason for hiding this comment

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

LGTM. Just curious, in which case should the rbacEnabled be set to true?

@cjc7373
Copy link
Contributor Author

cjc7373 commented Nov 18, 2024

It seems like rbacEnabled is used to "simulate" user defined rbac resources. @Y-Rookie What's this field designed for?

@zjx20
Copy link
Contributor

zjx20 commented Nov 18, 2024

I suggest deleting it if it's useless.

@ldming
Copy link
Collaborator

ldming commented Nov 20, 2024 •
edited
Loading

It seems like rbacEnabled is used to "simulate" user defined rbac resources. @Y-Rookie What's this field designed for?

In earlier versions of KubeBlokcs, it did not support the automatic creation of service accounts (SA) with specific roles for clusters. Therefore, they would be created in kbcli or helm charts. After KB started supporting this feature, in most cases, this parameter has become unnecessary.

But, to my knowledge, currently, Elasticsearch sets this parameter to true, referring to this PR apecloud/kbcli#460.

https://github.com/apecloud/kubeblocks/blob/26e2cf458382b8732d5e2ff54e3ac8b273f02272/controllers/apps/transformer_component_rbac.go#L268-L271
KubeBlocks will not create sa if probe, volume protection, and data protection are disabled at the same time.

IMO, KubeBlocks should create the cluster SA, and delete the rbacEnabled in helm chart and kbcli.

ldming
ldming previously approved these changes Nov 25, 2024
View reviewed changes
@cjc7373
Copy link
Contributor Author

cjc7373 commented Nov 25, 2024

I'll do the cleanup and remove rbacEnabled.

@cjc7373 cjc7373 marked this pull request as draft November 25, 2024 07:31
@cjc7373 cjc7373 dismissed stale reviews from ldming and zjx20 via d8060b7 November 29, 2024 08:04
@cjc7373 cjc7373 mentioned this pull request Nov 29, 2024
Merged
@cjc7373 cjc7373 force-pushed the chore/disable-manual-rbac branch from d8060b7 to fc22eaf Compare November 29, 2024 10:48
@cjc7373 cjc7373 changed the title chore: disable manual rbac by default chore: remove rbac related definition Nov 29, 2024
@cjc7373 cjc7373 marked this pull request as ready for review November 29, 2024 11:00
@cjc7373 cjc7373 force-pushed the chore/disable-manual-rbac branch from fb81bcb to 8b301d2 Compare January 20, 2025 09:38
@cjc7373 cjc7373 requested a review from a team as a code owner January 20, 2025 09:38
@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (d4eca60) to head (1d5db5e).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@          Coverage Diff          @@
##            main   #1197   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files         69      69           
  Lines       6900    6900           
=====================================
  Misses      6900    6900

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cjc7373 cjc7373 force-pushed the chore/disable-manual-rbac branch from 8b301d2 to e9b7c41 Compare January 20, 2025 09:50
zjx20
zjx20 previously approved these changes Jan 21, 2025
View reviewed changes
@cjc7373 cjc7373 force-pushed the chore/disable-manual-rbac branch from e9b7c41 to 1d5db5e Compare January 22, 2025 06:24
@cjc7373
Copy link
Contributor Author

cjc7373 commented Jan 22, 2025

/approve

@cjc7373 cjc7373 merged commit c707fc0 into main Jan 22, 2025
19 checks passed
@cjc7373 cjc7373 deleted the chore/disable-manual-rbac branch January 22, 2025 07:28
@cjc7373
Copy link
Contributor Author

cjc7373 commented Jan 23, 2025

/cherry-pick release-1.0-beta

@apecloud-bot
Copy link
Collaborator

🤖 says: Error cherry-picking.

Auto-merging addons-cluster/mongodb/values.yaml
CONFLICT (content): Merge conflict in addons-cluster/mongodb/values.yaml
Auto-merging addons-cluster/mysql/values.yaml
Auto-merging addons-cluster/rabbitmq/values.yaml
CONFLICT (content): Merge conflict in addons-cluster/rabbitmq/values.yaml
Auto-merging addons-cluster/tidb/templates/cluster.yaml
Auto-merging addons-cluster/tidb/values.yaml
CONFLICT (content): Merge conflict in addons-cluster/tidb/values.yaml
error: could not apply c707fc0... chore: remove rbac related definition (#1197)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"

@apecloud-bot
Copy link
Collaborator

🤖 says: ‼️ cherry pick action failed.
See: https://github.com/apecloud/kubeblocks-addons/actions/runs/12923369393

@apecloud-bot
Copy link
Collaborator

🤖 says: cherry pick action finished successfully 🎉!
See: https://github.com/apecloud/kubeblocks-addons/actions/runs/12923369393

apecloud-bot pushed a commit that referenced this pull request Jan 23, 2025
@cjc7373
chore: remove rbac related definition (#1197)
5e35ed2 
(cherry picked from commit c707fc0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leon-inf leon-inf leon-inf approved these changes

@apecloud-bot apecloud-bot apecloud-bot approved these changes

@ldming ldming ldming left review comments

@zjx20 zjx20 zjx20 left review comments

@nayutah nayutah Awaiting requested review from nayutah

@heng4fun heng4fun Awaiting requested review from heng4fun

@free6om free6om Awaiting requested review from free6om

@wangyelei wangyelei Awaiting requested review from wangyelei

@shanshanying shanshanying Awaiting requested review from shanshanying shanshanying is a code owner

@weicao weicao Awaiting requested review from weicao

@Y-Rookie Y-Rookie Awaiting requested review from Y-Rookie Y-Rookie is a code owner

@iziang iziang Awaiting requested review from iziang iziang is a code owner

@sophon-zt sophon-zt Awaiting requested review from sophon-zt sophon-zt is a code owner

@kizuna-lek kizuna-lek Awaiting requested review from kizuna-lek kizuna-lek is a code owner

@xuriwuyun xuriwuyun Awaiting requested review from xuriwuyun xuriwuyun is a code owner

@caiq1nyu caiq1nyu Awaiting requested review from caiq1nyu caiq1nyu is a code owner

@1aal 1aal Awaiting requested review from 1aal

@fengluodb fengluodb Awaiting requested review from fengluodb fengluodb is a code owner

@Chen-speculation Chen-speculation Awaiting requested review from Chen-speculation Chen-speculation is a code owner

@ahjing99 ahjing99 Awaiting requested review from ahjing99 ahjing99 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@cjc7373 @zjx20 @ldming @codecov-commenter @apecloud-bot @leon-inf