diff --git a/client/notices.go b/client/notices.go index d19cd423e..4ac487589 100644 --- a/client/notices.go +++ b/client/notices.go @@ -18,23 +18,31 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net/url" + "regexp" "time" ) type NotifyOptions struct { - // Key is the custom notice's key. Must be in "domain.com/key" format. + // Type is the notice's type. Currently only notices of type CustomNotice + // can be added. + Type NoticeType + + // Key is the notice's key. For "custom" notices, this must be in + // "domain.com/key" format. Key string // Data are optional key=value pairs for this occurrence of the notice. Data map[string]string - // RepeatAfter allows the notice to repeat after this duration. If this is - // zero (the default), the notice will always repeat. + // RepeatAfter, if not zero, prevents this notice from being observed if a + // notification with the same Type and Key has been made recently within + // the provided duration. RepeatAfter time.Duration } -// Notify records an occurrence of a "custom" notice with the specified options, +// Notify records an occurrence of a notice with the specified options, // returning the notice ID. func (client *Client) Notify(opts *NotifyOptions) (string, error) { var payload = struct { @@ -45,7 +53,7 @@ func (client *Client) Notify(opts *NotifyOptions) (string, error) { Data map[string]string `json:"data,omitempty"` }{ Action: "add", - Type: "custom", + Type: string(opts.Type), Key: opts.Key, Data: opts.Data, } @@ -78,8 +86,12 @@ type NoticesOptions struct { After time.Time } -// Notice is a notification whose identity is the combination of Type and Key -// that has occurred Occurrences number of times. +// Notice holds details of an event that was observed and reported either +// inside the server itself or externally via the API. Besides the ID field +// itself, the Type and Key fields together also uniquely identify a +// particular notice, and when a new notification is made with matching Type +// and Key, the previous notice is updated appropriately instead of a new one +// being created. type Notice struct { ID string `json:"id"` Type NoticeType `json:"type"` @@ -96,18 +108,10 @@ type Notice struct { type NoticeType string const ( - // Recorded whenever a change is updated: when the change is first spawned - // or its status is updated. - ChangeUpdateNotice NoticeType = "change-update" - // A custom notice reported via the Pebble client API or "pebble notify". // The key and data fields are provided by the user. The key must be in // the format "mydomain.io/mykey" to ensure well-namespaced notice keys. CustomNotice NoticeType = "custom" - - // Warnings are a subset of notices where the key is a human-readable - // warning message. - WarningNotice NoticeType = "warning" ) type jsonNotice struct { @@ -116,8 +120,17 @@ type jsonNotice struct { ExpireAfter string `json:"expire-after,omitempty"` } +// This is used to ensure we send a well-formed notice ID in the URL path. +// It's a little more permissive than the currently-valid notice IDs (which +// are always integers), but it will allow older clients to talk to newer +// servers which might start allowing letters too (for example). +var noticeIDRegexp = regexp.MustCompile(`^[a-z0-9]+$`) + // Notice fetches a single notice by ID. func (client *Client) Notice(id string) (*Notice, error) { + if !noticeIDRegexp.MatchString(id) { + return nil, fmt.Errorf("invalid notice ID %q", id) + } var jn *jsonNotice _, err := client.doSync("GET", "/v1/notices/"+id, nil, nil, nil, &jn) if err != nil { @@ -145,8 +158,6 @@ func (client *Client) WaitNotices(ctx context.Context, serverTimeout time.Durati query := makeNoticesQuery(opts) query.Set("timeout", serverTimeout.String()) - // We need to use client.raw() here to pass the context through (and we - // don't want retries). resp, err := client.Requester().Do(ctx, &RequestOptions{ Type: SyncRequest, Method: "GET", @@ -156,6 +167,7 @@ func (client *Client) WaitNotices(ctx context.Context, serverTimeout time.Durati if err != nil { return nil, err } + var jns []*jsonNotice err = resp.DecodeResult(&jns) if err != nil { diff --git a/client/notices_test.go b/client/notices_test.go index adbafb222..8eb38b5ea 100644 --- a/client/notices_test.go +++ b/client/notices_test.go @@ -55,6 +55,11 @@ func (cs *clientSuite) TestNotice(c *C) { }) } +func (cs *clientSuite) TestNoticeInvalidID(c *C) { + _, err := cs.cli.Notice("") + c.Assert(err, ErrorMatches, "invalid notice ID.*") +} + func (cs *clientSuite) TestNotices(c *C) { cs.rsp = `{"type": "sync", "result": [{ "id": "1", @@ -105,14 +110,14 @@ func (cs *clientSuite) TestNotices(c *C) { func (cs *clientSuite) TestNoticesFilters(c *C) { cs.rsp = `{"type": "sync", "result": []}` notices, err := cs.cli.Notices(&client.NoticesOptions{ - Types: []client.NoticeType{client.CustomNotice, client.WarningNotice}, + Types: []client.NoticeType{client.CustomNotice}, Keys: []string{"foo.com/bar", "example.com/x"}, After: time.Date(2023, 9, 5, 16, 43, 32, 123_456_789, time.UTC), }) c.Assert(err, IsNil) c.Assert(cs.req.URL.Path, Equals, "/v1/notices") c.Assert(cs.req.URL.Query(), DeepEquals, url.Values{ - "types": {"custom", "warning"}, + "types": {"custom"}, "keys": {"foo.com/bar", "example.com/x"}, "after": {"2023-09-05T16:43:32.123456789Z"}, }) @@ -121,13 +126,14 @@ func (cs *clientSuite) TestNoticesFilters(c *C) { func (cs *clientSuite) TestNotify(c *C) { cs.rsp = `{"type": "sync", "result": {"id": "7"}}` - noticeId, err := cs.cli.Notify(&client.NotifyOptions{ + noticeID, err := cs.cli.Notify(&client.NotifyOptions{ + Type: client.CustomNotice, Key: "foo.com/bar", RepeatAfter: time.Hour, Data: map[string]string{"k": "9"}, }) c.Assert(err, IsNil) - c.Check(noticeId, Equals, "7") + c.Check(noticeID, Equals, "7") c.Assert(cs.req.URL.Path, Equals, "/v1/notices") body, err := io.ReadAll(cs.req.Body) @@ -146,11 +152,12 @@ func (cs *clientSuite) TestNotify(c *C) { func (cs *clientSuite) TestNotifyMinimal(c *C) { cs.rsp = `{"type": "sync", "result": {"id": "1"}}` - noticeId, err := cs.cli.Notify(&client.NotifyOptions{ - Key: "a.b/c", + noticeID, err := cs.cli.Notify(&client.NotifyOptions{ + Type: client.CustomNotice, + Key: "a.b/c", }) c.Assert(err, IsNil) - c.Check(noticeId, Equals, "1") + c.Check(noticeID, Equals, "1") c.Assert(cs.req.URL.Path, Equals, "/v1/notices") body, err := io.ReadAll(cs.req.Body)