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

Use token based approach for system-agent #769

Merged

Conversation

Danil-Grigorev
Copy link
Contributor

@Danil-Grigorev Danil-Grigorev commented Oct 4, 2024

What this PR does / why we need it:
Reduce the footprint of the system-agent RBAC.

Per each cluster there will be created:

  • 1 system-agent ServiceAccount

Per each plan there will be temporarily created:

  • 1 Role with access to all plan secrets for each machine
  • 1 Rolebinging for the role and the cluster system-agent ServiceAccount

On plan completion/failure the role and rolebinding will be revoked

Per each machine there will be created:

  • 1 Secret for the system-agent authentication, with unique JWT bound to the secret existence in the API server, and a namespace/name pointer to the plan secret
  • 1 Secret for the plan execution

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #756

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

@Danil-Grigorev Danil-Grigorev requested a review from a team as a code owner October 4, 2024 11:43
@Danil-Grigorev Danil-Grigorev force-pushed the snapshot-restore-token-approach branch 3 times, most recently from 6b5657a to c9eeb3c Compare October 10, 2024 09:00
@Danil-Grigorev
Copy link
Contributor Author

To test issuing token we need a change which is not yet released in CR: kubernetes-sigs/controller-runtime#2969

Getting “ fakeSubResourceWriter does not support create for token”

@Danil-Grigorev Danil-Grigorev changed the title [WIP] Use token based approach for system-agent Use token based approach for system-agent Oct 18, 2024
Copy link
Member

@alexander-demicev alexander-demicev left a comment

Choose a reason for hiding this comment

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

lgtm, no comments are blocking merging this PR

exp/etcdrestore/webhooks/rke2config.go Show resolved Hide resolved
exp/etcdrestore/webhooks/rke2config.go Show resolved Hide resolved
exp/etcdrestore/webhooks/rke2config.go Show resolved Hide resolved
salasberryfin
salasberryfin previously approved these changes Oct 22, 2024
Copy link
Contributor

@salasberryfin salasberryfin left a comment

Choose a reason for hiding this comment

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

LGTM but it needs a rebase.

Reduce the footprint of the system-agent RBAC
Per each cluster there will be created:
- 1 system-agent ServiceAccount

Per each plan there will be temporarily created:
- 1 Role with access to all plan secrets for each machine
- 1 Rolebinging for the role and the cluster system-agent ServiceAccount
On plan completion/failure the role and rolebinding will be rewoked

Per each machine there will be created:
- 1 Secret for the system-agent authentication, with unique JWT bound to
  the secret existence in the API server, and a namespace/name pointer
  to the plan secret
- 1 Secret for the plan execution

Signed-off-by: Danil-Grigorev <[email protected]>
Signed-off-by: Danil-Grigorev <[email protected]>
@Danil-Grigorev
Copy link
Contributor Author

I’ll try to address the rest of the comments in the followup PRs.

@Danil-Grigorev Danil-Grigorev merged commit 81c0082 into rancher:main Oct 24, 2024
9 checks passed
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.

Investigate alternative authentication mechanisms
4 participants