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

i/p/requestprompts: add timeout for prompts #14390

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

i/p/requestprompts: add timeout for prompts #14390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1ef64ca
i/p/requestprompts: add timeout for prompts
olivercalder Aug 20, 2024
b166dbf
i/p/requestprompts: add helper to send reply for prompt
olivercalder Aug 20, 2024
d3cd196
i/p/requestprompts: do not expire prompts on access race
olivercalder Aug 20, 2024
391c1b6
i/p/requestprompts: avoid data races around close and expiration
olivercalder Aug 20, 2024
b09d8d8
i/p/requestprompts: leave prompts unchanged if timeout race happens
olivercalder Aug 21, 2024
a627ac2
i/p/requestprompts: use testutil.Mock instead of testutil.Backup
olivercalder Sep 10, 2024
d30a6d6
i/p/requestprompts: reorder user entry creation and map insertion
olivercalder Sep 18, 2024
baa5dc2
i/p/requestprompts: move timeout callback into dedicated method
olivercalder Sep 20, 2024
edda246
daemon,i/p/requestprompts,o/i/apparmorprompting: explicitly indicate …
olivercalder Sep 21, 2024
0588c48
daemon,i/p/requestprompts,o/i/apparmorprompting: add client activity …
olivercalder Sep 21, 2024
34c193a
daemon: add prompting client activity helper and improve comments
olivercalder Nov 18, 2024
b80456f
i/p/requestprompts: make internal errors noticef instead of debugf
olivercalder Nov 18, 2024
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
29 changes: 24 additions & 5 deletions daemon/api_prompting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ func getUserID(r *http.Request) (uint32, Response) {
return uint32(userIDInt), nil
}

// isClientActivity returns true if the request comes a prompting handler
// service.
func isClientActivity(c *Command, r *http.Request) bool {
// TODO: check that it's a handler service client making the API request
return true
}

type invalidReason string

