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(daemon): add ErrorResponse to allow external error creation #303

Merged
merged 7 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
22 changes: 11 additions & 11 deletions internals/daemon/api_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func v1GetChanges(c *Command, r *http.Request, _ *UserState) Response {
case "ready":
filter = func(chg *state.Change) bool { return chg.Status().Ready() }
default:
return statusBadRequest("select should be one of: all,in-progress,ready")
return StatusBadRequest("select should be one of: all,in-progress,ready")
}

if wantedName := query.Get("for"); wantedName != "" {
Expand Down Expand Up @@ -177,7 +177,7 @@ func v1GetChange(c *Command, r *http.Request, _ *UserState) Response {
defer st.Unlock()
chg := st.Change(changeID)
if chg == nil {
return statusNotFound("cannot find change with id %q", changeID)
return StatusNotFound("cannot find change with id %q", changeID)
}

return SyncResponse(change2changeInfo(chg))
Expand All @@ -190,7 +190,7 @@ func v1GetChangeWait(c *Command, r *http.Request, _ *UserState) Response {
change := st.Change(changeID)
st.Unlock()
if change == nil {
return statusNotFound("cannot find change with id %q", changeID)
return StatusNotFound("cannot find change with id %q", changeID)
}

timeoutStr := r.URL.Query().Get("timeout")
Expand All @@ -199,23 +199,23 @@ func v1GetChangeWait(c *Command, r *http.Request, _ *UserState) Response {
// whichever is first.
timeout, err := time.ParseDuration(timeoutStr)
if err != nil {
return statusBadRequest("invalid timeout %q: %v", timeoutStr, err)
return StatusBadRequest("invalid timeout %q: %v", timeoutStr, err)
}
timer := time.NewTimer(timeout)
select {
case <-change.Ready():
timer.Stop() // change ready, release timer resources
case <-timer.C:
return statusGatewayTimeout("timed out waiting for change after %s", timeout)
return StatusGatewayTimeout("timed out waiting for change after %s", timeout)
case <-r.Context().Done():
return statusInternalError("request cancelled")
return StatusInternalError("request cancelled")
}
} else {
// No timeout, wait indefinitely for change to be ready.
select {
case <-change.Ready():
case <-r.Context().Done():
return statusInternalError("request cancelled")
return StatusInternalError("request cancelled")
}
}

Expand All @@ -231,7 +231,7 @@ func v1PostChange(c *Command, r *http.Request, _ *UserState) Response {
defer state.Unlock()
chg := state.Change(chID)
if chg == nil {
return statusNotFound("cannot find change with id %q", chID)
return StatusNotFound("cannot find change with id %q", chID)
}

var reqData struct {
Expand All @@ -240,15 +240,15 @@ func v1PostChange(c *Command, r *http.Request, _ *UserState) Response {

decoder := json.NewDecoder(r.Body)
if err := decoder.Decode(&reqData); err != nil {
return statusBadRequest("cannot decode data from request body: %v", err)
return StatusBadRequest("cannot decode data from request body: %v", err)
}

if reqData.Action != "abort" {
return statusBadRequest("change action %q is unsupported", reqData.Action)
return StatusBadRequest("change action %q is unsupported", reqData.Action)
}

if chg.Status().Ready() {
return statusBadRequest("cannot abort change %s with nothing pending", chID)
return StatusBadRequest("cannot abort change %s with nothing pending", chID)
}

// flag the change
Expand Down
8 changes: 4 additions & 4 deletions internals/daemon/api_changes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ func (s *apiSuite) TestWaitChangeInvalidTimeout(c *check.C) {
c.Check(rec.Code, check.Equals, 400)
c.Check(rsp.Status, check.Equals, 400)
c.Check(rsp.Type, check.Equals, ResponseTypeError)
result := rsp.Result.(*errorResult)
result := rsp.Result.(*ErrorResult)
c.Check(result.Message, check.Matches, "invalid timeout .*")
}

Expand Down Expand Up @@ -419,7 +419,7 @@ func (s *apiSuite) TestWaitChangeCancel(c *check.C) {
c.Check(rec.Code, check.Equals, 500)
c.Check(rsp.Status, check.Equals, 500)
c.Check(rsp.Type, check.Equals, ResponseTypeError)
result := rsp.Result.(*errorResult)
result := rsp.Result.(*ErrorResult)
c.Check(result.Message, check.Equals, "request cancelled")
}

Expand All @@ -428,7 +428,7 @@ func (s *apiSuite) TestWaitChangeTimeout(c *check.C) {
c.Check(rec.Code, check.Equals, 504)
c.Check(rsp.Status, check.Equals, 504)
c.Check(rsp.Type, check.Equals, ResponseTypeError)
result := rsp.Result.(*errorResult)
result := rsp.Result.(*ErrorResult)
c.Check(result.Message, check.Matches, "timed out waiting for change .*")
}

Expand All @@ -440,7 +440,7 @@ func (s *apiSuite) TestWaitChangeTimeoutCancel(c *check.C) {
c.Check(rec.Code, check.Equals, 500)
c.Check(rsp.Status, check.Equals, 500)
c.Check(rsp.Type, check.Equals, ResponseTypeError)
result := rsp.Result.(*errorResult)
result := rsp.Result.(*ErrorResult)
c.Check(result.Message, check.Equals, "request cancelled")
}

Expand Down
4 changes: 2 additions & 2 deletions internals/daemon/api_checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ func v1GetChecks(c *Command, r *http.Request, _ *UserState) Response {
switch level {
case plan.UnsetLevel, plan.AliveLevel, plan.ReadyLevel:
default:
return statusBadRequest(`level must be "alive" or "ready"`)
return StatusBadRequest(`level must be "alive" or "ready"`)
}

names := strutil.MultiCommaSeparatedList(query["names"])

checkMgr := c.d.overlord.CheckManager()
checks, err := checkMgr.Checks()
if err != nil {
return statusInternalError("%v", err)
return StatusInternalError("%v", err)
}

infos := []checkInfo{} // if no checks, return [] instead of null
Expand Down
18 changes: 9 additions & 9 deletions internals/daemon/api_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,25 +48,25 @@ func v1PostExec(c *Command, req *http.Request, _ *UserState) Response {
var payload execPayload
decoder := json.NewDecoder(req.Body)
if err := decoder.Decode(&payload); err != nil {
return statusBadRequest("cannot decode request body: %v", err)
return StatusBadRequest("cannot decode request body: %v", err)
}
if len(payload.Command) < 1 {
return statusBadRequest("must specify command")
return StatusBadRequest("must specify command")
}

var timeout time.Duration
if payload.Timeout != "" {
var err error
timeout, err = time.ParseDuration(payload.Timeout)
if err != nil {
return statusBadRequest("invalid timeout: %v", err)
return StatusBadRequest("invalid timeout: %v", err)
}
}

// Check up-front that the executable exists.
_, err := exec.LookPath(payload.Command[0])
if err != nil {
return statusBadRequest("cannot find executable %q", payload.Command[0])
return StatusBadRequest("cannot find executable %q", payload.Command[0])
}

// Also check that the working directory exists, to avoid a confusing
Expand All @@ -78,13 +78,13 @@ func v1PostExec(c *Command, req *http.Request, _ *UserState) Response {
// correct to use it as a working directory, but this is a good start.
if payload.WorkingDir != "" {
if !osutil.IsDir(payload.WorkingDir) {
return statusBadRequest("cannot find working directory %q", payload.WorkingDir)
return StatusBadRequest("cannot find working directory %q", payload.WorkingDir)
}
}

p, err := c.d.overlord.ServiceManager().Plan()
if err != nil {
return statusBadRequest("%v", err)
return StatusBadRequest("%v", err)
}
overrides := plan.ContextOptions{
Environment: payload.Environment,
Expand All @@ -96,13 +96,13 @@ func v1PostExec(c *Command, req *http.Request, _ *UserState) Response {
}
merged, err := plan.MergeServiceContext(p, payload.ServiceContext, overrides)
if err != nil {
return statusBadRequest("%v", err)
return StatusBadRequest("%v", err)
}

// Convert User/UserID and Group/GroupID combinations into raw uid/gid.
uid, gid, err := osutil.NormalizeUidGid(merged.UserID, merged.GroupID, merged.User, merged.Group)
if err != nil {
return statusBadRequest("%v", err)
return StatusBadRequest("%v", err)
}

st := c.d.overlord.State()
Expand All @@ -124,7 +124,7 @@ func v1PostExec(c *Command, req *http.Request, _ *UserState) Response {
}
task, metadata, err := cmdstate.Exec(st, args)
if err != nil {
return statusInternalError("cannot call exec: %v", err)
return StatusInternalError("cannot call exec: %v", err)
}

change := st.NewChange("exec", fmt.Sprintf("Execute command %q", args.Command[0]))
Expand Down
54 changes: 27 additions & 27 deletions internals/daemon/api_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,31 +44,31 @@ func v1GetFiles(_ *Command, req *http.Request, _ *UserState) Response {
case "read":
paths := query["path"]
if len(paths) == 0 {
return statusBadRequest("must specify one or more paths")
return StatusBadRequest("must specify one or more paths")
}
if req.Header.Get("Accept") != "multipart/form-data" {
return statusBadRequest(`must accept multipart/form-data`)
return StatusBadRequest(`must accept multipart/form-data`)
}
return readFilesResponse{paths: paths}
case "list":
path := query.Get("path")
if path == "" {
return statusBadRequest("must specify path")
return StatusBadRequest("must specify path")
}
pattern := query.Get("pattern")
itself := query.Get("itself")
if itself != "true" && itself != "false" && itself != "" {
return statusBadRequest(`itself parameter must be "true" or "false"`)
return StatusBadRequest(`itself parameter must be "true" or "false"`)
}
return listFilesResponse(path, pattern, itself == "true")
default:
return statusBadRequest("invalid action %q", action)
return StatusBadRequest("invalid action %q", action)
}
}

type fileResult struct {
Path string `json:"path"`
Error *errorResult `json:"error,omitempty"`
Error *ErrorResult `json:"error,omitempty"`
}

// Reading files
Expand Down Expand Up @@ -160,20 +160,20 @@ func readFile(path string, mw *multipart.Writer) error {
return nil
}

func fileErrorToResult(err error) *errorResult {
func fileErrorToResult(err error) *ErrorResult {
if err == nil {
return nil
}
var kind errorKind
var kind ErrorKind
switch {
case errors.Is(err, os.ErrPermission):
kind = errorKindPermissionDenied
kind = ErrorKindPermissionDenied
case errors.Is(err, os.ErrNotExist):
kind = errorKindNotFound
kind = ErrorKindNotFound
default:
kind = errorKindGenericFileError
kind = ErrorKindGenericFileError
}
return &errorResult{
return &ErrorResult{
Kind: kind,
Message: err.Error(),
}
Expand Down Expand Up @@ -283,7 +283,7 @@ func fileInfoToResult(fullPath string, info os.FileInfo, userCache, groupCache m

func listFilesResponse(path, pattern string, itself bool) Response {
if !pathpkg.IsAbs(path) {
return statusBadRequest("path must be absolute, got %q", path)
return StatusBadRequest("path must be absolute, got %q", path)
}
result, err := listFiles(path, pattern, itself)
if err != nil {
Expand Down Expand Up @@ -341,14 +341,14 @@ func v1PostFiles(_ *Command, req *http.Request, _ *UserState) Response {
contentType := req.Header.Get("Content-Type")
mediaType, params, err := mime.ParseMediaType(contentType)
if err != nil {
return statusBadRequest("invalid Content-Type %q", contentType)
return StatusBadRequest("invalid Content-Type %q", contentType)
}

switch mediaType {
case "multipart/form-data":
boundary := params["boundary"]
if len(boundary) < minBoundaryLength {
return statusBadRequest("invalid boundary %q", boundary)
return StatusBadRequest("invalid boundary %q", boundary)
}
return writeFiles(req.Body, boundary)
case "application/json":
Expand All @@ -359,20 +359,20 @@ func v1PostFiles(_ *Command, req *http.Request, _ *UserState) Response {
}
decoder := json.NewDecoder(req.Body)
if err := decoder.Decode(&payload); err != nil {
return statusBadRequest("cannot decode request body: %v", err)
return StatusBadRequest("cannot decode request body: %v", err)
}
switch payload.Action {
case "make-dirs":
return makeDirs(payload.Dirs)
case "remove":
return removePaths(payload.Paths)
case "write":
return statusBadRequest(`must use multipart with "write" action`)
return StatusBadRequest(`must use multipart with "write" action`)
default:
return statusBadRequest("invalid action %q", payload.Action)
return StatusBadRequest("invalid action %q", payload.Action)
}
default:
return statusBadRequest("invalid media type %q", mediaType)
return StatusBadRequest("invalid media type %q", mediaType)
}
}

Expand All @@ -393,10 +393,10 @@ func writeFiles(body io.Reader, boundary string) Response {
mr := multipart.NewReader(body, boundary)
part, err := mr.NextPart()
if err != nil {
return statusBadRequest("cannot read request metadata: %v", err)
return StatusBadRequest("cannot read request metadata: %v", err)
}
if part.FormName() != "request" {
return statusBadRequest(`metadata field name must be "request", got %q`, part.FormName())
return StatusBadRequest(`metadata field name must be "request", got %q`, part.FormName())
}

// Decode metadata about files to write.
Expand All @@ -406,13 +406,13 @@ func writeFiles(body io.Reader, boundary string) Response {
}
decoder := json.NewDecoder(part)
if err := decoder.Decode(&payload); err != nil {
return statusBadRequest("cannot decode request metadata: %v", err)
return StatusBadRequest("cannot decode request metadata: %v", err)
}
if payload.Action != "write" {
return statusBadRequest(`multipart action must be "write", got %q`, payload.Action)
return StatusBadRequest(`multipart action must be "write", got %q`, payload.Action)
}
if len(payload.Files) == 0 {
return statusBadRequest("must specify one or more files")
return StatusBadRequest("must specify one or more files")
}
infos := make(map[string]writeFilesItem)
for _, file := range payload.Files {
Expand All @@ -426,15 +426,15 @@ func writeFiles(body io.Reader, boundary string) Response {
break
}
if err != nil {
return statusBadRequest("cannot read file part %d: %v", i, err)
return StatusBadRequest("cannot read file part %d: %v", i, err)
}
if part.FormName() != "files" {
return statusBadRequest(`field name must be "files", got %q`, part.FormName())
return StatusBadRequest(`field name must be "files", got %q`, part.FormName())
}
path := multipartFilename(part)
info, ok := infos[path]
if !ok {
return statusBadRequest("no metadata for path %q", path)
return StatusBadRequest("no metadata for path %q", path)
}
errors[path] = writeFile(info, part)
part.Close()
Expand Down
4 changes: 2 additions & 2 deletions internals/daemon/api_files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,9 @@ func (s *filesSuite) TestReadErrorOnRead(c *C) {
})
}

func checkFileResult(c *C, r testFileResult, path, errorKind, errorMsg string) {
func checkFileResult(c *C, r testFileResult, path, ErrorKind, errorMsg string) {
c.Check(r.Path, Equals, path)
c.Check(r.Error.Kind, Equals, errorKind)
c.Check(r.Error.Kind, Equals, ErrorKind)
c.Check(r.Error.Message, Matches, errorMsg)
}

Expand Down
Loading