[lxc-devel] [lxd/master] Implement ETag support

stgraber on Github lxc-bot at linuxcontainers.org
Thu Jun 16 16:18:41 UTC 2016


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 369 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160616/2a00f4df/attachment.bin>
-------------- next part --------------
From 771194b6d95a7b913565bce8a6314eb15d21abab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 16 Jun 2016 12:17:52 -0400
Subject: [PATCH] Implement ETag support
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #120

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 doc/api_extensions.md     | 13 +++++++++++++
 doc/rest-api.md           | 10 +++++-----
 lxd/api_1.0.go            |  9 +++++++--
 lxd/container.go          |  2 +-
 lxd/container_get.go      |  4 ++--
 lxd/container_lxc.go      | 13 ++++++++-----
 lxd/container_put.go      |  8 ++++++++
 lxd/container_snapshot.go |  4 ++--
 lxd/containers_get.go     |  2 +-
 lxd/images.go             | 38 ++++++++++++++++++++++++++------------
 lxd/profiles.go           | 20 +++++++++++++-------
 lxd/response.go           | 28 +++++++++++++++++++++++-----
 lxd/util.go               | 30 ++++++++++++++++++++++++++++++
 13 files changed, 139 insertions(+), 42 deletions(-)

diff --git a/doc/api_extensions.md b/doc/api_extensions.md
index 7f05af5..567c1a6 100644
--- a/doc/api_extensions.md
+++ b/doc/api_extensions.md
@@ -46,3 +46,16 @@ It is a timestamp of the last time the container was started.
 
 If a container has been created but not started yet, last\_used\_at field
 will be 1970-01-01T00:00:00Z
+
+## etag
+Add support for the ETag header on all relevant endpoints.
+
+This adds the following HTTP header on answers to GET:
+ - ETag (SHA-256 of user modifiable content)
+
+And adds support for the following HTTP header on PUT requests:
+ - If-Match (ETag value retrieved through previous GET)
+
+This makes it possible to GET a LXD object, modify it and PUT it without
+risking to hit a race condition where LXD or another client modified the
+object in the mean time.
diff --git a/doc/rest-api.md b/doc/rest-api.md
index a1390cc..b617b35 100644
--- a/doc/rest-api.md
+++ b/doc/rest-api.md
@@ -257,7 +257,7 @@ Return value (if guest or untrusted):
         "public": false,                        # Whether the server should be treated as a public (read-only) remote by the client
     }
 
-### PUT
+### PUT (ETag supported)
  * Description: Updates the server configuration or other properties
  * Authentication: trusted
  * Operation: sync
@@ -557,7 +557,7 @@ Output:
     }
 
 
-### PUT
+### PUT (ETag supported)
  * Description: update container configuration or restore snapshot
  * Authentication: trusted
  * Operation: async
@@ -1231,7 +1231,7 @@ Input (none at present):
 
 HTTP code for this should be 202 (Accepted).
 
-### PUT
+### PUT (ETag supported)
  * Description: Updates the image properties
  * Authentication: trusted
  * Operation: sync
@@ -1335,7 +1335,7 @@ Output:
         "target": "c9b6e738fae75286d52f497415463a8ecc61bbcb046536f220d797b0e500a41f"
     }
 
-### PUT
+### PUT (ETag supported)
  * Description: Updates the alias target or description
  * Authentication: trusted
  * Operation: sync
@@ -1537,7 +1537,7 @@ Output:
         }
     }
 
-### PUT
+### PUT (ETag supported)
  * Description: update the profile
  * Authentication: trusted
  * Operation: sync
diff --git a/lxd/api_1.0.go b/lxd/api_1.0.go
index 6ede473..45306ec 100644
--- a/lxd/api_1.0.go
+++ b/lxd/api_1.0.go
@@ -60,6 +60,7 @@ func api10Get(d *Daemon, r *http.Request) Response {
 			"container_syscall_filtering",
 			"auth_pki",
 			"container_last_used_at",
+			"etag",
 		},
 
 		"api_status":  "stable",
@@ -148,7 +149,7 @@ func api10Get(d *Daemon, r *http.Request) Response {
 		body["public"] = false
 	}
 
