Restore idempotency of PUT /api/nodes/{id}
HTTP PUT is supposed to be idempotent, so PUT for an existing node or profile should replace the node or profile, not throw an error. Generating an error on duplicate can now be requested using the If-None-Match header with a `*` value. Signed-off-by: Jonathon Anderson <janderson@ciq.com>
This commit is contained in:
@@ -9,6 +9,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
|
||||
### Added
|
||||
|
||||
- Added system and runtime overlay built time to REST API.
|
||||
- Support If-None-Match header in `PUT /api/nodes/{id}`
|
||||
|
||||
### Changed
|
||||
|
||||
- Restore default idempotency of `PUT /api/nodes/{id}`
|
||||
|
||||
### Changed
|
||||
|
||||
|
||||
@@ -170,18 +170,20 @@ func getNodeFields() usecase.Interactor {
|
||||
|
||||
func addNode() usecase.Interactor {
|
||||
type addNodeInput struct {
|
||||
ID string `path:"id" required:"true" description:"ID of node to be added"`
|
||||
Node node.Node `json:"node" required:"true" description:"Field values in JSON format for added node"`
|
||||
ID string `path:"id" required:"true" description:"ID of node to be added"`
|
||||
Node node.Node `json:"node" required:"true" description:"Field values in JSON format for added node"`
|
||||
IfNoneMatch string `header:"If-None-Match" description:"Set to '*' to indicate that the node should only be created if it does not already exist"`
|
||||
}
|
||||
|
||||
u := usecase.NewInteractor(func(ctx context.Context, input addNodeInput, output *node.Node) error {
|
||||
wwlog.Debug("api.addNode(ID:%v, Node:%+v)", input.ID, input.Node)
|
||||
// registry is the warewulf node "db" yaml file.
|
||||
if registry, err := node.New(); err != nil {
|
||||
return err
|
||||
} else {
|
||||
if _, ok := registry.Nodes[input.ID]; ok {
|
||||
return status.Wrap(fmt.Errorf("node name '%s' already exists", input.ID), status.InvalidArgument)
|
||||
if input.IfNoneMatch == "*" {
|
||||
if _, ok := registry.Nodes[input.ID]; ok {
|
||||
return status.Wrap(fmt.Errorf("node '%s' already exists", input.ID), status.InvalidArgument)
|
||||
}
|
||||
}
|
||||
for _, profile := range input.Node.Profiles {
|
||||
if _, ok := registry.NodeProfiles[profile]; !ok {
|
||||
|
||||
@@ -74,6 +74,60 @@ func TestNodeAPI(t *testing.T) {
|
||||
assert.JSONEq(t, `{"node1": {}, "test": {"system overlay": ["so1"], "runtime overlay": ["ro1"], "kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}}`, string(body))
|
||||
})
|
||||
|
||||
t.Run("test idempotency (put same node again)", func(t *testing.T) {
|
||||
// prepareration
|
||||
|
||||
testNode := `{
|
||||
"node":{
|
||||
"system overlay": ["so1"],
|
||||
"runtime overlay": ["ro1"],
|
||||
"kernel": {
|
||||
"version": "v1.0.0",
|
||||
"args": ["kernel-args"]
|
||||
}
|
||||
}
|
||||
}`
|
||||
req, err := http.NewRequest(http.MethodPut, srv.URL+"/api/nodes/test", bytes.NewBuffer([]byte(testNode)))
|
||||
assert.NoError(t, err)
|
||||
|
||||
resp, err := http.DefaultTransport.RoundTrip(req)
|
||||
assert.NoError(t, err)
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
assert.NoError(t, err)
|
||||
assert.NoError(t, resp.Body.Close())
|
||||
|
||||
assert.JSONEq(t, `{"system overlay": ["so1"], "runtime overlay": ["ro1"], "kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}`, string(body))
|
||||
})
|
||||
|
||||
t.Run("fail if node already exists (given appropriate header)", func(t *testing.T) {
|
||||
// prepareration
|
||||
|
||||
testNode := `{
|
||||
"node":{
|
||||
"system overlay": ["so1"],
|
||||
"runtime overlay": ["ro1"],
|
||||
"kernel": {
|
||||
"version": "v1.0.0",
|
||||
"args": ["kernel-args"]
|
||||
}
|
||||
}
|
||||
}`
|
||||
req, err := http.NewRequest(http.MethodPut, srv.URL+"/api/nodes/test", bytes.NewBuffer([]byte(testNode)))
|
||||
assert.NoError(t, err)
|
||||
req.Header.Set("If-None-Match", "*")
|
||||
|
||||
resp, err := http.DefaultTransport.RoundTrip(req)
|
||||
assert.NoError(t, err)
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
assert.NoError(t, err)
|
||||
assert.NoError(t, resp.Body.Close())
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
|
||||
assert.Contains(t, string(body), "node 'test' already exists")
|
||||
})
|
||||
|
||||
t.Run("get one specific node", func(t *testing.T) {
|
||||
req, err := http.NewRequest(http.MethodGet, srv.URL+"/api/nodes/test", nil)
|
||||
assert.NoError(t, err)
|
||||
@@ -228,6 +282,32 @@ func TestNodeAPI(t *testing.T) {
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("replace a node", func(t *testing.T) {
|
||||
// prepareration
|
||||
|
||||
testNode := `{
|
||||
"node":{
|
||||
"system overlay": ["so1"],
|
||||
"runtime overlay": ["ro1"],
|
||||
"kernel": {
|
||||
"version": "v1.0.0",
|
||||
"args": ["kernel-args"]
|
||||
}
|
||||
}
|
||||
}`
|
||||
req, err := http.NewRequest(http.MethodPut, srv.URL+"/api/nodes/test", bytes.NewBuffer([]byte(testNode)))
|
||||
assert.NoError(t, err)
|
||||
|
||||
resp, err := http.DefaultTransport.RoundTrip(req)
|
||||
assert.NoError(t, err)
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
assert.NoError(t, err)
|
||||
assert.NoError(t, resp.Body.Close())
|
||||
|
||||
assert.JSONEq(t, `{"system overlay": ["so1"], "runtime overlay": ["ro1"], "kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}`, string(body))
|
||||
})
|
||||
|
||||
t.Run("test delete nodes", func(t *testing.T) {
|
||||
req, err := http.NewRequest(http.MethodDelete, srv.URL+"/api/nodes/test", nil)
|
||||
assert.NoError(t, err)
|
||||
@@ -239,6 +319,6 @@ func TestNodeAPI(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
assert.NoError(t, resp.Body.Close())
|
||||
|
||||
assert.JSONEq(t, `{"system overlay": ["so1"], "runtime overlay": ["ro1"], "kernel": {"version": "v1.0.1-newversion", "args": ["kernel-args"]}}`, string(body))
|
||||
assert.JSONEq(t, `{"system overlay": ["so1"], "runtime overlay": ["ro1"], "kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}`, string(body))
|
||||
})
|
||||
}
|
||||
|
||||
@@ -57,8 +57,9 @@ func getProfileByID() usecase.Interactor {
|
||||
|
||||
func addProfile() usecase.Interactor {
|
||||
type addProfileInput struct {
|
||||
ID string `path:"id" required:"true" description:"ID of profile to add"`
|
||||
Profile node.Profile `json:"profile" required:"true" description:"Field values in JSON format for added profile"`
|
||||
ID string `path:"id" required:"true" description:"ID of profile to add"`
|
||||
Profile node.Profile `json:"profile" required:"true" description:"Field values in JSON format for added profile"`
|
||||
IfNoneMatch string `header:"If-None-Match" description:"Set to '*' to indicate that the profile should only be created if it does not already exist"`
|
||||
}
|
||||
|
||||
u := usecase.NewInteractor(func(ctx context.Context, input addProfileInput, output *node.Profile) error {
|
||||
@@ -66,6 +67,11 @@ func addProfile() usecase.Interactor {
|
||||
if registry, err := node.New(); err != nil {
|
||||
return err
|
||||
} else {
|
||||
if input.IfNoneMatch == "*" {
|
||||
if _, ok := registry.NodeProfiles[input.ID]; ok {
|
||||
return status.Wrap(fmt.Errorf("profile '%s' already exists", input.ID), status.InvalidArgument)
|
||||
}
|
||||
}
|
||||
for _, profile := range input.Profile.Profiles {
|
||||
if _, ok := registry.NodeProfiles[profile]; !ok {
|
||||
return status.Wrap(fmt.Errorf("profile '%s' does not exist", profile), status.InvalidArgument)
|
||||
|
||||
@@ -54,6 +54,37 @@ func TestProfileAPI(t *testing.T) {
|
||||
assert.JSONEq(t, `{"kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}`, string(body))
|
||||
})
|
||||
|
||||
t.Run("test idempotency", func(t *testing.T) {
|
||||
testProfile := `{"profile": {"kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}}`
|
||||
req, err := http.NewRequest(http.MethodPut, srv.URL+"/api/profiles/test", bytes.NewBuffer([]byte(testProfile)))
|
||||
assert.NoError(t, err)
|
||||
|
||||
resp, err := http.DefaultTransport.RoundTrip(req)
|
||||
assert.NoError(t, err)
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
assert.NoError(t, resp.Body.Close())
|
||||
assert.NoError(t, err)
|
||||
assert.JSONEq(t, `{"kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}`, string(body))
|
||||
})
|
||||
|
||||
t.Run("fail if profile already exists (given appropriate header)", func(t *testing.T) {
|
||||
testProfile := `{"profile": {"kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}}`
|
||||
req, err := http.NewRequest(http.MethodPut, srv.URL+"/api/profiles/test", bytes.NewBuffer([]byte(testProfile)))
|
||||
assert.NoError(t, err)
|
||||
req.Header.Set("If-None-Match", "*")
|
||||
|
||||
resp, err := http.DefaultTransport.RoundTrip(req)
|
||||
assert.NoError(t, err)
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
assert.NoError(t, resp.Body.Close())
|
||||
assert.NoError(t, err)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
|
||||
assert.Contains(t, string(body), "profile 'test' already exists")
|
||||
})
|
||||
|
||||
t.Run("re-read all profiles", func(t *testing.T) {
|
||||
req, err := http.NewRequest(http.MethodGet, srv.URL+"/api/profiles", nil)
|
||||
assert.NoError(t, err)
|
||||
@@ -107,6 +138,20 @@ func TestProfileAPI(t *testing.T) {
|
||||
assert.JSONEq(t, `{"kernel": {"version": "v1.0.1-newversion", "args": ["kernel-args"]}}`, string(body))
|
||||
})
|
||||
|
||||
t.Run("test overwrite", func(t *testing.T) {
|
||||
testProfile := `{"profile": {"kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}}`
|
||||
req, err := http.NewRequest(http.MethodPut, srv.URL+"/api/profiles/test", bytes.NewBuffer([]byte(testProfile)))
|
||||
assert.NoError(t, err)
|
||||
|
||||
resp, err := http.DefaultTransport.RoundTrip(req)
|
||||
assert.NoError(t, err)
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
assert.NoError(t, resp.Body.Close())
|
||||
assert.NoError(t, err)
|
||||
assert.JSONEq(t, `{"kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}`, string(body))
|
||||
})
|
||||
|
||||
t.Run("test delete a profile", func(t *testing.T) {
|
||||
req, err := http.NewRequest(http.MethodDelete, srv.URL+"/api/profiles/test", nil)
|
||||
assert.NoError(t, err)
|
||||
@@ -117,6 +162,6 @@ func TestProfileAPI(t *testing.T) {
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
assert.NoError(t, resp.Body.Close())
|
||||
assert.NoError(t, err)
|
||||
assert.JSONEq(t, `{"kernel": {"version": "v1.0.1-newversion", "args": ["kernel-args"]}}`, string(body))
|
||||
assert.JSONEq(t, `{"kernel": {"version": "v1.0.0", "args": ["kernel-args"]}}`, string(body))
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user