Watch Secret permission introduced by PR #963 breaks dynamic low-privilege multi-tenant set-ups

[armingerten]

Which component

controller

Describe the bug

Due to the implementation of #963 , versions of the sealed secrets controller >= v0.19.0 are no longer installable in dynamic low-privilege multi-tenant environments. The properties of the environment are:

• multi-tenant: Namespaces are isolated "per tenant"
• low-privilege: The sealed secret controller must only access resources of kind Secret in specific namespaces; No ClusterRoleBinding can be used.
• dynamic: Namespaces are added/removed in a high frequency.

The implementation of #568 already allows watching only specific namespaces. However, this static list must be adapted whenever a namespace is added / removed.

Possible solutions

Dynamically create Secret for namespaces that contain SealedSecret resources

Whenever the sealed secret controller picks up a new SealedSecret resource, it creates a watch for Secret resources within the same namespace, unless a watch already exists. Whenever a SealedSecret resource is deleted, the watch for Secret resources within the same namespace is removed, if it was the last SealedSecret in the same namespace.

Watch Namespaces for specific labels to include

Instead of maintaining a static list of namespaces when deploying the controller, the controller could watch namespace resources for a specific label (e.g. sealed-secrets: enabled). If the label is present, the controller creates watches for Secret (and SealedSecret?) resources.

Feature flag for #963

Another option could be a feature flag to disable the behavior introduced by #963 .

[armingerten]

This issue also relates to #233 .

[alemorcuq]

Hi, @armingerten.

Thanks for reporting this. Just to confirm I understand the issue here, you are using a single Sealed Secrets controller that watches for Sealed Secrets in all namespaces. However, this controller should not have access to the secrets in all namespaces but just in the ones that have Sealed Secrets. Is that right?

I understand the problem arises when adding new namespaces. The controller can see the Sealed Secrets in these new namespaces but not the Secrets, as you don't want to give the controller permission to see the Secrets in all namespaces (which makes total sense). Did I get that right?

Also, thanks for the proposed solutions. I think I'm leaning more towards (1), as (2) would disrupt the way users use Sealed Secrets (having to add a new label) and (3) is a bit ugly.

[armingerten]

Thanks for reporting this. Just to confirm I understand the issue here, you are using a single Sealed Secrets controller that watches for Sealed Secrets in all namespaces. However, this controller should not have access to the secrets in all namespaces but just in the ones that have Sealed Secrets. Is that right?

Correct. I am using one Sealed Secrets controller for most namespaces. Some namespaces (i.e. namespaces containing critical infrastructure components) are not considered for Sealed Secrets.

I understand the problem arises when adding new namespaces. The controller can see the Sealed Secrets in these new namespaces but not the Secrets, as you don't want to give the controller permission to see the Secrets in all namespaces (which makes total sense). Did I get that right?

Exactly.

[github-actions]

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

[tewfik-ghariani]

#Do_not_close

[github-actions]

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

[armingerten]

/bump

[alemorcuq]

We don't want to add the complexity in #1081 to the controller, so we will be working on a different solution to this problem.

[josvazg]

We will open a couple of separate issues due to this:

• To be backward compatible, a way to achieve that is to give up on recreation when lacking the permissions: PR #963 unintentional backwards compatibility break #1091
• Support an intermediate setup scenario in which namespace roles are used rather than cluster wide ones: Intermediate controller sharing scenario #1092

[github-actions]

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

[tewfik-ghariani]

#Not_stale

[github-actions]

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

[github-actions]

Due to the lack of activity in the last 7 days since it was marked as "stale", we proceed to close this Issue. Do not hesitate to reopen it later if necessary.

[tewfik-ghariani]

#Not_Stale

[josvazg]

@tewfik-ghariani @armingerten with #1118 merged we expect your use case can be covered by setting the new Helm chart value skipRecreate to false.

Please, let us know how it goes or if you have any concerns.

[tewfik-ghariani]

@josvazg Thank you so much for working on that!

[alemorcuq]

Hi, @tewfik-ghariani, @armingerten. A new version was released including a flag to disable secret recreation. Thanks for your patience.

[tewfik-ghariani]

Hi @alemorcuq , thank you so much for the follow-up!

Unfortunately, I have encountered an bug after enabling it. I created a new issue for it: #1151