From ab51050ce8fdbb0ea9ef086a3bf47cbe7a39b06f Mon Sep 17 00:00:00 2001 From: Isabel Suchanek Date: Thu, 2 Sep 2021 09:36:55 -0700 Subject: [PATCH] events: fix wildcard namespace handling (#10935) This fixes a bug in the event stream API where it currently interprets namespace=* as an actual namespace, not a wildcard. When Nomad parses incoming requests, it sets namespace to default if not specified, which means the request namespace will never be an empty string, which is what the event subscription was checking for. This changes the conditional logic to check for a wildcard namespace instead of an empty one. It also updates some event tests to include the default namespace in the subscription to match current behavior. Fixes #10903 --- .changelog/10935.txt | 3 +++ nomad/fsm_test.go | 2 ++ nomad/state/deployment_events_test.go | 1 + nomad/stream/event_broker_test.go | 30 ++++++++++++++++++++++++++- nomad/stream/subscription.go | 5 +++-- nomad/stream/subscription_test.go | 23 ++++++++++++++++++++ website/content/api-docs/events.mdx | 10 +++++++-- 7 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 .changelog/10935.txt diff --git a/.changelog/10935.txt b/.changelog/10935.txt new file mode 100644 index 000000000..f52b7199a --- /dev/null +++ b/.changelog/10935.txt @@ -0,0 +1,3 @@ +```release-note:bug +events: Fixed wildcard namespace handling +``` diff --git a/nomad/fsm_test.go b/nomad/fsm_test.go index ac1c07c22..42b3a7e25 100644 --- a/nomad/fsm_test.go +++ b/nomad/fsm_test.go @@ -3378,6 +3378,7 @@ func TestFSM_ACLEvents(t *testing.T) { Topics: map[structs.Topic][]string{ tc.reqTopic: {"*"}, }, + Namespace: "default", } sub, err := broker.Subscribe(subReq) @@ -3431,6 +3432,7 @@ func TestFSM_EventBroker_JobRegisterFSMEvents(t *testing.T) { Topics: map[structs.Topic][]string{ structs.TopicJob: {"*"}, }, + Namespace: "default", } sub, err := broker.Subscribe(subReq) diff --git a/nomad/state/deployment_events_test.go b/nomad/state/deployment_events_test.go index 0a38890d6..223812449 100644 --- a/nomad/state/deployment_events_test.go +++ b/nomad/state/deployment_events_test.go @@ -93,6 +93,7 @@ func EventsForIndex(t *testing.T, s *StateStore, index uint64) []structs.Event { Topics: map[structs.Topic][]string{ "*": {"*"}, }, + Namespace: "default", Index: index, StartExactlyAtIndex: true, }) diff --git a/nomad/stream/event_broker_test.go b/nomad/stream/event_broker_test.go index 0e3424710..a136031cb 100644 --- a/nomad/stream/event_broker_test.go +++ b/nomad/stream/event_broker_test.go @@ -270,6 +270,27 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) { Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}), }, }, + { + desc: "subscribed to evals in all namespaces and removed access", + policyBeforeRules: mock.NamespacePolicy("*", "", []string{acl.NamespaceCapabilityReadJob}), + policyAfterRules: mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadJob}), + shouldUnsubscribe: true, + event: structs.Event{ + Topic: structs.TopicEvaluation, + Type: structs.TypeEvalUpdated, + Namespace: "foo", + Payload: structs.EvaluationEvent{ + Evaluation: &structs.Evaluation{ + ID: "some-id", + }, + }, + }, + policyEvent: structs.Event{ + Topic: structs.TopicACLToken, + Type: structs.TypeACLTokenUpserted, + Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}), + }, + }, { desc: "subscribed to deployments and no access change", policyBeforeRules: mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadJob}), @@ -467,11 +488,18 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) { publisher, err := NewEventBroker(ctx, aclDelegate, EventBrokerCfg{}) require.NoError(t, err) + var ns string + if tc.event.Namespace != "" { + ns = tc.event.Namespace + } else { + ns = structs.DefaultNamespace + } + sub, err := publisher.SubscribeWithACLCheck(&SubscribeRequest{ Topics: map[structs.Topic][]string{ tc.event.Topic: {"*"}, }, - Namespace: structs.DefaultNamespace, + Namespace: ns, Token: secretID, }) diff --git a/nomad/stream/subscription.go b/nomad/stream/subscription.go index 81942e909..a9e37342e 100644 --- a/nomad/stream/subscription.go +++ b/nomad/stream/subscription.go @@ -123,14 +123,15 @@ func filter(req *SubscribeRequest, events []structs.Event) []structs.Event { allTopicKeys := req.Topics[structs.TopicAll] - if req.Namespace == "" && len(allTopicKeys) == 1 && allTopicKeys[0] == string(structs.TopicAll) { + // Return all events if subscribed to all namespaces and all topics + if req.Namespace == "*" && len(allTopicKeys) == 1 && allTopicKeys[0] == string(structs.TopicAll) { return events } var result []structs.Event for _, event := range events { - if req.Namespace != "" && event.Namespace != "" && event.Namespace != req.Namespace { + if req.Namespace != "*" && event.Namespace != "" && event.Namespace != req.Namespace { continue } diff --git a/nomad/stream/subscription_test.go b/nomad/stream/subscription_test.go index 0fc4e3af6..d1d40f350 100644 --- a/nomad/stream/subscription_test.go +++ b/nomad/stream/subscription_test.go @@ -147,6 +147,29 @@ func TestFilter_Namespace(t *testing.T) { require.Equal(t, 2, cap(actual)) } +func TestFilter_NamespaceAll(t *testing.T) { + events := make([]structs.Event, 0, 5) + events = append(events, + structs.Event{Topic: "Test", Key: "One", Namespace: "foo"}, + structs.Event{Topic: "Test", Key: "Two", Namespace: "bar"}, + structs.Event{Topic: "Test", Key: "Three", Namespace: "default"}, + ) + + req := &SubscribeRequest{ + Topics: map[structs.Topic][]string{ + "*": {"*"}, + }, + Namespace: "*", + } + actual := filter(req, events) + expected := []structs.Event{ + {Topic: "Test", Key: "One", Namespace: "foo"}, + {Topic: "Test", Key: "Two", Namespace: "bar"}, + {Topic: "Test", Key: "Three", Namespace: "default"}, + } + require.Equal(t, expected, actual) +} + func TestFilter_FilterKeys(t *testing.T) { events := make([]structs.Event, 0, 5) events = append(events, structs.Event{Topic: "Test", Key: "One", FilterKeys: []string{"extra-key"}}, structs.Event{Topic: "Test", Key: "Two"}, structs.Event{Topic: "Test", Key: "Two"}) diff --git a/website/content/api-docs/events.mdx b/website/content/api-docs/events.mdx index cd0a696dc..5cf0b0cdf 100644 --- a/website/content/api-docs/events.mdx +++ b/website/content/api-docs/events.mdx @@ -47,7 +47,8 @@ by default, requiring a management token. - `namespace` `(string: "default")` - Specifies the target namespace to filter on. Specifying `*` includes all namespaces for event types that support - namespaces. + namespaces. If you specify all namespaces (`*`) you'll either need a management + token, or an ACL Policy that explicitly applies to all namespaces (`*`). - `topic` `(topic:filter_key: "*:*")` - Specifies a topic to subscribe to and filter on. The default is to subscribe to all topics. Multiple topics may be @@ -100,10 +101,15 @@ by default, requiring a management token. ### Sample Request ```shell-session -# Subscribe to all events and topics +# Subscribe to all events and topics in the default namespace $ curl -s -v -N http://127.0.0.1:4646/v1/event/stream ``` +```shell-session +# Subscribe to all events and topics in all namespaces +$ curl -s -v -N http://127.0.0.1:4646/v1/event/stream?namespace=* +``` + ```shell-session # Start at index 100 and subscribe to all Evaluation events $ curl -s -v -N http://127.0.0.1:4646/v1/event/stream?index=100&topic=Evaluation