diff --git a/command/agent/secure_variable_endpoint.go b/command/agent/secure_variable_endpoint.go index a24cc878a..1c1f06fa9 100644 --- a/command/agent/secure_variable_endpoint.go +++ b/command/agent/secure_variable_endpoint.go @@ -1,7 +1,9 @@ package agent import ( + "fmt" "net/http" + "strconv" "strings" "github.com/hashicorp/nomad/nomad/structs" @@ -33,7 +35,7 @@ func (s *HTTPServer) SecureVariablesListRequest(resp http.ResponseWriter, req *h func (s *HTTPServer) SecureVariableSpecificRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { path := strings.TrimPrefix(req.URL.Path, "/v1/var/") if len(path) == 0 { - return nil, CodedError(http.StatusBadRequest, "Missing secure variable path") + return nil, CodedError(http.StatusBadRequest, "missing secure variable path") } switch req.Method { case http.MethodGet: @@ -63,7 +65,7 @@ func (s *HTTPServer) secureVariableQuery(resp http.ResponseWriter, req *http.Req setMeta(resp, &out.QueryMeta) if out.Data == nil { - return nil, CodedError(http.StatusNotFound, "Secure variable not found") + return nil, CodedError(http.StatusNotFound, "secure variable not found") } return out.Data, nil } @@ -75,23 +77,49 @@ func (s *HTTPServer) secureVariableUpsert(resp http.ResponseWriter, req *http.Re if err := decodeBody(req, &SecureVariable); err != nil { return nil, CodedError(http.StatusBadRequest, err.Error()) } - if SecureVariable.Items == nil { - return nil, CodedError(http.StatusBadRequest, "Secure variable missing required Items object.") + if len(SecureVariable.Items) == 0 { + return nil, CodedError(http.StatusBadRequest, "secure variable missing required Items object") } + SecureVariable.Path = path - // Format the request + // This function always makes an upsert request with length of 1, which is + // an important proviso for when we check for conflicts and return them args := structs.SecureVariablesUpsertRequest{ Data: []*structs.SecureVariableDecrypted{&SecureVariable}, } s.parseWriteRequest(req, &args.WriteRequest) + if err := parseCAS(req, &args); err != nil { + return nil, err + } var out structs.SecureVariablesUpsertResponse if err := s.agent.RPC(structs.SecureVariablesUpsertRPCMethod, &args, &out); err != nil { + + // This handles the cases where there is an error in the CAS checking + // function that renders it unable to return the conflicting variable + // so it returns a text error. We can at least consider these unknown + // moments to be CAS violations + if strings.Contains(err.Error(), "cas error:") { + resp.WriteHeader(http.StatusConflict) + } + + // Otherwise it's a non-CAS error + setIndex(resp, out.WriteMeta.Index) return nil, err } - setIndex(resp, out.WriteMeta.Index) + // As noted earlier, the upsert request generated by this endpoint always + // has length of 1, so we expect a non-Nil Conflicts slice to have len(1). + // We then extract the conflict value at index 0 + if len(out.Conflicts) == 1 { + setIndex(resp, out.Conflicts[0].ModifyIndex) + resp.WriteHeader(http.StatusConflict) + return out.Conflicts[0], nil + } + + // Finally, we know that this is a success response, send it to the caller + setIndex(resp, out.WriteMeta.Index) return nil, nil } @@ -102,11 +130,49 @@ func (s *HTTPServer) secureVariableDelete(resp http.ResponseWriter, req *http.Re Path: path, } s.parseWriteRequest(req, &args.WriteRequest) + if err := parseCAS(req, &args); err != nil { + return nil, err + } var out structs.SecureVariablesDeleteResponse if err := s.agent.RPC(structs.SecureVariablesDeleteRPCMethod, &args, &out); err != nil { + + // This handles the cases where there is an error in the CAS checking + // function that renders it unable to return the conflicting variable + // so it returns a text error. We can at least consider these unknown + // moments to be CAS violations + if strings.HasPrefix(err.Error(), "cas error:") { + resp.WriteHeader(http.StatusConflict) + } + setIndex(resp, out.WriteMeta.Index) return nil, err } + + // If the CAS validation can decode the conflicting value, Conflict is + // non-Nil. Write out a 409 Conflict response. + if out.Conflict != nil { + setIndex(resp, out.Conflict.ModifyIndex) + resp.WriteHeader(http.StatusConflict) + return out.Conflict, nil + } + + // Finally, we know that this is a success response, send it to the caller setIndex(resp, out.WriteMeta.Index) + resp.WriteHeader(http.StatusNoContent) return nil, nil } + +func parseCAS(req *http.Request, rpc CheckIndexSetter) error { + if cq := req.URL.Query().Get("cas"); cq != "" { + ci, err := strconv.ParseUint(cq, 10, 64) + if err != nil { + return CodedError(http.StatusBadRequest, fmt.Sprintf("can not parse cas: %v", err)) + } + rpc.SetCheckIndex(ci) + } + return nil +} + +type CheckIndexSetter interface { + SetCheckIndex(uint64) +} diff --git a/command/agent/secure_variable_endpoint_test.go b/command/agent/secure_variable_endpoint_test.go index 177648840..3ac7fb74b 100644 --- a/command/agent/secure_variable_endpoint_test.go +++ b/command/agent/secure_variable_endpoint_test.go @@ -3,6 +3,7 @@ package agent import ( "bytes" "encoding/json" + "fmt" "io" "io/ioutil" "net/http" @@ -131,7 +132,7 @@ func TestHTTP_SecureVariables(t *testing.T) { require.NoError(t, err) respW := httptest.NewRecorder() obj, err := s.Server.SecureVariableSpecificRequest(respW, req) - require.EqualError(t, err, "Missing secure variable path") + require.EqualError(t, err, "missing secure variable path") require.Nil(t, obj) }) t.Run("query_unset_variable", func(t *testing.T) { @@ -140,7 +141,7 @@ func TestHTTP_SecureVariables(t *testing.T) { require.NoError(t, err) respW := httptest.NewRecorder() obj, err := s.Server.SecureVariableSpecificRequest(respW, req) - require.EqualError(t, err, "Secure variable not found") + require.EqualError(t, err, "secure variable not found") require.Nil(t, obj) }) t.Run("query", func(t *testing.T) { @@ -192,7 +193,7 @@ func TestHTTP_SecureVariables(t *testing.T) { require.NoError(t, err) respW := httptest.NewRecorder() obj, err := s.Server.SecureVariableSpecificRequest(respW, req) - require.EqualError(t, err, "Secure variable missing required Items object.") + require.EqualError(t, err, "secure variable missing required Items object") require.Nil(t, obj) }) t.Run("create", func(t *testing.T) { @@ -220,10 +221,10 @@ func TestHTTP_SecureVariables(t *testing.T) { }) rpcResetSV(s) - sv1U := sv1.Copy() - sv1U.Items["new"] = "new" - t.Run("error_parse_update", func(t *testing.T) { + sv1U := sv1.Copy() + sv1U.Items["new"] = "new" + // break the request body badBuf := encodeBrokenReq(&sv1U) @@ -240,6 +241,9 @@ func TestHTTP_SecureVariables(t *testing.T) { require.Nil(t, obj) }) t.Run("error_rpc_update", func(t *testing.T) { + sv1U := sv1.Copy() + sv1U.Items["new"] = "new" + // test broken rpc error buf := encodeReq(&sv1U) req, err := http.NewRequest("PUT", "/v1/var/"+sv1.Path+"?region=bad", buf) @@ -252,11 +256,16 @@ func TestHTTP_SecureVariables(t *testing.T) { require.Nil(t, obj) }) t.Run("update", func(t *testing.T) { - require.NoError(t, rpcWriteSV(s, sv1)) + sv := mock.SecureVariable() + require.NoError(t, rpcWriteSV(s, sv)) + sv, err := rpcReadSV(s, sv.Namespace, sv.Path) + require.NoError(t, err) + svU := sv.Copy() + svU.Items["new"] = "new" // Make the HTTP request - buf := encodeReq(&sv1U) - req, err := http.NewRequest("PUT", "/v1/var/"+sv1.Path, buf) + buf := encodeReq(&svU) + req, err := http.NewRequest("PUT", "/v1/var/"+sv.Path, buf) require.NoError(t, err) respW := httptest.NewRecorder() @@ -268,15 +277,92 @@ func TestHTTP_SecureVariables(t *testing.T) { // Check for the index require.NotZero(t, respW.HeaderMap.Get("X-Nomad-Index")) - // Check the variable was created - out, err := rpcReadSV(s, sv1.Namespace, sv1.Path) - require.NoError(t, err) - require.NotNil(t, out) + { + out, err := rpcReadSV(s, sv.Namespace, sv.Path) + require.NoError(t, err) + require.NotNil(t, out) - sv1.CreateIndex, sv1.ModifyIndex = out.CreateIndex, out.ModifyIndex - require.Equal(t, sv1.Path, out.Path) - require.NotEqual(t, sv1, out) - require.Equal(t, "new", out.Items["new"]) + // Check that written varible does not equal the input to rule out input mutation + require.NotEqual(t, svU.SecureVariableMetadata, out.SecureVariableMetadata) + + // Update the input token with the updated metadata so that we + // can use a simple equality check + svU.CreateIndex, svU.ModifyIndex = out.CreateIndex, out.ModifyIndex + svU.CreateTime, svU.ModifyTime = out.CreateTime, out.ModifyTime + require.Equal(t, svU.SecureVariableMetadata, out.SecureVariableMetadata) + + // fmt writes sorted output of maps for testability. + require.Equal(t, fmt.Sprint(svU.Items), fmt.Sprint(out.Items)) + } + }) + t.Run("update-cas", func(t *testing.T) { + sv := mock.SecureVariable() + require.NoError(t, rpcWriteSV(s, sv)) + sv, err := rpcReadSV(s, sv.Namespace, sv.Path) + require.NoError(t, err) + + svU := sv.Copy() + svU.Items["new"] = "new" + + // Make the HTTP request + { + buf := encodeReq(&svU) + req, err := http.NewRequest("PUT", "/v1/var/"+svU.Path+"?cas=0", buf) + require.NoError(t, err) + respW := httptest.NewRecorder() + + // Make the request + obj, err := s.Server.SecureVariableSpecificRequest(respW, req) + require.Equal(t, http.StatusConflict, respW.Result().StatusCode) + + // Evaluate the conflict variable + require.NotNil(t, obj) + conflict, ok := obj.(*structs.SecureVariableDecrypted) + require.True(t, ok, "Expected *structs.SecureVariableDecrypted, got %T", obj) + require.True(t, sv.Equals(*conflict)) + + // Check for the index + require.NotZero(t, respW.HeaderMap.Get("X-Nomad-Index")) + } + // Check the variable was not updated + { + out, err := rpcReadSV(s, sv.Namespace, sv.Path) + require.NoError(t, err) + require.Equal(t, sv, out) + } + // Make the HTTP request + { + buf := encodeReq(&svU) + req, err := http.NewRequest("PUT", "/v1/var/"+svU.Path+"?cas="+fmt.Sprint(sv.ModifyIndex), buf) + require.NoError(t, err) + respW := httptest.NewRecorder() + + // Make the request + obj, err := s.Server.SecureVariableSpecificRequest(respW, req) + require.NoError(t, err) + require.Nil(t, obj) + + // Check for the index + require.NotZero(t, respW.HeaderMap.Get("X-Nomad-Index")) + } + // Check the variable was created correctly + { + out, err := rpcReadSV(s, sv.Namespace, sv.Path) + require.NoError(t, err) + require.NotNil(t, out) + + require.NotEqual(t, sv, out) + require.NotEqual(t, svU.SecureVariableMetadata, out.SecureVariableMetadata) + + // Update the input token with the updated metadata so that we + // can use a simple equality check + svU.CreateIndex, svU.ModifyIndex = out.CreateIndex, out.ModifyIndex + svU.CreateTime, svU.ModifyTime = out.CreateTime, out.ModifyTime + require.Equal(t, svU.SecureVariableMetadata, out.SecureVariableMetadata) + + // fmt writes sorted output of maps for testability. + require.Equal(t, fmt.Sprint(svU.Items), fmt.Sprint(out.Items)) + } }) rpcResetSV(s) @@ -294,6 +380,58 @@ func TestHTTP_SecureVariables(t *testing.T) { require.EqualError(t, err, "No path to region") require.Nil(t, obj) }) + t.Run("delete-cas", func(t *testing.T) { + sv := mock.SecureVariable() + require.NoError(t, rpcWriteSV(s, sv)) + sv, err := rpcReadSV(s, sv.Namespace, sv.Path) + require.NoError(t, err) + + // Make the HTTP request + { + req, err := http.NewRequest("DELETE", "/v1/var/"+sv.Path+"?cas=0", nil) + require.NoError(t, err) + respW := httptest.NewRecorder() + + // Make the request + obj, err := s.Server.SecureVariableSpecificRequest(respW, req) + require.NoError(t, err) + require.Equal(t, http.StatusConflict, respW.Result().StatusCode) + + // Evaluate the conflict variable + require.NotNil(t, obj) + conflict, ok := obj.(*structs.SecureVariableDecrypted) + require.True(t, ok, "Expected *structs.SecureVariableDecrypted, got %T", obj) + require.True(t, sv.Equals(*conflict)) + + // Check for the index + require.NotZero(t, respW.HeaderMap.Get("X-Nomad-Index")) + } + + // Check variable was not deleted + { + svChk, err := rpcReadSV(s, sv.Namespace, sv.Path) + require.NoError(t, err) + require.NotNil(t, svChk) + require.Equal(t, sv, svChk) + } + // Make the HTTP request + { + req, err := http.NewRequest("DELETE", "/v1/var/"+sv.Path+"?cas="+fmt.Sprint(sv.ModifyIndex), nil) + require.NoError(t, err) + respW := httptest.NewRecorder() + + // Make the request + obj, err := s.Server.SecureVariableSpecificRequest(respW, req) + require.NoError(t, err) + require.Nil(t, obj) + } + // Check variable was deleted + { + svChk, err := rpcReadSV(s, sv.Namespace, sv.Path) + require.NoError(t, err) + require.Nil(t, svChk) + } + }) t.Run("delete", func(t *testing.T) { sv1 := mock.SecureVariable() require.NoError(t, rpcWriteSV(s, sv1)) @@ -310,6 +448,7 @@ func TestHTTP_SecureVariables(t *testing.T) { // Check for the index require.NotZero(t, respW.HeaderMap.Get("X-Nomad-Index")) + require.Equal(t, http.StatusNoContent, respW.Result().StatusCode) // Check variable was deleted sv, err := rpcReadSV(s, sv1.Namespace, sv1.Path) diff --git a/nomad/encrypter.go b/nomad/encrypter.go index d56d0a4a3..ff77e6df0 100644 --- a/nomad/encrypter.go +++ b/nomad/encrypter.go @@ -298,7 +298,7 @@ func (e *Encrypter) activeKeySetLocked() (*keyset, error) { func (e *Encrypter) keysetByIDLocked(keyID string) (*keyset, error) { keyset, ok := e.keyring[keyID] if !ok { - return nil, fmt.Errorf("no such key %s in keyring", keyID) + return nil, fmt.Errorf("no such key %q in keyring", keyID) } return keyset, nil } diff --git a/nomad/mock/acl.go b/nomad/mock/acl.go index 584ddd675..71afd9007 100644 --- a/nomad/mock/acl.go +++ b/nomad/mock/acl.go @@ -38,6 +38,46 @@ func NamespacePolicy(namespace string, policy string, capabilities []string) str return policyHCL } +// NamespacePolicy is a helper for generating the policy hcl for a given +// namespace. Either policy or capabilities may be nil but not both. +func NamespacePolicyWithSecureVariables(namespace string, policy string, capabilities []string, svars map[string][]string) string { + policyHCL := fmt.Sprintf("namespace %q {", namespace) + if policy != "" { + policyHCL += fmt.Sprintf("\n\tpolicy = %q", policy) + } + if len(capabilities) != 0 { + for i, s := range capabilities { + if !strings.HasPrefix(s, "\"") { + capabilities[i] = strconv.Quote(s) + } + } + policyHCL += fmt.Sprintf("\n\tcapabilities = [%v]", strings.Join(capabilities, ",")) + } + + policyHCL += SecureVariablePolicy(svars) + policyHCL += "\n}" + return policyHCL +} + +// SecureVariablePolicy is a helper for generating the policy hcl for a given +// secure_variable block inside of a namespace. +func SecureVariablePolicy(svars map[string][]string) string { + policyHCL := "" + if len(svars) > 0 { + policyHCL = "\n\n\tsecure_variables {" + for p, c := range svars { + for i, s := range c { + if !strings.HasPrefix(s, "\"") { + c[i] = strconv.Quote(s) + } + } + policyHCL += fmt.Sprintf("\n\t\tpath %q { capabilities = [%v]}", p, strings.Join(c, ",")) + } + policyHCL += "\n\t}" + } + return policyHCL +} + // HostVolumePolicy is a helper for generating the policy hcl for a given // host-volume. Either policy or capabilities may be nil but not both. func HostVolumePolicy(vol string, policy string, capabilities []string) string { diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index cf3126b74..09c781296 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -2303,33 +2303,13 @@ func ServiceRegistrations() []*structs.ServiceRegistration { type MockSecureVariables map[string]*structs.SecureVariableDecrypted func SecureVariable() *structs.SecureVariableDecrypted { - envs := []string{"dev", "test", "prod"} - envIdx := rand.Intn(3) - env := envs[envIdx] - domain := fake.DomainName() - path := strings.ReplaceAll(env+"."+domain, ".", "/") - createIdx := uint64(rand.Intn(100) + 100) - createDT := fake.DateRange(time.Now().AddDate(0, -1, 0), time.Now()) - sv := &structs.SecureVariableDecrypted{ - SecureVariableMetadata: structs.SecureVariableMetadata{ - Path: path, - Namespace: structs.DefaultNamespace, - CreateIndex: createIdx, - ModifyIndex: createIdx, - CreateTime: createDT, - ModifyTime: createDT, - }, + return &structs.SecureVariableDecrypted{ + SecureVariableMetadata: mockSecureVariableMetadata(), Items: structs.SecureVariableItems{ "key1": "value1", "key2": "value2", }, } - // Flip a coin to see if we should return a "modified" object - if fake.Bool() { - sv.ModifyTime = fake.DateRange(sv.CreateTime, time.Now()) - sv.ModifyIndex = sv.CreateIndex + uint64(rand.Intn(100)) - } - return sv } // SecureVariables returns a random number of secure variables between min @@ -2381,34 +2361,13 @@ func (svs MockSecureVariables) List() []*structs.SecureVariableDecrypted { type MockSecureVariablesEncrypted map[string]*structs.SecureVariableEncrypted func SecureVariableEncrypted() *structs.SecureVariableEncrypted { - envs := []string{"dev", "test", "prod"} - envIdx := rand.Intn(3) - env := envs[envIdx] - domain := fake.DomainName() - path := strings.ReplaceAll(env+"."+domain, ".", "/") - // owner := fake.Person() - createIdx := uint64(rand.Intn(100) + 100) - createDT := fake.DateRange(time.Now().AddDate(0, -1, 0), time.Now()) - sv := &structs.SecureVariableEncrypted{ - SecureVariableMetadata: structs.SecureVariableMetadata{ - Path: path, - Namespace: structs.DefaultNamespace, - CreateIndex: createIdx, - ModifyIndex: createIdx, - CreateTime: createDT, - ModifyTime: createDT, - }, + return &structs.SecureVariableEncrypted{ + SecureVariableMetadata: mockSecureVariableMetadata(), SecureVariableData: structs.SecureVariableData{ KeyID: "foo", Data: []byte("foo"), }, } - // Flip a coin to see if we should return a "modified" object - if fake.Bool() { - sv.ModifyTime = fake.DateRange(sv.CreateTime, time.Now()) - sv.ModifyIndex = sv.CreateIndex + uint64(rand.Intn(100)) - } - return sv } // SecureVariables returns a random number of secure variables between min @@ -2456,3 +2415,26 @@ func (svs MockSecureVariablesEncrypted) List() []*structs.SecureVariableEncrypte } return out } + +func mockSecureVariableMetadata() structs.SecureVariableMetadata { + envs := []string{"dev", "test", "prod"} + envIdx := rand.Intn(3) + env := envs[envIdx] + domain := fake.DomainName() + + out := structs.SecureVariableMetadata{ + Namespace: "default", + Path: strings.ReplaceAll(env+"."+domain, ".", "/"), + CreateIndex: uint64(rand.Intn(100) + 100), + CreateTime: fake.DateRange(time.Now().AddDate(0, -1, 0), time.Now()).UnixNano(), + } + out.ModifyIndex = out.CreateIndex + out.ModifyTime = out.CreateTime + + // Flip a coin to see if we should return a "modified" object + if fake.Bool() { + out.ModifyTime = fake.DateRange(time.Unix(0, out.CreateTime), time.Now()).UnixNano() + out.ModifyIndex = out.CreateIndex + uint64(rand.Intn(100)) + } + return out +} diff --git a/nomad/secure_variables_endpoint.go b/nomad/secure_variables_endpoint.go index 7f42d9585..b7a9b09e6 100644 --- a/nomad/secure_variables_endpoint.go +++ b/nomad/secure_variables_endpoint.go @@ -2,6 +2,7 @@ package nomad import ( "encoding/json" + "errors" "fmt" "net/http" "strings" @@ -65,6 +66,16 @@ func (sv *SecureVariables) Upsert( mErr.Errors = append(mErr.Errors, err) continue } + if args.CheckIndex != nil { + var conflict *structs.SecureVariableDecrypted + if err := sv.validateCASUpdate(*args.CheckIndex, v, &conflict); err != nil { + if reply.Conflicts == nil { + reply.Conflicts = make([]*structs.SecureVariableDecrypted, len(args.Data)) + } + reply.Conflicts[i] = conflict + continue + } + } ev, err := sv.encrypt(v) if err != nil { mErr.Errors = append(mErr.Errors, err) @@ -72,8 +83,14 @@ func (sv *SecureVariables) Upsert( } uArgs.Data[i] = ev } + if len(reply.Conflicts) != 0 { + // This is a reply with CAS conflicts so it needs to return here + // "successfully". The caller needs to check to see if Conflicts + // is non-Nil. + return nil + } if err := mErr.ErrorOrNil(); err != nil { - return err + return &mErr } if err := sv.enforceQuota(uArgs); err != nil { @@ -116,7 +133,23 @@ func (sv *SecureVariables) Delete( return structs.ErrPermissionDenied } } + if args.CheckIndex != nil { + if err := sv.validateCASDelete(*args.CheckIndex, args.Namespace, args.Path, &reply.Conflict); err != nil { + + // If the validateCASDelete func sends back the conflict sentinel + // error value then it will have put the conflict into the reply, + // and we need to "succeed". + if err.Error() == "conflict" { + reply.Index = reply.Conflict.ModifyIndex + return nil + } + + // There are a few cases where validateCASDelete can error that + // aren't conflicts. + return err + } + } // Update via Raft. out, index, err := sv.srv.raftApply(structs.SecureVariableDeleteRequestType, args) if err != nil { @@ -468,3 +501,53 @@ func (sv *SecureVariables) authValidatePrefix(claims *structs.IdentityClaims, ns } return nil } + +func (s *SecureVariables) validateCASUpdate(cidx uint64, sv *structs.SecureVariableDecrypted, conflict **structs.SecureVariableDecrypted) error { + return s.validateCAS(cidx, sv.Namespace, sv.Path, conflict) +} + +func (s *SecureVariables) validateCASDelete(cidx uint64, namespace, path string, conflict **structs.SecureVariableDecrypted) error { + return s.validateCAS(cidx, namespace, path, conflict) +} + +func (s *SecureVariables) validateCAS(cidx uint64, namespace, path string, conflictOut **structs.SecureVariableDecrypted) error { + casConflict := errors.New("conflict") + // lookup any existing key and validate the update + snap, err := s.srv.fsm.State().Snapshot() + if err != nil { + return err + } + ws := memdb.NewWatchSet() + exist, err := snap.GetSecureVariable(ws, namespace, path) + if err != nil { + return fmt.Errorf("cas error: %w", err) + } + if exist == nil && cidx != 0 { + // return a zero value with the namespace and path applied + zeroVal := &structs.SecureVariableDecrypted{ + SecureVariableMetadata: structs.SecureVariableMetadata{ + Namespace: namespace, + Path: path, + CreateIndex: 0, + CreateTime: 0, + ModifyIndex: 0, + ModifyTime: 0, + }, + Items: nil, + } + *conflictOut = zeroVal + return casConflict + } + if exist != nil && exist.ModifyIndex != cidx { + dec, err := s.decrypt(exist) + if err != nil { + // we can't return the conflict and we will have to bail out + decErrStr := fmt.Sprintf(". Additional error decrypting conflict: %s", err) + return fmt.Errorf("cas error: requested index %v; found index %v%s", cidx, exist.ModifyIndex, decErrStr) + } + *conflictOut = dec + return casConflict + } + + return nil +} diff --git a/nomad/state/state_store_secure_variables.go b/nomad/state/state_store_secure_variables.go index 3c7b808eb..d8285f0ee 100644 --- a/nomad/state/state_store_secure_variables.go +++ b/nomad/state/state_store_secure_variables.go @@ -147,7 +147,7 @@ func (s *StateStore) upsertSecureVariableImpl(index uint64, txn *txn, sv *struct } // Setup the indexes correctly - now := time.Now().Round(0) + nowNano := time.Now().UnixNano() if existing != nil { exist := existing.(*structs.SecureVariableEncrypted) if !shouldWrite(sv, exist) { @@ -157,13 +157,13 @@ func (s *StateStore) upsertSecureVariableImpl(index uint64, txn *txn, sv *struct sv.CreateIndex = exist.CreateIndex sv.CreateTime = exist.CreateTime sv.ModifyIndex = index - sv.ModifyTime = now + sv.ModifyTime = nowNano } else { sv.CreateIndex = index - sv.CreateTime = now + sv.CreateTime = nowNano sv.ModifyIndex = index - sv.ModifyTime = now + sv.ModifyTime = nowNano } // Insert the secure variable diff --git a/nomad/structs/secure_variables.go b/nomad/structs/secure_variables.go index 126cc0d8f..9ea5fe816 100644 --- a/nomad/structs/secure_variables.go +++ b/nomad/structs/secure_variables.go @@ -1,6 +1,7 @@ package structs import ( + "bytes" "errors" "fmt" "reflect" @@ -50,10 +51,10 @@ const ( type SecureVariableMetadata struct { Namespace string Path string - CreateTime time.Time CreateIndex uint64 + CreateTime int64 ModifyIndex uint64 - ModifyTime time.Time + ModifyTime int64 } // SecureVariableEncrypted structs are returned from the Encrypter's encrypt @@ -81,15 +82,55 @@ type SecureVariableDecrypted struct { // are always encrypted and decrypted as a single unit. type SecureVariableItems map[string]string -func (sv SecureVariableData) Copy() SecureVariableData { - out := make([]byte, len(sv.Data)) - copy(out, sv.Data) - return SecureVariableData{ - Data: out, - KeyID: sv.KeyID, +// Equals checks both the metadata and items in a SecureVariableDecrypted +// struct +func (v1 SecureVariableDecrypted) Equals(v2 SecureVariableDecrypted) bool { + return v1.SecureVariableMetadata.Equals(v2.SecureVariableMetadata) && + v1.Items.Equals(v2.Items) +} + +// Equals is a convenience method to provide similar equality checking +// syntax for metadata and the SecureVariablesData or SecureVariableItems +// struct +func (sv SecureVariableMetadata) Equals(sv2 SecureVariableMetadata) bool { + return sv == sv2 +} + +// Equals performs deep equality checking on the cleartext items +// of a SecureVariableDecrypted. Uses reflect.DeepEqual +func (i1 SecureVariableItems) Equals(i2 SecureVariableItems) bool { + return reflect.DeepEqual(i1, i2) +} + +// Equals checks both the metadata and encrypted data for a +// SecureVariableEncrypted struct +func (v1 SecureVariableEncrypted) Equals(v2 SecureVariableEncrypted) bool { + return v1.SecureVariableMetadata.Equals(v2.SecureVariableMetadata) && + v1.SecureVariableData.Equals(v2.SecureVariableData) +} + +// Equals performs deep equality checking on the encrypted data part +// of a SecureVariableEncrypted +func (d1 SecureVariableData) Equals(d2 SecureVariableData) bool { + return d1.KeyID == d2.KeyID && + bytes.Equal(d1.Data, d2.Data) +} + +func (sv SecureVariableDecrypted) Copy() SecureVariableDecrypted { + return SecureVariableDecrypted{ + SecureVariableMetadata: sv.SecureVariableMetadata, + Items: sv.Items.Copy(), } } +func (sv SecureVariableItems) Copy() SecureVariableItems { + out := make(SecureVariableItems, len(sv)) + for k, v := range sv { + out[k] = v + } + return out +} + func (sv SecureVariableEncrypted) Copy() SecureVariableEncrypted { return SecureVariableEncrypted{ SecureVariableMetadata: sv.SecureVariableMetadata, @@ -97,34 +138,13 @@ func (sv SecureVariableEncrypted) Copy() SecureVariableEncrypted { } } -func (sv SecureVariableMetadata) Equals(sv2 SecureVariableMetadata) bool { - return sv == sv2 -} - -func (sv SecureVariableDecrypted) Equals(sv2 SecureVariableDecrypted) bool { - // FIXME: This should be a smarter equality check - return sv.SecureVariableMetadata.Equals(sv2.SecureVariableMetadata) && - len(sv.Items) == len(sv2.Items) && - reflect.DeepEqual(sv.Items, sv2.Items) -} - -func (sv SecureVariableDecrypted) Copy() SecureVariableDecrypted { - out := SecureVariableDecrypted{ - SecureVariableMetadata: sv.SecureVariableMetadata, - Items: make(SecureVariableItems, len(sv.Items)), +func (sv SecureVariableData) Copy() SecureVariableData { + out := make([]byte, len(sv.Data)) + copy(out, sv.Data) + return SecureVariableData{ + Data: out, + KeyID: sv.KeyID, } - for k, v := range sv.Items { - out.Items[k] = v - } - return out -} - -func (sv SecureVariableEncrypted) Equals(sv2 SecureVariableEncrypted) bool { - // FIXME: This should be a smarter equality check - return sv.SecureVariableMetadata.Equals(sv2.SecureVariableMetadata) && - sv.KeyID == sv2.KeyID && - - reflect.DeepEqual(sv.SecureVariableData, sv2.SecureVariableData) } func (sv SecureVariableDecrypted) Validate() error { @@ -140,6 +160,12 @@ func (sv *SecureVariableDecrypted) Canonicalize() { } } +// GetNamespace returns the secure variable's namespace. Used for pagination. +func (sv *SecureVariableMetadata) Copy() *SecureVariableMetadata { + var out SecureVariableMetadata = *sv + return &out +} + // GetNamespace returns the secure variable's namespace. Used for pagination. func (sv SecureVariableMetadata) GetNamespace() string { return sv.Namespace @@ -176,21 +202,26 @@ func (svq *SecureVariablesQuota) Copy() *SecureVariablesQuota { } type SecureVariablesUpsertRequest struct { - Data []*SecureVariableDecrypted + Data []*SecureVariableDecrypted + CheckIndex *uint64 WriteRequest } +func (svur *SecureVariablesUpsertRequest) SetCheckIndex(ci uint64) { + svur.CheckIndex = &ci +} + type SecureVariablesEncryptedUpsertRequest struct { Data []*SecureVariableEncrypted WriteRequest } type SecureVariablesUpsertResponse struct { + Conflicts []*SecureVariableDecrypted WriteMeta } type SecureVariablesListRequest struct { - // TODO: do we need any fields here? QueryOptions } @@ -210,11 +241,17 @@ type SecureVariablesReadResponse struct { } type SecureVariablesDeleteRequest struct { - Path string + Path string + CheckIndex *uint64 WriteRequest } +func (svdr *SecureVariablesDeleteRequest) SetCheckIndex(ci uint64) { + svdr.CheckIndex = &ci +} + type SecureVariablesDeleteResponse struct { + Conflict *SecureVariableDecrypted WriteMeta } diff --git a/nomad/structs/secure_variables_test.go b/nomad/structs/secure_variables_test.go index 73e3fcd0e..02a1da2a9 100644 --- a/nomad/structs/secure_variables_test.go +++ b/nomad/structs/secure_variables_test.go @@ -15,9 +15,9 @@ func TestStructs_SecureVariableDecrypted_Copy(t *testing.T) { Namespace: "a", Path: "a/b/c", CreateIndex: 1, - CreateTime: n, + CreateTime: n.UnixNano(), ModifyIndex: 2, - ModifyTime: n.Add(48 * time.Hour), + ModifyTime: n.Add(48 * time.Hour).UnixNano(), } sv := SecureVariableDecrypted{ SecureVariableMetadata: a,