const (
Expand Down Expand Up @@ -331,7 +338,9 @@ func getPrompts(c *Command, r *http.Request, user *auth.UserState) Response {
return promptingNotRunningError()
}

prompts, err := getInterfaceManager(c).InterfacesRequestsManager().Prompts(userID)
clientActivity := isClientActivity(c, r)

prompts, err := getInterfaceManager(c).InterfacesRequestsManager().Prompts(userID, clientActivity)
if err != nil {
return promptingError(err)
}
Expand Down Expand Up @@ -360,7 +369,9 @@ func getPrompt(c *Command, r *http.Request, user *auth.UserState) Response {
return promptingNotRunningError()
}

prompt, err := getInterfaceManager(c).InterfacesRequestsManager().PromptWithID(userID, promptID)
clientActivity := isClientActivity(c, r)

prompt, err := getInterfaceManager(c).InterfacesRequestsManager().PromptWithID(userID, promptID, clientActivity)
if err != nil {
return promptingError(err)
}
Expand Down Expand Up @@ -392,7 +403,9 @@ func postPrompt(c *Command, r *http.Request, user *auth.UserState) Response {
return promptingError(fmt.Errorf("cannot decode request body into prompt reply: %w", err))
}

satisfiedPromptIDs, err := getInterfaceManager(c).InterfacesRequestsManager().HandleReply(userID, promptID, reply.Constraints, reply.Outcome, reply.Lifespan, reply.Duration)
clientActivity := isClientActivity(c, r)

satisfiedPromptIDs, err := getInterfaceManager(c).InterfacesRequestsManager().HandleReply(userID, promptID, reply.Constraints, reply.Outcome, reply.Lifespan, reply.Duration, clientActivity)
if err != nil {
return promptingError(err)
}
Expand Down Expand Up @@ -441,6 +454,9 @@ func postRules(c *Command, r *http.Request, user *auth.UserState) Response {
return promptingNotRunningError()
}

// Do not treat activity on the rules endpoints as prompt client activity.
clientActivity := false
olivercalder marked this conversation as resolved.
Show resolved Hide resolved

var postBody postRulesRequestBody
decoder := json.NewDecoder(r.Body)
if err := decoder.Decode(&postBody); err != nil {
Expand All @@ -452,7 +468,7 @@ func postRules(c *Command, r *http.Request, user *auth.UserState) Response {
if postBody.AddRule == nil {
return BadRequest(`must include "rule" field in request body when action is "add"`)
}
newRule, err := getInterfaceManager(c).InterfacesRequestsManager().AddRule(userID, postBody.AddRule.Snap, postBody.AddRule.Interface, postBody.AddRule.Constraints, postBody.AddRule.Outcome, postBody.AddRule.Lifespan, postBody.AddRule.Duration)
newRule, err := getInterfaceManager(c).InterfacesRequestsManager().AddRule(userID, postBody.AddRule.Snap, postBody.AddRule.Interface, postBody.AddRule.Constraints, postBody.AddRule.Outcome, postBody.AddRule.Lifespan, postBody.AddRule.Duration, clientActivity)
if err != nil {
return promptingError(err)
}
Expand Down Expand Up @@ -518,6 +534,9 @@ func postRule(c *Command, r *http.Request, user *auth.UserState) Response {
return promptingNotRunningError()
}

// Do not treat activity on the rules endpoints as prompt client activity.
clientActivity := false

var postBody postRuleRequestBody
decoder := json.NewDecoder(r.Body)
if err := decoder.Decode(&postBody); err != nil {
Expand All @@ -529,7 +548,7 @@ func postRule(c *Command, r *http.Request, user *auth.UserState) Response {
if postBody.PatchRule == nil {
return BadRequest(`must include "rule" field in request body when action is "patch"`)
}
patchedRule, err := getInterfaceManager(c).InterfacesRequestsManager().PatchRule(userID, ruleID, postBody.PatchRule.Constraints, postBody.PatchRule.Outcome, postBody.PatchRule.Lifespan, postBody.PatchRule.Duration)
patchedRule, err := getInterfaceManager(c).InterfacesRequestsManager().PatchRule(userID, ruleID, postBody.PatchRule.Constraints, postBody.PatchRule.Outcome, postBody.PatchRule.Lifespan, postBody.PatchRule.Duration, clientActivity)
if err != nil {
return promptingError(err)
}
Expand Down
37 changes: 24 additions & 13 deletions daemon/api_prompting_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,34 +52,38 @@ type fakeInterfacesRequestsManager struct {
err error

// Store most recent received values
userID uint32
snap string
iface string
id prompting.IDType // used for prompt ID or rule ID
constraints *prompting.Constraints
outcome prompting.OutcomeType
lifespan prompting.LifespanType
duration string
userID uint32
snap string
iface string
id prompting.IDType // used for prompt ID or rule ID
constraints *prompting.Constraints
outcome prompting.OutcomeType
lifespan prompting.LifespanType
duration string
clientActivity bool
}

func (m *fakeInterfacesRequestsManager) Prompts(userID uint32) ([]*requestprompts.Prompt, error) {
func (m *fakeInterfacesRequestsManager) Prompts(userID uint32, clientActivity bool) ([]*requestprompts.Prompt, error) {
m.userID = userID
m.clientActivity = clientActivity
return m.prompts, m.err
}

func (m *fakeInterfacesRequestsManager) PromptWithID(userID uint32, promptID prompting.IDType) (*requestprompts.Prompt, error) {
func (m *fakeInterfacesRequestsManager) PromptWithID(userID uint32, promptID prompting.IDType, clientActivity bool) (*requestprompts.Prompt, error) {
m.userID = userID
m.id = promptID
m.clientActivity = clientActivity
return m.prompt, m.err
}

func (m *fakeInterfacesRequestsManager) HandleReply(userID uint32, promptID prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) ([]prompting.IDType, error) {
func (m *fakeInterfacesRequestsManager) HandleReply(userID uint32, promptID prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string, clientActivity bool) ([]prompting.IDType, error) {
m.userID = userID
m.id = promptID
m.constraints = constraints
m.outcome = outcome
m.lifespan = lifespan
m.duration = duration
m.clientActivity = clientActivity
return m.satisfiedIDs, m.err
}

Expand All @@ -90,14 +94,15 @@ func (m *fakeInterfacesRequestsManager) Rules(userID uint32, snap string, iface
return m.rules, m.err
}

func (m *fakeInterfacesRequestsManager) AddRule(userID uint32, snap string, iface string, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (*requestrules.Rule, error) {
func (m *fakeInterfacesRequestsManager) AddRule(userID uint32, snap string, iface string, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string, clientActivity bool) (*requestrules.Rule, error) {
m.userID = userID
m.snap = snap
m.iface = iface
m.constraints = constraints
m.outcome = outcome
m.lifespan = lifespan
m.duration = duration
m.clientActivity = clientActivity
return m.rule, m.err
}

Expand All @@ -114,13 +119,14 @@ func (m *fakeInterfacesRequestsManager) RuleWithID(userID uint32, ruleID prompti
return m.rule, m.err
}

func (m *fakeInterfacesRequestsManager) PatchRule(userID uint32, ruleID prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string) (*requestrules.Rule, error) {
func (m *fakeInterfacesRequestsManager) PatchRule(userID uint32, ruleID prompting.IDType, constraints *prompting.Constraints, outcome prompting.OutcomeType, lifespan prompting.LifespanType, duration string, clientActivity bool) (*requestrules.Rule, error) {
m.userID = userID
m.id = ruleID
m.constraints = constraints
m.outcome = outcome
m.lifespan = lifespan
m.duration = duration
m.clientActivity = clientActivity
return m.rule, m.err
}

Expand Down Expand Up @@ -651,6 +657,7 @@ func (s *promptingSuite) TestGetPromptsHappy(c *C) {

// Check parameters
c.Check(s.manager.userID, Equals, uint32(1000))
c.Check(s.manager.clientActivity, Equals, true)

// Check return value
prompts, ok := rsp.Result.([]*requestprompts.Prompt)
Expand Down Expand Up @@ -681,6 +688,7 @@ func (s *promptingSuite) TestGetPromptHappy(c *C) {
// Check parameters
c.Check(s.manager.userID, Equals, uint32(1000))
c.Check(s.manager.id, Equals, prompting.IDType(0x0123456789abcdef))
c.Check(s.manager.clientActivity, Equals, true)

// Check return value
prompt, ok := rsp.Result.(*requestprompts.Prompt)
Expand Down Expand Up @@ -722,6 +730,7 @@ func (s *promptingSuite) TestPostPromptHappy(c *C) {
c.Check(s.manager.outcome, Equals, contents.Outcome)
c.Check(s.manager.lifespan, Equals, contents.Lifespan)
c.Check(s.manager.duration, Equals, contents.Duration)
c.Check(s.manager.clientActivity, Equals, true)

// Check return value
satisfiedIDs, ok := rsp.Result.([]prompting.IDType)
Expand Down Expand Up @@ -848,6 +857,7 @@ func (s *promptingSuite) TestPostRulesAddHappy(c *C) {
c.Check(s.manager.outcome, Equals, contents.Outcome)
c.Check(s.manager.lifespan, Equals, contents.Lifespan)
c.Check(s.manager.duration, Equals, contents.Duration)
c.Check(s.manager.clientActivity, Equals, false)

// Check return value
rule, ok := rsp.Result.(*requestrules.Rule)
Expand Down Expand Up @@ -1015,6 +1025,7 @@ func (s *promptingSuite) TestPostRulePatchHappy(c *C) {
c.Check(s.manager.outcome, Equals, contents.Outcome)
c.Check(s.manager.lifespan, Equals, contents.Lifespan)
c.Check(s.manager.duration, Equals, contents.Duration)
c.Check(s.manager.clientActivity, Equals, false)

// Check return value
rule, ok := rsp.Result.(*requestrules.Rule)
Expand Down
9 changes: 9 additions & 0 deletions interfaces/prompting/requestprompts/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"time"

"github.com/snapcore/snapd/interfaces/prompting"
"github.com/snapcore/snapd/testutil"
)

const MaxOutstandingPromptsPerUser = maxOutstandingPromptsPerUser
Expand Down Expand Up @@ -51,3 +52,11 @@ func (pdb *PromptDB) PerUser() map[uint32]*userPromptDB {
func (pdb *PromptDB) NextID() (prompting.IDType, error) {
return pdb.maxIDMmap.NextID()
}

func MockInitialTimeout(timeout time.Duration) (restore func()) {
return testutil.Mock(&initialTimeout, timeout)
}

func MockActivityTimeout(timeout time.Duration) (restore func()) {
return testutil.Mock(&activityTimeout, timeout)
}
Loading
Loading