Skip to content

Revert "More fine-grained control over powerful RBAC permission granted via Helm chart"#7836

Merged
cert-manager-prow[bot] merged 1 commit intomasterfrom
revert-7666-feat/alnoor/adding-fine-grained-control-over-controller-challenges-rbac
Jul 2, 2025
Merged

Revert "More fine-grained control over powerful RBAC permission granted via Helm chart"#7836
cert-manager-prow[bot] merged 1 commit intomasterfrom
revert-7666-feat/alnoor/adding-fine-grained-control-over-controller-challenges-rbac

Conversation

@inteon
Copy link
Copy Markdown
Member

@inteon inteon commented Jul 2, 2025

Reverts #7666.

As noted in #7598, there is an issue with informers that are failing due to the new RBAC.

We have a proposed fix here: #7823.
I'm reverting the PR to give ourselves a bit more time to finetune this fix and maybe consider disabling these informers instead of adding more RBAC.
The goal is to include the fix in v1.19.

Kind

/kind bug

Release Note

Reverted adding the `global.rbac.disableHTTPChallengesRole` Helm option.

@cert-manager-prow cert-manager-prow bot added dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. area/deploy Indicates a PR modifies deployment configuration needs-kind Indicates a PR lacks a `kind/foo` label and requires one. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 2, 2025
…ed via Helm chart"

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
@inteon inteon force-pushed the revert-7666-feat/alnoor/adding-fine-grained-control-over-controller-challenges-rbac branch from 127b951 to 587f0c2 Compare July 2, 2025 10:01
@cert-manager-prow cert-manager-prow bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Jul 2, 2025
Copy link
Copy Markdown
Member

@wallrj wallrj left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

Yes, I agree. The fix proposed in #7823 does not seem quite right.
For example, the ACME challenges controller not only relies on a Pod informer; it also relies on an ingress informer, unconditionally. So in the same way that the DNS-01 role needs permission to list and watch pods, it would strictly need permission to list and watch Ingress...and if we go down that path, the DNS-01 role would look overly permissive to a casual observer. It happens to work, because the Ingress list and watch permissions are also granted by roles designed for ingress-shim.
Let's try and figure out a better UX for this feature; something like global.disableHTTP01.

@cert-manager-prow cert-manager-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2025
@cert-manager-prow
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wallrj

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

The pull request process is described here

Details 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 approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 2, 2025
@inteon
Copy link
Copy Markdown
Member Author

inteon commented Jul 2, 2025

/cherrypick release-1.18

@cert-manager-bot
Copy link
Copy Markdown
Contributor

@inteon: once the present PR merges, I will cherry-pick it on top of release-1.18 in a new PR and assign it to you.

Details

In response to this:

/cherrypick release-1.18

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@cert-manager-prow cert-manager-prow bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 2, 2025
@inteon inteon added kind/bug Categorizes issue or PR as related to a bug. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jul 2, 2025
@cert-manager-prow cert-manager-prow bot merged commit e27cd10 into master Jul 2, 2025
6 checks passed
@cert-manager-bot
Copy link
Copy Markdown
Contributor

@inteon: new pull request created: #7837

Details

In response to this:

/cherrypick release-1.18

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

james-callahan added a commit to james-callahan/cert-manager-kustomize that referenced this pull request Jul 11, 2025
@erikgb erikgb deleted the revert-7666-feat/alnoor/adding-fine-grained-control-over-controller-challenges-rbac branch September 14, 2025 11:01
hswong3i added a commit to alvistack/ansible-role-helm_cert_manager that referenced this pull request Oct 3, 2025
…1.18.1

See cert-manager/cert-manager#7836

