Skip to content

Delay role/rolebinding creation to gha-runner-scale-set installation time#2363

Merged
TingluoHuang merged 1 commit into
masterfrom
users/tihuang/splitclusterrole
Mar 14, 2023
Merged

Delay role/rolebinding creation to gha-runner-scale-set installation time#2363
TingluoHuang merged 1 commit into
masterfrom
users/tihuang/splitclusterrole

Conversation

@TingluoHuang

Copy link
Copy Markdown
Member

Part 2 of #2275

  • Keep ClusterRole/ClusterRoleBinding to manage CRD and List/Watch low-risk resources.
  • New Role/RoleBinding to make sure the manager has permission to create an AutoScalingListener in the controller installed namespace.
  • New Role/RoleBinding to make sure the manager has permission to do everything in the namespace where the gha-runner-scale-set is installed.

@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch from 7a35c25 to fb10c0d Compare March 6, 2023 20:03
@TingluoHuang TingluoHuang changed the title [WIP] Split permission per namespace. Delay role/rolebinding creation to gha-runner-scale-set installation time Mar 6, 2023
@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch 2 times, most recently from 2eaa39f to e393667 Compare March 6, 2023 20:38
@TingluoHuang TingluoHuang force-pushed the users/tihuang/nolistsecrets branch from fbfc944 to b54ab93 Compare March 6, 2023 20:40
@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch 3 times, most recently from e25d041 to fedc29a Compare March 6, 2023 21:40
@TingluoHuang TingluoHuang marked this pull request as ready for review March 6, 2023 21:42
@TingluoHuang

Copy link
Copy Markdown
Member Author

the Check (Validate Helm Chart / Lint Chart (push)) is failing because we haven't published any image that supports trimming permission down.

@wherka-ama

Copy link
Copy Markdown
Contributor

the Check (Validate Helm Chart / Lint Chart (push)) is failing because we haven't published any image that supports trimming permission down.

@TingluoHuang : thanks for your great work! Much appreciated.

Any idea on the timeline please? Do you know when this version will become generally available?

@uhthomas

uhthomas commented Mar 7, 2023

Copy link
Copy Markdown
Contributor

This is exactly the change we've been looking for before deploying the controller in our cluster, thank you for your effort. Is it possible to remove the cluster roles entirely?

@TingluoHuang

Copy link
Copy Markdown
Member Author

This is exactly the change we've been looking for before deploying the controller in our cluster, thank you for your effort. Is it possible to remove the cluster roles entirely?

@uhthomas #2374 can remove the cluster role, but the trade off is the controller can only create AutoScalingRunnerSet in a single pre-defined namespace.

@uhthomas

uhthomas commented Mar 9, 2023

Copy link
Copy Markdown
Contributor

This is exactly the change we've been looking for before deploying the controller in our cluster, thank you for your effort. Is it possible to remove the cluster roles entirely?

@uhthomas #2374 can remove the cluster role, but the trade off is the controller can only create AutoScalingRunnerSet in a single pre-defined namespace.

Awesome, thank you. This is exactly what we are looking for. We have very strict access controls and intend to run the operator in a single predefined namespace. Is there anything I can do to help get this over the line?

@TingluoHuang TingluoHuang force-pushed the users/tihuang/nolistsecrets branch from b54ab93 to 6729502 Compare March 9, 2023 18:40
@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch 2 times, most recently from 1725f23 to b1d9c03 Compare March 9, 2023 22:31
@TingluoHuang TingluoHuang force-pushed the users/tihuang/nolistsecrets branch 2 times, most recently from 926a55e to c92720f Compare March 10, 2023 21:34
@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch from b1d9c03 to 8d924ae Compare March 10, 2023 22:05
@TingluoHuang TingluoHuang force-pushed the users/tihuang/nolistsecrets branch from c92720f to 95bd192 Compare March 11, 2023 00:37
@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch from 8d924ae to fb2b6a0 Compare March 11, 2023 00:38
@TingluoHuang TingluoHuang force-pushed the users/tihuang/nolistsecrets branch from 95bd192 to 86c3186 Compare March 11, 2023 02:25
@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch from fb2b6a0 to b9c7561 Compare March 11, 2023 02:26
@TingluoHuang TingluoHuang force-pushed the users/tihuang/nolistsecrets branch from 86c3186 to d1e4297 Compare March 11, 2023 05:09
@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch from b9c7561 to 2d1e2d2 Compare March 11, 2023 05:10
Comment thread charts/gha-runner-scale-set/templates/_helpers.tpl Outdated
Comment thread charts/gha-runner-scale-set/templates/_helpers.tpl Outdated
Comment thread charts/gha-runner-scale-set/templates/_helpers.tpl Outdated
Comment thread charts/gha-runner-scale-set/templates/_helpers.tpl Outdated
Comment thread charts/gha-runner-scale-set/templates/_helpers.tpl Outdated
Comment thread charts/gha-runner-scale-set/templates/_helpers.tpl Outdated
rentziass
rentziass previously approved these changes Mar 13, 2023

@rentziass rentziass left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left a few nits but looks good to me! :shipit:

@nikola-jokic nikola-jokic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! 🚀

@TingluoHuang TingluoHuang force-pushed the users/tihuang/nolistsecrets branch from d1e4297 to 92bb99c Compare March 13, 2023 19:39
@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch 2 times, most recently from ca31c3e to 9b97a9f Compare March 13, 2023 19:42
@TingluoHuang TingluoHuang force-pushed the users/tihuang/nolistsecrets branch from 92bb99c to 5973a04 Compare March 13, 2023 20:06
@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch from 9b97a9f to 0825c8b Compare March 13, 2023 20:08
@TingluoHuang TingluoHuang force-pushed the users/tihuang/nolistsecrets branch from 5973a04 to 932d85c Compare March 14, 2023 13:01
@TingluoHuang TingluoHuang force-pushed the users/tihuang/splitclusterrole branch from 0825c8b to 1744fe4 Compare March 14, 2023 13:02
nikola-jokic
nikola-jokic previously approved these changes Mar 14, 2023
Base automatically changed from users/tihuang/nolistsecrets to master March 14, 2023 13:23
@TingluoHuang TingluoHuang merged commit 9e6c7d0 into master Mar 14, 2023
@TingluoHuang TingluoHuang deleted the users/tihuang/splitclusterrole branch March 14, 2023 13:45
unpollito pushed a commit to DistruApp/actions-runner-controller that referenced this pull request Jan 21, 2026
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.

5 participants