Merge pull request #355 from xiangli-cmu/root_readonly

WIP fix root should be rdonly

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

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) {

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()