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

Allow configuration of namespaces watched for secrets #3154

Closed
hafe opened this issue Oct 15, 2022 · 6 comments · Fixed by #3170
Closed

Allow configuration of namespaces watched for secrets #3154

hafe opened this issue Oct 15, 2022 · 6 comments · Fixed by #3170
Labels
proposal An issue that proposes a feature request

Comments

@hafe
Copy link
Contributor

hafe commented Oct 15, 2022

Is your feature request related to a problem? Please describe.

Either all (by default) or a subset (new 2.4 feature) of namespaces are watched for ingress related resources BUT ALSO SECRETS..

This means all found secrets are cached in the ingress controller even if not needed/used. This of course is a security concern.

With multi tenancy and namespace isolation as described by:

"Enabling Multi-Tenancy and Namespace Isolation in Kubernetes with NGINX - NGINX" https://www.nginx.com/blog/enabling-multi-tenancy-namespace-isolation-in-kubernetes-with-nginx/

There is no requirement/need for the controller to read secrets from the application namespaces.

Thus security can be improved by allowing to configure what namespaces should be watched for secrets.

Using custom RBAC roles and bindings the cluster admin can fine tune the controller privileges to the minimum for it to work.

Describe the solution you'd like

One possible and simple solution is to provide an additional flag similar to "watch-namespace" but for secrets. Let's call it "watch-secret-namespaces".

For this I have a patch I will submit for review.

Describe alternatives you've considered

Other implementations are more complex using for example annotations/labels on namespaces

Additional context

@hafe hafe added the proposal An issue that proposes a feature request label Oct 15, 2022
@github-actions
Copy link

Hi @hafe thanks for reporting!

Be sure to check out the docs while you wait for a human to take a look at this 🙂

Cheers!

@brianehlert
Copy link
Collaborator

brianehlert commented Oct 21, 2022

@hafe I think I am starting to understand what you are attempting to do with the proposed extra command line option.
Let me describe back to you and see if I have it correct.
I have the understanding that you don't want the controller process to even know about (list) a secret that it does not need. As we have chatted in other threads about list vs read.

You have this ingress controller project. And you have that restricted to watching a list of namespaces.
Within each of those namespaces you have the services that the controller process should be watching - secrets, Ingress, VirtualServer, etc.

However, in your case you also have additional artifacts that you don't want the controller process to enumerate. Specifically, secrets. Essentially your RBAC logic is: "watch the relative things in these namespaces except the secrets".
And in order to support a configuration like that, you might have a separate namespace that only has secrets that the ingress controller cares about.

The reason that I am asking for clarification is that we are trying to fully understand the use case behind an additional flag, and not simply adding an additional secrets namespace to the list of watched namespaces.

If all of that is true, what is your business requirement is behind having secrets in the application space that are somehow not related? I am guessing there is some business requirement behind what your organization is doing that we are not fully understanding.

Also, if you could help me understand this statement:

Other implementations are more complex using for example annotations/labels on namespaces

@hafe
Copy link
Contributor Author

hafe commented Oct 23, 2022

@hafe I think I am starting to understand what you are attempting to do with the proposed extra command line option. Let me describe back to you and see if I have it correct. I have the understanding that you don't want the controller process to even know about (list) a secret that it does not need. As we have chatted in other threads about list vs read.

Correct. List (unfortunately) means read with K8s RBAC.

You have this ingress controller project. And you have that restricted to watching a list of namespaces. Within each of those namespaces you have the services that the controller process should be watching - secrets, Ingress, VirtualServer, etc.

However, in your case you also have additional artifacts that you don't want the controller process to enumerate. Specifically, secrets. Essentially your RBAC logic is: "watch the relative things in these namespaces except the secrets". And in order to support a configuration like that, you might have a separate namespace that only has secrets that the ingress controller cares about.

Well kind of. I found the blog post, especially the "Restricted Self-Service" model but also cross-namespace-configuration.

In that setup, an Ingress (master) or VirtualServer together with secrets are created in e.g. the controller namespace. An Ingress (minion) and/or VirtualServerRoute are located in application namespaces.

With this separation, there is no need for the ingress controller to read secrets in the application namespace. It only needs to protected with RBAC since the controller still needs to list/read secrets. Otherwise it will never become ready!

