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

WIP: POC of dynamic authority #468

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

erikgb
Copy link
Contributor

@erikgb erikgb commented Nov 5, 2024

This PR was created to show how trust-manager could look like without a requirement for cert-manager to be pre-installed. It is not supposed to be merged - at least not in the present state.

Inspired by the bootstrap mechanism cert-manager uses to bootstrap it's webhooks, I created https://github.com/erikgb/dynamic-authority. I have no plans to maintain such a project alone, and most of the code comes from cert-manager, so I hope this eventually could become a new project under the cert-manager org. 😺

I also had to copy some awesome PKI code from cert-manager into the new project. IMO we should create a dedicated cert-manager PKI project with all the nice PKI helpers battle-tested as part of cert-manager. This could allow reuse across other cert-manager projects without the unfortunate dependency to cert-manager/cert-manager.

@cert-manager-prow
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@cert-manager-prow cert-manager-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. labels Nov 5, 2024
@cert-manager-prow
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from erikgb. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cert-manager-prow cert-manager-prow bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 5, 2024
@erikgb
Copy link
Contributor Author

erikgb commented Nov 5, 2024

/test all

@erikgb erikgb force-pushed the dynamic-authority-poc branch 2 times, most recently from 0e09db6 to 9facda4 Compare November 5, 2024 09:43
@erikgb
Copy link
Contributor Author

erikgb commented Nov 5, 2024

/test all

@cert-manager-prow cert-manager-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 5, 2024
@erikgb
Copy link
Contributor Author

erikgb commented Nov 5, 2024

/test pull-trust-manager-smoke

@erikgb
Copy link
Contributor Author

erikgb commented Nov 5, 2024

/test pull-trust-manager-smoke

@erikgb
Copy link
Contributor Author

erikgb commented Nov 5, 2024

/test pull-trust-manager-smoke

@erikgb
Copy link
Contributor Author

erikgb commented Nov 5, 2024

/test pull-trust-manager-smoke

@erikgb
Copy link
Contributor Author

erikgb commented Nov 5, 2024

/test pull-trust-manager-smoke

1 similar comment
@erikgb
Copy link
Contributor Author

erikgb commented Nov 5, 2024

/test pull-trust-manager-smoke

@cert-manager-prow cert-manager-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 6, 2024
@cert-manager-prow cert-manager-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2024
@erikgb
Copy link
Contributor Author

erikgb commented Nov 9, 2024

/test pull-trust-manager-smoke

@erikgb
Copy link
Contributor Author

erikgb commented Nov 9, 2024

/test pull-trust-manager-smoke

@erikgb erikgb force-pushed the dynamic-authority-poc branch 2 times, most recently from e67c46a to 55cec37 Compare November 15, 2024 19:28
@erikgb
Copy link
Contributor Author

erikgb commented Nov 15, 2024

/test pull-trust-manager-smoke

@erikgb
Copy link
Contributor Author

erikgb commented Nov 15, 2024

/test pull-trust-manager-smoke

@erikgb erikgb force-pushed the dynamic-authority-poc branch 3 times, most recently from 2212acd to 1d48d46 Compare November 15, 2024 20:07
@erikgb
Copy link
Contributor Author

erikgb commented Nov 15, 2024

/test pull-trust-manager-smoke

@erikgb
Copy link
Contributor Author

erikgb commented Nov 15, 2024

/hold

@cert-manager-prow cert-manager-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 15, 2024
@erikgb erikgb marked this pull request as ready for review November 15, 2024 20:13
Signed-off-by: Erik Godding Boye <[email protected]>
@erikgb
Copy link
Contributor Author

erikgb commented Nov 23, 2024

@SgtCoDFish I have now reconfigured the RBAC to (almost) least-privilege, as you commented in one of our stand-ups where we discussed this PR. The new control loops still need RBAC to read all validation webhook configurations (cluster-wide) and secrets in the controller namespace (it already has this permission). But the create/patch permissions are now targeted on resources by name. I personally don't see a big benefit of narrowing it even more, as I have a feeling controller-runtime doesn't support watching resources by name out-of-the-box and will require even more complex code in the new module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant