From 978cb54fc7eb7b0a8600151710e9ca131b2bf3a8 Mon Sep 17 00:00:00 2001 From: Brandon Philips Date: Wed, 27 Nov 2013 14:37:31 -0800 Subject: [PATCH 01/11] fix(README): use machine instead of node Internally we call every member of an etcd cluster a "machine" so stay consistent when talking in the README. --- README.md | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 3caf06fd0..b18e42b66 100644 --- a/README.md +++ b/README.md @@ -56,18 +56,18 @@ go version ``` -### Running a single node +### Running a single machine -These examples will use a single node cluster to show you the basics of the etcd REST API. +These examples will use a single machine cluster to show you the basics of the etcd REST API. Let's start etcd: ```sh -./etcd -data-dir node0 -name node0 +./etcd -data-dir machine0 -name machine0 ``` -This will bring up an etcd node listening on port 4001 for client communication and on port 7001 for server-to-server communication. -The `-data-dir node0` argument tells etcd to write node configuration, logs and snapshots to the `./node0/` directory. -The `-name node0` tells the rest of the cluster that this node is named node0. +This will bring up etcd listening on port 4001 for client communication and on port 7001 for server-to-server communication. +The `-data-dir machine0` argument tells etcd to write machine configuration, logs and snapshots to the `./machine0/` directory. +The `-name machine` tells the rest of the cluster that this machine is named machine0. @@ -75,7 +75,7 @@ The `-name node0` tells the rest of the cluster that this node is named node0. ### Setting the value to a key -Let’s set the first key-value pair to the node. +Let’s set the first key-value pair to the datastore. In this case the key is `/message` and the value is `Hello world`. ```sh @@ -164,7 +164,7 @@ Note the two new fields in response: 2. The `ttl` is the time to live for the key, in seconds. -_NOTE_: Keys can only be expired by a cluster leader so if a node gets disconnected from the cluster, its keys will not expire until it rejoins. +_NOTE_: Keys can only be expired by a cluster leader so if a machine gets disconnected from the cluster, its keys will not expire until it rejoins. Now you can try to get the key by sending a `GET` request: @@ -378,12 +378,12 @@ For testing you can use the certificates in the `fixtures/ca` directory. Let's configure etcd to use this keypair: ```sh -./etcd -f -name node0 -data-dir node0 -cert-file=./fixtures/ca/server.crt -key-file=./fixtures/ca/server.key.insecure +./etcd -f -name machine0 -data-dir machine0 -cert-file=./fixtures/ca/server.crt -key-file=./fixtures/ca/server.key.insecure ``` There are a few new options we're using: -* `-f` - forces a new node configuration, even if an existing configuration is found. (WARNING: data loss!) +* `-f` - forces a new machine configuration, even if an existing configuration is found. (WARNING: data loss!) * `-cert-file` and `-key-file` specify the location of the cert and key files to be used for for transport layer security between the client and server. You can now test the configuration using HTTPS: @@ -413,7 +413,7 @@ We can also do authentication using CA certs. The clients will provide their cert to the server and the server will check whether the cert is signed by the CA and decide whether to serve the request. ```sh -./etcd -f -name node0 -data-dir node0 -ca-file=./fixtures/ca/ca.crt -cert-file=./fixtures/ca/server.crt -key-file=./fixtures/ca/server.key.insecure +./etcd -f -name machine0 -data-dir machine0 -ca-file=./fixtures/ca/ca.crt -cert-file=./fixtures/ca/server.crt -key-file=./fixtures/ca/server.key.insecure ``` ```-ca-file``` is the path to the CA cert. @@ -463,20 +463,20 @@ We use Raft as the underlying distributed protocol which provides consistency an Let start by creating 3 new etcd instances. -We use `-peer-addr` to specify server port and `-addr` to specify client port and `-data-dir` to specify the directory to store the log and info of the node in the cluster: +We use `-peer-addr` to specify server port and `-addr` to specify client port and `-data-dir` to specify the directory to store the log and info of the machine in the cluster: ```sh -./etcd -peer-addr 127.0.0.1:7001 -addr 127.0.0.1:4001 -data-dir nodes/node1 -name node1 +./etcd -peer-addr 127.0.0.1:7001 -addr 127.0.0.1:4001 -data-dir machines/machine1 -name machine1 ``` **Note:** If you want to run etcd on an external IP address and still have access locally, you'll need to add `-bind-addr 0.0.0.0` so that it will listen on both external and localhost addresses. A similar argument `-peer-bind-addr` is used to setup the listening address for the server port. -Let's join two more nodes to this cluster using the `-peers` argument: +Let's join two more machines to this cluster using the `-peers` argument: ```sh -./etcd -peer-addr 127.0.0.1:7002 -addr 127.0.0.1:4002 -peers 127.0.0.1:7001 -data-dir nodes/node2 -name node2 -./etcd -peer-addr 127.0.0.1:7003 -addr 127.0.0.1:4003 -peers 127.0.0.1:7001 -data-dir nodes/node3 -name node3 +./etcd -peer-addr 127.0.0.1:7002 -addr 127.0.0.1:4002 -peers 127.0.0.1:7001 -data-dir machines/machine2 -name machine2 +./etcd -peer-addr 127.0.0.1:7003 -addr 127.0.0.1:4003 -peers 127.0.0.1:7001 -data-dir machines/machine3 -name machine3 ``` We can retrieve a list of machines in the cluster using the HTTP API: @@ -485,7 +485,7 @@ We can retrieve a list of machines in the cluster using the HTTP API: curl -L http://127.0.0.1:4001/v1/machines ``` -We should see there are three nodes in the cluster +We should see there are three machines in the cluster ``` http://127.0.0.1:4001, http://127.0.0.1:4002, http://127.0.0.1:4003 @@ -498,7 +498,7 @@ curl -L http://127.0.0.1:4001/v1/keys/_etcd/machines ``` ```json -[{"action":"get","key":"/_etcd/machines/node1","value":"raft=http://127.0.0.1:7001\u0026etcd=http://127.0.0.1:4001","index":1},{"action":"get","key":"/_etcd/machines/node2","value":"raft=http://127.0.0.1:7002\u0026etcd=http://127.0.0.1:4002","index":1},{"action":"get","key":"/_etcd/machines/node3","value":"raft=http://127.0.0.1:7003\u0026etcd=http://127.0.0.1:4003","index":1}] +[{"action":"get","key":"/_etcd/machines/machine1","value":"raft=http://127.0.0.1:7001\u0026etcd=http://127.0.0.1:4001","index":1},{"action":"get","key":"/_etcd/machines/machine2","value":"raft=http://127.0.0.1:7002\u0026etcd=http://127.0.0.1:4002","index":1},{"action":"get","key":"/_etcd/machines/machine3","value":"raft=http://127.0.0.1:7003\u0026etcd=http://127.0.0.1:4003","index":1}] ``` We can also get the current leader in the cluster: @@ -551,8 +551,8 @@ http://127.0.0.1:7003 ### Testing Persistence -Next we'll kill all the nodes to test persistence. -Type `CTRL-C` on each terminal and then rerun the same command you used to start each node. +Next we'll kill all the machines to test persistence. +Type `CTRL-C` on each terminal and then rerun the same command you used to start each machine. Your request for the `foo` key will return the correct value: @@ -654,8 +654,8 @@ The command is not committed until the majority of the cluster peers receive tha Because of this majority voting property, the ideal cluster should be kept small to keep speed up and be made up of an odd number of peers. Odd numbers are good because if you have 8 peers the majority will be 5 and if you have 9 peers the majority will still be 5. -The result is that an 8 peer cluster can tolerate 3 peer failures and a 9 peer cluster can tolerate 4 nodes failures. -And in the best case when all 9 peers are responding the cluster will perform at the speed of the fastest 5 nodes. +The result is that an 8 peer cluster can tolerate 3 peer failures and a 9 peer cluster can tolerate 4 machine failures. +And in the best case when all 9 peers are responding the cluster will perform at the speed of the fastest 5 machines. ### Why SSLv3 alert handshake failure when using SSL client auth? @@ -677,7 +677,7 @@ Add the following section to your openssl.cnf: When creating the cert be sure to reference it in the `-extensions` flag: ``` -openssl ca -config openssl.cnf -policy policy_anything -extensions ssl_client -out certs/node.crt -infiles node.csr +openssl ca -config openssl.cnf -policy policy_anything -extensions ssl_client -out certs/machine.crt -infiles machine.csr ``` From 2a5030b3ca02d650ea7b6899c1056445678b0782 Mon Sep 17 00:00:00 2001 From: Brandon Philips Date: Tue, 26 Nov 2013 10:29:03 -0600 Subject: [PATCH 02/11] fix(README): fix 9 instances of v1 remove references to the v1 API and consistently use the v2 API. --- README.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index b18e42b66..9d38186bd 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ curl -L http://127.0.0.1:4001/v2/keys/message You can change the value of `/message` from `Hello world` to `Hello etcd` with another `PUT` request to the key: ```sh -curl -L http://127.0.0.1:4001/v1/keys/message -XPUT -d value="Hello etcd" +curl -L http://127.0.0.1:4001/v2/keys/message -XPUT -d value="Hello etcd" ``` ```json @@ -235,14 +235,14 @@ Here is a simple example. Let's create a key-value pair first: `foo=one`. ```sh -curl -L http://127.0.0.1:4001/v1/keys/foo -XPUT -d value=one +curl -L http://127.0.0.1:4001/v2/keys/foo -XPUT -d value=one ``` Let's try an invalid `CompareAndSwap` command first. We can provide the `prevValue` parameter to the set command to make it a `CompareAndSwap` command. ```sh -curl -L http://127.0.0.1:4001/v1/keys/foo?prevValue=two -XPUT -d value=three +curl -L http://127.0.0.1:4001/v2/keys/foo?prevValue=two -XPUT -d value=three ``` This will try to compare the previous value of the key and the previous value we provided. If they are equal, the value of the key will change to three. @@ -435,7 +435,7 @@ routines:SSL3_READ_BYTES:sslv3 alert bad certificate We need to give the CA signed cert to the server. ```sh -curl --key ./fixtures/ca/server2.key.insecure --cert ./fixtures/ca/server2.crt --cacert ./fixtures/ca/server-chain.pem -L https://127.0.0.1:4001/v1/keys/foo -XPUT -d value=bar -v +curl --key ./fixtures/ca/server2.key.insecure --cert ./fixtures/ca/server2.crt --cacert ./fixtures/ca/server-chain.pem -L https://127.0.0.1:4001/v2/keys/foo -XPUT -d value=bar -v ``` You should able to see: @@ -482,7 +482,7 @@ Let's join two more machines to this cluster using the `-peers` argument: We can retrieve a list of machines in the cluster using the HTTP API: ```sh -curl -L http://127.0.0.1:4001/v1/machines +curl -L http://127.0.0.1:4001/v2/machines ``` We should see there are three machines in the cluster @@ -494,7 +494,7 @@ http://127.0.0.1:4001, http://127.0.0.1:4002, http://127.0.0.1:4003 The machine list is also available via the main key API: ```sh -curl -L http://127.0.0.1:4001/v1/keys/_etcd/machines +curl -L http://127.0.0.1:4001/v2/keys/_etcd/machines ``` ```json @@ -529,13 +529,13 @@ curl -L http://127.0.0.1:4001/v2/keys/foo -XPUT -d value=bar Now if we kill the leader of the cluster, we can get the value from one of the other two machines: ```sh -curl -L http://127.0.0.1:4002/v1/keys/foo +curl -L http://127.0.0.1:4002/v2/keys/foo ``` We can also see that a new leader has been elected: ``` -curl -L http://127.0.0.1:4002/v1/leader +curl -L http://127.0.0.1:4002/v2/leader ``` ``` @@ -557,7 +557,7 @@ Type `CTRL-C` on each terminal and then rerun the same command you used to start Your request for the `foo` key will return the correct value: ```sh -curl -L http://127.0.0.1:4002/v1/keys/foo +curl -L http://127.0.0.1:4002/v2/keys/foo ``` ```json From 5097a2adee0ca27d3af6a2225e62b1d2aba93626 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sun, 1 Dec 2013 00:47:23 -0500 Subject: [PATCH 03/11] fix(event_history.go) should not scan prefix --- store/event_history.go | 13 ++++++++++--- store/event_test.go | 8 ++++---- store/store.go | 9 ++++----- store/watcher_hub.go | 12 ++++++------ 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/store/event_history.go b/store/event_history.go index 4fd077184..d61d54d60 100644 --- a/store/event_history.go +++ b/store/event_history.go @@ -2,6 +2,7 @@ package store import ( "fmt" + "path" "strings" "sync" @@ -39,8 +40,8 @@ func (eh *EventHistory) addEvent(e *Event) *Event { } // scan function is enumerating events from the index in history and -// stops till the first point where the key has identified prefix -func (eh *EventHistory) scan(prefix string, index uint64) (*Event, *etcdErr.Error) { +// stops till the first point where the key has identified key +func (eh *EventHistory) scan(key string, recursive bool, index uint64) (*Event, *etcdErr.Error) { eh.rwl.RLock() defer eh.rwl.RUnlock() @@ -62,7 +63,13 @@ func (eh *EventHistory) scan(prefix string, index uint64) (*Event, *etcdErr.Erro for { e := eh.Queue.Events[i] - if strings.HasPrefix(e.Key, prefix) && index <= e.Index() { // make sure we bypass the smaller one + ok := (e.Key == key) + + if recursive { + ok = ok || strings.HasPrefix(e.Key, path.Join(key, "/")) + } + + if ok && index <= e.Index() { // make sure we bypass the smaller one return e, nil } diff --git a/store/event_test.go b/store/event_test.go index dc30ce44d..1d82ebf74 100644 --- a/store/event_test.go +++ b/store/event_test.go @@ -41,24 +41,24 @@ func TestScanHistory(t *testing.T) { eh.addEvent(newEvent(Create, "/foo/bar/bar", 4)) eh.addEvent(newEvent(Create, "/foo/foo/foo", 5)) - e, err := eh.scan("/foo", 1) + e, err := eh.scan("/foo", false, 1) if err != nil || e.Index() != 1 { t.Fatalf("scan error [/foo] [1] %v", e.Index) } - e, err = eh.scan("/foo/bar", 1) + e, err = eh.scan("/foo/bar", false, 1) if err != nil || e.Index() != 2 { t.Fatalf("scan error [/foo/bar] [2] %v", e.Index) } - e, err = eh.scan("/foo/bar", 3) + e, err = eh.scan("/foo/bar", true, 3) if err != nil || e.Index() != 4 { t.Fatalf("scan error [/foo/bar/bar] [4] %v", e.Index) } - e, err = eh.scan("/foo/bar", 6) + e, err = eh.scan("/foo/bar", true, 6) if e != nil { t.Fatalf("bad index shoud reuturn nil") diff --git a/store/store.go b/store/store.go index 80f9622af..1edb02f29 100644 --- a/store/store.go +++ b/store/store.go @@ -280,8 +280,8 @@ func (s *store) Delete(nodePath string, recursive bool) (*Event, error) { return e, nil } -func (s *store) Watch(prefix string, recursive bool, sinceIndex uint64) (<-chan *Event, error) { - prefix = path.Clean(path.Join("/", prefix)) +func (s *store) Watch(key string, recursive bool, sinceIndex uint64) (<-chan *Event, error) { + key = path.Clean(path.Join("/", key)) nextIndex := s.CurrentIndex + 1 @@ -292,10 +292,10 @@ func (s *store) Watch(prefix string, recursive bool, sinceIndex uint64) (<-chan var err *etcdErr.Error if sinceIndex == 0 { - c, err = s.WatcherHub.watch(prefix, recursive, nextIndex) + c, err = s.WatcherHub.watch(key, recursive, nextIndex) } else { - c, err = s.WatcherHub.watch(prefix, recursive, sinceIndex) + c, err = s.WatcherHub.watch(key, recursive, sinceIndex) } if err != nil { @@ -396,7 +396,6 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla expireTime = Permanent } - dir, newNodeName := path.Split(nodePath) // walk through the nodePath, create dirs and get the last directory node diff --git a/store/watcher_hub.go b/store/watcher_hub.go index b952aec6d..19eef7a57 100644 --- a/store/watcher_hub.go +++ b/store/watcher_hub.go @@ -33,11 +33,11 @@ func newWatchHub(capacity int) *watcherHub { } // watch function returns an Event channel. -// If recursive is true, the first change after index under prefix will be sent to the event channel. -// If recursive is false, the first change after index at prefix will be sent to the event channel. +// If recursive is true, the first change after index under key will be sent to the event channel. +// If recursive is false, the first change after index at key will be sent to the event channel. // If index is zero, watch will start from the current index + 1. -func (wh *watcherHub) watch(prefix string, recursive bool, index uint64) (<-chan *Event, *etcdErr.Error) { - event, err := wh.EventHistory.scan(prefix, index) +func (wh *watcherHub) watch(key string, recursive bool, index uint64) (<-chan *Event, *etcdErr.Error) { + event, err := wh.EventHistory.scan(key, recursive, index) if err != nil { return nil, err @@ -57,7 +57,7 @@ func (wh *watcherHub) watch(prefix string, recursive bool, index uint64) (<-chan sinceIndex: index, } - l, ok := wh.watchers[prefix] + l, ok := wh.watchers[key] if ok { // add the new watcher to the back of the list l.PushBack(w) @@ -65,7 +65,7 @@ func (wh *watcherHub) watch(prefix string, recursive bool, index uint64) (<-chan } else { // create a new list and add the new watcher l := list.New() l.PushBack(w) - wh.watchers[prefix] = l + wh.watchers[key] = l } atomic.AddInt64(&wh.count, 1) From 08dffe85d6227d055c6226e8228c63db445d463f Mon Sep 17 00:00:00 2001 From: Baruch Even Date: Sun, 1 Dec 2013 08:35:56 +0200 Subject: [PATCH 04/11] Fix spelling mistakes in internal-protocol-versioning.md --- Documentation/internal-protocol-versioning.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/internal-protocol-versioning.md b/Documentation/internal-protocol-versioning.md index 4602f91b6..ec3874d25 100644 --- a/Documentation/internal-protocol-versioning.md +++ b/Documentation/internal-protocol-versioning.md @@ -4,9 +4,9 @@ Goal: We want to be able to upgrade an individual peer in an etcd cluster to a n The process will take the form of individual followers upgrading to the latest version until the entire cluster is on the new version. Immediate need: etcd is moving too fast to version the internal API right now. -But, we need to keep mixed version clusters from being started by a rollowing upgrade process (e.g. the CoreOS developer alpha). +But, we need to keep mixed version clusters from being started by a rolling upgrade process (e.g. the CoreOS developer alpha). -Longer term need: Having a mixed version cluster where all peers are not be running the exact same version of etcd itself but are able to speak one version of the internal protocol. +Longer term need: Having a mixed version cluster where all peers are not running the exact same version of etcd itself but are able to speak one version of the internal protocol. Solution: The internal protocol needs to be versioned just as the client protocol is. Initially during the 0.\*.\* series of etcd releases we won't allow mixed versions at all. From cc9c75acbee32b042ebf8af0e1ef455eae35832c Mon Sep 17 00:00:00 2001 From: Brandon Philips Date: Sat, 30 Nov 2013 22:46:12 -0800 Subject: [PATCH 05/11] fix(README): repair some noisy spaces --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9d38186bd..0cc358d61 100644 --- a/README.md +++ b/README.md @@ -219,9 +219,9 @@ The watch command returns immediately with the same response as previous. ### Atomic Compare-and-Swap (CAS) -Etcd can be used as a centralized coordination service in a cluster and `CompareAndSwap` is the most basic operation to build distributed lock service. +Etcd can be used as a centralized coordination service in a cluster and `CompareAndSwap` is the most basic operation to build distributed lock service. -This command will set the value of a key only if the client-provided conditions are equal to the current conditions. +This command will set the value of a key only if the client-provided conditions are equal to the current conditions. The current comparable conditions are: From 1d02a70802637b4940938314e0058763ea6b18d8 Mon Sep 17 00:00:00 2001 From: Brandon Philips Date: Sun, 1 Dec 2013 13:28:08 -0800 Subject: [PATCH 06/11] feat(README): add a prevExist example On the mailing list Dustin Oprea suggested adding a prevExist concrete example would make it more clear that the value must be true or false. --- README.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0cc358d61..3db3f8642 100644 --- a/README.md +++ b/README.md @@ -238,8 +238,20 @@ Let's create a key-value pair first: `foo=one`. curl -L http://127.0.0.1:4001/v2/keys/foo -XPUT -d value=one ``` -Let's try an invalid `CompareAndSwap` command first. -We can provide the `prevValue` parameter to the set command to make it a `CompareAndSwap` command. +Let's try some invalid `CompareAndSwap` commands first. + +Trying to set this existing key with `prevExist=false` fails as expected: +```sh +curl -L http://127.0.0.1:4001/v2/keys/foo?prevExist=false -XPUT -d value=three +``` + +The error code explains the problem: + +```json +{"errorCode":105,"message":"Already exists","cause":"/foo","index":39776} +``` + +Now lets provide a `prevValue` parameter: ```sh curl -L http://127.0.0.1:4001/v2/keys/foo?prevValue=two -XPUT -d value=three From 78e382cb6b687c5de1e0d0f7c090da86f904a0d3 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sun, 1 Dec 2013 17:35:22 -0500 Subject: [PATCH 07/11] test add watcher prefix test --- store/event_history.go | 8 +++++++- store/watcher_test.go | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/store/event_history.go b/store/event_history.go index d61d54d60..8ca9e8f50 100644 --- a/store/event_history.go +++ b/store/event_history.go @@ -66,7 +66,13 @@ func (eh *EventHistory) scan(key string, recursive bool, index uint64) (*Event, ok := (e.Key == key) if recursive { - ok = ok || strings.HasPrefix(e.Key, path.Join(key, "/")) + // add tailing slash + key := path.Clean(key) + if key[len(key)-1] != '/' { + key = key + "/" + } + + ok = ok || strings.HasPrefix(e.Key, key) } if ok && index <= e.Index() { // make sure we bypass the smaller one diff --git a/store/watcher_test.go b/store/watcher_test.go index 386fec440..3afd44c75 100644 --- a/store/watcher_test.go +++ b/store/watcher_test.go @@ -68,4 +68,24 @@ func TestWatcher(t *testing.T) { t.Fatal("recv != send") } + // ensure we are doing exact matching rather than prefix matching + c, _ = wh.watch("/fo", true, 1) + + select { + case re = <-c: + t.Fatal("should not receive from channel:", re) + default: + // do nothing + } + + e = newEvent(Create, "/fo/bar", 3) + + wh.notify(e) + + re = <-c + + if e != re { + t.Fatal("recv != send") + } + } From 6252037376917169d02880954c6880ec37ccfd20 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sun, 1 Dec 2013 18:01:24 -0500 Subject: [PATCH 08/11] fix root should be rdonly --- error/error.go | 2 ++ store/store.go | 25 +++++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/error/error.go b/error/error.go index dddad19b4..61eb7c3f7 100644 --- a/error/error.go +++ b/error/error.go @@ -32,6 +32,7 @@ const ( EcodeNotDir = 104 EcodeNodeExist = 105 EcodeKeyIsPreserved = 106 + EcodeRootROnly = 107 EcodeValueRequired = 200 EcodePrevValueRequired = 201 @@ -56,6 +57,7 @@ func init() { errors[EcodeNoMorePeer] = "Reached the max number of peers in the cluster" errors[EcodeNotDir] = "Not A Directory" errors[EcodeNodeExist] = "Already exists" // create + errors[EcodeRootROnly] = "Root is read only" errors[EcodeKeyIsPreserved] = "The prefix of given key is a keyword in etcd" // Post form related errors diff --git a/store/store.go b/store/store.go index 1edb02f29..688e2ccf8 100644 --- a/store/store.go +++ b/store/store.go @@ -156,8 +156,6 @@ func (s *store) Get(nodePath string, recursive, sorted bool) (*Event, error) { // If the node has already existed, create will fail. // If any node on the path is a file, create will fail. func (s *store) Create(nodePath string, value string, unique bool, expireTime time.Time) (*Event, error) { - nodePath = path.Clean(path.Join("/", nodePath)) - s.worldLock.Lock() defer s.worldLock.Unlock() e, err := s.internalCreate(nodePath, value, unique, false, expireTime, Create) @@ -173,8 +171,6 @@ func (s *store) Create(nodePath string, value string, unique bool, expireTime ti // Set function creates or replace the Node at nodePath. func (s *store) Set(nodePath string, value string, expireTime time.Time) (*Event, error) { - nodePath = path.Clean(path.Join("/", nodePath)) - s.worldLock.Lock() defer s.worldLock.Unlock() e, err := s.internalCreate(nodePath, value, false, true, expireTime, Set) @@ -192,6 +188,10 @@ func (s *store) CompareAndSwap(nodePath string, prevValue string, prevIndex uint value string, expireTime time.Time) (*Event, error) { nodePath = path.Clean(path.Join("/", nodePath)) + // we do not allow the user to change "/" + if nodePath == "/" { + return nil, etcdErr.NewError(etcdErr.EcodeRootROnly, "/", s.CurrentIndex) + } s.worldLock.Lock() defer s.worldLock.Unlock() @@ -238,6 +238,10 @@ func (s *store) CompareAndSwap(nodePath string, prevValue string, prevIndex uint // If the node is a directory, recursive must be true to delete it. func (s *store) Delete(nodePath string, recursive bool) (*Event, error) { nodePath = path.Clean(path.Join("/", nodePath)) + // we do not allow the user to change "/" + if nodePath == "/" { + return nil, etcdErr.NewError(etcdErr.EcodeRootROnly, "/", s.CurrentIndex) + } s.worldLock.Lock() defer s.worldLock.Unlock() @@ -334,13 +338,17 @@ func (s *store) walk(nodePath string, walkFunc func(prev *Node, component string // If the node is a file, the value and the ttl can be updated. // If the node is a directory, only the ttl can be updated. func (s *store) Update(nodePath string, newValue string, expireTime time.Time) (*Event, error) { + nodePath = path.Clean(path.Join("/", nodePath)) + // we do not allow the user to change "/" + if nodePath == "/" { + return nil, etcdErr.NewError(etcdErr.EcodeRootROnly, "/", s.CurrentIndex) + } + s.worldLock.Lock() defer s.worldLock.Unlock() currIndex, nextIndex := s.CurrentIndex, s.CurrentIndex+1 - nodePath = path.Clean(path.Join("/", nodePath)) - n, err := s.internalGet(nodePath) if err != nil { // if the node does not exist, return error @@ -390,6 +398,11 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla nodePath = path.Clean(path.Join("/", nodePath)) + // we do not allow the user to change "/" + if nodePath == "/" { + return nil, etcdErr.NewError(etcdErr.EcodeRootROnly, "/", currIndex) + } + // Assume expire times that are way in the past are not valid. // This can occur when the time is serialized to JSON and read back in. if expireTime.Before(minExpireTime) { From b929e71948fbb2fcc5c960390b999fef29916162 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sun, 1 Dec 2013 18:16:32 -0500 Subject: [PATCH 09/11] tests add root readonly test --- store/store_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/store/store_test.go b/store/store_test.go index 29b2986b9..863874fcb 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -213,6 +213,26 @@ func TestStoreDeleteDiretory(t *testing.T) { assert.Equal(t, e.Action, "delete", "") } +func TestRootRdOnly(t *testing.T) { + s := newStore() + + _, err := s.Set("/", "", Permanent) + assert.NotNil(t, err, "") + + _, err = s.Delete("/", true) + assert.NotNil(t, err, "") + + _, err = s.Create("/", "", false, Permanent) + assert.NotNil(t, err, "") + + _, err = s.Update("/", "", Permanent) + assert.NotNil(t, err, "") + + _, err = s.CompareAndSwap("/", "", 0, "", Permanent) + assert.NotNil(t, err, "") + +} + // Ensure that the store cannot delete a directory if recursive is not specified. func TestStoreDeleteDiretoryFailsIfNonRecursive(t *testing.T) { s := newStore() From 72bf216cb47504309e37f05ee94469e9073516ae Mon Sep 17 00:00:00 2001 From: Brandon Philips Date: Mon, 2 Dec 2013 18:20:11 -0800 Subject: [PATCH 10/11] fix(server/v2): redirect to ClientURL not PeerURL If consistent is set you must redirect the client to the leader's ClientURL not the PeerURL. --- server/server.go | 5 +++++ server/v2/get_handler.go | 2 +- server/v2/v2.go | 1 + tests/mock/server_v2.go | 5 +++++ 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/server/server.go b/server/server.go index 4f75df2e0..00c39227a 100644 --- a/server/server.go +++ b/server/server.go @@ -96,6 +96,11 @@ func (s *Server) PeerURL(name string) (string, bool) { return s.registry.PeerURL(name) } +// ClientURL retrieves the Client URL for a given node name. +func (s *Server) ClientURL(name string) (string, bool) { + return s.registry.ClientURL(name) +} + // Returns a reference to the Store. func (s *Server) Store() store.Store { return s.store diff --git a/server/v2/get_handler.go b/server/v2/get_handler.go index dd0bdc180..2f48fc32a 100644 --- a/server/v2/get_handler.go +++ b/server/v2/get_handler.go @@ -23,7 +23,7 @@ func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { // Help client to redirect the request to the current leader if req.FormValue("consistent") == "true" && s.State() != raft.Leader { leader := s.Leader() - hostname, _ := s.PeerURL(leader) + hostname, _ := s.ClientURL(leader) url := hostname + req.URL.Path log.Debugf("Redirect consistent get to %s", url) http.Redirect(w, req, url, http.StatusTemporaryRedirect) diff --git a/server/v2/v2.go b/server/v2/v2.go index 135479d29..0cdfb1cf7 100644 --- a/server/v2/v2.go +++ b/server/v2/v2.go @@ -13,6 +13,7 @@ type Server interface { CommitIndex() uint64 Term() uint64 PeerURL(string) (string, bool) + ClientURL(string) (string, bool) Store() store.Store Dispatch(raft.Command, http.ResponseWriter, *http.Request) error } diff --git a/tests/mock/server_v2.go b/tests/mock/server_v2.go index d48d5c0ef..a4e8821a1 100644 --- a/tests/mock/server_v2.go +++ b/tests/mock/server_v2.go @@ -45,6 +45,11 @@ func (s *ServerV2) PeerURL(name string) (string, bool) { return args.String(0), args.Bool(1) } +func (s *ServerV2) ClientURL(name string) (string, bool) { + args := s.Called(name) + return args.String(0), args.Bool(1) +} + func (s *ServerV2) Store() store.Store { return s.store } From 6729776e25c271dfd7d38d105c951538d02ee8a2 Mon Sep 17 00:00:00 2001 From: Brandon Philips Date: Mon, 2 Dec 2013 18:41:03 -0800 Subject: [PATCH 11/11] fix(server/usage): fixup the usage based on feedback People were getting confused by the lack of explanation on which port to use. Fix this. /cc @polvi --- server/usage.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/usage.go b/server/usage.go index ff23aac53..3809fb04b 100644 --- a/server/usage.go +++ b/server/usage.go @@ -26,8 +26,9 @@ Options: -vv Enabled very verbose logging. Cluster Configuration Options: - -peers= Comma-separated list of peers (ip + port) in the cluster. - -peers-file= Path to a file containing the peer list. + -peers-file= Path to a file containing the peer list. + -peers=, Comma-separated list of peers. The members + should match the peer's '-peer-addr' flag. Client Communication Options: -addr= The public host:port used for client communication.