Signed-off-by: Wong Hoi Sing Edison <hswong3i@pantarei-design.com>
alexlebens pushed a commit to alexlebens/infrastructure that referenced this pull request Oct 8, 2025
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [cert-manager](https://cert-manager.io) ([source](https://github.com/cert-manager/cert-manager)) | minor | `v1.18.2` -> `v1.19.0` |

---

### Release Notes

<details>
<summary>cert-manager/cert-manager (cert-manager)</summary>

### [`v1.19.0`](https://github.com/cert-manager/cert-manager/releases/tag/v1.19.0)

[Compare Source](cert-manager/cert-manager@v1.18.2...v1.19.0)

cert-manager is the easiest way to automatically manage certificates in Kubernetes and OpenShift clusters.

This release focuses on expanding platform compatibility, improving deployment flexibility, enhancing observability, and addressing key reliability issues.

> 📖  Read the full release notes at cert-manager.io: <https://cert-manager.io/docs/releases/release-notes/release-notes-1.19>

Changes since `v1.18.0`:

#### Feature

- Add IPv6 rules to the default network policy ([#&#8203;7726](cert-manager/cert-manager#7726), [@&#8203;jcpunk](https://github.com/jcpunk))
- Add `global.nodeSelector` to helm chart to allow for a single `nodeSelector` to be set across all services. ([#&#8203;7818](cert-manager/cert-manager#7818), [@&#8203;StingRayZA](https://github.com/StingRayZA))
- Add a feature gate to default to Ingress `pathType` `Exact` in ACME HTTP01 Ingress challenge solvers. ([#&#8203;7795](cert-manager/cert-manager#7795), [@&#8203;sspreitzer](https://github.com/sspreitzer))
- Add generated `applyconfigurations` allowing clients to make type-safe server-side apply requests for cert-manager resources. ([#&#8203;7866](cert-manager/cert-manager#7866), [@&#8203;erikgb](https://github.com/erikgb))
- Added API defaults to issuer references group (cert-manager.io) and kind (Issuer). ([#&#8203;7414](cert-manager/cert-manager#7414), [@&#8203;erikgb](https://github.com/erikgb))
- Added `certmanager_certificate_challenge_status` Prometheus metric. ([#&#8203;7736](cert-manager/cert-manager#7736), [@&#8203;hjoshi123](https://github.com/hjoshi123))
- Added `protocol` field for `rfc2136` DNS01 provider ([#&#8203;7881](cert-manager/cert-manager#7881), [@&#8203;hjoshi123](https://github.com/hjoshi123))
- Added experimental field `hostUsers` flag to all pods. Not set by default. ([#&#8203;7973](cert-manager/cert-manager#7973), [@&#8203;hjoshi123](https://github.com/hjoshi123))
- Support configurable resource requests and limits for ACME HTTP01 solver pods through ClusterIssuer and Issuer specifications, allowing granular resource management that overrides global `--acme-http01-solver-resource-*` settings. ([#&#8203;7972](cert-manager/cert-manager#7972), [@&#8203;lunarwhite](https://github.com/lunarwhite))
- The `CAInjectorMerging` feature has been promoted to BETA and is now enabled by default ([#&#8203;8017](cert-manager/cert-manager#8017), [@&#8203;ThatsMrTalbot](https://github.com/ThatsMrTalbot))
- The controller, webhook and ca-injector now log their version and git commit on startup for easier debugging and support. ([#&#8203;8072](cert-manager/cert-manager#8072), [@&#8203;prasad89](https://github.com/prasad89))
- Updated `certificate` metrics to the collector approach. ([#&#8203;7856](cert-manager/cert-manager#7856), [@&#8203;hjoshi123](https://github.com/hjoshi123))

#### Bug or Regression

- ACME: Increased challenge authorization timeout to 2 minutes to fix `error waiting for authorization` ([#&#8203;7796](cert-manager/cert-manager#7796), [@&#8203;hjoshi123](https://github.com/hjoshi123))
- BUGFIX: permitted URI domains were incorrectly used to set the excluded URI domains in the CSR's name constraints ([#&#8203;7816](cert-manager/cert-manager#7816), [@&#8203;kinolaev](https://github.com/kinolaev))
- Enforced ACME HTTP-01 solver validation to properly reject configurations when multiple ingress options (`class`, `ingressClassName`, `name`) are specified simultaneously ([#&#8203;8021](cert-manager/cert-manager#8021), [@&#8203;lunarwhite](https://github.com/lunarwhite))
- Increase maximum sizes of PEM certificates and chains which can be parsed in cert-manager, to handle leaf certificates with large numbers of DNS names or other identities ([#&#8203;7961](cert-manager/cert-manager#7961), [@&#8203;SgtCoDFish](https://github.com/SgtCoDFish))
- Reverted adding the `global.rbac.disableHTTPChallengesRole` Helm option. ([#&#8203;7836](cert-manager/cert-manager#7836), [@&#8203;inteon](https://github.com/inteon))
- This change removes the `path` label of core ACME client metrics and will require users to update their monitoring dashboards and alerting rules if using those metrics. ([#&#8203;8109](cert-manager/cert-manager#8109), [@&#8203;mladen-rusev-cyberark](https://github.com/mladen-rusev-cyberark))
- Use the latest version of `ingress-nginx` in E2E tests to ensure compatibility ([#&#8203;7792](cert-manager/cert-manager#7792), [@&#8203;wallrj](https://github.com/wallrj))

#### Other (Cleanup or Flake)

- Helm: Fix naming template of `tokenrequest` RoleBinding resource to improve consistency ([#&#8203;7761](cert-manager/cert-manager#7761), [@&#8203;lunarwhite](https://github.com/lunarwhite))
- Improve error messages when certificates, CRLs or private keys fail admission due to malformed or missing PEM data ([#&#8203;7928](cert-manager/cert-manager#7928), [@&#8203;SgtCoDFish](https://github.com/SgtCoDFish))
- Major upgrade of Akamai SDK. NOTE: The new version has not been fully tested end-to-end due to the lack of cloud infrastructure. ([#&#8203;8003](cert-manager/cert-manager#8003), [@&#8203;hjoshi123](https://github.com/hjoshi123))
- Update kind images to include the Kubernetes 1.33 node image ([#&#8203;7786](cert-manager/cert-manager#7786), [@&#8203;wallrj](https://github.com/wallrj))
- Use `maps.Copy` for cleaner map handling ([#&#8203;8092](cert-manager/cert-manager#8092), [@&#8203;quantpoet](https://github.com/quantpoet))
- Vault: Migrate Vault E2E add-on tests from deprecated `vault-client-go` to the new `vault/api` client. ([#&#8203;8059](cert-manager/cert-manager#8059), [@&#8203;armagankaratosun](https://github.com/armagankaratosun))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xMzUuNCIsInVwZGF0ZWRJblZlciI6IjQxLjEzNS40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJjaGFydCJdfQ==-->

Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/1711
Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net>
Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/deploy Indicates a PR modifies deployment configuration dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants