cert-manager / trust-manager

trust-manager is an operator for distributing trust bundles across a Kubernetes cluster.
https://cert-manager.io/docs/projects/trust-manager/
Apache License 2.0
244 stars 66 forks source link

Custom trust namespace - permissions issue #224

Closed tspearconquest closed 10 months ago

tspearconquest commented 10 months ago

Hello,

I've configured trust-manager as a subchart in order to install it alongside my cert-manager deployment, and I added the below values:

app:
  namespace: trust-manager
  trust:
    namespace: trust

When installing trust-manager, the role and rolebinding for trust-manager to manage its leases is configured against the trust namespace and trust-manager uses the trust namespace as per the --trust-namespace flag.

However, as per the documentation:

An ideal deployment would be a fresh namespace dedicated entirely to trust-manager, to minimize the number of actors in your cluster that can modify your trust sources.

To me, this means that the leases would be kept in the namespace where trust manager is installed, and the "trust" namespace will only contain secrets to be read by trust-manager for creating a bundle.

I've created a patch for the helm chart to setup permissions for the leases separately so that trust-manager will be granted privileges to manage the leases in its namespace instead of the trust namespace, but in order to make this patch effective, we also need for trust-manager to not use the trust namespace for its leases, and instead either read the POD_NAMESPACE variable from the environment to set the namespace for the leases, or provide a separate flag to specify the trust-manager namespace to the binary to be able to properly set the namespace for the leases.

tspearconquest commented 10 months ago

225

erikgb commented 10 months ago

@tspearconquest Thanks for this issue. This seems like a bug. I would also expect the leader election lease to be located in the same namespace as trust-manager is running, and not the trust namespace. But I think the fix should be quite simple. See: https://github.com/kubernetes-sigs/controller-runtime/blob/9f9f369134d82816648b691c168008c2d0aa9bfe/pkg/leaderelection/leader_election.go#L74-L80.

I think we just have to make a simple change in Go code. Just delete this line: https://github.com/cert-manager/trust-manager/blob/cb1bef1552a8d175e61c11793832661869e3c5fe/cmd/trust-manager/app/app.go#L81

But we need to configure RBAC correctly.

tspearconquest commented 10 months ago

225 should resolve the RBAC issue because it separates the lease permission to its own role and rolebinding, and they are both pointed at the deployment namespace rather than the trust namespace. I will submit a separate PR for the deletion of line 81 in leader_election.go.

tspearconquest commented 10 months ago

https://github.com/cert-manager/trust-manager/pull/226 deletes the line in question.

erikgb commented 10 months ago

I think the change should go in the same PR. RBAC and controller behavior must be in sync.

tspearconquest commented 10 months ago

Closing #226, and have updated #225

SgtCoDFish commented 10 months ago

Closing as #225 has merged 👍