From 54806d9ba6737e12199ed12b53323674b66bc66d Mon Sep 17 00:00:00 2001 From: Dean Roehrich Date: Fri, 13 Dec 2024 09:04:08 -0600 Subject: [PATCH] Add negative API vesion tests for server Signed-off-by: Dean Roehrich --- daemons/copy-offload/pkg/server/server.go | 2 +- .../copy-offload/pkg/server/server_test.go | 99 +++++++++++++++++++ daemons/lib-copy-offload/copy-offload.c | 20 ++-- 3 files changed, 111 insertions(+), 10 deletions(-) diff --git a/daemons/copy-offload/pkg/server/server.go b/daemons/copy-offload/pkg/server/server.go index 37990216..1688dcaa 100644 --- a/daemons/copy-offload/pkg/server/server.go +++ b/daemons/copy-offload/pkg/server/server.go @@ -45,7 +45,7 @@ func validateVersion(w http.ResponseWriter, req *http.Request) string { // This applies to the format of the request sent by the client as well // as the format of our response. apiVersion := req.Header.Get("Accepts-version") - if apiVersion == "" || apiVersion != "1.0" { + if apiVersion != "1.0" { // The RFC says Not Accceptable should return a list of valid versions. http.Error(w, "Valid versions: 1.0", http.StatusNotAcceptable) return "" diff --git a/daemons/copy-offload/pkg/server/server_test.go b/daemons/copy-offload/pkg/server/server_test.go index 3787562c..7b214b9f 100644 --- a/daemons/copy-offload/pkg/server/server_test.go +++ b/daemons/copy-offload/pkg/server/server_test.go @@ -368,6 +368,105 @@ func TestE_Lifecycle(t *testing.T) { }) } +func TestF_BadAPIVersion(t *testing.T) { + + crLog := setupLog() + drvr := &driver.Driver{Log: crLog, Mock: true} + httpHandler := &UserHttp{Log: crLog, Drvr: drvr, Mock: true} + + testCases := []struct { + name string + method string + url string + handler func(http.ResponseWriter, *http.Request) + body []byte + skipApiVersion bool + }{ + { + name: "bad api version for hello", + method: http.MethodGet, + url: "/hello", + handler: httpHandler.Hello, + }, + { + name: "skip api version for hello", + method: http.MethodGet, + url: "/hello", + handler: httpHandler.Hello, + skipApiVersion: true, + }, + { + name: "bad api version for list", + method: http.MethodGet, + url: "/list", + handler: httpHandler.ListRequests, + }, + { + name: "skip api version for list", + method: http.MethodGet, + url: "/list", + handler: httpHandler.ListRequests, + skipApiVersion: true, + }, + { + name: "bad api version for cancel", + method: http.MethodDelete, + url: "/cancel/nnf-copy-offload-node-9ae2a136-4", + handler: httpHandler.CancelRequest, + }, + { + name: "skip api version for cancel", + method: http.MethodDelete, + url: "/cancel/nnf-copy-offload-node-9ae2a136-4", + handler: httpHandler.CancelRequest, + skipApiVersion: true, + }, + { + name: "bad api version for copy", + method: http.MethodPost, + url: "/trial", + body: []byte("{\"computeName\": \"rabbit-compute-3\", \"workflowName\": \"yellow\", \"sourcePath\": \"/mnt/nnf/dc51a384-99bd-4ef1-8444-4ee3b0cdc8a8-0\", \"destinationPath\": \"/lus/global/dean/foo\", \"dryrun\": true}"), + handler: httpHandler.TrialRequest, + }, + { + name: "skip api version for copy", + method: http.MethodPost, + url: "/trial", + body: []byte("{\"computeName\": \"rabbit-compute-3\", \"workflowName\": \"yellow\", \"sourcePath\": \"/mnt/nnf/dc51a384-99bd-4ef1-8444-4ee3b0cdc8a8-0\", \"destinationPath\": \"/lus/global/dean/foo\", \"dryrun\": true}"), + handler: httpHandler.TrialRequest, + skipApiVersion: true, + }, + } + + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + var readerBody io.Reader = nil + if len(test.body) > 0 { + readerBody = bytes.NewReader(test.body) + } + request, _ := http.NewRequest(test.method, test.url, readerBody) + if !test.skipApiVersion { + request.Header.Set("Accepts-version", "0.0") + } + response := httptest.NewRecorder() + + test.handler(response, request) + + res := response.Result() + got := response.Body.String() + statusWant := http.StatusNotAcceptable + wantPrefix := "Valid versions: " + + if res.StatusCode != statusWant { + t.Errorf("got status %d, want status %d", res.StatusCode, statusWant) + } + if !strings.HasPrefix(got, wantPrefix) { + t.Errorf("got %q, want \"%s[...]\"", got, wantPrefix) + } + }) + } +} + // Just touch ginkgo, so it's here to interpret any ginkgo args from // "make test", so that doesn't fail on this test file. var _ = BeforeSuite(func() {}) diff --git a/daemons/lib-copy-offload/copy-offload.c b/daemons/lib-copy-offload/copy-offload.c index 09d4c122..9bfd46df 100644 --- a/daemons/lib-copy-offload/copy-offload.c +++ b/daemons/lib-copy-offload/copy-offload.c @@ -158,6 +158,14 @@ static long copy_offload_perform(COPY_OFFLOAD *offload, struct memory *chunk) { return http_code; } +/* Chop the newline from the end of the string. */ +static void chop(char **output) { + if (*output != NULL) { + char *newline = strchr(*output, '\n'); + *newline = '\0'; + } +} + /* List the active copy-offload requests. * The caller is responsible for calling free() on @output if *output is non-NULL. */ @@ -175,9 +183,7 @@ int copy_offload_list(COPY_OFFLOAD *offload, char **output) { ret = 0; if (chunk.response != NULL) { *output = strdup(chunk.response); - // chop - char *newline = strchr(*output, '\n'); - *newline = '\0'; + chop(output); free(chunk.response); } return ret; @@ -199,9 +205,7 @@ int copy_offload_cancel(COPY_OFFLOAD *offload, char *job_name, char **output) { ret = 0; if (chunk.response != NULL) { *output = strdup(chunk.response); - // chop - char *newline = strchr(*output, '\n'); - *newline = '\0'; + chop(output); free(chunk.response); } return ret; @@ -230,9 +234,7 @@ int copy_offload_copy(COPY_OFFLOAD *offload, char *compute_name, char *workflow_ char *delim = strchr(chunk.response, '='); if (delim != NULL) { *output = strdup(delim+1); - // chop - char *newline = strchr(*output, '\n'); - *newline = '\0'; + chop(output); } ret = 0; } else if (http_code != -1) {