Skip to content

Commit

Permalink
clair: use Etag header to communicate indexer state change
Browse files Browse the repository at this point in the history
Adding this fixes a race condition that might occur when doing a deploy,
where a manifest would be associated with the wrong indexer state.

In the course of this change, the Etag validation logic is also
corrected.

Signed-off-by: Hank Donnay <hdonnay@redhat.com>
  • Loading branch information
hdonnay committed Mar 13, 2020
1 parent 485a7f6 commit 42b1ba9
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 13 deletions.
3 changes: 0 additions & 3 deletions cmd/clair/httptransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ import (
"github.com/quay/claircore/libvuln"
"go.opentelemetry.io/otel/plugin/othttp"

"github.com/quay/clair/v4/middleware/auth"
"github.com/quay/clair/v4/middleware/compress"

"github.com/quay/clair/v4/config"
"github.com/quay/clair/v4/indexer"
"github.com/quay/clair/v4/matcher"
Expand Down
44 changes: 34 additions & 10 deletions indexer/httptransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"net/http"
"path"
"sort"
"strings"
"time"

Expand Down Expand Up @@ -58,8 +57,11 @@ func NewHTTPTransport(service Service) (*HTTP, error) {

func unmodified(r *http.Request, v string) bool {
if vs, ok := r.Header["If-None-Match"]; ok {
sort.Strings(vs)
return sort.SearchStrings(vs, v) != -1
for _, rv := range vs {
if rv == v {
return true
}
}
}
return false
}
Expand Down Expand Up @@ -109,7 +111,7 @@ func (h *HTTP) IndexReportHandler(w http.ResponseWriter, r *http.Request) {
je.Error(w, resp, http.StatusInternalServerError)
return
}
validator := fmt.Sprintf(`"%s|%s"`, state, manifest.String())
validator := `"` + state + `"`
if unmodified(r, validator) {
w.WriteHeader(http.StatusNotModified)
return
Expand Down Expand Up @@ -166,19 +168,42 @@ func (h *HTTP) IndexHandler(w http.ResponseWriter, r *http.Request) {
je.Error(w, resp, http.StatusMethodNotAllowed)
return
}
state, err := h.serv.State(ctx)
if err != nil {
resp := &je.Response{
Code: "internal error",
Message: "could not retrieve indexer state " + err.Error(),
}
je.Error(w, resp, http.StatusInternalServerError)
return
}

var m claircore.Manifest
err := json.NewDecoder(r.Body).Decode(&m)
if err != nil {
if err := json.NewDecoder(r.Body).Decode(&m); err != nil {
resp := &je.Response{
Code: "bad-request",
Message: fmt.Sprintf("failed to deserialize manifest: %v", err),
}
je.Error(w, resp, http.StatusBadRequest)
return
}
if m.Hash.String() == "" || len(m.Layers) == 0 {
resp := &je.Response{
Code: "bad-request",
Message: "bogus manifest",
}
je.Error(w, resp, http.StatusBadRequest)
return
}
next := path.Join(IndexReportAPIPath, m.Hash.String())

// TODO Validate manifest structure.
w.Header().Add("link", fmt.Sprintf(linkIndex, next))
w.Header().Add("link", fmt.Sprintf(linkReport, path.Join(v1Root, "vulnerabilty_report", m.Hash.String())))
validator := `"` + state + `"`
if unmodified(r, validator) {
w.WriteHeader(http.StatusPreconditionFailed)
return
}

// TODO Do we need some sort of background context embedded in the HTTP
// struct?
Expand All @@ -188,13 +213,12 @@ func (h *HTTP) IndexHandler(w http.ResponseWriter, r *http.Request) {
Code: "index-error",
Message: fmt.Sprintf("failed to start scan: %v", err),
}
w.Header().Del("link")
je.Error(w, resp, http.StatusInternalServerError)
return
}

next := path.Join(IndexReportAPIPath, m.Hash.String())
w.Header().Add("link", fmt.Sprintf(linkReport, path.Join(v1Root, "vulnerabilty_report", m.Hash.String())))
w.Header().Add("link", fmt.Sprintf(linkIndex, next))
w.Header().Set("etag", validator)
w.Header().Set("location", next)
w.WriteHeader(http.StatusCreated)
if err := json.NewEncoder(w).Encode(report); err != nil {
Expand Down

0 comments on commit 42b1ba9

Please sign in to comment.