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 wildcard value for namespaces in Alert eventSources #504

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

alekspog
Copy link
Contributor

Allow wildcard value for namespaces in Alert eventSources.

Fix: #71

At the moment we have to create a list of all HelmRelease objects, for example, in the eventSources section to be able to get notifications about them. We also need to update the list whenever a new object is added. That requires lots of effort. It seems that the best way is to allow wildcards for namespaces.

How the Alert eventSources definition looks now:

  eventSources:
    - kind: HelmRelease
      name: '*'
      namespace: 'service1'
    - kind: HelmRelease
      name: '*'
      namespace: 'service2'
...
    - kind: HelmRelease
      name: '*'
      namespace: 'service99'

How it would be after the change:

  eventSources:
    - kind: HelmRelease
      name: '*'
      namespace: '*'

@alekspog alekspog force-pushed the feature/allow-wildcard-for-namespaces branch from 4dce5ea to 5df068a Compare March 30, 2023 19:39
Signed-off-by: alekspog <aleksey.pogibelev@gmail.com>
Signed-off-by: alekspog <aleksey.pogibelev@gmail.com>
Signed-off-by: alekspog <aleksey.pogibelev@gmail.com>
Signed-off-by: alekspog <aleksey.pogibelev@gmail.com>
Signed-off-by: alekspog <aleksey.pogibelev@gmail.com>
Signed-off-by: alekspog <aleksey.pogibelev@gmail.com>
Signed-off-by: alekspog <aleksey.pogibelev@gmail.com>
Signed-off-by: alekspog <aleksey.pogibelev@gmail.com>
@alekspog alekspog force-pushed the feature/allow-wildcard-for-namespaces branch from 5d412ad to 371ea46 Compare March 30, 2023 19:58
Signed-off-by: alekspog <aleksey.pogibelev@gmail.com>
@makkes makkes added the hold Issues and pull requests put on hold label Mar 31, 2023
@makkes
Copy link
Member

makkes commented Mar 31, 2023

There are security concerns related to this feature that were articulated in #71 (e.g. #71 (comment)). Given the mentioned RFC has never been merged, we need to think this over.

In any case this will need to be held until notification-controller 1.0.0 has been released. Let's please take the time to completely discuss the threat model and potential mitigations here.

@alekspog
Copy link
Contributor Author

alekspog commented Apr 3, 2023

I thought that disabling cross-namespace references should solve the security concern as was said there:
#71 (comment). I agree that using ReferenceGrant should improve the security boundaries. Still, it seems to be orthogonal to the Alerts setup. notification-controller can listen to events in any namespace anyway if the cross-namespace references flag is enabled. If this is inappropriate cluster admin can disable cross-namespace references.

Are there any ongoing discussions about the threat model? I see that fluxcd/flux2#2092 and fluxcd/flux2#2093 were closed both to wait for https://github.com/kubernetes/enhancements/blob/master/keps/sig-auth/3766-referencegrant/README.md

@xTrekStorex
Copy link

any update on this?

@sebastiangaiser
Copy link

How about using regex matching for the namespace(s)?

@gecube
Copy link

gecube commented Nov 1, 2023

Hi! Is it correct to say that this PR is still on hold and we are waiting?

@makkes
Copy link
Member

makkes commented Nov 9, 2023

Hi! Is it correct to say that this PR is still on hold and we are waiting?

Yes, pretty much. Current consensus is that we want to wait for the Kubernetes ReferenceGrant API to become GA which will definitely take a couple of Kubernetes releases to come.

@MOZGIII
Copy link

MOZGIII commented Dec 20, 2023

This feature request seems irrelevant to ReferenceGrant API; what ReferenceGrant will affect is all of the cross-namespace references filtering - but that'd have to work at a different level. I see no point in holding on with this PR.
The current model is pretty secure - this PR does not make it weaker.

@rinzler-17
Copy link

Hi, any update on this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hold Issues and pull requests put on hold
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add possibility to configure wildcard namespace value in Alert eventSources
7 participants