Skip to content

Commit

Permalink
Version the API using an HTTP header (#247)
Browse files Browse the repository at this point in the history
Signed-off-by: Dean Roehrich <[email protected]>
  • Loading branch information
roehrich-hpe authored Dec 12, 2024
1 parent b19f7ab commit 51989cc
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 39 deletions.
2 changes: 1 addition & 1 deletion daemons/copy-offload-testing/e2e-mocked.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ echo "Waiting for daemon to start"
while : ; do
sleep 1
# shellcheck disable=SC2086
if curl $CURLCERTS "$PROTO://$SRVR/hello"; then
if curl -H "Accepts-version: 1.0" $CURLCERTS "$PROTO://$SRVR/hello"; then
break
fi
done
Expand Down
41 changes: 36 additions & 5 deletions daemons/copy-offload/pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,42 @@ type UserHttp struct {
Mock bool
}

func validateVersion(w http.ResponseWriter, req *http.Request) string {
// See COPY_OFFLOAD_API_VERSION in copy-offload.h.
// 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" {
// The RFC says Not Accceptable should return a list of valid versions.
http.Error(w, "Valid versions: 1.0", http.StatusNotAcceptable)
return ""
}
return apiVersion
}

func (user *UserHttp) Hello(w http.ResponseWriter, req *http.Request) {
var apiVersion string
if req.Method != "GET" {
http.Error(w, "method not supported", http.StatusNotImplemented)
return
}
if apiVersion = validateVersion(w, req); apiVersion == "" {
return
}

user.Log.Info("Hello")
fmt.Fprintf(w, "hello back at ya\n")
}

