search

cloudfoundry-incubator / eirinix

Extensions Library for Cloud Foundry Eirini
Apache License 2.0
2 stars 5 forks source link

Allow empty namespace to monitor any namespace #47

Closed jimmykarily closed 4 years ago

jimmykarily commented 4 years ago

and handle permissions errors when creating the webhook. Also create the webhook secret in the same namespace where the webhook is deployed, rather than the monitored namespace.

[Fixes #45]

Signed-off-by: Kieron Browne kbrowne@pivotal.io

jimmykarily commented 4 years ago

I can't tell if this is the correct value or not: https://github.com/cloudfoundry-incubator/eirinix/blob/master/manager.go#L316 . @mudler ? I mean, should it be changed to WebhookNamespace instead? My understanding is that this can be empty to allow monitoring any namespace. Is that correct?

mudler commented 4 years ago

I can't tell if this is the correct value or not: https://github.com/cloudfoundry-incubator/eirinix/blob/master/manager.go#L316 . @mudler ? I mean, should it be changed to WebhookNamespace instead? My understanding is that this can be empty to allow monitoring any namespace. Is that correct?

Yes, I think that's what we want at the end:

It looks good, I think we should just extend the same mechanism for Watchers to be consistent (maybe in a later roundup?). On the other hand isn't hard to address, just needs to optionally set the namespace, for e.g. here : https://github.com/cloudfoundry-incubator/eirinix/commit/5b4be51749d2b60c5433d3ea83d36749b5458c1a

jimmykarily commented 4 years ago

@mudler a test has been added to check that it works for Watchers too: https://github.com/cloudfoundry-incubator/eirinix/pull/47/files#diff-3032bdd5c889553a7a1a73af5a64173fR310 (it was working already).

We can merge this I think.