The reason that I am asking for clarification is that we are trying to fully understand the use case behind an additional flag, and not simply adding an additional secrets namespace to the list of watched namespaces.

Using an additional secrets namespace is not possible since Ingress, Secret, Service and POD needs to be in the same namespace. I believe the same is true for VirtualServer.

The use case of the new additional flag is to support RBAC for the "Restricted Self-Service" model .

If all of that is true, what is your business requirement is behind having secrets in the application space that are somehow not related? I am guessing there is some business requirement behind what your organization is doing that we are not fully understanding.

Just security requirements. Considering that the ingress controller is usually Internet facing, it should not have access to secrets not needed.

Also, if you could help me understand this statement:

Other implementations are more complex using for example annotations/labels on namespaces

I just realised that there could other ways of implementing the same thing. Just found it more complex to start reading annotations/labels from namespaces and settled with a new command line option.

But again the ideal solution is for the ingress controller to only watch secrets actually referenced from Ingress & VirtualServer but that seems a lot harder to implement.

@brianehlert
Copy link
Collaborator

In doing a bit of additional searching and reading...
I think the behavior that you might be looking for is an explicit get and watch for individual secrets.
So that only secrets that are expressly configured would be queried for and watched.

I thought this was a useful description of combinations:
https://stackoverflow.com/questions/58069073/kubernetes-rbac-verbs-get-without-list-and-vice-versa-watch-without-list/58077113#58077113

it would require some type of namespace/name pattern to refer to a secret, as opposed to simply name.
get to return the secret details and watch to get notified on change.

That would be a fundamental behavior change specific to secrets enumeration.
The secret that would be the exception is the secret the admission controller grants to the ingress controller to access the API. This would be present in the ingress controller deployment namespace.

@hafe
Copy link
Contributor Author

hafe commented Oct 24, 2022

I read that get+watch does not work. Meaning that solution might not be possible to implement.

My patch is just supporting hardened rbac for the "cross namespace" configuration. Which effectively achieves the same level of security, in a different way.

@brianehlert
Copy link
Collaborator

We have come to detail your use case in the following way (to help folks understand - as you are taking the RBAC model to a different level of granularity):

  • NIC watches all resources BUT secrets in namespaces admin1,admin2,application1,application2
  • NIC watches JUST secrets in namespaces admin1,admin2
  • Admin deploys secret vs-secret, and VirtualServer vs1, in namespace admin1
  • Admin deploys secret ingress-secret, and master Ingress master1 in namespace admin2
  • App developer 1 deploys VirtualServerRoute vsr1 in namespace application1, along with the service and application
  • App developer 2 deploys minion Ingress minion1 in namespace application2, along with the service and application
  • VirtualServer admin1/vs1 references secret admin1/vs-secret, and VirtualServerRoute application1/vsr1
  • Master Ingress admin2/ing1 references secret admin2/ingress-secret, and has same hostname as minion Ingress application2/minion1
  • No secret access is required in namespaces application1 or application2
    • this could be rephrased to say the app developer does not have any access to any of the secrets. Or, there are specific secrets in these namespaces that NIC does not need / should not access.

hafe added a commit to hafe/kubernetes-ingress that referenced this issue Nov 8, 2022
Add new command line option "watch-secret-namespace" that can be used to
configure namespaces watched for secrets.

Closes nginx#3154
ciarams87 added a commit that referenced this issue Nov 10, 2022
* Watch subset of namespaces for secrets

Add new command line option "watch-secret-namespace" that can be used to
configure namespaces watched for secrets.

Closes #3154

* Apply suggestions from code review

Co-authored-by: Ciara Stacke <[email protected]>

* Update cmd/nginx-ingress/flags.go

Co-authored-by: Ciara Stacke <[email protected]>
coolbry95 pushed a commit to coolbry95/kubernetes-ingress that referenced this issue Nov 18, 2022
* Watch subset of namespaces for secrets

Add new command line option "watch-secret-namespace" that can be used to
configure namespaces watched for secrets.

Closes nginx#3154

* Apply suggestions from code review

Co-authored-by: Ciara Stacke <[email protected]>

* Update cmd/nginx-ingress/flags.go

Co-authored-by: Ciara Stacke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal An issue that proposes a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants