diff --git a/api/machines.go b/api/machines.go index 990d7d80c..6de8024f7 100644 --- a/api/machines.go +++ b/api/machines.go @@ -15,9 +15,11 @@ package api import ( - "fmt" + "encoding/json" + "errors" "net/http" "path" + "regexp" "github.com/coreos/fleet/client" "github.com/coreos/fleet/log" @@ -25,6 +27,10 @@ import ( "github.com/coreos/fleet/schema" ) +var ( + metadataPathRegex = regexp.MustCompile("^/([^/]+)/metadata/([A-Za-z0-9_.-]+$)") +) + func wireUpMachinesResource(mux *http.ServeMux, prefix string, cAPI client.API) { res := path.Join(prefix, "machines") mr := machinesResource{cAPI} @@ -35,12 +41,24 @@ type machinesResource struct { cAPI client.API } +type machineMetadataOp struct { + Operation string `json:"op"` + Path string + Value string +} + func (mr *machinesResource) ServeHTTP(rw http.ResponseWriter, req *http.Request) { - if req.Method != "GET" { - sendError(rw, http.StatusBadRequest, fmt.Errorf("only HTTP GET supported against this resource")) - return + switch req.Method { + case "GET": + mr.list(rw, req) + case "PATCH": + mr.patch(rw, req) + default: + sendError(rw, http.StatusMethodNotAllowed, errors.New("only GET and PATCH supported against this resource")) } +} +func (mr *machinesResource) list(rw http.ResponseWriter, req *http.Request) { token, err := findNextPageToken(req.URL) if err != nil { sendError(rw, http.StatusBadRequest, err) @@ -62,6 +80,54 @@ func (mr *machinesResource) ServeHTTP(rw http.ResponseWriter, req *http.Request) sendResponse(rw, http.StatusOK, page) } +func (mr *machinesResource) patch(rw http.ResponseWriter, req *http.Request) { + ops := make([]machineMetadataOp, 0) + dec := json.NewDecoder(req.Body) + if err := dec.Decode(&ops); err != nil { + sendError(rw, http.StatusBadRequest, err) + return + } + + for _, op := range ops { + if op.Operation != "add" && op.Operation != "remove" && op.Operation != "replace" { + sendError(rw, http.StatusBadRequest, errors.New("invalid op: expect add, remove, or replace")) + return + } + + if metadataPathRegex.FindStringSubmatch(op.Path) == nil { + sendError(rw, http.StatusBadRequest, errors.New("machine metadata path invalid")) + return + } + + if op.Operation != "remove" && len(op.Value) == 0 { + sendError(rw, http.StatusBadRequest, errors.New("invalid value: add and replace require a value")) + return + } + } + + for _, op := range ops { + // regex already validated above + s := metadataPathRegex.FindStringSubmatch(op.Path) + machID := s[1] + key := s[2] + + if op.Operation == "remove" { + err := mr.cAPI.DeleteMachineMetadata(machID, key) + if err != nil { + sendError(rw, http.StatusInternalServerError, err) + return + } + } else { + err := mr.cAPI.SetMachineMetadata(machID, key, op.Value) + if err != nil { + sendError(rw, http.StatusInternalServerError, err) + return + } + } + } + sendResponse(rw, http.StatusNoContent, "") +} + func getMachinePage(cAPI client.API, tok PageToken) (*schema.MachinePage, error) { all, err := cAPI.Machines() if err != nil { diff --git a/api/machines_test.go b/api/machines_test.go index 969b5c590..674680371 100644 --- a/api/machines_test.go +++ b/api/machines_test.go @@ -19,6 +19,7 @@ import ( "net/http/httptest" "reflect" "strconv" + "strings" "testing" "github.com/coreos/fleet/client" @@ -26,16 +27,22 @@ import ( "github.com/coreos/fleet/registry" ) -func TestMachinesList(t *testing.T) { +func fakeMachinesSetup() (*machinesResource, *httptest.ResponseRecorder) { fr := registry.NewFakeRegistry() fr.SetMachines([]machine.MachineState{ - {ID: "XXX", PublicIP: "", Metadata: nil}, + {ID: "XXX", PublicIP: "", Metadata: map[string]string{}}, {ID: "YYY", PublicIP: "1.2.3.4", Metadata: map[string]string{"ping": "pong"}}, }) fAPI := &client.RegistryClient{Registry: fr} resource := &machinesResource{cAPI: fAPI} rw := httptest.NewRecorder() - req, err := http.NewRequest("GET", "http://example.com", nil) + + return resource, rw +} + +func TestMachinesList(t *testing.T) { + resource, rw := fakeMachinesSetup() + req, err := http.NewRequest("GET", "http://example.com/machines", nil) if err != nil { t.Fatalf("Failed creating http.Request: %v", err) } @@ -63,11 +70,23 @@ func TestMachinesList(t *testing.T) { } } +func TestMachinesListBadMethod(t *testing.T) { + resource, rw := fakeMachinesSetup() + req, err := http.NewRequest("POST", "http://example.com/machines", nil) + if err != nil { + t.Fatalf("Failed creating http.Request: %v", err) + } + + resource.ServeHTTP(rw, req) + + err = assertErrorResponse(rw, http.StatusMethodNotAllowed) + if err != nil { + t.Error(err.Error()) + } +} + func TestMachinesListBadNextPageToken(t *testing.T) { - fr := registry.NewFakeRegistry() - fAPI := &client.RegistryClient{Registry: fr} - resource := &machinesResource{fAPI} - rw := httptest.NewRecorder() + resource, rw := fakeMachinesSetup() req, err := http.NewRequest("GET", "http://example.com/machines?nextPageToken=EwBMLg==", nil) if err != nil { t.Fatalf("Failed creating http.Request: %v", err) @@ -136,3 +155,126 @@ func TestExtractMachinePage(t *testing.T) { } } } + +func TestMachinesPatchAddModify(t *testing.T) { + reqBody := ` + [{"op": "add", "path": "/XXX/metadata/foo", "value": "bar"}, + {"op": "replace", "path": "/YYY/metadata/ping", "value": "splat"}] + ` + + resource, rw := fakeMachinesSetup() + req, err := http.NewRequest("PATCH", "http://example.com/machines", strings.NewReader(reqBody)) + if err != nil { + t.Fatalf("Failed creating http.Request: %v", err) + } + + resource.ServeHTTP(rw, req) + if rw.Code != http.StatusNoContent { + t.Errorf("Expected 204, got %d", rw.Code) + } + + // fetch machine to make sure data has been added + req, err = http.NewRequest("GET", "http://example.com/machines", nil) + if err != nil { + t.Fatalf("Failed creating http.Request: %v", err) + } + rw.Body.Reset() + resource.ServeHTTP(rw, req) + + if rw.Body == nil { + t.Error("Received nil response body") + } else { + body := rw.Body.String() + expected := `{"machines":[{"id":"XXX","metadata":{"foo":"bar"}},{"id":"YYY","metadata":{"ping":"splat"},"primaryIP":"1.2.3.4"}]}` + if body != expected { + t.Errorf("Expected body:\n%s\n\nReceived body:\n%s\n", expected, body) + } + } +} + +func TestMachinesPatchDelete(t *testing.T) { + reqBody := ` + [{"op": "remove", "path": "/XXX/metadata/foo"}, + {"op": "remove", "path": "/YYY/metadata/ping"}] + ` + + resource, rw := fakeMachinesSetup() + req, err := http.NewRequest("PATCH", "http://example.com/machines", strings.NewReader(reqBody)) + if err != nil { + t.Fatalf("Failed creating http.Request: %v", err) + } + + resource.ServeHTTP(rw, req) + if rw.Code != http.StatusNoContent { + t.Errorf("Expected 204, got %d", rw.Code) + } + + // fetch machine to make sure data has been added + req, err = http.NewRequest("GET", "http://example.com/machines", nil) + if err != nil { + t.Fatalf("Failed creating http.Request: %v", err) + } + rw.Body.Reset() + resource.ServeHTTP(rw, req) + + if rw.Body == nil { + t.Error("Received nil response body") + } else { + body := rw.Body.String() + expected := `{"machines":[{"id":"XXX"},{"id":"YYY","primaryIP":"1.2.3.4"}]}` + if body != expected { + t.Errorf("Expected body:\n%s\n\nReceived body:\n%s\n", expected, body) + } + } +} + +func TestMachinesPatchBadOp(t *testing.T) { + reqBody := ` + [{"op": "noop", "path": "/XXX/metadata/foo", "value": "bar"}] + ` + + resource, rw := fakeMachinesSetup() + req, err := http.NewRequest("PATCH", "http://example.com/machines", strings.NewReader(reqBody)) + if err != nil { + t.Fatalf("Failed creating http.Request: %v", err) + } + + resource.ServeHTTP(rw, req) + if rw.Code != http.StatusBadRequest { + t.Errorf("Expected 400, got %d", rw.Code) + } +} + +func TestMachinesPatchBadPath(t *testing.T) { + reqBody := ` + [{"op": "add", "path": "/XXX/foo", "value": "bar"}] + ` + + resource, rw := fakeMachinesSetup() + req, err := http.NewRequest("PATCH", "http://example.com/machines", strings.NewReader(reqBody)) + if err != nil { + t.Fatalf("Failed creating http.Request: %v", err) + } + + resource.ServeHTTP(rw, req) + if rw.Code != http.StatusBadRequest { + t.Errorf("Expected 400, got %d", rw.Code) + } +} + +func TestMachinesPatchBadValue(t *testing.T) { + reqBody := ` + [{"op": "add", "path": "/XXX/foo"}] + ` + + resource, rw := fakeMachinesSetup() + req, err := http.NewRequest("PATCH", "http://example.com/machines", strings.NewReader(reqBody)) + if err != nil { + t.Fatalf("Failed creating http.Request: %v", err) + } + + resource.ServeHTTP(rw, req) + if rw.Code != http.StatusBadRequest { + t.Errorf("Expected 400, got %d", rw.Code) + } +} diff --git a/client/api.go b/client/api.go index 470007dfe..b3c15e2c5 100644 --- a/client/api.go +++ b/client/api.go @@ -21,6 +21,8 @@ import ( type API interface { Machines() ([]machine.MachineState, error) + SetMachineMetadata(machID string, key string, value string) error + DeleteMachineMetadata(machID string, key string) error Unit(string) (*schema.Unit, error) Units() ([]*schema.Unit, error) diff --git a/machine/state.go b/machine/state.go index 821f07524..2167da007 100644 --- a/machine/state.go +++ b/machine/state.go @@ -23,7 +23,7 @@ const ( type MachineState struct { ID string PublicIP string - Metadata map[string]string + Metadata map[string]string `json:"-"` Version string } diff --git a/registry/fake.go b/registry/fake.go index 2a6c8bc5a..e6613ae87 100644 --- a/registry/fake.go +++ b/registry/fake.go @@ -279,6 +279,24 @@ func (f *FakeRegistry) UnitHeartbeat(name, machID string, ttl time.Duration) err func (f *FakeRegistry) ClearUnitHeartbeat(string) {} +func (f *FakeRegistry) SetMachineMetadata(machID string, key string, value string) error { + for _, mach := range f.machines { + if mach.ID == machID { + mach.Metadata[key] = value + } + } + return nil +} + +func (f *FakeRegistry) DeleteMachineMetadata(machID string, key string) error { + for _, mach := range f.machines { + if mach.ID == machID { + delete(mach.Metadata, key) + } + } + return nil +} + func NewFakeClusterRegistry(dVersion *semver.Version, eVersion int) *FakeClusterRegistry { return &FakeClusterRegistry{ dVersion: dVersion, diff --git a/registry/interface.go b/registry/interface.go index 270a2ef9d..13b3da642 100644 --- a/registry/interface.go +++ b/registry/interface.go @@ -37,6 +37,8 @@ type Registry interface { SetUnitTargetState(name string, state job.JobState) error SetMachineState(ms machine.MachineState, ttl time.Duration) (uint64, error) UnscheduleUnit(name, machID string) error + SetMachineMetadata(machID, key, value string) error + DeleteMachineMetadata(machID, key string) error UnitRegistry } diff --git a/registry/machine.go b/registry/machine.go index 3ed8a1812..4bf416b2e 100644 --- a/registry/machine.go +++ b/registry/machine.go @@ -43,17 +43,28 @@ func (r *EtcdRegistry) Machines() (machines []machine.MachineState, err error) { } for _, node := range resp.Node.Nodes { - for _, obj := range node.Nodes { - if !strings.HasSuffix(obj.Key, "/object") { - continue - } + var mach machine.MachineState + + mach.ID = path.Base(node.Key) - var mach machine.MachineState - err = unmarshal(obj.Value, &mach) - if err != nil { - return + for _, obj := range node.Nodes { + // Load machine object + if strings.HasSuffix(obj.Key, "/object") { + err = unmarshal(obj.Value, &mach) + if err != nil { + return + } + } else if strings.HasSuffix(obj.Key, "/metadata") { + // Load metadata + mach.Metadata = make(map[string]string, len(obj.Nodes)) + for _, mdnode := range obj.Nodes { + mach.Metadata[path.Base(mdnode.Key)] = mdnode.Value + } } + } + // only use the machine if there was an object key + if len(mach.Version) > 0 { machines = append(machines, mach) } } @@ -91,9 +102,61 @@ func (r *EtcdRegistry) SetMachineState(ms machine.MachineState, ttl time.Duratio return uint64(0), err } + // Set the initial metadata on creation + if err = r.setMachineMetadata(ms.ID, ms.Metadata); err != nil { + return uint64(0), err + } + return resp.Node.ModifiedIndex, nil } +func (r *EtcdRegistry) SetMachineMetadata(machID string, key string, value string) error { + //Attempt to update key + update := etcd.Set{ + Key: path.Join(r.keyPrefix, machinePrefix, machID, "metadata", key), + Value: value, + } + + _, err := r.etcd.Do(&update) + return err +} + +func (r *EtcdRegistry) DeleteMachineMetadata(machID string, key string) error { + del := etcd.Delete{ + Key: path.Join(r.keyPrefix, machinePrefix, machID, "metadata", key), + } + + _, err := r.etcd.Do(&del) + if etcd.IsKeyNotFound(err) { + err = nil + } + return err +} + +// Remove and reset all metadata +func (r *EtcdRegistry) setMachineMetadata(machID string, metadata map[string]string) error { + del := etcd.Delete{ + Key: path.Join(r.keyPrefix, machinePrefix, machID, "metadata"), + Recursive: true, + } + + _, err := r.etcd.Do(&del) + if etcd.IsKeyNotFound(err) { + err = nil + } + + if err != nil { + return err + } + + for key, val := range metadata { + if err = r.SetMachineMetadata(machID, key, val); err != nil { + return err + } + } + return nil +} + func (r *EtcdRegistry) RemoveMachineState(machID string) error { req := etcd.Delete{ Key: path.Join(r.keyPrefix, machinePrefix, machID, "object"), @@ -102,5 +165,16 @@ func (r *EtcdRegistry) RemoveMachineState(machID string) error { if etcd.IsKeyNotFound(err) { err = nil } + + // Delete metadata + req = etcd.Delete{ + Key: path.Join(r.keyPrefix, machinePrefix, machID, "metadata"), + Recursive: true, + } + _, err = r.etcd.Do(&req) + if etcd.IsKeyNotFound(err) { + err = nil + } + return err } diff --git a/registry/unit_state_test.go b/registry/unit_state_test.go index 72a2a4b11..978cebd1e 100644 --- a/registry/unit_state_test.go +++ b/registry/unit_state_test.go @@ -108,7 +108,7 @@ func TestSaveUnitState(t *testing.T) { us.UnitHash = "quickbrownfox" r.SaveUnitState(j, us, time.Second) - json := `{"loadState":"abc","activeState":"def","subState":"ghi","machineState":{"ID":"mymachine","PublicIP":"","Metadata":null,"Version":""},"unitHash":"quickbrownfox"}` + json := `{"loadState":"abc","activeState":"def","subState":"ghi","machineState":{"ID":"mymachine","PublicIP":"","Version":""},"unitHash":"quickbrownfox"}` p1 := "/fleet/state/foo.service" p2 := "/fleet/states/foo.service/mymachine" want := []action{