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

feat(client): add Go client for notices #297

Merged
merged 35 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
7fbc993
feat(state): add core support for Pebble Notices
benhoyt Sep 4, 2023
79bed4e
Make AddNotice return the notice ID so POST /v1/notices can return it
benhoyt Sep 5, 2023
1b5f188
Clean up tests a bit
benhoyt Sep 5, 2023
a973de6
feat(daemon): add Pebble Notices API
benhoyt Sep 5, 2023
313c894
Tweak comment wording
benhoyt Sep 5, 2023
70f8a7d
Merge branch 'notices-state' into notices-api
benhoyt Sep 5, 2023
41bd09b
feat(client): add Go client for notices
benhoyt Sep 5, 2023
fde015f
Ensure client uses full nanosecond-resultion time for "after"
benhoyt Sep 6, 2023
f462099
Save last notice ID in state to avoid accidental reuse
benhoyt Sep 6, 2023
709ac37
Note that WaitNotices returns nil slice if timeout elapses
benhoyt Sep 6, 2023
31b6bb7
Comment tweaks per Fred's review; tweaks to repeatAfter logic
benhoyt Sep 14, 2023
a555411
More slight comment tweaks
benhoyt Sep 14, 2023
acde18d
Merge branch 'notices-state' into notices-api
benhoyt Sep 15, 2023
b0e88f8
Make client key regexp a bit tighter (and add ^ and $ anchors!)
benhoyt Sep 15, 2023
7c7a143
Make /v1/notices timeout return empty list of notices (not an error)
benhoyt Sep 15, 2023
54036a2
Merge branch 'notices-api' into notices-cli
benhoyt Sep 15, 2023
8ab3981
Update as /v1/notices no longer returns HTTP gateway timeout
benhoyt Sep 15, 2023
c4c6429
Tweak change-update comment
benhoyt Sep 15, 2023
f1263d3
Reorder WaitNotices args so options are last; allow nil NoticesOptions
benhoyt Sep 22, 2023
4d25e3c
Updates: client->custom, repeat-after=0 means always, multi types&keys
benhoyt Sep 27, 2023
de13c72
Merge branch 'notices-state' into notices-api
benhoyt Sep 27, 2023
85efd41
Updates per spec review
benhoyt Sep 27, 2023
29cf984
Remove max notice length from state (enforce in API)
benhoyt Sep 27, 2023
286b0fd
Merge branch 'notices-state' into notices-api
benhoyt Sep 27, 2023
6383cae
Merge branch 'notices-api' into notices-cli
benhoyt Sep 27, 2023
49e2a86
Updated from spec review
benhoyt Sep 27, 2023
1fa3639
Updates from Gustavo's code review
benhoyt Oct 3, 2023
580a95a
Merge branch 'notices-state' into notices-api
benhoyt Oct 3, 2023
7840544
Updates after merge
benhoyt Oct 3, 2023
403be41
Merge branch 'notices-api' into notices-cli
benhoyt Oct 3, 2023
f69171d
Renames as per state changes
benhoyt Oct 3, 2023
b715b16
Merge branch 'master' into notices-api
benhoyt Oct 6, 2023
01dd5ee
Merge branch 'notices-api' into notices-cli
benhoyt Oct 6, 2023
eeaaae0
Merge branch 'master' into notices-cli
benhoyt Oct 13, 2023
32a1540
Updates per code review
benhoyt Oct 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,10 @@ func (client *Client) doSync(method, path string, query url.Values, headers map[
if err := rsp.err(client); err != nil {
return nil, err
}
return client.finishSync(rsp, v)
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
}

func (client *Client) finishSync(rsp response, v interface{}) (*ResultInfo, error) {
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
if rsp.Type != "sync" {
return nil, fmt.Errorf("expected sync response, got %q", rsp.Type)
}
Expand Down
189 changes: 189 additions & 0 deletions client/notices.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
// Copyright (c) 2023 Canonical Ltd
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License version 3 as
// published by the Free Software Foundation.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.

package client

import (
"bytes"
"context"
"encoding/json"
"net/http"
"net/url"
"time"
)

type NotifyOptions struct {
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
// Key is the client notice's key. Must be in "domain.com/key" format.
Key string
benhoyt marked this conversation as resolved.
Show resolved Hide resolved

// RepeatAfter, if provided, allows the notice to repeat after this duration.
RepeatAfter time.Duration

// Data are optional key=value pairs for this occurrence of the notice.
Data map[string]string
}

// Notify records an occurrence of a "client" notice with the specified options,
// returning the notice ID.
func (client *Client) Notify(opts *NotifyOptions) (string, error) {
var payload = struct {
Action string `json:"action"`
Type string `json:"type"`
Key string `json:"key"`
RepeatAfter string `json:"repeat-after,omitempty"`
Data map[string]string `json:"data,omitempty"`
}{
Action: "add",
Type: "client",
Key: opts.Key,
Data: opts.Data,
}
if opts.RepeatAfter != 0 {
payload.RepeatAfter = opts.RepeatAfter.String()
}
var body bytes.Buffer
if err := json.NewEncoder(&body).Encode(&payload); err != nil {
return "", err
}

result := struct {
ID string `json:"id"`
}{}
_, err := client.doSync("POST", "/v1/notices", nil, nil, &body, &result)
if err != nil {
return "", err
}
return result.ID, err
}

type NoticesOptions struct {
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
// Type, if set, includes only notices of this type.
Type NoticeType
// Key, if set, includes only notices with this key.
Key string
// After, if set, includes only notices that were last repeated after this time.
After time.Time
}

// Notice is a notification whose identity is the combination of Type and Key
// that has occurred Occurrences number of times.
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
type Notice struct {
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
ID string `json:"id"`
Type NoticeType `json:"type"`
Key string `json:"key"`
FirstOccurred time.Time `json:"first-occurred"`
LastOccurred time.Time `json:"last-occurred"`
LastRepeated time.Time `json:"last-repeated"`
Occurrences int `json:"occurrences"`
LastData map[string]string `json:"last-data,omitempty"`
RepeatAfter time.Duration `json:"repeat-after,omitempty"`
ExpireAfter time.Duration `json:"expire-after,omitempty"`
}

type NoticeType string

const (
// Recorded whenever a change is updated: when it is first spawned or its
// status was updated.
NoticeChangeUpdate NoticeType = "change-update"

// A client 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.
NoticeClient NoticeType = "client"

// Warnings are a subset of notices where the key is a human-readable
// warning message.
NoticeWarning NoticeType = "warning"
)

type jsonNotice struct {
Notice
RepeatAfter string `json:"repeat-after,omitempty"`
ExpireAfter string `json:"expire-after,omitempty"`
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
}

// Notice fetches a single notice by ID.
func (client *Client) Notice(id string) (*Notice, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting here to avoid mixing with the unrelated thread below.

Taking a raw string from the outside world and blindly appending it to the GET path with zero validation is anxiety inducing. This is the kind of thing that can be exploited in an application we have no visibility over and become an actual hack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see what you're saying, but I'm not so sure. Go will safely and properly percent-escape any disallowed characters, and the server will unescape (and will not find the notice).

However, I've added a simple client-side validation -- that's slightly more permissive than the integers we produce now for future-proofing -- see what you think.

If you think this is a good approach, I can create a PR to do something similar in client/changes.go, where we do the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened #318 to track that.

var jn *jsonNotice
_, err := client.doSync("GET", "/v1/notices/"+id, nil, nil, nil, &jn)
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
return jsonNoticeToNotice(jn), nil
}

// Notices returns a list of notices that match the filters given in opts,
// ordered by the last-repeated time.
func (client *Client) Notices(opts *NoticesOptions) ([]*Notice, error) {
query := makeNoticesQuery(opts)
var jns []*jsonNotice
_, err := client.doSync("GET", "/v1/notices", query, nil, nil, &jns)
return jsonNoticesToNotices(jns), err
}

// WaitNotices returns a list of notices that match the filters given in opts,
// waiting up to the given timeout. They are ordered by the last-repeated time.
func (client *Client) WaitNotices(ctx context.Context, opts *NoticesOptions, timeout time.Duration) ([]*Notice, error) {
query := makeNoticesQuery(opts)
query.Set("timeout", timeout.String())

res, err := client.raw(ctx, "GET", "/v1/notices", query, nil, nil)
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
defer res.Body.Close()

if res.StatusCode == http.StatusGatewayTimeout {
return nil, nil
benhoyt marked this conversation as resolved.
Show resolved Hide resolved
}

var rsp response
err = decodeInto(res.Body, &rsp)
if err != nil {
return nil, err
}
var jns []*jsonNotice
_, err = client.finishSync(rsp, &jns)
return jsonNoticesToNotices(jns), err
}

func makeNoticesQuery(opts *NoticesOptions) url.Values {
query := make(url.Values)
if opts.Type != "" {
query.Set("type", string(opts.Type))
}
if opts.Key != "" {
query.Set("key", opts.Key)
}
if !opts.After.IsZero() {
query.Set("after", opts.After.Format(time.RFC3339Nano))
}
return query
}

func jsonNoticesToNotices(jns []*jsonNotice) []*Notice {
ns := make([]*Notice, len(jns))
for i, jn := range jns {
ns[i] = jsonNoticeToNotice(jn)
}
return ns
}

func jsonNoticeToNotice(jn *jsonNotice) *Notice {
n := &jn.Notice
n.ExpireAfter, _ = time.ParseDuration(jn.ExpireAfter)
n.RepeatAfter, _ = time.ParseDuration(jn.RepeatAfter)
return n
}
Loading