-	return SyncResponse(true, body)
+	return SyncResponseETag(true, body, body["config"])
 }
 
 type apiPut struct {
@@ -161,8 +162,12 @@ func api10Put(d *Daemon, r *http.Request) Response {
 		return InternalError(err)
 	}
 
-	req := apiPut{}
+	err = etagCheck(r, oldConfig)
+	if err != nil {
+		return PreconditionFailed(err)
+	}
 
+	req := apiPut{}
 	if err := shared.ReadToJSON(r.Body, &req); err != nil {
 		return BadRequest(err)
 	}
diff --git a/lxd/container.go b/lxd/container.go
index f4a6307..ecd74e1 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -402,7 +402,7 @@ type container interface {
 	Exec(command []string, env map[string]string, stdin *os.File, stdout *os.File, stderr *os.File) (int, error)
 
 	// Status
-	Render() (interface{}, error)
+	Render() (interface{}, interface{}, error)
 	RenderState() (*shared.ContainerState, error)
 	IsPrivileged() bool
 	IsRunning() bool
diff --git a/lxd/container_get.go b/lxd/container_get.go
index 3df94ea..a1db3a4 100644
--- a/lxd/container_get.go
+++ b/lxd/container_get.go
@@ -13,10 +13,10 @@ func containerGet(d *Daemon, r *http.Request) Response {
 		return SmartError(err)
 	}
 
-	state, err := c.Render()
+	state, etag, err := c.Render()
 	if err != nil {
 		return InternalError(err)
 	}
 
-	return SyncResponse(true, state)
+	return SyncResponseETag(true, state, etag)
 }
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 2623174..c75d29c 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1612,16 +1612,19 @@ func (c *containerLXC) getLxcState() (lxc.State, error) {
 	}
 }
 
-func (c *containerLXC) Render() (interface{}, error) {
+func (c *containerLXC) Render() (interface{}, interface{}, error) {
 	// Load the go-lxc struct
 	err := c.initLXC()
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	// Ignore err as the arch string on error is correct (unknown)
 	architectureName, _ := shared.ArchitectureName(c.architecture)
 
+	// Prepare the ETag
+	etag := []interface{}{c.architecture, c.localConfig, c.localDevices, c.ephemeral, c.profiles}
+
 	if c.IsSnapshot() {
 		return &shared.SnapshotInfo{
 			Architecture:    architectureName,
@@ -1635,12 +1638,12 @@ func (c *containerLXC) Render() (interface{}, error) {
 			Name:            c.name,
 			Profiles:        c.profiles,
 			Stateful:        c.stateful,
-		}, nil
+		}, etag, nil
 	} else {
 		// FIXME: Render shouldn't directly access the go-lxc struct
 		cState, err := c.getLxcState()
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		statusCode := shared.FromLXCState(int(cState))
 
@@ -1658,7 +1661,7 @@ func (c *containerLXC) Render() (interface{}, error) {
 			Status:          statusCode.String(),
 			StatusCode:      statusCode,
 			Stateful:        c.stateful,
-		}, nil
+		}, etag, nil
 	}
 }
 
