From 264fd30956469a70786ff4667b6d62677cd887ca Mon Sep 17 00:00:00 2001 From: Pablo Chacin Date: Thu, 21 Nov 2024 13:23:19 +0100 Subject: [PATCH 1/2] add request headers Signed-off-by: Pablo Chacin --- pkg/client/client.go | 50 +++++++++++++++++++++++++++++++---- pkg/client/client_test.go | 55 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 6 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 51aade8..2c72718 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -14,6 +14,10 @@ import ( "github.com/grafana/k6build/pkg/api" ) +const ( + defaultAuthType = "Bearer" +) + var ( ErrBuildFailed = errors.New("build failed") //nolint:revive ErrRequestFailed = errors.New("request failed") //nolint:revive @@ -21,24 +25,39 @@ var ( // BuildServiceClientConfig defines the configuration for accessing a remote build service type BuildServiceClientConfig struct { + // URL to build service URL string + // Authorization credentials passed in the Authorization: header + // See AuthorizationType + Authorization string + // AuthorizationType type of credentials in the Authorization: header + // For example, "Bearer", "Token", "Basic". Defaults to "Bearer" + AuthorizationType string + // Headers custom request headers + Headers map[string]string } // NewBuildServiceClient returns a new client for a remote build service func NewBuildServiceClient(config BuildServiceClientConfig) (k6build.BuildService, error) { return &BuildClient{ - srv: config.URL, + srvURL: config.URL, + auth: config.Authorization, + authType: config.AuthorizationType, + headers: config.Headers, }, nil } // BuildClient defines a client of a build service type BuildClient struct { - srv string + srvURL string + authType string + auth string + headers map[string]string } // Build request building an artidact to a build service func (r *BuildClient) Build( - _ context.Context, + ctx context.Context, platform string, k6Constrains string, deps []k6build.Dependency, @@ -54,12 +73,33 @@ func (r *BuildClient) Build( return k6build.Artifact{}, fmt.Errorf("%w: %w", ErrRequestFailed, err) } - url, err := url.Parse(r.srv) + url, err := url.Parse(r.srvURL) if err != nil { return k6build.Artifact{}, fmt.Errorf("invalid server %w", err) } url.Path = "/build/" - resp, err := http.Post(url.String(), "application/json", marshaled) //nolint:noctx + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url.String(), marshaled) + if err != nil { + return k6build.Artifact{}, fmt.Errorf("%w: %w", ErrRequestFailed, err) + } + req.Header.Add("Content-Type", "application/json") + + // add authorization header "Authorization: " + if r.auth != "" { + authType := r.authType + if authType == "" { + authType = defaultAuthType + } + req.Header.Add("Authorization", fmt.Sprintf("%s %s", authType, r.auth)) + } + + // add custom headers + for h, v := range r.headers { + req.Header.Add(h, v) + } + + resp, err := http.DefaultClient.Do(req) if err != nil { return k6build.Artifact{}, fmt.Errorf("%w: %w", ErrRequestFailed, err) } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 515bf94..d7e7719 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "testing" @@ -16,11 +17,31 @@ import ( type testSrv struct { status int response api.BuildResponse + auth string + authType string + headers map[string]string } func (t testSrv) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Header().Add("Content-Type", "application/json") + // validate authorization + if t.auth != "" { + authHeader := fmt.Sprintf("%s %s", t.authType, t.auth) + if r.Header.Get("Authorization") != authHeader { + w.WriteHeader(http.StatusUnauthorized) + return + } + } + + // validate headers + for h, v := range t.headers { + if r.Header.Get(h) != v { + w.WriteHeader(http.StatusBadRequest) + return + } + } + // validate request req := api.BuildRequest{} err := json.NewDecoder(r.Body).Decode(&req) @@ -47,6 +68,9 @@ func TestRemote(t *testing.T) { testCases := []struct { title string + headers map[string]string + auth string + authType string status int resp api.BuildResponse expectErr error @@ -77,6 +101,29 @@ func TestRemote(t *testing.T) { }, expectErr: ErrBuildFailed, }, + { + title: "auth header", + auth: "token", + authType: "Bearer", + status: http.StatusOK, + resp: api.BuildResponse{}, + expectErr: nil, + }, + { + title: "failed auth", + status: http.StatusUnauthorized, + resp: api.BuildResponse{}, + expectErr: ErrRequestFailed, + }, + { + title: "custom headers", + headers: map[string]string{ + "Custom-Header": "Custom-Value", + }, + status: http.StatusOK, + resp: api.BuildResponse{}, + expectErr: nil, + }, } for _, tc := range testCases { @@ -87,11 +134,17 @@ func TestRemote(t *testing.T) { srv := httptest.NewServer(testSrv{ status: tc.status, response: tc.resp, + auth: tc.auth, + authType: tc.authType, + headers: tc.headers, }) client, err := NewBuildServiceClient( BuildServiceClientConfig{ - URL: srv.URL, + URL: srv.URL, + Headers: tc.headers, + Authorization: tc.auth, + AuthorizationType: tc.authType, }, ) if err != nil { From 6cdb0bdbe966467f236cd429b290201609fc3fee Mon Sep 17 00:00:00 2001 From: Pablo Chacin Date: Thu, 21 Nov 2024 14:13:58 +0100 Subject: [PATCH 2/2] refactor client tests Signed-off-by: Pablo Chacin --- pkg/client/client_test.go | 156 ++++++++++++++++++++++---------------- 1 file changed, 92 insertions(+), 64 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index d7e7719..ebc9e31 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -15,51 +15,83 @@ import ( ) type testSrv struct { - status int - response api.BuildResponse - auth string - authType string - headers map[string]string + handlers []requestHandler } -func (t testSrv) ServeHTTP(w http.ResponseWriter, r *http.Request) { - w.Header().Add("Content-Type", "application/json") +// process request and return a boolean indicating if request should be passed to the next handler in the chain +type requestHandler func(w http.ResponseWriter, r *http.Request) bool + +func withValidateRequest() requestHandler { + return func(w http.ResponseWriter, r *http.Request) bool { + req := api.BuildRequest{} + err := json.NewDecoder(r.Body).Decode(&req) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return false + } - // validate authorization - if t.auth != "" { - authHeader := fmt.Sprintf("%s %s", t.authType, t.auth) + if req.K6Constrains == "" || req.Platform == "" || len(req.Dependencies) == 0 { + w.WriteHeader(http.StatusBadRequest) + return false + } + + return true + } +} + +func withAuthorizationCheck(authType string, auth string) requestHandler { + return func(w http.ResponseWriter, r *http.Request) bool { + authHeader := fmt.Sprintf("%s %s", authType, auth) if r.Header.Get("Authorization") != authHeader { w.WriteHeader(http.StatusUnauthorized) - return + return false } + return true } +} - // validate headers - for h, v := range t.headers { - if r.Header.Get(h) != v { - w.WriteHeader(http.StatusBadRequest) - return +func withHeadersCheck(headers map[string]string) requestHandler { + return func(w http.ResponseWriter, r *http.Request) bool { + for h, v := range headers { + if r.Header.Get(h) != v { + w.WriteHeader(http.StatusBadRequest) + return false + } } + return true } +} + +func withResponse(status int, response api.BuildResponse) requestHandler { + return func(w http.ResponseWriter, _ *http.Request) bool { + resp := &bytes.Buffer{} + err := json.NewEncoder(resp).Encode(response) + if err != nil { + panic("unexpected error encoding response") + } - // validate request - req := api.BuildRequest{} - err := json.NewDecoder(r.Body).Decode(&req) - if err != nil { - w.WriteHeader(http.StatusBadRequest) - return + w.WriteHeader(status) + _, _ = w.Write(resp.Bytes()) + + return false } +} - // send canned response - resp := &bytes.Buffer{} - err = json.NewEncoder(resp).Encode(t.response) - if err != nil { - // set uncommon status code to signal something unexpected happened - w.WriteHeader(http.StatusTeapot) - return +func (t testSrv) ServeHTTP(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Content-Type", "application/json") + + // check headers + for _, check := range t.handlers { + if !check(w, r) { + return + } } - w.WriteHeader(t.status) + // by default return ok and an empty artifact + resp := &bytes.Buffer{} + _ = json.NewEncoder(resp).Encode(api.BuildResponse{Artifact: k6build.Artifact{}}) //nolint:errchkjson + + w.WriteHeader(http.StatusOK) _, _ = w.Write(resp.Bytes()) } @@ -71,48 +103,45 @@ func TestRemote(t *testing.T) { headers map[string]string auth string authType string - status int - resp api.BuildResponse + handlers []requestHandler expectErr error }{ { - title: "normal build", - status: http.StatusOK, - resp: api.BuildResponse{ - Error: "", - Artifact: k6build.Artifact{}, + title: "normal build", + handlers: []requestHandler{ + withValidateRequest(), }, }, { - title: "request failed", - status: http.StatusInternalServerError, - resp: api.BuildResponse{ - Error: "request failed", - Artifact: k6build.Artifact{}, + title: "build request failed", + handlers: []requestHandler{ + withResponse(http.StatusOK, api.BuildResponse{Error: ErrBuildFailed.Error()}), }, - expectErr: ErrRequestFailed, + expectErr: ErrBuildFailed, }, { - title: "failed build", - status: http.StatusOK, - resp: api.BuildResponse{ - Error: "failed build", - Artifact: k6build.Artifact{}, + title: "auth header", + auth: "token", + authType: "Bearer", + handlers: []requestHandler{ + withAuthorizationCheck("Bearer", "token"), }, - expectErr: ErrBuildFailed, + expectErr: nil, }, { - title: "auth header", - auth: "token", - authType: "Bearer", - status: http.StatusOK, - resp: api.BuildResponse{}, + title: "with default auth type", + auth: "token", + authType: "", + handlers: []requestHandler{ + withAuthorizationCheck("Bearer", "token"), + }, expectErr: nil, }, { - title: "failed auth", - status: http.StatusUnauthorized, - resp: api.BuildResponse{}, + title: "failed auth", + handlers: []requestHandler{ + withResponse(http.StatusUnauthorized, api.BuildResponse{Error: "Authorization Required"}), + }, expectErr: ErrRequestFailed, }, { @@ -120,8 +149,9 @@ func TestRemote(t *testing.T) { headers: map[string]string{ "Custom-Header": "Custom-Value", }, - status: http.StatusOK, - resp: api.BuildResponse{}, + handlers: []requestHandler{ + withHeadersCheck(map[string]string{"Custom-Header": "Custom-Value"}), + }, expectErr: nil, }, } @@ -132,13 +162,11 @@ func TestRemote(t *testing.T) { t.Parallel() srv := httptest.NewServer(testSrv{ - status: tc.status, - response: tc.resp, - auth: tc.auth, - authType: tc.authType, - headers: tc.headers, + handlers: tc.handlers, }) + defer srv.Close() + client, err := NewBuildServiceClient( BuildServiceClientConfig{ URL: srv.URL,