Add negative API vesion tests for server (#248)

Signed-off-by: Dean Roehrich <[email protected]>

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 ""

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() {})

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) {