diff --git a/commands/deploy.go b/commands/deploy.go index c7217a89..8a51399c 100644 --- a/commands/deploy.go +++ b/commands/deploy.go @@ -276,9 +276,15 @@ Error: %s`, fprocessErr.Error()) if msg := checkTLSInsecure(services.Provider.GatewayURL, deploySpec.TLSInsecure); len(msg) > 0 { fmt.Println(msg) } - statusCode := proxyClient.DeployFunction(ctx, deploySpec) - if badStatusCode(statusCode) { - failedStatusCodes[k] = statusCode + dRes, res, err := proxyClient.DeployFunction(ctx, deploySpec) + + if err == nil { + fmt.Println(dRes.Message) + fmt.Printf("URL: %s\n\n", dRes.URL) + } + + if badStatusCode(res.StatusCode) { + failedStatusCodes[k] = res.StatusCode } } } else { @@ -375,9 +381,14 @@ func deployImage( fmt.Println(msg) } - statusCode = client.DeployFunction(ctx, deploySpec) + dRes, res, err := client.DeployFunction(ctx, deploySpec) + if err != nil { + return res.StatusCode, err + } - return statusCode, nil + fmt.Println(dRes.Message) + fmt.Printf("URL: %s\n\n", dRes.URL) + return res.StatusCode, nil } func mergeSlice(values []string, overlay []string) []string { diff --git a/commands/describe.go b/commands/describe.go index 04876302..01c1b927 100644 --- a/commands/describe.go +++ b/commands/describe.go @@ -82,9 +82,9 @@ func runDescribe(cmd *cobra.Command, args []string) error { } //To get correct value for invocation count from /system/functions endpoint - functionList, err := cliClient.ListFunctions(ctx, functionNamespace) + functionList, _, err := cliClient.ListFunctions(ctx, functionNamespace) if err != nil { - return err + return actionableErrorMessage(err) } var invocationCount int diff --git a/commands/errors.go b/commands/errors.go index 25ff7f6a..c493626e 100644 --- a/commands/errors.go +++ b/commands/errors.go @@ -4,7 +4,10 @@ package commands import ( + "fmt" "strings" + + "github.com/openfaas/faas-cli/proxy" ) const ( @@ -24,3 +27,13 @@ func checkTLSInsecure(gateway string, tlsInsecure bool) string { } return "" } + +//actionableErrorMessage print actionable error message based on APIError check +func actionableErrorMessage(err error) error { + if proxy.IsUnknown(err) { + return fmt.Errorf("server returned unexpected status response: %s", err.Error()) + } else if proxy.IsUnauthorized(err) { + return fmt.Errorf("unauthorized access, run \"faas-cli login\" to setup authentication for this server") + } + return err +} diff --git a/commands/list.go b/commands/list.go index 28a52538..dba7ba11 100644 --- a/commands/list.go +++ b/commands/list.go @@ -73,9 +73,9 @@ func runList(cmd *cobra.Command, args []string) error { return err } - functions, err := proxyClient.ListFunctions(context.Background(), functionNamespace) + functions, _, err := proxyClient.ListFunctions(context.Background(), functionNamespace) if err != nil { - return err + return actionableErrorMessage(err) } if sortOrder == "name" { diff --git a/proxy/client.go b/proxy/client.go index bb32e387..d1d0d23c 100644 --- a/proxy/client.go +++ b/proxy/client.go @@ -1,9 +1,12 @@ package proxy import ( + "bytes" "context" + "encoding/json" "fmt" "io" + "io/ioutil" "net/http" "net/http/httputil" "net/url" @@ -129,3 +132,30 @@ func addQueryParams(u string, params map[string]string) (string, error) { func (c *Client) AddCheckRedirect(checkRedirect func(*http.Request, []*http.Request) error) { c.httpClient.CheckRedirect = checkRedirect } + +// parseResponse function parses HTTP response body into a typed struct +// or copies to io.Writer object. If it fails to decode response body +// then it re-construct it so that it can be read later +func parseResponse(res *http.Response, v interface{}) error { + var err error + defer res.Body.Close() + + switch v := v.(type) { + case nil: + case io.Writer: + _, err = io.Copy(v, res.Body) + default: + data, err := ioutil.ReadAll(res.Body) + if err == io.EOF { + err = nil // ignore EOF errors caused by empty response body + } + + decErr := json.Unmarshal(data, v) + if decErr != nil { + err = decErr + // In case of JSON decode error, re-construct response body + res.Body = io.NopCloser(bytes.NewBuffer(data)) + } + } + return err +} diff --git a/proxy/deploy.go b/proxy/deploy.go index b85e4e46..867376ba 100644 --- a/proxy/deploy.go +++ b/proxy/deploy.go @@ -8,7 +8,6 @@ import ( "context" "encoding/json" "fmt" - "io/ioutil" "net/http" "time" @@ -57,29 +56,48 @@ func generateFuncStr(spec *DeployFunctionSpec) string { return spec.FunctionName } +type DeployResponse struct { + Message string + RollingUpdate bool + URL string +} + // DeployFunction first tries to deploy a function and if it exists will then attempt // a rolling update. Warnings are suppressed for the second API call (if required.) -func (c *Client) DeployFunction(context context.Context, spec *DeployFunctionSpec) int { +func (c *Client) DeployFunction(context context.Context, spec *DeployFunctionSpec) (*DeployResponse, *http.Response, error) { rollingUpdateInfo := fmt.Sprintf("Function %s already exists, attempting rolling-update.", spec.FunctionName) - statusCode, deployOutput := c.deploy(context, spec, spec.Update) + res, err := c.deploy(context, spec, spec.Update) + + if err != nil && IsUnknown(err) { + return nil, res, err + } + + var deployResponse DeployResponse + if err == nil { + deployResponse.Message = fmt.Sprintf("Deployed. %s.", res.Status) + deployResponse.URL = fmt.Sprintf("%s/function/%s", c.GatewayURL.String(), generateFuncStr(spec)) + } - if spec.Update == true && statusCode == http.StatusNotFound { + if spec.Update == true && IsNotFound(err) { // Re-run the function with update=false + res, err = c.deploy(context, spec, false) + if err == nil { + deployResponse.Message = fmt.Sprintf("Deployed. %s.", res.Status) + deployResponse.URL = fmt.Sprintf("%s/function/%s", c.GatewayURL.String(), generateFuncStr(spec)) + } + + } else if res.StatusCode == http.StatusOK { + deployResponse.Message += rollingUpdateInfo + deployResponse.RollingUpdate = true - statusCode, deployOutput = c.deploy(context, spec, false) - } else if statusCode == http.StatusOK { - fmt.Println(rollingUpdateInfo) } - fmt.Println() - fmt.Println(deployOutput) - return statusCode + + return &deployResponse, res, err } // deploy a function to an OpenFaaS gateway over REST -func (c *Client) deploy(context context.Context, spec *DeployFunctionSpec, update bool) (int, string) { - - var deployOutput string +func (c *Client) deploy(context context.Context, spec *DeployFunctionSpec, update bool) (*http.Response, error) { // Need to alter Gateway to allow nil/empty string as fprocess, to avoid this repetition. var fprocessTemplate string if len(spec.FProcess) > 0 { @@ -146,37 +164,20 @@ func (c *Client) deploy(context context.Context, spec *DeployFunctionSpec, updat request, err = c.newRequest(method, "/system/functions", reader) if err != nil { - deployOutput += fmt.Sprintln(err) - return http.StatusInternalServerError, deployOutput + return nil, err } res, err := c.doRequest(context, request) if err != nil { - deployOutput += fmt.Sprintln("Is OpenFaaS deployed? Do you need to specify the --gateway flag?") - deployOutput += fmt.Sprintln(err) - return http.StatusInternalServerError, deployOutput - } - - if res.Body != nil { - defer res.Body.Close() + errMessage := fmt.Sprintln("Is OpenFaaS deployed? Do you need to specify the --gateway flag?") + errMessage += fmt.Sprintln(err) + return res, NewUnknown(errMessage, 0) } - switch res.StatusCode { - case http.StatusOK, http.StatusCreated, http.StatusAccepted: - deployOutput += fmt.Sprintf("Deployed. %s.\n", res.Status) - - deployedURL := fmt.Sprintf("URL: %s/function/%s", c.GatewayURL.String(), generateFuncStr(spec)) - deployOutput += fmt.Sprintln(deployedURL) - case http.StatusUnauthorized: - deployOutput += fmt.Sprintln("unauthorized access, run \"faas-cli login\" to setup authentication for this server") - - default: - bytesOut, err := ioutil.ReadAll(res.Body) - if err == nil { - deployOutput += fmt.Sprintf("Unexpected status: %d, message: %s\n", res.StatusCode, string(bytesOut)) - } + err = checkForAPIError(res) + if err != nil { + return res, err } - - return res.StatusCode, deployOutput + return res, nil } diff --git a/proxy/deploy_test.go b/proxy/deploy_test.go index dfd6bd98..87418240 100644 --- a/proxy/deploy_test.go +++ b/proxy/deploy_test.go @@ -21,7 +21,8 @@ type deployProxyTest struct { mockServerResponses []int replace bool update bool - expectedOutput string + expectedMessage string + statusCode int } func runDeployProxyTest(t *testing.T, deployTest deployProxyTest) { @@ -34,32 +35,34 @@ func runDeployProxyTest(t *testing.T, deployTest deployProxyTest) { cliAuth := NewTestAuth(nil) proxyClient, _ := NewClient(cliAuth, s.URL, nil, &defaultCommandTimeout) - stdout := test.CaptureStdout(func() { - proxyClient.DeployFunction(context.TODO(), &DeployFunctionSpec{ - "fprocess", - "function", - "image", - "dXNlcjpwYXNzd29yZA==", - "language", - deployTest.replace, - nil, - "network", - []string{}, - deployTest.update, - []string{}, - map[string]string{}, - map[string]string{}, - FunctionResourceRequest{}, - false, - tlsNoVerify, - "", - "", - }) + dRes, httpRes, _ := proxyClient.DeployFunction(context.TODO(), &DeployFunctionSpec{ + "fprocess", + "function", + "image", + "dXNlcjpwYXNzd29yZA==", + "language", + deployTest.replace, + nil, + "network", + []string{}, + deployTest.update, + []string{}, + map[string]string{}, + map[string]string{}, + FunctionResourceRequest{}, + false, + tlsNoVerify, + "", + "", }) - r := regexp.MustCompile(deployTest.expectedOutput) - if !r.MatchString(stdout) { - t.Fatalf("Output not matched: %s", stdout) + if httpRes.StatusCode != deployTest.statusCode { + t.Fatalf("StatuCode did not match. expected: %d, got: %d", deployTest.statusCode, httpRes.StatusCode) + } + + r := regexp.MustCompile(deployTest.expectedMessage) + if !r.MatchString(dRes.Message) { + t.Fatalf("Output not matched: %s", dRes.Message) } } @@ -70,21 +73,24 @@ func Test_RunDeployProxyTests(t *testing.T) { mockServerResponses: []int{http.StatusOK, http.StatusOK}, replace: true, update: false, - expectedOutput: `(?m:Deployed)`, + statusCode: http.StatusOK, + expectedMessage: `(?m:Deployed)`, }, { title: "404_Deploy", mockServerResponses: []int{http.StatusOK, http.StatusNotFound}, replace: true, update: false, - expectedOutput: `(?m:Unexpected status: 404)`, + statusCode: http.StatusNotFound, + expectedMessage: "", }, { title: "UpdateFailedDeployed", mockServerResponses: []int{http.StatusNotFound, http.StatusOK}, replace: false, update: true, - expectedOutput: `(?m:Deployed)`, + statusCode: http.StatusOK, + expectedMessage: `(?m:Deployed)`, }, } for _, tst := range deployProxyTests { diff --git a/proxy/errors.go b/proxy/errors.go new file mode 100644 index 00000000..47538f4f --- /dev/null +++ b/proxy/errors.go @@ -0,0 +1,270 @@ +package proxy + +import ( + "bytes" + "errors" + "io/ioutil" + "net/http" +) + +type APIError struct { + Reason StatusReason + Code int + Status string + Message string +} + +func (e *APIError) Error() string { + return e.Message +} + +const ( + StatusSuccess = "Success" + StatusFailure = "Failure" +) + +type StatusReason string + +const ( + StatusReasonUnknown StatusReason = "Unknown" + + // Status code 500 + StatusReasonInternalError StatusReason = "InternalError" + + // Status code 401 + StatusReasonUnauthorized StatusReason = "Unauthorized" + + // Status code 403 + StatusReasonForbidden StatusReason = "Forbidden" + + // Status code 404 + StatusReasonNotFound StatusReason = "NotFound" + + // Status code 409 + StatusReasonConflict StatusReason = "Conflict" + + // Status code 504 + StatusReasonGatewayTimeout StatusReason = "Timeout" + + // Status code 429 + StatusReasonTooManyRquests StatusReason = "TooManyRequests" + + // Status code 400 + StatusReasonBadRequest StatusReason = "BadRequest" + + // Status code 405 + StatusReasonMethodNotAllowed StatusReason = "MethodNotAllowed" + + // Status code 503 + StatusReasonServiceUnavailable StatusReason = "ServiceUnavailable" +) + +func NewInternalServer(message string) *APIError { + return &APIError{ + Reason: StatusReasonInternalError, + Code: http.StatusInternalServerError, + Status: StatusFailure, + Message: message, + } +} + +func NewUnauthorized(message string) *APIError { + return &APIError{ + Reason: StatusReasonUnauthorized, + Code: http.StatusUnauthorized, + Status: StatusFailure, + Message: message, + } +} + +func NewForbidden(message string) *APIError { + return &APIError{ + Reason: StatusReasonForbidden, + Code: http.StatusForbidden, + Status: StatusFailure, + Message: message, + } +} + +func NewNotFound(message string) *APIError { + return &APIError{ + Reason: StatusReasonNotFound, + Code: http.StatusNotFound, + Status: StatusFailure, + Message: message, + } +} + +func NewConflict(message string) *APIError { + return &APIError{ + Reason: StatusReasonConflict, + Code: http.StatusConflict, + Status: StatusFailure, + Message: message, + } +} + +func NewGatewayTimeout(message string) *APIError { + return &APIError{ + Reason: StatusReasonGatewayTimeout, + Code: http.StatusGatewayTimeout, + Status: StatusFailure, + Message: message, + } +} + +func NewTooManyRequests(message string) *APIError { + return &APIError{ + Reason: StatusReasonTooManyRquests, + Code: http.StatusTooManyRequests, + Status: StatusFailure, + Message: message, + } +} + +func NewBadRequest(message string) *APIError { + return &APIError{ + Reason: StatusReasonBadRequest, + Code: http.StatusBadRequest, + Status: StatusFailure, + Message: message, + } +} + +func NewMethodNotAllowed(message string) *APIError { + return &APIError{ + Reason: StatusReasonMethodNotAllowed, + Code: http.StatusMethodNotAllowed, + Status: StatusFailure, + Message: message, + } +} + +func NewServiceUnavailable(message string) *APIError { + return &APIError{ + Reason: StatusReasonServiceUnavailable, + Code: http.StatusServiceUnavailable, + Status: StatusFailure, + Message: message, + } +} + +func NewUnknown(message string, statusCode int) *APIError { + return &APIError{ + Reason: StatusReasonUnknown, + Code: statusCode, + Message: message, + Status: StatusFailure, + } +} + +func checkForAPIError(resp *http.Response) error { + //HTTP status in 2xx range are success + if c := resp.StatusCode; 200 <= c && c <= 299 { + return nil + } + + if resp.Body != nil { + defer resp.Body.Close() + } + + respBytes, err := ioutil.ReadAll(resp.Body) + + // Re-construct response body in case of error + resp.Body = ioutil.NopCloser(bytes.NewBuffer(respBytes)) + if err != nil { + return err + } + + message := string(respBytes) + + switch resp.StatusCode { + + case http.StatusInternalServerError: + return NewInternalServer(message) + + case http.StatusUnauthorized: + return NewUnauthorized(message) + + case http.StatusForbidden: + return NewForbidden(message) + + case http.StatusNotFound: + return NewNotFound(message) + + case http.StatusConflict: + return NewConflict(message) + + case http.StatusGatewayTimeout: + return NewGatewayTimeout(message) + + case http.StatusTooManyRequests: + return NewTooManyRequests(message) + + case http.StatusMethodNotAllowed: + return NewMethodNotAllowed(message) + + case http.StatusBadRequest: + return NewBadRequest(message) + + case http.StatusServiceUnavailable: + return NewServiceUnavailable(message) + + default: + return NewUnknown(message, resp.StatusCode) + } +} + +func IsUnknown(err error) bool { + return ReasonForError(err) == StatusReasonUnknown +} + +func IsNotFound(err error) bool { + return ReasonForError(err) == StatusReasonNotFound +} + +func IsUnauthorized(err error) bool { + return ReasonForError(err) == StatusReasonUnauthorized +} + +func IsBadRequest(err error) bool { + return ReasonForError(err) == StatusReasonBadRequest +} + +func IsForbidden(err error) bool { + return ReasonForError(err) == StatusReasonForbidden +} + +func IsInternalServerError(err error) bool { + return ReasonForError(err) == StatusReasonInternalError +} + +func IsServiceUnavailable(err error) bool { + return ReasonForError(err) == StatusReasonServiceUnavailable +} + +func IsMethodNotAllowed(err error) bool { + return ReasonForError(err) == StatusReasonMethodNotAllowed +} + +func IsTooManyRequests(err error) bool { + return ReasonForError(err) == StatusReasonTooManyRquests +} + +func IsGatewayTimeout(err error) bool { + return ReasonForError(err) == StatusReasonGatewayTimeout +} + +func IsConflict(err error) bool { + return ReasonForError(err) == StatusReasonConflict +} + +func ReasonForError(err error) StatusReason { + var e *APIError + + if errors.As(err, &e) { + return e.Reason + } + + return StatusReasonUnknown +} diff --git a/proxy/list.go b/proxy/list.go index 5681f985..5419f497 100644 --- a/proxy/list.go +++ b/proxy/list.go @@ -5,17 +5,15 @@ package proxy import ( "context" - "encoding/json" "fmt" - "io/ioutil" "net/http" types "github.com/openfaas/faas-provider/types" ) // ListFunctions list deployed functions -func (c *Client) ListFunctions(ctx context.Context, namespace string) ([]types.FunctionStatus, error) { +func (c *Client) ListFunctions(ctx context.Context, namespace string) ([]types.FunctionStatus, *http.Response, error) { var ( results []types.FunctionStatus listEndpoint string @@ -30,42 +28,29 @@ func (c *Client) ListFunctions(ctx context.Context, namespace string) ([]types.F if len(namespace) > 0 { listEndpoint, err = addQueryParams(listEndpoint, map[string]string{namespaceKey: namespace}) if err != nil { - return results, err + return results, nil, err } } getRequest, err := c.newRequest(http.MethodGet, listEndpoint, nil) if err != nil { - return nil, fmt.Errorf("cannot connect to OpenFaaS on URL: %s", c.GatewayURL.String()) + return results, nil, fmt.Errorf("cannot connect to OpenFaaS on URL: %s", c.GatewayURL.String()) } res, err := c.doRequest(ctx, getRequest) if err != nil { - return nil, fmt.Errorf("cannot connect to OpenFaaS on URL: %s", c.GatewayURL.String()) + return results, res, fmt.Errorf("cannot connect to OpenFaaS on URL: %s", c.GatewayURL.String()) } - if res.Body != nil { - defer res.Body.Close() + err = checkForAPIError(res) + if err != nil { + return results, res, err } - switch res.StatusCode { - case http.StatusOK: - - bytesOut, err := ioutil.ReadAll(res.Body) - if err != nil { - return nil, fmt.Errorf("cannot read result from OpenFaaS on URL: %s", c.GatewayURL.String()) - } - jsonErr := json.Unmarshal(bytesOut, &results) - if jsonErr != nil { - return nil, fmt.Errorf("cannot parse result from OpenFaaS on URL: %s\n%s", c.GatewayURL.String(), jsonErr.Error()) - } - case http.StatusUnauthorized: - return nil, fmt.Errorf("unauthorized access, run \"faas-cli login\" to setup authentication for this server") - default: - bytesOut, err := ioutil.ReadAll(res.Body) - if err == nil { - return nil, fmt.Errorf("server returned unexpected status code: %d - %s", res.StatusCode, string(bytesOut)) - } + err = parseResponse(res, &results) + if err != nil { + return results, res, err } - return results, nil + + return results, res, nil } diff --git a/proxy/list_test.go b/proxy/list_test.go index 67990e21..6c720c31 100644 --- a/proxy/list_test.go +++ b/proxy/list_test.go @@ -7,7 +7,6 @@ import ( "context" "net/http" "reflect" - "regexp" "testing" @@ -44,7 +43,7 @@ func Test_ListFunctions(t *testing.T) { cliAuth := NewTestAuth(nil) client, _ := NewClient(cliAuth, s.URL, nil, &defaultCommandTimeout) - result, err := client.ListFunctions(context.Background(), "") + result, _, err := client.ListFunctions(context.Background(), "") if err != nil { t.Fatalf("Error returned: %s", err) @@ -61,14 +60,13 @@ func Test_ListFunctions_Not200(t *testing.T) { cliAuth := NewTestAuth(nil) client, _ := NewClient(cliAuth, s.URL, nil, &defaultCommandTimeout) - _, err := client.ListFunctions(context.Background(), "") + _, _, err := client.ListFunctions(context.Background(), "") if err == nil { t.Fatalf("Error was not returned") } - r := regexp.MustCompile(`(?m:server returned unexpected status code)`) - if !r.MatchString(err.Error()) { + if !IsBadRequest(err) { t.Fatalf("Error not matched: %s", err) } }