Added the purge API on node endpoints #3447
Conversation
nomad/node_endpoint.go
Outdated
| @@ -232,7 +246,6 @@ func (n *Node) Deregister(args *structs.NodeDeregisterRequest, reply *structs.No | |||
| } | |||
|
|
|||
| // Determine if there are any Vault accessors on the node | |||
| ws := memdb.NewWatchSet() | |||
| accessors, err := n.srv.State().VaultAccessorsByNode(ws, args.NodeID) | |||
There was a problem hiding this comment.
@dadgar Can we use the snap we created earlier?
There was a problem hiding this comment.
Yeah you can switch it over
nomad/node_endpoint.go
Outdated
| @@ -232,7 +246,6 @@ func (n *Node) Deregister(args *structs.NodeDeregisterRequest, reply *structs.No | |||
| } | |||
|
|
|||
| // Determine if there are any Vault accessors on the node | |||
| ws := memdb.NewWatchSet() | |||
| accessors, err := n.srv.State().VaultAccessorsByNode(ws, args.NodeID) | |||
There was a problem hiding this comment.
Yeah you can switch it over
| @@ -213,6 +213,20 @@ func (n *Node) Deregister(args *structs.NodeDeregisterRequest, reply *structs.No | |||
| if args.NodeID == "" { | |||
| return fmt.Errorf("missing node ID for client deregistration") | |||
| } | |||
| // Look for the node | |||
There was a problem hiding this comment.
Before the // Verify the arguments section can you add:
// Check node write permissions
if aclObj, err := n.srv.ResolveToken(args.AuthToken); err != nil {
return err
} else if aclObj != nil && !aclObj.AllowNodeWrite() {
return structs.ErrPermissionDenied
}
There was a problem hiding this comment.
We will need to add a test similar to this: https://github.com/hashicorp/nomad/blob/master/nomad/node_endpoint_test.go#L613
What you need to test is that an ACL with node write/management token work and that another token does not.
| @@ -42,6 +42,9 @@ func (s *HTTPServer) NodeSpecificRequest(resp http.ResponseWriter, req *http.Req | |||
| case strings.HasSuffix(path, "/drain"): | |||
| nodeName := strings.TrimSuffix(path, "/drain") | |||
| return s.nodeToggleDrain(resp, req, nodeName) | |||
| case strings.HasSuffix(path, "/purge"): | |||
|
Addressed all the comments. |
| @@ -142,3 +145,19 @@ func (s *HTTPServer) nodeQuery(resp http.ResponseWriter, req *http.Request, | |||
| } | |||
| return out.Node, nil | |||
| } | |||
|
|
|||
| func (s *HTTPServer) nodePurge(resp http.ResponseWriter, req *http.Request, nodeID string) (interface{}, error) { | |||
| if req.Method != "POST" { | |||
There was a problem hiding this comment.
I am going to merge and also allow PUT
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #3444
Tested by creating a client and calling the purge API. The server removed the client, however, the client could register if it was still alive which I believe is the expected behavior.
I will add the http api docs on the website once the PR is merged.