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.

Sign up for GitHub

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

Jump to bottom

fix(notices): replace "select=all" with "users=all" #368

Merged
merged 1 commit into from
Feb 25, 2024
Merged

fix(notices): replace "select=all" with "users=all" #368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions client/notices.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ func (client *Client) Notify(opts *NotifyOptions) (string, error) {
}

type NoticesOptions struct {
// Select allows returning broader sets of notices.
Select NoticesSelect
// Users allows returning notices for all users.
Users NoticesUsers

// UserID, if set, includes only notices that have this user ID or are public.
UserID *uint32
Expand All @@ -93,10 +93,10 @@ type NoticesOptions struct {
After time.Time
}

type NoticesSelect string
type NoticesUsers string

const (
NoticesSelectAll NoticesSelect = "all"
NoticesUsersAll NoticesUsers = "all"
)

// Notice holds details of an event that was observed and reported either
Expand Down Expand Up @@ -195,8 +195,8 @@ func makeNoticesQuery(opts *NoticesOptions) url.Values {
if opts == nil {
return query
}
if opts.Select != "" {
query.Add("select", string(opts.Select))
if opts.Users != "" {
query.Add("users", string(opts.Users))
}
if opts.UserID != nil {
query.Add("user-id", strconv.FormatUint(uint64(*opts.UserID), 10))
Expand Down
4 changes: 2 additions & 2 deletions client/notices_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (cs *clientSuite) TestNoticesFilters(c *C) {
cs.rsp = `{"type": "sync", "result": []}`
uid := uint32(1000)
notices, err := cs.cli.Notices(&client.NoticesOptions{
Select: client.NoticesSelectAll,
Users: client.NoticesUsersAll,
UserID: &uid,
Types: []client.NoticeType{client.CustomNotice},
Keys: []string{"foo.com/bar", "example.com/x"},
Expand All @@ -128,7 +128,7 @@ func (cs *clientSuite) TestNoticesFilters(c *C) {
c.Assert(err, IsNil)
c.Assert(cs.req.URL.Path, Equals, "/v1/notices")
c.Assert(cs.req.URL.Query(), DeepEquals, url.Values{
"select": {"all"},
"users": {"all"},
"user-id": {"1000"},
"types": {"custom"},
"keys": {"foo.com/bar", "example.com/x"},
Expand Down
16 changes: 8 additions & 8 deletions internals/cli/cmd_notices.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ type cmdNotices struct {
client *client.Client

timeMixin
Select client.NoticesSelect `long:"select"`
UID *uint32 `long:"uid"`
Type []client.NoticeType `long:"type"`
Key []string `long:"key"`
Timeout time.Duration `long:"timeout"`
Users client.NoticesUsers `long:"users"`
UID *uint32 `long:"uid"`
Type []client.NoticeType `long:"type"`
Key []string `long:"key"`
Timeout time.Duration `long:"timeout"`
}

func init() {
Expand All @@ -51,8 +51,8 @@ func init() {
Summary: cmdNoticesSummary,
Description: cmdNoticesDescription,
ArgsHelp: merge(timeArgsHelp, map[string]string{
"--select": "Show all notices with any user ID (admin only; cannot be used with --uid)",
"--uid": "Only list notices with this user ID (admin only; cannot be used with --select)",
"--users": "Show all notices with any user ID (admin only; cannot be used with --uid)",
"--uid": "Only list notices with this user ID (admin only; cannot be used with --users)",
"--type": "Only list notices of this type (multiple allowed)",
"--key": "Only list notices with this key (multiple allowed)",
"--timeout": "Wait up to this duration for matching notices to arrive",
Expand All @@ -73,7 +73,7 @@ func (cmd *cmdNotices) Execute(args []string) error {
return fmt.Errorf("cannot load CLI state: %w", err)
}
options := client.NoticesOptions{
Select: cmd.Select,
Users: cmd.Users,
UserID: cmd.UID,
Types: cmd.Type,
Keys: cmd.Key,
Expand Down
10 changes: 5 additions & 5 deletions internals/cli/cmd_notices_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ ID User Type Key First Repeated Occurre
})
}

func (s *PebbleSuite) TestNoticesFiltersSelect(c *C) {
func (s *PebbleSuite) TestNoticesFiltersUsers(c *C) {
s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
c.Check(r.Method, Equals, "GET")
c.Check(r.URL.Path, Equals, "/v1/notices")
c.Check(r.URL.Query(), DeepEquals, url.Values{
"select": {"all"},
"types": {"custom", "warning"},
"keys": {"a.b/c"},
"users": {"all"},
"types": {"custom", "warning"},
"keys": {"a.b/c"},
})

fmt.Fprint(w, `{
Expand All @@ -103,7 +103,7 @@ func (s *PebbleSuite) TestNoticesFiltersSelect(c *C) {
})

rest, err := cli.Parser(cli.Client()).ParseArgs([]string{
"notices", "--abs-time", "--select", "all", "--type", "custom", "--key", "a.b/c", "--type", "warning"})
"notices", "--abs-time", "--users", "all", "--type", "custom", "--key", "a.b/c", "--type", "warning"})
c.Assert(err, IsNil)
c.Check(rest, HasLen, 0)
c.Check(s.Stdout(), Equals, `
Expand Down
10 changes: 5 additions & 5 deletions internals/daemon/api_notices.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,15 @@ func v1GetNotices(c *Command, r *http.Request, _ *UserState) Response {
}
}

if len(query["select"]) > 0 {
if len(query["users"]) > 0 {
if !isAdmin(requestUID, daemonUID) {
return Forbidden(`only admins may use the "select" filter`)
return Forbidden(`only admins may use the "users" filter`)
}
if len(query["user-id"]) > 0 {
return BadRequest(`cannot use both "select" and "user-id" parameters`)
return BadRequest(`cannot use both "users" and "user-id" parameters`)
}
if query.Get("select") != "all" {
return BadRequest(`invalid "select" filter: must be "all"`)
if query.Get("users") != "all" {
return BadRequest(`invalid "users" filter: must be "all"`)
}
// Clear the userID filter so all notices will be returned.
userID = nil
Expand Down
20 changes: 10 additions & 10 deletions internals/daemon/api_notices_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ func (s *apiSuite) TestNoticesUserIDNonAdminFilter(c *C) {
c.Assert(ok, Equals, true)
}

func (s *apiSuite) TestNoticesSelectAdminFilter(c *C) {
func (s *apiSuite) TestNoticesUsersAdminFilter(c *C) {
s.daemon(c)
restore := fakeSysGetuid(0)
defer restore()
Expand All @@ -425,8 +425,8 @@ func (s *apiSuite) TestNoticesSelectAdminFilter(c *C) {

noticesCmd := apiCmd("/v1/notices")

// Test that admin user may get all notices with --select=all filter
reqUrl := "/v1/notices?select=all"
// Test that admin user may get all notices with --users=all filter
reqUrl := "/v1/notices?users=all"
req, err := http.NewRequest("GET", reqUrl, nil)
c.Check(err, IsNil)
req.RemoteAddr = "pid=100;uid=0;socket=;"
Expand All @@ -452,7 +452,7 @@ func (s *apiSuite) TestNoticesSelectAdminFilter(c *C) {
c.Assert(n["key"], Equals, "danger")
}

func (s *apiSuite) TestNoticesSelectNonAdminFilter(c *C) {
func (s *apiSuite) TestNoticesUsersNonAdminFilter(c *C) {
s.daemon(c)
restore := fakeSysGetuid(0)
defer restore()
Expand All @@ -465,8 +465,8 @@ func (s *apiSuite) TestNoticesSelectNonAdminFilter(c *C) {

noticesCmd := apiCmd("/v1/notices")

// Test that non-admin user may not use --select filter
reqUrl := "/v2/notices?select=all"
// Test that non-admin user may not use --users filter
reqUrl := "/v2/notices?users=all"
req, err := http.NewRequest("GET", reqUrl, nil)
c.Check(err, IsNil)
req.RemoteAddr = "pid=100;uid=1000;socket=;"
Expand Down Expand Up @@ -609,16 +609,16 @@ func (s *apiSuite) TestNoticesInvalidUserIDLow(c *C) {
s.testNoticesBadRequest(c, "user-id=-1", `invalid "user-id" filter:.*`)
}

func (s *apiSuite) TestNoticesInvalidSelect(c *C) {
func (s *apiSuite) TestNoticesInvalidUsers(c *C) {
restore := fakeSysGetuid(0)
defer restore()
s.testNoticesBadRequest(c, "select=foo", `invalid "select" filter:.*`)
s.testNoticesBadRequest(c, "users=foo", `invalid "users" filter:.*`)
}

func (s *apiSuite) TestNoticesInvalidUserIDWithSelect(c *C) {
func (s *apiSuite) TestNoticesInvalidUserIDWithUsers(c *C) {
restore := fakeSysGetuid(0)
defer restore()
s.testNoticesBadRequest(c, "user-id=1234&select=all", `cannot use both "select" and "user-id" parameters`)
s.testNoticesBadRequest(c, "user-id=1234&users=all", `cannot use both "users" and "user-id" parameters`)
}

func (s *apiSuite) TestNoticesInvalidAfter(c *C) {
Expand Down
Loading