-
Notifications
You must be signed in to change notification settings - Fork 74
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
Bundle: namespaceSelector
#37
Bundle: namespaceSelector
#37
Conversation
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
ConfigMaps for namespaces which are no longer matching namespaces Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
/hold
Looks like a good improvement to me 😁
I'm a little squeamish about syncing to all namespaces as the default. I'd probably prefer to default only to syncing to the trust namespace; IMO since this project is so new it's reasonable for us to make a breaking change like that.
But that's a tangent from this PR which is good as-is 😁
EDIT: I added a hold since this wasn't assigned to me; maybe you want more eyes on it. If not, happy to unhold and proceed!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoshVanL, SgtCoDFish The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@SgtCoDFish As discussed on the standup, I think I agree that syncing to all namespaces by default is probably not the way to go, and we should make the namespace selector a required field in future PRs. /hold cancel |
/test pull-cert-manager-trust-verify |
/test pull-cert-manager-trust-smoke |
Cool cool; I'll create issues now for that and for the root cert bundle thing we mentioned! |
Hello @SgtCoDFish, a new version (0.1.2) of the Helm chart with support on CRD level was released, but the functionality in the application itself was not released. It there a plan to align this, please? |
Hi! maybe @JoshVanL can help, I can't I'm afraid |
@jaygridley I'll go ahead with a minor release with this feature now 🙂 |
This PR adds a namespaceSelector option to the bundle.spec.target API.
The namespace selector currently only contains a
MatchLabels map[string]string
field.If the label selector is empty, it matches on all Namespaces (the current behaviour).
Hopefully this is quite self explanatory slightly_smiling_face
A target ConfigMap will now receive an event with a relevant message when the target ConfigMap is not deleted because it is not owned by the trust operator.
/assign @munnerz
fixes #1
Docs PR