Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validation for xnames in BootparametersPost and enhance tests #52

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cmd/boot-script-service/default_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,16 @@ func BootparametersPost(w http.ResponseWriter, r *http.Request) {
fmt.Sprintf("Bad Request: %s", err))
return
}
// Check that the xnames are valid
err = args.CheckXnames()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(No change requested here, just commenting for posterity)

See comment in pkg/bssTypes/types.go on line 61. When a solution comes for the mentioned issue, this may change to be conditional in the future.

if err != nil {
// Invalid xname format (if included), invalid request
LogBootParameters(fmt.Sprintf("/bootparameters POST FAILED: %s", err.Error()), args)
base.SendProblemDetailsGeneric(w, http.StatusBadRequest,
fmt.Sprintf("Bad Request: %s", err))
return
}
// Fields appear to be correct. Continue with processing.
debugf("Received boot parameters: %v\n", args)
err, referralToken := StoreNew(args)
if err == nil {
Expand Down
132 changes: 132 additions & 0 deletions cmd/boot-script-service/default_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,15 @@
package main

import (
"bytes"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"regexp"
"testing"

"github.com/OpenCHAMI/bss/pkg/bssTypes"
)

func mockGetSignedS3Url(s3Url string) (string, error) {
Expand Down Expand Up @@ -179,3 +185,129 @@ func TestReplaceS3Params_error(t *testing.T) {
t.Errorf("replaceS3Params failed.\n expected: %s\n actual: %s\n", expected_params, newParams)
}
}

func TestBootparametersPost_Success(t *testing.T) {

args := bssTypes.BootParams{
Macs: []string{"00:11:22:33:44:55"},
Hosts: []string{"x1c1s1b1n1"},
Nids: []int32{1},
Kernel: "kernel",
Initrd: "initrd",
Params: "params",
}

body, _ := json.Marshal(args)
req, err := http.NewRequest("POST", "/bootparameters", bytes.NewBuffer(body))
if err != nil {
t.Fatal(err)
}

rr := httptest.NewRecorder()
handler := http.HandlerFunc(BootparametersPost)
handler.ServeHTTP(rr, req)

if status := rr.Code; status != http.StatusCreated {
t.Errorf("handler returned wrong status code: got %v want %v",
status, http.StatusCreated)
}

}

func TestBootparametersPost_BadRequest(t *testing.T) {

req, err := http.NewRequest("POST", "/bootparameters", bytes.NewBuffer([]byte("invalid body")))
if err != nil {
t.Fatal(err)
}

rr := httptest.NewRecorder()
handler := http.HandlerFunc(BootparametersPost)
handler.ServeHTTP(rr, req)

if status := rr.Code; status != http.StatusBadRequest {
t.Errorf("handler returned wrong status code: got %v want %v",
status, http.StatusBadRequest)
}
}

func TestBootparametersPost_InvalidMac(t *testing.T) {

args := bssTypes.BootParams{
Macs: []string{"invalid-mac"},
Hosts: []string{"x1c1s1b1n1"},
Nids: []int32{1},
Kernel: "kernel",
Initrd: "initrd",
Params: "params",
}

body, _ := json.Marshal(args)
req, err := http.NewRequest("POST", "/bootparameters", bytes.NewBuffer(body))
if err != nil {
t.Fatal(err)
}

rr := httptest.NewRecorder()
handler := http.HandlerFunc(BootparametersPost)
handler.ServeHTTP(rr, req)

if status := rr.Code; status != http.StatusBadRequest {
t.Errorf("handler returned wrong status code: got %v want %v",
status, http.StatusBadRequest)
}
}

func TestBootparametersPost_InvalidXname(t *testing.T) {

args := bssTypes.BootParams{
Macs: []string{"00:11:22:33:44:55"},
Hosts: []string{"invalid-xname"},
Nids: []int32{1},
Kernel: "kernel",
Initrd: "initrd",
Params: "params",
}

body, _ := json.Marshal(args)
req, err := http.NewRequest("POST", "/bootparameters", bytes.NewBuffer(body))
if err != nil {
t.Fatal(err)
}

rr := httptest.NewRecorder()
handler := http.HandlerFunc(BootparametersPost)
handler.ServeHTTP(rr, req)

if status := rr.Code; status != http.StatusBadRequest {
t.Errorf("handler returned wrong status code: got %v want %v",
status, http.StatusBadRequest)
}
}

func TestBootparametersPost_StoreError(t *testing.T) {

args := bssTypes.BootParams{
Macs: []string{"00:11:22:33:44:55"},
Hosts: []string{"xname1"},
Nids: []int32{1},
Kernel: "kernel",
Initrd: "initrd",
Params: "params",
}

body, _ := json.Marshal(args)
req, err := http.NewRequest("POST", "/bootparameters", bytes.NewBuffer(body))
if err != nil {
t.Fatal(err)
}

rr := httptest.NewRecorder()
handler := http.HandlerFunc(BootparametersPost)
handler.ServeHTTP(rr, req)

if status := rr.Code; status != http.StatusBadRequest {
t.Errorf("handler returned wrong status code: got %v want %v",
status, http.StatusBadRequest)
}
}
19 changes: 18 additions & 1 deletion pkg/bssTypes/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ package bssTypes
import (
"fmt"
"regexp"

"github.com/Cray-HPE/hms-xname/xnames"
)

type PhoneHome struct {
Expand Down Expand Up @@ -56,7 +58,7 @@ type CloudInit struct {
// provide a "default" selection which provides a way to supply default
// parameters for any node which is not explicitly configured.
type BootParams struct {
Hosts []string `json:"hosts,omitempty"`
Hosts []string `json:"hosts,omitempty"` // This list of hosts must be xnames
Comment on lines -59 to +61
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(No change requested here, just commenting for posterity)

Unfortunately, it looks like there may have been premature optimization here as this field (according to the comment above) is supposed to support xnames and "special names" (i.e. groups). However, the current BSS does not support boot groups with Postgres (yet). This may change with a solution to #50. However for now, this is fine as-is.

Macs []string `json:"macs,omitempty"`
Nids []int32 `json:"nids,omitempty"`
Params string `json:"params,omitempty"`
Expand All @@ -65,6 +67,7 @@ type BootParams struct {
CloudInit CloudInit `json:"cloud-init,omitempty"`
}

// Validate the MACs in the boot parameters
func (bp BootParams) CheckMacs() (err error) {
if len(bp.Macs) > 0 {
re := regexp.MustCompile(`^([0-9A-Fa-f]{2}:){5}[0-9a-fA-F]{2}$`)
Expand All @@ -80,6 +83,20 @@ func (bp BootParams) CheckMacs() (err error) {
return
}

// Validate the xnames in the boot parameters. They must be of type "Node"
func (bp BootParams) CheckXnames() (err error) {
for _, xname := range bp.Hosts {
myXname := xnames.FromString(xname)
if myXname == nil {
return fmt.Errorf("invalid xname: %s", xname)
}
if myXname.Type() != "Node" {
return fmt.Errorf("invalid xname type: %s", myXname.Type())
}
}
return nil
}

// The following structures and types all related to the last access information for bootscripts and cloud-init data.

type EndpointType string
Expand Down
Loading