func (user *UserHttp) ListRequests(w http.ResponseWriter, req *http.Request) {

var apiVersion string
if req.Method != "GET" {
http.Error(w, "method not supported", http.StatusNotImplemented)
return
}
if apiVersion = validateVersion(w, req); apiVersion == "" {
return
}

drvrReq := driver.DriverRequest{Drvr: user.Drvr}
items, err := drvrReq.ListRequests(context.TODO())
Expand All @@ -64,12 +89,15 @@ func (user *UserHttp) ListRequests(w http.ResponseWriter, req *http.Request) {
}

func (user *UserHttp) CancelRequest(w http.ResponseWriter, req *http.Request) {

var apiVersion string
if req.Method != "DELETE" {
http.Error(w, "method not supported", http.StatusNotImplemented)
return
}
user.Log.Info("In DELETE", "url", req.URL)
if apiVersion = validateVersion(w, req); apiVersion == "" {
return
}
user.Log.Info("In DELETE", "version", apiVersion, "url", req.URL)
urlParts, err := url.Parse(req.URL.String())
if err != nil {
http.Error(w, "unable to parse URL", http.StatusBadRequest)
Expand All @@ -86,12 +114,15 @@ func (user *UserHttp) CancelRequest(w http.ResponseWriter, req *http.Request) {
}

func (user *UserHttp) TrialRequest(w http.ResponseWriter, req *http.Request) {

var apiVersion string
if req.Method != "POST" {
http.Error(w, "method not supported", http.StatusNotImplemented)
return
}
user.Log.Info("In TrialRequest", "url", req.URL)
if apiVersion = validateVersion(w, req); apiVersion == "" {
return
}
user.Log.Info("In TrialRequest", "version", apiVersion, "url", req.URL)

var dmreq driver.DMRequest
if err := json.NewDecoder(req.Body).Decode(&dmreq); err != nil {
Expand Down
10 changes: 8 additions & 2 deletions daemons/copy-offload/pkg/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func setupLog() logr.Logger {
func TestA_Hello(t *testing.T) {
t.Run("returns hello response", func(t *testing.T) {
request, _ := http.NewRequest(http.MethodGet, "/hello", nil)
request.Header.Set("Accepts-version", "1.0")
response := httptest.NewRecorder()

httpHandler := &UserHttp{Log: setupLog()}
Expand Down Expand Up @@ -104,6 +105,7 @@ func TestB_ListRequests(t *testing.T) {
for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
request, _ := http.NewRequest(test.method, "/list", nil)
request.Header.Set("Accepts-version", "1.0")
response := httptest.NewRecorder()

httpHandler.ListRequests(response, request)
Expand Down Expand Up @@ -155,6 +157,7 @@ func TestC_CancelRequest(t *testing.T) {
for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
request, _ := http.NewRequest(test.method, "/cancel/nnf-copy-offload-node-9ae2a136-4", nil)
request.Header.Set("Accepts-version", "1.0")
response := httptest.NewRecorder()

httpHandler.CancelRequest(response, request)
Expand Down Expand Up @@ -219,7 +222,7 @@ func TestD_TrialRequest(t *testing.T) {
readerBody = bytes.NewReader(test.body)
}
request, _ := http.NewRequest(test.method, "/trial", readerBody)

request.Header.Set("Accepts-version", "1.0")
response := httptest.NewRecorder()

httpHandler.TrialRequest(response, request)
Expand Down Expand Up @@ -282,7 +285,7 @@ func TestE_Lifecycle(t *testing.T) {
readerBody = bytes.NewReader(test.body)
}
request, _ := http.NewRequest(test.method, "/trial", readerBody)

request.Header.Set("Accepts-version", "1.0")
response := httptest.NewRecorder()

httpHandler.TrialRequest(response, request)
Expand All @@ -306,6 +309,7 @@ func TestE_Lifecycle(t *testing.T) {
stringWanted := strings.Join(listWanted, ",")
t.Run("list all jobs", func(t *testing.T) {
request, _ := http.NewRequest(http.MethodGet, "/list", nil)
request.Header.Set("Accepts-version", "1.0")
response := httptest.NewRecorder()

httpHandler.ListRequests(response, request)
Expand All @@ -327,6 +331,7 @@ func TestE_Lifecycle(t *testing.T) {
// get a null pointer reference in CancelRequest(), where 'req' will
// be null.
request, _ := http.NewRequest(http.MethodDelete, "/cancel/nnf-copy-offload-node-0", nil)
request.Header.Set("Accepts-version", "1.0")
response := httptest.NewRecorder()

httpHandler.CancelRequest(response, request)
Expand All @@ -345,6 +350,7 @@ func TestE_Lifecycle(t *testing.T) {
stringWanted = strings.Join(listWanted[1:], ",")
t.Run("list remaining jobs", func(t *testing.T) {
request, _ := http.NewRequest(http.MethodGet, "/list", nil)
request.Header.Set("Accepts-version", "1.0")
response := httptest.NewRecorder()

httpHandler.ListRequests(response, request)
Expand Down
37 changes: 22 additions & 15 deletions daemons/lib-copy-offload/copy-offload.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ void copy_offload_configure(COPY_OFFLOAD *offload, char **host_and_port, int ski
offload->clientcert = clientcert;
strncpy(offload->proto, "http", sizeof(offload->proto));

struct curl_slist *chunk = NULL;
chunk = curl_slist_append(chunk, COPY_OFFLOAD_API_VERSION);
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, chunk);

curl_easy_setopt(curl, CURLOPT_TRANSFERTEXT, 1L);
if (!skip_tls) {
strncpy(offload->proto, "https", sizeof(offload->proto));
Expand Down Expand Up @@ -167,21 +171,20 @@ int copy_offload_list(COPY_OFFLOAD *offload, char **output) {
curl_easy_setopt(offload->curl, CURLOPT_URL, urlbuf);

http_code = copy_offload_perform(offload, &chunk);
if (http_code == 200) {
if (chunk.response != NULL) {
*output = strdup(chunk.response);
// chop
char *newline = strchr(*output, '\n');
*newline = '\0';
free(chunk.response);
}
if (http_code == 200)
ret = 0;
if (chunk.response != NULL) {
*output = strdup(chunk.response);
// chop
char *newline = strchr(*output, '\n');
*newline = '\0';
free(chunk.response);
}
return ret;
}

/* Cancel a specific copy-offload request. */
int copy_offload_cancel(COPY_OFFLOAD *offload, char *job_name) {
int copy_offload_cancel(COPY_OFFLOAD *offload, char *job_name, char **output) {
long http_code;
struct memory chunk = {NULL, 0};
int ret = 1;
Expand All @@ -192,18 +195,22 @@ int copy_offload_cancel(COPY_OFFLOAD *offload, char *job_name) {
curl_easy_setopt(offload->curl, CURLOPT_CUSTOMREQUEST, "DELETE");

http_code = copy_offload_perform(offload, &chunk);
if (http_code == 204) {
if (http_code == 204)
ret = 0;
} else if (chunk.response != NULL) {
if (chunk.response != NULL) {
*output = strdup(chunk.response);
// chop
char *newline = strchr(*output, '\n');
*newline = '\0';
free(chunk.response);
}
return ret;
}

/* Submit a new copy-offload request.
* The caller is responsible for calling free() on @job_name if *job_name is non-NULL.
* The caller is responsible for calling free() on @output if *output is non-NULL.
*/
int copy_offload_copy(COPY_OFFLOAD *offload, char *compute_name, char *workflow_name, char *source_path, char *dest_path, char **job_name) {
int copy_offload_copy(COPY_OFFLOAD *offload, char *compute_name, char *workflow_name, char *source_path, char *dest_path, char **output) {
long http_code;
struct memory chunk = {NULL, 0};
int ret = 1;
Expand All @@ -222,9 +229,9 @@ int copy_offload_copy(COPY_OFFLOAD *offload, char *compute_name, char *workflow_
if (http_code == 200) {
char *delim = strchr(chunk.response, '=');
if (delim != NULL) {
*job_name = strdup(delim+1);
*output = strdup(delim+1);
// chop
char *newline = strchr(*job_name, '\n');
char *newline = strchr(*output, '\n');
*newline = '\0';
}
ret = 0;
Expand Down
15 changes: 11 additions & 4 deletions daemons/lib-copy-offload/copy-offload.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@

#include <curl/curl.h>

// Define the API version. This applies to the request format sent by the client
// as well as the response format sent by the server.
// See validateVersion() in the server.
#define COPY_OFFLOAD_API_VERSION "Accepts-version: 1.0"

#define COPY_OFFLOAD_MSG_SIZE CURL_ERROR_SIZE * 2

struct copy_offload_s {
Expand Down Expand Up @@ -67,18 +72,20 @@ void copy_offload_verbose(COPY_OFFLOAD *offload);
int copy_offload_list(COPY_OFFLOAD *offload, char **output);

/* Cancel a specific copy-offload request.
* Any output from the server, if present, will be placed in @output. The caller
* is responsible for calling free() on @output if *output is non-NULL.
* Returns 0 on success.
* On failure it returns 1 and places an error message in @offload->err_message.
*/
int copy_offload_cancel(COPY_OFFLOAD *offload, char *job_name);
int copy_offload_cancel(COPY_OFFLOAD *offload, char *job_name, char **output);

/* Submit a new copy-offload request.
* The new job's name will be placed in @job_name. The caller is responsible
* for calling free() on @job_name if *job_name is non-NULL.
* The new job's name will be placed in @output. The caller is responsible
* for calling free() on @output if *output is non-NULL.
* Returns 0 on success.
* On failure it returns 1 and places an error message in @offload->err_message.
*/
int copy_offload_copy(COPY_OFFLOAD *offload, char *compute_name, char *workflow_name, char *source_path, char *dest_path, char **job_name);
int copy_offload_copy(COPY_OFFLOAD *offload, char *compute_name, char *workflow_name, char *source_path, char *dest_path, char **output);

/* Clean up the handle's resources. */
void copy_offload_cleanup(COPY_OFFLOAD *offload);
22 changes: 10 additions & 12 deletions daemons/lib-copy-offload/test-tool/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,28 +150,26 @@ int main(int argc, const char **argv) {
copy_offload_verbose(offload);
}

char *output = NULL;

if (l_opt) {
char *output = NULL;
ret = copy_offload_list(offload, &output);
if (output != NULL) {
printf("%s\n", output);
free(output);
}
} else if (c_opt) {
ret = copy_offload_cancel(offload, job_name);
ret = copy_offload_cancel(offload, job_name, &output);
} else if (o_opt) {
char *job_name = NULL;
ret = copy_offload_copy(offload, compute_name, workflow_name, source_path, dest_path, &job_name);
if (job_name != NULL) {
printf("%s\n", job_name);
free(job_name);
}
ret = copy_offload_copy(offload, compute_name, workflow_name, source_path, dest_path, &output);
} else {
fprintf(stderr, "What action?\n");
copy_offload_cleanup(offload);
exit(1);
}

if (output != NULL) {
printf("%s\n", output);
free(output);
output = NULL;
}

if (verbose) {
printf("ret %d, http_code %ld\n", ret, offload->http_code);
}
Expand Down

0 comments on commit 51989cc

Please sign in to comment.