diff --git a/lxd/container_put.go b/lxd/container_put.go
index 747a234..1b19cea 100644
--- a/lxd/container_put.go
+++ b/lxd/container_put.go
@@ -26,12 +26,20 @@ type containerPutReq struct {
  * the named snapshot
  */
 func containerPut(d *Daemon, r *http.Request) Response {
+	// Get the container
 	name := mux.Vars(r)["name"]
 	c, err := containerLoadByName(d, name)
 	if err != nil {
 		return NotFound
 	}
 
+	// Validate the ETag
+	etag := []interface{}{c.Architecture(), c.LocalConfig(), c.LocalDevices(), c.IsEphemeral(), c.Profiles()}
+	err = etagCheck(r, etag)
+	if err != nil {
+		return PreconditionFailed(err)
+	}
+
 	configRaw := containerPutReq{}
 	if err := json.NewDecoder(r.Body).Decode(&configRaw); err != nil {
 		return BadRequest(err)
diff --git a/lxd/container_snapshot.go b/lxd/container_snapshot.go
index 803ac11..7d5e409 100644
--- a/lxd/container_snapshot.go
+++ b/lxd/container_snapshot.go
@@ -44,7 +44,7 @@ func containerSnapshotsGet(d *Daemon, r *http.Request) Response {
 			url := fmt.Sprintf("/%s/containers/%s/snapshots/%s", shared.APIVersion, cname, snapName)
 			resultString = append(resultString, url)
 		} else {
-			render, err := snap.Render()
+			render, _, err := snap.Render()
 			if err != nil {
 				continue
 			}
@@ -183,7 +183,7 @@ func snapshotHandler(d *Daemon, r *http.Request) Response {
 }
 
 func snapshotGet(sc container, name string) Response {
-	render, err := sc.Render()
+	render, _, err := sc.Render()
 	if err != nil {
 		return SmartError(err)
 	}
diff --git a/lxd/containers_get.go b/lxd/containers_get.go
index 750782d..cb0be7e 100644
--- a/lxd/containers_get.go
+++ b/lxd/containers_get.go
@@ -69,7 +69,7 @@ func doContainerGet(d *Daemon, cname string) (*shared.ContainerInfo, error) {
 		return nil, err
 	}
 
-	cts, err := c.Render()
+	cts, _, err := c.Render()
 	if err != nil {
 		return nil, err
 	}
diff --git a/lxd/images.go b/lxd/images.go
index 3410b6b..8118d8f 100644
--- a/lxd/images.go
+++ b/lxd/images.go
@@ -1016,7 +1016,8 @@ func imageGet(d *Daemon, r *http.Request) Response {
 		return response
 	}
 
-	return SyncResponse(true, info)
+	etag := []interface{}{info.Public, info.AutoUpdate, info.Properties}
+	return SyncResponseETag(true, info, etag)
 }
 
 type imagePutReq struct {
@@ -1026,18 +1027,25 @@ type imagePutReq struct {
 }
 
 func imagePut(d *Daemon, r *http.Request) Response {
+	// Get current value
 	fingerprint := mux.Vars(r)["fingerprint"]
+	id, info, err := dbImageGet(d.db, fingerprint, false, false)
+	if err != nil {
+		return SmartError(err)
+	}
+
+	// Validate ETag
+	etag := []interface{}{info.Public, info.AutoUpdate, info.Properties}
+	err = etagCheck(r, etag)
+	if err != nil {
+		return PreconditionFailed(err)
+	}
 
 	req := imagePutReq{}
 	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
 		return BadRequest(err)
 	}
 
-	id, info, err := dbImageGet(d.db, fingerprint, false, false)
-	if err != nil {
-		return SmartError(err)
-	}
-
 	err = dbImageUpdate(d.db, id, info.Filename, info.Size, req.Public, req.AutoUpdate, info.Architecture, info.CreationDate, info.ExpiryDate, req.Properties)
 	if err != nil {
 		return SmartError(err)
@@ -1131,7 +1139,7 @@ func aliasGet(d *Daemon, r *http.Request) Response {
 		return SmartError(err)
 	}
 
-	return SyncResponse(true, alias)
+	return SyncResponseETag(true, alias, alias)
 }
 
 func aliasDelete(d *Daemon, r *http.Request) Response {
@@ -1150,7 +1158,18 @@ func aliasDelete(d *Daemon, r *http.Request) Response {
 }
 
 func aliasPut(d *Daemon, r *http.Request) Response {
+	// Get current value
 	name := mux.Vars(r)["name"]
+	id, alias, err := dbImageAliasGet(d.db, name, true)
+	if err != nil {
+		return SmartError(err)
+	}
+
+	// Validate ETag
+	err = etagCheck(r, alias)
+	if err != nil {
+		return PreconditionFailed(err)
+	}
 
 	req := aliasPutReq{}
 	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
@@ -1161,11 +1180,6 @@ func aliasPut(d *Daemon, r *http.Request) Response {
 		return BadRequest(fmt.Errorf("The target field is required"))
 	}
 
-	id, _, err := dbImageAliasGet(d.db, name, true)
-	if err != nil {
-		return SmartError(err)
-	}
-
 	imageId, _, err := dbImageGet(d.db, req.Target, false, false)
 	if err != nil {
 		return SmartError(err)
diff --git a/lxd/profiles.go b/lxd/profiles.go
index 17bb997..32a498a 100644
--- a/lxd/profiles.go
+++ b/lxd/profiles.go
@@ -104,7 +104,7 @@ func profileGet(d *Daemon, r *http.Request) Response {
 		return SmartError(err)
 	}
 
-	return SyncResponse(true, resp)
+	return SyncResponseETag(true, resp, resp)
 }
 
 func getRunningContainersWithProfile(d *Daemon, profile string) []container {
@@ -127,7 +127,18 @@ func getRunningContainersWithProfile(d *Daemon, profile string) []container {
 }
 
 func profilePut(d *Daemon, r *http.Request) Response {
+	// Get the profile
 	name := mux.Vars(r)["name"]
+	id, profile, err := dbProfileGet(d.db, name)
+	if err != nil {
+		return InternalError(fmt.Errorf("Failed to retrieve profile='%s'", name))
+	}
+
+	// Validate the ETag
+	err = etagCheck(r, profile)
+	if err != nil {
+		return PreconditionFailed(err)
+	}
 
 	req := profilesPostReq{}
 	if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
@@ -135,7 +146,7 @@ func profilePut(d *Daemon, r *http.Request) Response {
 	}
 
 	// Sanity checks
-	err := containerValidConfig(d, req.Config, true, false)
+	err = containerValidConfig(d, req.Config, true, false)
 	if err != nil {
 		return BadRequest(err)
 	}
@@ -157,11 +168,6 @@ func profilePut(d *Daemon, r *http.Request) Response {
 	}
 
 	// Update the database
-	id, profile, err := dbProfileGet(d.db, name)
-	if err != nil {
-		return InternalError(fmt.Errorf("Failed to retrieve profile='%s'", name))
-	}
-
 	tx, err := dbBegin(d.db)
 	if err != nil {
 		return InternalError(err)
diff --git a/lxd/response.go b/lxd/response.go
index 59a7a66..2aa9a05 100644
--- a/lxd/response.go
+++ b/lxd/response.go
@@ -39,10 +39,20 @@ type Response interface {
 // Sync response
 type syncResponse struct {
 	success  bool
+	etag     interface{}
 	metadata interface{}
 }
 
 func (r *syncResponse) Render(w http.ResponseWriter) error {
+	// Set an appropriate ETag header
+	if r.etag != nil {
+		etag, err := etagHash(r.etag)
+		if err == nil {
+			w.Header().Set("ETag", etag)
+		}
+	}
+
+	// Prepare the JSON response
 	status := shared.Success
 	if !r.success {
 		status = shared.Failure
@@ -52,10 +62,6 @@ func (r *syncResponse) Render(w http.ResponseWriter) error {
 	return WriteJSON(w, resp)
 }
 
-func SyncResponse(success bool, metadata interface{}) Response {
-	return &syncResponse{success, metadata}
-}
-
 func (r *syncResponse) String() string {
 	if r.success {
 		return "success"
@@ -64,7 +70,15 @@ func (r *syncResponse) String() string {
 	return "failure"
 }
 
-var EmptySyncResponse = &syncResponse{true, make(map[string]interface{})}
+func SyncResponse(success bool, metadata interface{}) Response {
+	return &syncResponse{success: success, metadata: metadata}
+}
+
+func SyncResponseETag(success bool, metadata interface{}, etag interface{}) Response {
+	return &syncResponse{success: success, metadata: metadata, etag: etag}
+}
+
+var EmptySyncResponse = &syncResponse{success: true, metadata: make(map[string]interface{})}
 
 // File transfer response
 type fileResponseEntry struct {
@@ -253,6 +267,10 @@ func InternalError(err error) Response {
 	return &errorResponse{http.StatusInternalServerError, err.Error()}
 }
 
+func PreconditionFailed(err error) Response {
+	return &errorResponse{http.StatusPreconditionFailed, err.Error()}
+}
+
 /*
  * SmartError returns the right error message based on err.
  */
diff --git a/lxd/util.go b/lxd/util.go
index 331d284..a496f82 100644
--- a/lxd/util.go
+++ b/lxd/util.go
@@ -2,7 +2,9 @@ package main
 
 import (
 	"bytes"
+	"crypto/sha256"
 	"encoding/json"
+	"fmt"
 	"io"
 	"net/http"
 
@@ -27,3 +29,31 @@ func WriteJSON(w http.ResponseWriter, body interface{}) error {
 
 	return err
 }
+
+func etagHash(data interface{}) (string, error) {
+	etag := sha256.New()
+	err := json.NewEncoder(etag).Encode(data)
+	if err != nil {
+		return "", err
+	}
+
+	return fmt.Sprintf("%x", etag.Sum(nil)), nil
+}
+
+func etagCheck(r *http.Request, data interface{}) error {
+	match := r.Header.Get("If-Match")
+	if match == "" {
+		return nil
+	}
+
+	hash, err := etagHash(data)
+	if err != nil {
+		return err
+	}
+
+	if hash != match {
+		return fmt.Errorf("ETag doesn't match: %s vs %s", hash, match)
+	}
+
+	return nil
+}


More information about the lxc-devel mailing list