diff --git a/command/agent/variable_endpoint.go b/command/agent/variable_endpoint.go index 4e5234b15f0a..82a3f431b63f 100644 --- a/command/agent/variable_endpoint.go +++ b/command/agent/variable_endpoint.go @@ -4,6 +4,7 @@ package agent import ( + "errors" "fmt" "net/http" "net/url" @@ -13,10 +14,11 @@ import ( "github.com/hashicorp/nomad/nomad/structs" ) -const ( - renewLockQueryParam = "renew" - acquireLockQueryParam = "acquire" - releaseLockQueryParam = "release" +var ( + renewLockQueryParam = "renew" + + acquireLockQueryParam = string(structs.VarOpLockAcquire) + releaseLockQueryParam = string(structs.VarOpLockRelease) ) func (s *HTTPServer) VariablesListRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { @@ -52,20 +54,28 @@ func (s *HTTPServer) VariableSpecificRequest(resp http.ResponseWriter, req *http case http.MethodGet: return s.variableQuery(resp, req, path) case http.MethodPut, http.MethodPost: + urlParams := req.URL.Query() + lockOperation, err := getLockOperation(urlParams) + if err != nil { + return nil, CodedError(http.StatusBadRequest, err.Error()) + } - queryParams := req.URL.Query() - _, renewLock := queryParams[renewLockQueryParam] - _, acquireLock := queryParams[acquireLockQueryParam] - _, releaseLock := queryParams[releaseLockQueryParam] + cq := req.URL.Query().Get("cas") + + if cq != "" && lockOperation != "" { + return nil, CodedError(http.StatusBadRequest, "CAS can't be used with lock operations") + } - if renewLock || acquireLock || releaseLock { - if !isOneAndOnlyOneSet(renewLock, acquireLock, releaseLock) { - return nil, CodedError(http.StatusBadRequest, "multiple lock operations") - } - return s.variableLockOperation(resp, req, queryParams) + if lockOperation == "" { + return s.variableUpsert(resp, req, path) } - return s.variableUpsert(resp, req, path) + if lockOperation == renewLockQueryParam { + return s.variableLockRenew(resp, req) + } + + return s.variableLockOperation(resp, req, path, lockOperation) + case http.MethodDelete: return s.variableDelete(resp, req, path) default: @@ -73,8 +83,7 @@ func (s *HTTPServer) VariableSpecificRequest(resp http.ResponseWriter, req *http } } -func (s *HTTPServer) variableLockOperation(resp http.ResponseWriter, req *http.Request, - operation url.Values) (interface{}, error) { +func (s *HTTPServer) variableLockRenew(resp http.ResponseWriter, req *http.Request) (interface{}, error) { // Parse the Variable var Variable structs.VariableDecrypted @@ -82,37 +91,47 @@ func (s *HTTPServer) variableLockOperation(resp http.ResponseWriter, req *http.R return nil, CodedError(http.StatusBadRequest, err.Error()) } - if operation[renewLockQueryParam][0] == renewLockQueryParam { - args := structs.VariablesRenewLockRequest{ - Path: Variable.Path, + args := structs.VariablesRenewLockRequest{ + Path: Variable.Path, - LockID: Variable.Lock.ID, - } + LockID: Variable.Lock.ID, + } - s.parseWriteRequest(req, &args.WriteRequest) + s.parseWriteRequest(req, &args.WriteRequest) - var out structs.VariablesRenewLockResponse - if err := s.agent.RPC(structs.VariablesRenewLockRPCMethod, &args, &out); err != nil { - return nil, err - } + var out structs.VariablesRenewLockResponse + if err := s.agent.RPC(structs.VariablesRenewLockRPCMethod, &args, &out); err != nil { + return nil, err + } + + return out.VarMeta, nil +} + +func (s *HTTPServer) variableLockOperation(resp http.ResponseWriter, req *http.Request, + path, operation string) (interface{}, error) { - return out.VarMeta, nil + // Parse the Variable + var Variable structs.VariableDecrypted + if err := decodeBody(req, &Variable); err != nil { + return nil, CodedError(http.StatusBadRequest, err.Error()) } // At this point, the operation can be either acquire or release, and they are // both handled by the VariablesApplyRPCMethod. args := structs.VariablesApplyRequest{ - Op: structs.VarOpSet, + Op: structs.VarOp(operation), Var: &Variable, } + Variable.Path = path + s.parseWriteRequest(req, &args.WriteRequest) var out structs.VariablesApplyResponse err := s.agent.RPC(structs.VariablesApplyRPCMethod, &args, &out) defer setIndex(resp, out.WriteMeta.Index) if err != nil { - return nil, CodedError(http.StatusInternalServerError, err.Error()) + return nil, err } if out.Conflict != nil { @@ -263,3 +282,30 @@ func parseCAS(req *http.Request) (bool, uint64, error) { func isOneAndOnlyOneSet(a, b, c bool) bool { return (a || b || c) && !a != !b != !c != !(a && b && c) } + +// getLockOperation returns the lock operation to be performed in case there is +// one. It returns error if more than one is set. +func getLockOperation(queryParams url.Values) (string, error) { + _, renewLock := queryParams[renewLockQueryParam] + _, acquireLock := queryParams[acquireLockQueryParam] + _, releaseLock := queryParams[releaseLockQueryParam] + + if !renewLock && !acquireLock && !releaseLock { + return "", nil + } + + if !isOneAndOnlyOneSet(renewLock, acquireLock, releaseLock) { + return "", errors.New("multiple lock operations") + } + + switch { + case renewLock: + return renewLockQueryParam, nil + case acquireLock: + return acquireLockQueryParam, nil + case releaseLock: + return releaseLockQueryParam, nil + default: + return "", errors.New("unspecified lock operation") + } +} diff --git a/nomad/state/state_store_variables.go b/nomad/state/state_store_variables.go index 2b1e936797fc..dfaaf3a842e2 100644 --- a/nomad/state/state_store_variables.go +++ b/nomad/state/state_store_variables.go @@ -204,6 +204,7 @@ func (s *StateStore) varSetTxn(tx WriteTxn, idx uint64, req *structs.VarApplySta if err != nil { return req.ErrorResponse(idx, fmt.Errorf("failed sve lookup: %s", err)) } + existing, _ := existingRaw.(*structs.VariableEncrypted) existingQuota, err := tx.First(TableVariablesQuotas, indexID, sv.Namespace) @@ -609,6 +610,7 @@ func (s *StateStore) VarLockRelease(idx uint64, VariableMetadata: structs.VariableMetadata{ Namespace: sv.Namespace, Path: sv.Path, + Lock: &structs.VariableLock{}, }, } return req.ConflictResponse(idx, zeroVal) @@ -618,6 +620,11 @@ func (s *StateStore) VarLockRelease(idx uint64, updated.Lock = nil updated.ModifyIndex = idx + // Avoid overwriting the variable data when releasing the lock, to prevent + // a delay release to remove customer data. + + updated.Data = sv.Data + err = s.updateVarsAndIndexTxn(tx, idx, &updated) if err != nil { req.ErrorResponse(idx, fmt.Errorf("failed lock release: %s", err)) diff --git a/nomad/structs/variables.go b/nomad/structs/variables.go index c71f9c0c0414..11ba8af28b06 100644 --- a/nomad/structs/variables.go +++ b/nomad/structs/variables.go @@ -5,6 +5,7 @@ package structs import ( "bytes" + "encoding/json" "errors" "fmt" "reflect" @@ -153,6 +154,77 @@ func (vl *VariableLock) Equal(vl2 *VariableLock) bool { return true } +// MarshalJSON implements the json.Marshaler interface and allows +// VariableLock.TTL and VariableLock.Delay to be marshaled correctly. +func (vl *VariableLock) MarshalJSON() ([]byte, error) { + type Alias VariableLock + exported := &struct { + TTL string + LockDelay string + *Alias + }{ + TTL: vl.TTL.String(), + LockDelay: vl.LockDelay.String(), + Alias: (*Alias)(vl), + } + + if vl.TTL == 0 { + exported.TTL = "" + } + + if vl.LockDelay == 0 { + exported.LockDelay = "" + } + return json.Marshal(exported) +} + +// UnmarshalJSON implements the json.Unmarshaler interface and allows +// VariableLock.TTL and VariableLock.Delay to be unmarshalled correctly. +func (vl *VariableLock) UnmarshalJSON(data []byte) (err error) { + type Alias VariableLock + aux := &struct { + TTL interface{} + LockDelay interface{} + *Alias + }{ + Alias: (*Alias)(vl), + } + + if err = json.Unmarshal(data, &aux); err != nil { + return err + } + + if aux.TTL != nil { + switch v := aux.TTL.(type) { + case string: + if v != "" { + if vl.TTL, err = time.ParseDuration(v); err != nil { + return err + } + } + case float64: + vl.TTL = time.Duration(v) + } + + } + + if aux.LockDelay != nil { + switch v := aux.LockDelay.(type) { + case string: + if v != "" { + if vl.LockDelay, err = time.ParseDuration(v); err != nil { + return err + } + } + case float64: + vl.LockDelay = time.Duration(v) + } + + } + + return nil +} + // Copy creates a deep copy of the variable lock. This copy can then be safely // modified. It handles nil objects. func (vl *VariableLock) Copy() *VariableLock { @@ -192,7 +264,7 @@ func (vl *VariableLock) Validate() error { mErr = multierror.Append(mErr, errInvalidTTL) } - return mErr + return mErr.ErrorOrNil() } func (vi VariableItems) Size() uint64 { @@ -303,6 +375,7 @@ func (vd VariableDecrypted) Validate() error { if len(vd.Items) == 0 { return errors.New("empty variables are invalid") } + if vd.Items.Size() > maxVariableSize { return errors.New("variables are limited to 64KiB in total size") } @@ -311,6 +384,10 @@ func (vd VariableDecrypted) Validate() error { return err } + if vd.Lock != nil { + return vd.Lock.Validate() + } + return nil } diff --git a/nomad/variables_endpoint.go b/nomad/variables_endpoint.go index 4a7cb2830b06..2990385973cf 100644 --- a/nomad/variables_endpoint.go +++ b/nomad/variables_endpoint.go @@ -93,11 +93,8 @@ func (sv *Variables) Apply(args *structs.VariablesApplyRequest, reply *structs.V return err } - // The read permission modify the way the response is populated. If ACL is not - // used, read permission is granted by default. - var canRead bool = true + // IF ACL is being used, if aclObj != nil { - canRead = hasReadPermission(aclObj, args.Var.Namespace, args.Var.Path) err := hasOperationPermissions(aclObj, args.Var.Namespace, args.Var.Path, args.Op) if err != nil { return err @@ -112,7 +109,8 @@ func (sv *Variables) Apply(args *structs.VariablesApplyRequest, reply *structs.V var ev *structs.VariableEncrypted switch args.Op { - case structs.VarOpSet, structs.VarOpCAS: + case structs.VarOpSet, structs.VarOpCAS, structs.VarOpLockAcquire, + structs.VarOpLockRelease: ev, err = sv.encrypt(args.Var) if err != nil { return fmt.Errorf("variable error: encrypt: %w", err) @@ -129,15 +127,6 @@ func (sv *Variables) Apply(args *structs.VariablesApplyRequest, reply *structs.V ModifyIndex: args.Var.ModifyIndex, }, } - - case structs.VarOpLockAcquire, structs.VarOpLockRelease: - ev, err = sv.encrypt(args.Var) - if err != nil { - return fmt.Errorf("variable error: encrypt: %w", err) - } - - now := time.Now().UnixNano() - ev.ModifyTime = now } // Make a SVEArgs @@ -153,18 +142,11 @@ func (sv *Variables) Apply(args *structs.VariablesApplyRequest, reply *structs.V return fmt.Errorf("raft apply failed: %w", err) } - r, err := sv.makeVariablesApplyResponse(args, out.(*structs.VarApplyStateResponse), canRead) + r, err := sv.makeVariablesApplyResponse(args, out.(*structs.VarApplyStateResponse), aclObj) if err != nil { return err } - // Verify the caller is providing the correct lockID, meaning it is the - // lock holder and has access to the lock information or is management call. - // If not, remove the lock information from response. - if isVarLocked(args, r) || !aclObj.IsManagement() { - r.Output.VariableMetadata.Lock = nil - } - *reply = *r reply.Index = index @@ -210,16 +192,8 @@ func hasOperationPermissions(aclObj *acl.ACL, namespace, path string, op structs func canonicalizeAndValidate(args *structs.VariablesApplyRequest) error { switch args.Op { case structs.VarOpLockAcquire: - // If the variable doesn't have an ID, it means a lock is - // being created on a variable that doesn't exist, the variable needs to - // be created. - if args.Var.GetID() == "" { - args.Var.Canonicalize() - } - - if err := args.Var.ValidateForLock(); err != nil { - return err - } + args.Var.Canonicalize() + return args.Var.ValidateForLock() case structs.VarOpSet, structs.VarOpCAS: args.Var.Canonicalize() @@ -247,7 +221,7 @@ func canonicalizeAndValidate(args *structs.VariablesApplyRequest) error { // VariableDataItems func (sv *Variables) makeVariablesApplyResponse( req *structs.VariablesApplyRequest, eResp *structs.VarApplyStateResponse, - canRead bool) (*structs.VariablesApplyResponse, error) { + aclObj *acl.ACL) (*structs.VariablesApplyResponse, error) { out := structs.VariablesApplyResponse{ Op: eResp.Op, @@ -257,6 +231,15 @@ func (sv *Variables) makeVariablesApplyResponse( WriteMeta: eResp.WriteMeta, } + // The read permission modify the way the response is populated. If ACL is not + // used, read permission is granted by default. + var canRead bool = true + var isManagement = true + if aclObj != nil { + canRead = hasReadPermission(aclObj, eResp.Conflict.Namespace, eResp.Conflict.Path) + isManagement = aclObj.IsManagement() + } + if eResp.IsOk() { if eResp.WrittenSVMeta != nil { // The writer is allowed to read their own write @@ -264,7 +247,15 @@ func (sv *Variables) makeVariablesApplyResponse( VariableMetadata: *eResp.WrittenSVMeta, Items: req.Var.Items.Copy(), } + + // Verify the caller is providing the correct lockID, meaning it is the + // lock holder and has access to the lock information or is a management call. + // If locked, remove the lock information from response. + if isVarLocked(req, eResp.WrittenSVMeta) || !isManagement { + out.Output.VariableMetadata.Lock = nil + } } + return &out, nil } @@ -319,7 +310,7 @@ func (sv *Variables) Read(args *structs.VariablesReadRequest, reply *structs.Var defer metrics.MeasureSince([]string{"nomad", "variables", "read"}, time.Now()) - _, _, err := sv.handleMixedAuthEndpoint(args.QueryOptions, + aclObj, _, err := sv.handleMixedAuthEndpoint(args.QueryOptions, acl.PolicyRead, args.Path) if err != nil { return err @@ -342,7 +333,12 @@ func (sv *Variables) Read(args *structs.VariablesReadRequest, reply *structs.Var if err != nil { return err } + ov := dv.Copy() + if aclObj != nil && !aclObj.IsManagement() { + ov.Lock = nil + } + reply.Data = &ov reply.Index = out.ModifyIndex } else { @@ -431,6 +427,11 @@ func (sv *Variables) List( func(raw interface{}) error { sv := raw.(*structs.VariableEncrypted) svStub := sv.VariableMetadata + + if aclObj != nil && !aclObj.IsManagement() { + sv.VariableMetadata.Lock = nil + } + svs = append(svs, &svStub) return nil }) @@ -724,7 +725,11 @@ func (sv *Variables) RenewLock(args *structs.VariablesRenewLockRequest, reply *s return nil } -func isVarLocked(req *structs.VariablesApplyRequest, eResp *structs.VariablesApplyResponse) bool { - return req.Var.Lock == nil || eResp.Output.VariableMetadata.Lock == nil || - req.Var.Lock.ID != eResp.Output.VariableMetadata.Lock.ID +func isVarLocked(req *structs.VariablesApplyRequest, respVarMeta *structs.VariableMetadata) bool { + reqLock := req.Var.VariableMetadata.Lock + savedLock := respVarMeta.Lock + + return reqLock != nil && + savedLock != nil && + reqLock.ID != savedLock.ID }