-
Notifications
You must be signed in to change notification settings - Fork 562
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
i/prompting: add package for prompting common types/functions #13849
Changes from 5 commits
dbddf6b
6a71217
74cc30b
a428025
29920e0
0f9b8d2
05030d3
617e192
a9031f6
01d2a87
4adc766
4fe163d
ccca57a
6a8609a
9875909
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,154 @@ | ||||||
// -*- Mode: Go; indent-tabs-mode: t -*- | ||||||
|
||||||
/* | ||||||
* Copyright (C) 2024 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 prompting | ||||||
|
||||||
import ( | ||||||
"encoding/base32" | ||||||
"encoding/binary" | ||||||
"fmt" | ||||||
"time" | ||||||
) | ||||||
|
||||||
type OutcomeType string | ||||||
|
||||||
const ( | ||||||
OutcomeUnset OutcomeType = "" | ||||||
OutcomeAllow OutcomeType = "allow" | ||||||
OutcomeDeny OutcomeType = "deny" | ||||||
) | ||||||
|
||||||
// AsBool returns the outcome as a boolean, or an error if it cannot be parsed. | ||||||
func (outcome OutcomeType) AsBool() (bool, error) { | ||||||
olivercalder marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
switch outcome { | ||||||
case OutcomeAllow: | ||||||
return true, nil | ||||||
case OutcomeDeny: | ||||||
return false, nil | ||||||
default: | ||||||
return false, fmt.Errorf(`invalid outcome: must be %q or %q: %q`, OutcomeAllow, OutcomeDeny, outcome) | ||||||
} | ||||||
} | ||||||
|
||||||
// ValidateOutcome returns nil if the given outcome is valid, otherwise an error. | ||||||
func ValidateOutcome(outcome OutcomeType) error { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a bit strange, shouldn't it be a parse function taking a string and returning the OutcomeType or an error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or maybe what you need is MarshalJSON UnmarshalJSON methods? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm if we don't care about defining new values of OutcomeType manually, then I think a trivial UnmarshalJSON would be preferred. You do not need MarshalJSON in such case. |
||||||
switch outcome { | ||||||
case OutcomeAllow, OutcomeDeny: | ||||||
return nil | ||||||
default: | ||||||
return fmt.Errorf(`invalid outcome: must be %q or %q: %q`, OutcomeAllow, OutcomeDeny, outcome) | ||||||
} | ||||||
} | ||||||
|
||||||
type LifespanType string | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||||||
|
||||||
const ( | ||||||
LifespanUnset LifespanType = "" | ||||||
LifespanForever LifespanType = "forever" | ||||||
LifespanSession LifespanType = "session" | ||||||
olivercalder marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
LifespanSingle LifespanType = "single" | ||||||
LifespanTimespan LifespanType = "timespan" | ||||||
bboozzoo marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
) | ||||||
|
||||||
// ValidateLifespanExpiration checks that the given lifespan is valid and that | ||||||
// the given expiration is valid for that lifespan. | ||||||
// | ||||||
// If the lifespan is LifespanTimespan LifespanTimespan, then expiration must | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
// be a string parsable as time.Duration with RFC3339 format. Otherwise, it must | ||||||
// be empty. Returns an error if any of the above are invalid. | ||||||
func ValidateLifespanExpiration(lifespan LifespanType, expiration string, currTime time.Time) error { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same question There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More strongly than for |
||||||
switch lifespan { | ||||||
case LifespanForever, LifespanSession, LifespanSingle: | ||||||
if expiration != "" { | ||||||
return fmt.Errorf(`invalid expiration: expiration must be empty when lifespan is %q, but received non-empty expiration: %q`, lifespan, expiration) | ||||||
} | ||||||
case LifespanTimespan: | ||||||
if expiration == "" { | ||||||
return fmt.Errorf(`invalid expiration: expiration must be non-empty when lifespan is %q, but received empty expiration`, lifespan) | ||||||
} | ||||||
parsedTime, err := time.Parse(time.RFC3339, expiration) | ||||||
if err != nil { | ||||||
return fmt.Errorf("invalid expiration: expiration not parsable as a time in RFC3339 format: %q", expiration) | ||||||
} | ||||||
if currTime.After(parsedTime) { | ||||||
return fmt.Errorf("invalid expiration: expiration time has already passed: %q", expiration) | ||||||
} | ||||||
default: | ||||||
return fmt.Errorf(`invalid lifespan: %q`, lifespan) | ||||||
} | ||||||
return nil | ||||||
} | ||||||
|
||||||
// ValidateLifespanParseDuration checks that the given lifespan is valid and | ||||||
// that the given duration is valid for that lifespan. | ||||||
// | ||||||
// If the lifespan is LifespanTimespan, then duration must be a string parsable | ||||||
// by time.ParseDuration(), representing the duration of time for which the rule | ||||||
// should be valid. Otherwise, it must be empty. Returns an error if any of the | ||||||
// above are invalid, otherwise computes the expiration time of the rule based | ||||||
// on the current time and the given duration and returns it. | ||||||
func ValidateLifespanParseDuration(lifespan LifespanType, duration string) (string, error) { | ||||||
expirationString := "" | ||||||
switch lifespan { | ||||||
case LifespanForever, LifespanSession, LifespanSingle: | ||||||
if duration != "" { | ||||||
return "", fmt.Errorf(`invalid duration: duration must be empty when lifespan is %q, but received non-empty duration: %q`, lifespan, duration) | ||||||
} | ||||||
case LifespanTimespan: | ||||||
if duration == "" { | ||||||
return "", fmt.Errorf(`invalid duration: duration must be non-empty when lifespan is %q, but received empty expiration`, lifespan) | ||||||
} | ||||||
parsedDuration, err := time.ParseDuration(duration) | ||||||
if err != nil { | ||||||
return "", fmt.Errorf(`invalid duration: error parsing duration string: %q`, duration) | ||||||
} | ||||||
if parsedDuration <= 0 { | ||||||
return "", fmt.Errorf(`invalid duration: duration must be greater than zero: %q`, duration) | ||||||
} | ||||||
expirationString = time.Now().Add(parsedDuration).Format(time.RFC3339) | ||||||
default: | ||||||
return "", fmt.Errorf(`invalid lifespan: %q`, lifespan) | ||||||
} | ||||||
return expirationString, nil | ||||||
} | ||||||
|
||||||
// TimestampToTime converts the given timestamp string to a time.Time in Local | ||||||
// time. The timestamp string is expected to be of the format time.RFC3339Nano. | ||||||
// If it cannot be parsed as such, returns an error. | ||||||
func TimestampToTime(timestamp string) (time.Time, error) { | ||||||
t, err := time.Parse(time.RFC3339Nano, timestamp) | ||||||
olivercalder marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if err != nil { | ||||||
return t, err | ||||||
olivercalder marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
return t.Local(), nil | ||||||
} | ||||||
|
||||||
// NewIDAndTimestamp returns a new unique ID and corresponding timestamp. | ||||||
// | ||||||
// The ID is the current unix time in nanoseconds encoded as a string in base32. | ||||||
// The timestamp is the same time, encoded as a string in time.RFC3339Nano. | ||||||
func NewIDAndTimestamp() (id string, timestamp string) { | ||||||
now := time.Now() | ||||||
nowUnix := uint64(now.UnixNano()) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens when the time reports the same unixnano? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just use a counter. Store with rules storage? And use with request prompts as well. Or combine boot ID with monotonic boot time. (look for helper, reads from
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the time package documentation (https://pkg.go.dev/time#hdr-Monotonic_Clocks),
Thus, the monotonic time is only stripped when rules/prompts are read from disk. This occurs when snapd is restarted, including on system start. Unfortunately, this means There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where and what are this IDs used for ? in general snapd prefers to use sequential increasing IDs? at least for norification from the kernel don't we get them in a single goroutine? in that context using sequential IDs would be cheap, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIU the IDs are stored within the prompting namespace in notices and we need them to be unique across reboots too (@olivercalder please correct me if I'm wrong), so to keep them sequential we'd either need to store some state. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, we could restart counting from the max of the pending ones when we restart There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, that's a good point There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could start from the max of the pending ones, though that would allow for the potential that IDs are reused for different rules, if a rule is removed, then snapd restarted. This isn't necessarily a problem, as we don't guarantee that IDs are never reused, but I'd prefer to keep them unique if possible. I think storing a count as part of the rule storage would solve this problem, and it can be atomically incremented whenever a new rule is added. For prompt requests, which are not written to disk (because of listener/notify socket behavior, but that can be discussed in a later PR), we can just keep a count in memory and atomically increment it. So, my opinion is that we should remove this function for now and add a |
||||||
nowBytes := make([]byte, 8) | ||||||
binary.BigEndian.PutUint64(nowBytes, nowUnix) | ||||||
id = base32.StdEncoding.EncodeToString(nowBytes) | ||||||
timestamp = now.Format(time.RFC3339Nano) | ||||||
return id, timestamp | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,173 @@ | ||
// -*- Mode: Go; indent-tabs-mode: t -*- | ||
|
||
/* | ||
* Copyright (C) 2024 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 prompting_test | ||
|
||
import ( | ||
"encoding/base32" | ||
"encoding/binary" | ||
"testing" | ||
"time" | ||
|
||
. "gopkg.in/check.v1" | ||
|
||
"github.com/snapcore/snapd/dirs" | ||
"github.com/snapcore/snapd/interfaces/prompting" | ||
) | ||
|
||
func Test(t *testing.T) { TestingT(t) } | ||
|
||
type promptingSuite struct { | ||
tmpdir string | ||
} | ||
|
||
var _ = Suite(&promptingSuite{}) | ||
|
||
func (s *promptingSuite) SetUpTest(c *C) { | ||
s.tmpdir = c.MkDir() | ||
dirs.SetRootDir(s.tmpdir) | ||
} | ||
|
||
func (s *promptingSuite) TestOutcomeAsBool(c *C) { | ||
result, err := prompting.OutcomeAllow.AsBool() | ||
c.Check(err, IsNil) | ||
c.Check(result, Equals, true) | ||
result, err = prompting.OutcomeDeny.AsBool() | ||
c.Check(err, IsNil) | ||
c.Check(result, Equals, false) | ||
_, err = prompting.OutcomeUnset.AsBool() | ||
c.Check(err, ErrorMatches, `invalid outcome.*`) | ||
_, err = prompting.OutcomeType("foo").AsBool() | ||
c.Check(err, ErrorMatches, `invalid outcome.*`) | ||
} | ||
|
||
func (s *promptingSuite) TestValidateOutcome(c *C) { | ||
c.Assert(prompting.ValidateOutcome(prompting.OutcomeAllow), Equals, nil) | ||
c.Assert(prompting.ValidateOutcome(prompting.OutcomeDeny), Equals, nil) | ||
c.Assert(prompting.ValidateOutcome(prompting.OutcomeUnset), ErrorMatches, `invalid outcome.*`) | ||
c.Assert(prompting.ValidateOutcome(prompting.OutcomeType("foo")), ErrorMatches, `invalid outcome.*`) | ||
} | ||
|
||
func (s *promptingSuite) TestValidateLifespanExpiration(c *C) { | ||
unsetExpiration := "" | ||
invalidExpiration := "foo" | ||
currTime := time.Now() | ||
negativeExpiration := currTime.Add(-5 * time.Second).Format(time.RFC3339) | ||
validExpiration := currTime.Add(10 * time.Minute).Format(time.RFC3339) | ||
|
||
for _, lifespan := range []prompting.LifespanType{ | ||
prompting.LifespanForever, | ||
prompting.LifespanSession, | ||
prompting.LifespanSingle, | ||
} { | ||
err := prompting.ValidateLifespanExpiration(lifespan, unsetExpiration, currTime) | ||
c.Check(err, IsNil) | ||
for _, exp := range []string{invalidExpiration, negativeExpiration, validExpiration} { | ||
err = prompting.ValidateLifespanExpiration(lifespan, exp, currTime) | ||
c.Check(err, ErrorMatches, `invalid expiration: expiration must be empty.*`) | ||
} | ||
} | ||
|
||
err := prompting.ValidateLifespanExpiration(prompting.LifespanTimespan, unsetExpiration, currTime) | ||
c.Check(err, ErrorMatches, `invalid expiration: expiration must be non-empty.*`) | ||
|
||
err = prompting.ValidateLifespanExpiration(prompting.LifespanTimespan, invalidExpiration, currTime) | ||
c.Check(err, ErrorMatches, `invalid expiration: expiration not parsable.*`) | ||
|
||
err = prompting.ValidateLifespanExpiration(prompting.LifespanTimespan, negativeExpiration, currTime) | ||
c.Check(err, ErrorMatches, `invalid expiration: expiration time has already passed.*`) | ||
|
||
err = prompting.ValidateLifespanExpiration(prompting.LifespanTimespan, validExpiration, currTime) | ||
c.Check(err, IsNil) | ||
} | ||
|
||
func (s *promptingSuite) TestValidateLifespanParseDuration(c *C) { | ||
unsetDuration := "" | ||
invalidDuration := "foo" | ||
negativeDuration := "-5s" | ||
validDuration := "10m" | ||
parsedValidDuration, err := time.ParseDuration(validDuration) | ||
c.Assert(err, IsNil) | ||
|
||
for _, lifespan := range []prompting.LifespanType{ | ||
prompting.LifespanForever, | ||
prompting.LifespanSession, | ||
prompting.LifespanSingle, | ||
} { | ||
expiration, err := prompting.ValidateLifespanParseDuration(lifespan, unsetDuration) | ||
c.Check(expiration, Equals, "") | ||
c.Check(err, IsNil) | ||
for _, dur := range []string{invalidDuration, negativeDuration, validDuration} { | ||
expiration, err = prompting.ValidateLifespanParseDuration(lifespan, dur) | ||
c.Check(expiration, Equals, "") | ||
c.Check(err, ErrorMatches, `invalid duration: duration must be empty.*`) | ||
} | ||
} | ||
|
||
expiration, err := prompting.ValidateLifespanParseDuration(prompting.LifespanTimespan, unsetDuration) | ||
c.Check(expiration, Equals, "") | ||
c.Check(err, ErrorMatches, `invalid duration: duration must be non-empty.*`) | ||
|
||
expiration, err = prompting.ValidateLifespanParseDuration(prompting.LifespanTimespan, invalidDuration) | ||
c.Check(expiration, Equals, "") | ||
c.Check(err, ErrorMatches, `invalid duration: error parsing duration string.*`) | ||
|
||
expiration, err = prompting.ValidateLifespanParseDuration(prompting.LifespanTimespan, negativeDuration) | ||
c.Check(expiration, Equals, "") | ||
c.Check(err, ErrorMatches, `invalid duration: duration must be greater than zero.*`) | ||
|
||
expiration, err = prompting.ValidateLifespanParseDuration(prompting.LifespanTimespan, validDuration) | ||
c.Check(err, IsNil) | ||
expirationTime, err := time.Parse(time.RFC3339, expiration) | ||
c.Check(err, IsNil) | ||
c.Check(expirationTime.After(time.Now()), Equals, true) | ||
c.Check(expirationTime.Before(time.Now().Add(parsedValidDuration)), Equals, true) | ||
} | ||
|
||
func (s *promptingSuite) TestTimestampToTime(c *C) { | ||
before := time.Now() | ||
ts := time.Now().Format(time.RFC3339Nano) | ||
after := time.Now() | ||
parsedTime, err := prompting.TimestampToTime(ts) | ||
c.Assert(err, IsNil) | ||
c.Assert(parsedTime.After(before), Equals, true) | ||
c.Assert(parsedTime.Before(after), Equals, true) | ||
} | ||
|
||
func (s *promptingSuite) TestNewIDAndTimestamp(c *C) { | ||
before := time.Now() | ||
id, _ := prompting.NewIDAndTimestamp() | ||
idPaired, timestampPaired := prompting.NewIDAndTimestamp() | ||
after := time.Now() | ||
data1, err := base32.StdEncoding.DecodeString(id) | ||
c.Assert(err, IsNil) | ||
data2, err := base32.StdEncoding.DecodeString(idPaired) | ||
c.Assert(err, IsNil) | ||
parsedNs := int64(binary.BigEndian.Uint64(data1)) | ||
parsedNsPaired := int64(binary.BigEndian.Uint64(data2)) | ||
parsedTime := time.Unix(parsedNs/1000000000, parsedNs%1000000000) | ||
parsedTimePaired := time.Unix(parsedNsPaired/1000000000, parsedNsPaired%1000000000) | ||
c.Assert(parsedTime.After(before), Equals, true) | ||
c.Assert(parsedTime.Before(after), Equals, true) | ||
c.Assert(parsedTimePaired.After(before), Equals, true) | ||
c.Assert(parsedTimePaired.Before(after), Equals, true) | ||
parsedTimestamp, err := prompting.TimestampToTime(timestampPaired) | ||
c.Assert(err, IsNil) | ||
c.Assert(parsedTimePaired, Equals, parsedTimestamp) | ||
} |
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.
this needs a doc string