diff --git a/pkg/gateway/objects.go b/pkg/gateway/objects.go index 113a258..fa95e20 100644 --- a/pkg/gateway/objects.go +++ b/pkg/gateway/objects.go @@ -3,7 +3,6 @@ package gateway import ( "bytes" "encoding/json" - "fmt" "net/http" "github.com/go-chi/chi/v5" @@ -65,7 +64,7 @@ func (o *ObjectHandler) get(w http.ResponseWriter, r *http.Request) { } func (o *ObjectHandler) apply(w http.ResponseWriter, r *http.Request) { - manager := r.Header.Get(hz.HeaderFieldManager) + manager := r.Header.Get(hz.HeaderApplyFieldManager) client := hz.NewClient( o.Conn, hz.WithClientSessionFromRequest(r), @@ -80,7 +79,6 @@ func (o *ObjectHandler) apply(w http.ResponseWriter, r *http.Request) { ) return } - fmt.Println("GENERIC OBJECT: ", obj) if err := client.Apply(r.Context(), hz.WithApplyObject(obj)); err != nil { httpError(w, err) return diff --git a/pkg/hz/client.go b/pkg/hz/client.go index b41e74c..672e1d4 100644 --- a/pkg/hz/client.go +++ b/pkg/hz/client.go @@ -23,9 +23,10 @@ const ( ) const ( - HeaderStatus = "Hz-Status" - HeaderAuthorization = "Hz-Authorization" - HeaderFieldManager = "Hz-Field-Manager" + HeaderStatus = "Hz-Status" + HeaderAuthorization = "Hz-Authorization" + HeaderApplyFieldManager = "Hz-Apply-Field-Manager" + HeaderApplyForceConflicts = "Hz-Apply-Force-Conflicts" ) const ( @@ -269,7 +270,6 @@ func SessionFromRequest(req *http.Request) string { if sessionCookie, err := req.Cookie(CookieSession); err == nil { return sessionCookie.Value } - fmt.Println("REQ HEADER:", req.Header.Get(HeaderAuthorization)) return req.Header.Get(HeaderAuthorization) } @@ -476,6 +476,7 @@ type applyOptions struct { object Objecter data []byte objectKey ObjectKeyer + force bool } func WithApplyObject(object Objecter) ApplyOption { @@ -497,6 +498,12 @@ func WithApplyKey(key ObjectKeyer) ApplyOption { } } +func WithApplyForce(force bool) ApplyOption { + return func(ao *applyOptions) { + ao.force = force + } +} + func (c Client) Apply( ctx context.Context, opts ...ApplyOption, @@ -526,27 +533,26 @@ func (c Client) Apply( if err != nil { return fmt.Errorf("marshalling object: %w", err) } - key, err = keyFromObjectStrict(ao.object) + key, err = KeyFromObjectConcrete(ao.object) if err != nil { return fmt.Errorf("invalid object: %w", err) } } if ao.objectKey != nil { var err error - key, err = keyFromObjectStrict(ao.objectKey) + key, err = KeyFromObjectConcrete(ao.objectKey) if err != nil { return fmt.Errorf("invalid object: %w", err) } } - fmt.Println("key:", key) - msg := nats.NewMsg( c.SubjectPrefix() + fmt.Sprintf( SubjectStoreApply, key, ), ) - msg.Header.Set(HeaderFieldManager, c.Manager) + msg.Header.Set(HeaderApplyFieldManager, c.Manager) + msg.Header.Set(HeaderApplyForceConflicts, strconv.FormatBool(ao.force)) msg.Header.Set(HeaderAuthorization, c.Session) msg.Data = ao.data ctx, cancel := context.WithTimeout(ctx, time.Second) diff --git a/pkg/hz/error.go b/pkg/hz/error.go index 06794d5..384cef2 100644 --- a/pkg/hz/error.go +++ b/pkg/hz/error.go @@ -46,6 +46,33 @@ func ErrorFromHTTP(resp *http.Response) error { } } +// ErrorWrap takes an error and checks if it is an [Error]. +// If it is, it will make a copy of the [Error], add the given message and +// return it. The status will remain the same. +// +// If it is not an [Error], it will wrap the given error in an [Error] with the +// given status and message. +func ErrorWrap( + err error, + status int, + message string, +) error { + if err == nil { + return nil + } + var hErr *Error + if errors.As(err, &hErr) { + return &Error{ + Status: hErr.Status, + Message: fmt.Sprintf("%s: %s", message, hErr.Message), + } + } + return &Error{ + Status: status, + Message: fmt.Sprintf("%s: %s", message, err.Error()), + } +} + // respondError responds to a NATS message with an error. // It expects the err to be an *Error and will use the status and message for // the response. diff --git a/pkg/hz/httpclient.go b/pkg/hz/httpclient.go index 3e6baab..9a80dd6 100644 --- a/pkg/hz/httpclient.go +++ b/pkg/hz/httpclient.go @@ -161,7 +161,7 @@ func (c *HTTPClient) Apply(ctx context.Context, opts ...HTTPApplyOption) error { req.Header.Add("Content-Type", "application/json") req.Header.Add(HeaderAuthorization, c.Session) - req.Header.Add(HeaderFieldManager, c.Manager) + req.Header.Add(HeaderApplyFieldManager, c.Manager) resp, err := http.DefaultClient.Do(req) if err != nil { diff --git a/pkg/hz/managedfields.go b/pkg/hz/managedfields.go index 3e41844..8538465 100644 --- a/pkg/hz/managedfields.go +++ b/pkg/hz/managedfields.go @@ -4,7 +4,7 @@ import ( "encoding/json" "fmt" - "github.com/verifa/horizon/pkg/managedfields" + "github.com/verifa/horizon/pkg/internal/managedfields" ) // ExtractManagedFields creates an object containing the fields managed by the diff --git a/pkg/hz/managedfields_test.go b/pkg/hz/managedfields_test.go index 73401ca..f257822 100644 --- a/pkg/hz/managedfields_test.go +++ b/pkg/hz/managedfields_test.go @@ -7,7 +7,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/verifa/horizon/pkg/hz" - "github.com/verifa/horizon/pkg/managedfields" + "github.com/verifa/horizon/pkg/internal/managedfields" tu "github.com/verifa/horizon/pkg/testutil" ) diff --git a/pkg/hz/object.go b/pkg/hz/object.go index f373926..1b7bc26 100644 --- a/pkg/hz/object.go +++ b/pkg/hz/object.go @@ -7,7 +7,7 @@ import ( "strings" "time" - "github.com/verifa/horizon/pkg/managedfields" + "github.com/verifa/horizon/pkg/internal/managedfields" ) type Objecter interface { @@ -28,6 +28,14 @@ type ObjectKeyer interface { ObjectGroup() string } +// KeyFromObject takes an ObjectKeyer and returns a string key. +// Any empty fields in the ObjectKeyer are replaced with "*" which works well +// for nats subjects to list objects. +// +// If performing an action on a specific object (e.g. get, create, apply) the +// key cannot contain "*". +// In this case you can use [KeyFromObjectConcrete] which makes sure the +// ObjectKeyer is concrete. func KeyFromObject(obj ObjectKeyer) string { account := "*" if obj.ObjectAccount() != "" { @@ -54,7 +62,11 @@ func KeyFromObject(obj ObjectKeyer) string { ) } -func keyFromObjectStrict(obj ObjectKeyer) (string, error) { +// KeyFromObjectConcrete takes an ObjectKeyer and returns a string key. +// It returns an error if any of the fields are empty. +// This is useful when you want to ensure the key is concrete when performing +// operations on specific objects (e.g. get, create, apply). +func KeyFromObjectConcrete(obj ObjectKeyer) (string, error) { var errs error if obj.ObjectAccount() == "" { errs = errors.Join(errs, fmt.Errorf("account is required")) diff --git a/pkg/hzctl/login/login.go b/pkg/hzctl/login/login.go index 15fc9fd..e78b466 100644 --- a/pkg/hzctl/login/login.go +++ b/pkg/hzctl/login/login.go @@ -44,7 +44,6 @@ func Login(ctx context.Context, req LoginRequest) (*LoginResponse, error) { // Add return_url to login request. form.Add("return_url", returnURL) loginURL.RawQuery = form.Encode() - fmt.Println("loginURL: ", loginURL.String()) if err := openBrowser(loginURL.String()); err != nil { return nil, fmt.Errorf("opening browser: %w", err) diff --git a/pkg/managedfields/extract.go b/pkg/internal/managedfields/extract.go similarity index 100% rename from pkg/managedfields/extract.go rename to pkg/internal/managedfields/extract.go diff --git a/pkg/managedfields/fields.go b/pkg/internal/managedfields/fields.go similarity index 75% rename from pkg/managedfields/fields.go rename to pkg/internal/managedfields/fields.go index 9042df0..959b639 100644 --- a/pkg/managedfields/fields.go +++ b/pkg/internal/managedfields/fields.go @@ -20,26 +20,50 @@ func (m ManagedFields) FieldManager(manager string) (FieldManager, bool) { return FieldManager{}, false } +type FieldsType string + +const ( + FieldsTypeV1 FieldsType = "FieldsV1" +) + +// FieldManager is a manager of fields for a given object. +// An object can have multiple field managers, and those field managers make up +// the managed fields for the object. type FieldManager struct { + // Manager is the unique name of the manager. Manager string `json:"manager" cue:"=~\"^[a-zA-Z0-9-_]+$\""` - // Operation Operation `json:"operation" cue:"=~\"^[a-zA-Z0-9-_]+$\""` - // Time time.Time `json:"time" cue:",opt"` - FieldsType string `json:"fieldsType" cue:"=~\"^[a-zA-Z0-9-_]+$\""` - FieldsV1 FieldsV1 `json:"fieldsV1"` + // FieldsType is the type of fields that are managed. + // Only supported type is right now is "FieldsV1". + FieldsType FieldsType `json:"fieldsType" cue:"=~\"^[a-zA-Z0-9-_]+$\""` + // FieldsV1 is the actual fields that are managed. + FieldsV1 FieldsV1 `json:"fieldsV1"` } +// FieldsV1 is the actual fields that are managed. type FieldsV1 struct { + // Parent is a pointer to the parent field. + // It is only used when creating and operating on the managed fields, and + // not stored together with the object. Parent *FieldsV1Step `json:"-"` - // Fields = Object + // Fields represents an object, and all of its fields. Fields map[FieldsV1Key]FieldsV1 `json:"-"` - // Elements = Array + // Elements represents an array. + // To allow managing indexes of an array, we use a key (not a numerical + // index) for the array index. + // + // The fancy term for this is "associative list". Elements map[FieldsV1Key]FieldsV1 `json:"-"` } +// IsLeaf returns true if the field is a leaf node and does not have any fields +// (object) or elements (array). func (f FieldsV1) IsLeaf() bool { return len(f.Fields) == 0 && len(f.Elements) == 0 } +// Path constructs a path from this node to the root. +// It only works if the parent is set, which is only the case when creating a +// [FieldsV1]. func (f FieldsV1) Path() FieldsV1Path { if f.Parent == nil { return FieldsV1Path{} @@ -47,6 +71,7 @@ func (f FieldsV1) Path() FieldsV1Path { return append(f.Parent.Field.Path(), *f.Parent) } +// FieldsV1Path is a series of steps from a node (root) to a leaf node. type FieldsV1Path []FieldsV1Step func (p FieldsV1Path) String() string { @@ -73,6 +98,8 @@ func (s FieldsV1Step) String() string { return strings.Join(steps, ".") } +// FieldsV1Key represents a key in a FieldsV1 object. +// It can be either an object key (string) or an array (key-value). type FieldsV1Key struct { Type FieldsV1KeyType `json:"-"` Key string `json:"-"` @@ -214,11 +241,3 @@ func (f FieldsV1Key) String() string { } return fmt.Sprintf("{%s:%s}", f.Key, f.Value) } - -type Operation string - -const ( - OperationCreate Operation = "Create" - OperationUpdate Operation = "Update" - OperationApply Operation = "Apply" -) diff --git a/pkg/managedfields/managedfields.go b/pkg/internal/managedfields/managedfields.go similarity index 100% rename from pkg/managedfields/managedfields.go rename to pkg/internal/managedfields/managedfields.go diff --git a/pkg/managedfields/managedfields_test.go b/pkg/internal/managedfields/managedfields_test.go similarity index 100% rename from pkg/managedfields/managedfields_test.go rename to pkg/internal/managedfields/managedfields_test.go diff --git a/pkg/internal/managedfields/merge.go b/pkg/internal/managedfields/merge.go new file mode 100644 index 0000000..76f6dea --- /dev/null +++ b/pkg/internal/managedfields/merge.go @@ -0,0 +1,352 @@ +package managedfields + +import ( + "fmt" + "slices" + "strings" +) + +var _ (error) = (*Conflict)(nil) + +type Conflict struct { + // Fields is a list of fields that are in conflict. + Fields []FieldsV1 +} + +func (c *Conflict) Error() string { + conflicts := make([]string, len(c.Fields)) + for i, f := range c.Fields { + conflicts[i] = f.Path().String() + } + return fmt.Sprintf( + "conflicting fields: [%s]", + strings.Join(conflicts, ", "), + ) +} + +func MergeManagedFields( + managedFields []FieldManager, + reqFM FieldManager, + force bool, +) (MergeResult, error) { + mr := MergeResult{} + conflicts := Conflict{} + for i, mgrs := range managedFields { + // Don't compare the same manager (if it exists). + // That's just asking for trouble (and conflicts). + if mgrs.Manager == reqFM.Manager { + continue + } + newFields := conflictOrForceOverrideFields( + &conflicts, + mgrs.FieldsV1, + reqFM.FieldsV1, + force, + ) + mgrs.FieldsV1 = newFields + managedFields[i] = mgrs + } + // If any managers are now "empty" (i.e. they own no fields or elements), + // remove them. + managedFields = slices.DeleteFunc( + managedFields, + func(mgr FieldManager) bool { + return mgr.FieldsV1.IsLeaf() + }, + ) + if len(conflicts.Fields) > 0 { + return MergeResult{}, &conflicts + } + // fieldsIndex returns the index of the field manager in the managedFields. + fieldsIndex := func(mgFields []FieldManager, req FieldManager) int { + for i, mgrs := range mgFields { + if mgrs.Manager == req.Manager { + return i + } + } + return -1 + } + index := fieldsIndex(managedFields, reqFM) + if index >= 0 { + fieldsDiff(&mr, managedFields[index].FieldsV1, reqFM.FieldsV1) + // Overwrite the existing field manager. + managedFields[index] = reqFM + } else { + managedFields = append(managedFields, reqFM) + } + mr.ManagedFields = managedFields + return mr, nil +} + +type MergeResult struct { + // ManagedFields is the updated list of field managers after the merge. + ManagedFields []FieldManager + // Removed contains a list of fields that were previously owned by the field + // manager and have been removed in this merge request. + Removed []FieldsV1 +} + +func conflictOrForceOverrideFields( + conflicts *Conflict, + old FieldsV1, + req FieldsV1, + force bool, +) FieldsV1 { + // Object values. + for key, value := range req.Fields { + // If the key exists and the field is a leaf (no children), then we have + // a conflict (unless force). + // If the key exists, but it is not a leaf there is no conflict and we + // need to recurse. + if subField, ok := old.Fields[key]; ok { + if value.IsLeaf() { + if !force { + conflicts.Fields = append(conflicts.Fields, value) + break + } + // If force is true, remove ownership of the field from old. + delete(old.Fields, key) + break + } + subField = conflictOrForceOverrideFields( + conflicts, + subField, + value, + force, + ) + // After traversing subField, if it is not a leaf (i.e. it has no + // fields or elements) we want to remove it altogether. + if subField.IsLeaf() { + delete(old.Fields, key) + continue + } + old.Fields[key] = subField + } + } + // Same as for above but with elements (arrays). + for key, value := range req.Elements { + if subField, ok := old.Elements[key]; ok { + if value.IsLeaf() { + if !force { + conflicts.Fields = append(conflicts.Fields, value) + break + } + // If force is true, remove ownership of the field from old. + delete(old.Elements, key) + break + } + subField = conflictOrForceOverrideFields( + conflicts, + subField, + value, + force, + ) + // After traversing subField, if it is not a leaf (i.e. it has no + // fields or elements) we want to remove it altogether. + if subField.IsLeaf() { + delete(old.Fields, key) + continue + } + old.Elements[key] = subField + } + } + return old +} + +func fieldsDiff(mr *MergeResult, oldFields, newFields FieldsV1) { + // Check diff in fields (objects). + for oldKey, oldValue := range oldFields.Fields { + newValue, ok := newFields.Fields[oldKey] + if !ok { + mr.Removed = append(mr.Removed, oldValue) + continue + } + fieldsDiff(mr, oldValue, newValue) + } + // Check diff in elements (arrays). + for oldKey, oldValue := range oldFields.Elements { + newValue, ok := newFields.Elements[oldKey] + if !ok { + mr.Removed = append(mr.Removed, oldValue) + continue + } + fieldsDiff(mr, oldValue, newValue) + } +} + +func PurgeRemovedFields( + obj map[string]interface{}, + removed []FieldsV1, +) error { + for _, field := range removed { + if err := purgeRemovedFieldsObject(obj, field.Path()); err != nil { + return err + } + } + return nil +} + +func purgeRemovedFieldsObject( + obj map[string]interface{}, + path []FieldsV1Step, +) error { + step := path[0] + if step.Key.Type != FieldsV1KeyObject { + return fmt.Errorf( + "expected array but got object at %s", + step.String(), + ) + } + + stepObj, ok := obj[step.Key.Key] + if !ok { + return fmt.Errorf( + "key %q not found at %q", + step.Key.Key, + step.String(), + ) + } + if len(path) == 1 { + delete(obj, step.Key.Key) + return nil + } + nextStep := path[1] + if nextStep.Key.Type == FieldsV1KeyObject { + v, ok := stepObj.(map[string]interface{}) + if !ok { + return fmt.Errorf( + "expected map[string]interface{}, got %T at %q", + stepObj, + step.String(), + ) + } + if err := purgeRemovedFieldsObject(v, path[1:]); err != nil { + return err + } + return nil + } + + if _, ok := stepObj.([]interface{}); !ok { + return fmt.Errorf( + "expected []interface{}, got %T at %q", + stepObj, + step.String(), + ) + } + + arrayVal, err := purgeRemovedFieldsArray(stepObj.([]interface{}), path[1:]) + if err != nil { + return err + } + obj[step.Key.Key] = arrayVal + + return nil +} + +func purgeRemovedFieldsArray( + obj []interface{}, + path []FieldsV1Step, +) ([]interface{}, error) { + // Get the next step in the path and find the index of the array element. + step := path[0] + if step.Key.Type == FieldsV1KeyObject { + return nil, fmt.Errorf( + "expected array but got object at %q", + step.String(), + ) + } + index := FindIndexArrayByKey(obj, step.Key) + if index == -1 { + return nil, fmt.Errorf( + "key %q not found at %q", + step.Key.String(), + step.String(), + ) + } + // If there are no more steps in the path, we remove the found element from + // the array. + if len(path) == 1 { + // Remove the element from the array. + return append(obj[:index], obj[index+1:]...), nil + } + // If there more steps in the path, we need to recurse. + nextObj, ok := obj[index].(map[string]interface{}) + if !ok { + return nil, fmt.Errorf( + "expected map[string]interface{}, got %T at %q", + obj[index], + step.String(), + ) + } + if err := purgeRemovedFieldsObject(nextObj, path[1:]); err != nil { + return nil, err + } + return obj, nil +} + +// MergeObjects merges the src map into the dst map according to the fields. +// It is expected that the fields have been generated from the src object/data. +func MergeObjects( + dst map[string]interface{}, + src map[string]interface{}, + fields FieldsV1, +) { + mergeObjects(dst, src, fields) +} + +func mergeObjects( + dst map[string]interface{}, + src map[string]interface{}, + fields FieldsV1, +) { + for key, subFields := range fields.Fields { + // If the field is a leaf, we can just copy the value from src to dst. + // Else, we need to recurse. + if subFields.IsLeaf() { + dst[key.Key] = src[key.Key] + continue + } + dstField, ok := dst[key.Key] + if !ok { + dst[key.Key] = src[key.Key] + continue + } + if len(subFields.Fields) > 0 { + mergeObjects( + dstField.(map[string]interface{}), + src[key.Key].(map[string]interface{}), + subFields, + ) + } + if len(subFields.Elements) > 0 { + dstField = mergeArray( + dstField.([]interface{}), + src[key.Key].([]interface{}), + subFields, + ) + } + dst[key.Key] = dstField + } +} + +func mergeArray( + dst []interface{}, + src []interface{}, + fields FieldsV1, +) []interface{} { + for key, subFields := range fields.Elements { + dstIndex := FindIndexArrayByKey(dst, key) + srcIndex := FindIndexArrayByKey(src, key) + // If the field is not found in dst, we can just append the value from + // src to dst. + if dstIndex == -1 { + dst = append(dst, src[srcIndex]) + continue + } + dstObj := dst[dstIndex].(map[string]interface{}) + srcObj := src[srcIndex].(map[string]interface{}) + mergeObjects(dstObj, srcObj, subFields) + dst[dstIndex] = dstObj + } + return dst +} diff --git a/pkg/managedfields/merge_test.go b/pkg/internal/managedfields/merge_test.go similarity index 75% rename from pkg/managedfields/merge_test.go rename to pkg/internal/managedfields/merge_test.go index 9187088..67e772e 100644 --- a/pkg/managedfields/merge_test.go +++ b/pkg/internal/managedfields/merge_test.go @@ -2,7 +2,6 @@ package managedfields import ( "encoding/json" - "fmt" "testing" tu "github.com/verifa/horizon/pkg/testutil" @@ -10,11 +9,13 @@ import ( func TestMergeManagedFields(t *testing.T) { type test struct { - name string - managedFields string - merge string - expConflict func(FieldsV1) []FieldsV1 - expRemoved func([]FieldManager) []FieldsV1 + name string + managedFields string + merge string + force bool + expManagedFields string + expConflict func(FieldsV1) []FieldsV1 + expRemoved func([]FieldManager) []FieldsV1 } tests := []test{ { @@ -42,6 +43,29 @@ func TestMergeManagedFields(t *testing.T) { } } }`, + expManagedFields: `[ + { + "manager": "m1", + "fieldsV1": { + "f:metadata": { + "f:name": {}, + "f:labels": { + "f:app": {} + } + } + } + }, + { + "manager": "m2", + "fieldsV1": { + "f:metadata": { + "f:labels": { + "f:test": {} + } + } + } + } + ]`, expConflict: func(fields FieldsV1) []FieldsV1 { return nil }, expRemoved: func(fms []FieldManager) []FieldsV1 { return nil }, }, @@ -65,6 +89,24 @@ func TestMergeManagedFields(t *testing.T) { } } }`, + expManagedFields: `[ + { + "manager": "m1", + "fieldsV1": { + "f:slice": { + "k:{\"id\":\"1\"}": {} + } + } + }, + { + "manager": "m2", + "fieldsV1": { + "f:slice": { + "k:{\"id\":\"2\"}": {} + } + } + } + ]`, expConflict: func(fields FieldsV1) []FieldsV1 { return nil }, expRemoved: func(fms []FieldManager) []FieldsV1 { return nil }, }, @@ -91,6 +133,16 @@ func TestMergeManagedFields(t *testing.T) { } } }`, + expManagedFields: `[ + { + "manager": "m1", + "fieldsV1": { + "f:metadata": { + "f:name": {} + } + } + } + ]`, expConflict: func(fields FieldsV1) []FieldsV1 { return nil }, expRemoved: func(fms []FieldManager) []FieldsV1 { return []FieldsV1{ @@ -119,6 +171,16 @@ func TestMergeManagedFields(t *testing.T) { } } }`, + expManagedFields: `[ + { + "manager": "m1", + "fieldsV1": { + "f:slice": { + "k:{\"id\":\"1\"}": {} + } + } + } + ]`, expConflict: func(fields FieldsV1) []FieldsV1 { return nil }, expRemoved: func(fms []FieldManager) []FieldsV1 { return []FieldsV1{ @@ -157,6 +219,40 @@ func TestMergeManagedFields(t *testing.T) { }, expRemoved: func(fms []FieldManager) []FieldsV1 { return nil }, }, + { + name: "conflict object force", + managedFields: `[ + { + "manager": "m1", + "fieldsV1": { + "f:metadata": { + "f:name": {} + } + } + } + ]`, + merge: `{ + "manager": "m2", + "fieldsV1": { + "f:metadata": { + "f:name": {} + } + } + }`, + expManagedFields: `[ + { + "manager": "m2", + "fieldsV1": { + "f:metadata": { + "f:name": {} + } + } + } + ]`, + force: true, + expConflict: func(fields FieldsV1) []FieldsV1 { return nil }, + expRemoved: func(fms []FieldManager) []FieldsV1 { return nil }, + }, { name: "conflict array", managedFields: `[ @@ -206,7 +302,7 @@ func TestMergeManagedFields(t *testing.T) { } } expRm := tc.expRemoved(managedFields) - result, err := MergeManagedFields(managedFields, merge) + result, err := MergeManagedFields(managedFields, merge, tc.force) tu.AssertEqual(t, expErr, err, cmpOptIgnoreParent) tu.AssertEqual( t, @@ -214,68 +310,31 @@ func TestMergeManagedFields(t *testing.T) { result.Removed, cmpOptIgnoreParent, ) + if err == nil { + var expManagedFields []FieldManager + err := json.Unmarshal( + []byte(tc.expManagedFields), + &expManagedFields, + ) + tu.AssertNoError(t, err, "parsing exp managed fields json") + tu.AssertEqual( + t, + expManagedFields, + result.ManagedFields, + cmpOptIgnoreParent, + ) + } }) } - mf := []FieldManager{ - { - Manager: "m1", - FieldsV1: FieldsV1{ - Fields: map[FieldsV1Key]FieldsV1{ - fkey("metadata"): { - Fields: map[FieldsV1Key]FieldsV1{ - fkey("name"): {}, - }, - }, - fkey("spec"): { - Fields: map[FieldsV1Key]FieldsV1{ - fkey("replicas"): {}, - }, - }, - }, - }, - }, - { - Manager: "m2", - FieldsV1: FieldsV1{ - Fields: map[FieldsV1Key]FieldsV1{ - fkey("metadata"): { - Fields: map[FieldsV1Key]FieldsV1{ - fkey("name"): {}, - }, - }, - fkey("spec"): { - Fields: map[FieldsV1Key]FieldsV1{ - fkey("field"): {}, - }, - }, - }, - }, - }, - } - - m2 := FieldManager{ - Manager: "m2", - FieldsV1: FieldsV1{ - Fields: map[FieldsV1Key]FieldsV1{ - fkey("metadata"): { - Fields: map[FieldsV1Key]FieldsV1{ - fkey("name"): {}, - }, - }, - }, - }, - } - - result, _ := MergeManagedFields(mf, m2) - fmt.Println(result.Removed) } func TestMergeObjects(t *testing.T) { type test struct { - name string - dst string - src string - exp string + name string + dst string + src string + fields string + exp string } tests := []test{ { @@ -296,6 +355,14 @@ func TestMergeObjects(t *testing.T) { } } }`, + fields: `{ + "f:metadata": { + "f:name": {}, + "f:labels": { + "f:test": {} + } + } + }`, exp: `{ "metadata": { "name": "test", @@ -349,6 +416,25 @@ func TestMergeObjects(t *testing.T) { } ] }`, + fields: `{ + "f:slice": {}, + "f:objects": { + "k:{\"id\":\"1\"}": { + "f:field": {} + }, + "k:{\"id\":\"2\"}": { + "f:field": {} + } + }, + "f:nested": { + "k:{\"id\":\"1\"}": { + "f:slice": {}, + "f:object": { + "f:b": {} + } + } + } + }`, exp: `{ "slice": ["overwrite"], "objects":[ @@ -404,6 +490,22 @@ func TestMergeObjects(t *testing.T) { ] } }`, + fields: `{ + "f:metadata": { + "f:name": {}, + "f:labels": { + "f:test": {} + } + }, + "f:spec": { + "f:replicas": {}, + "f:objslice": { + "k:{\"id\":\"2\"}": { + "f:field": {} + } + } + } + }`, exp: `{ "metadata": { "name": "test", @@ -431,7 +533,10 @@ func TestMergeObjects(t *testing.T) { tu.AssertNoError(t, err, "parsing src json") err = json.Unmarshal([]byte(tc.exp), &exp) tu.AssertNoError(t, err, "parsing exp json") - MergeObjects(dst, src) + var fields FieldsV1 + err = json.Unmarshal([]byte(tc.fields), &fields) + tu.AssertNoError(t, err, "parsing fields json") + MergeObjects(dst, src, fields) tu.AssertEqual(t, exp, dst) }) } diff --git a/pkg/managedfields/merge.go b/pkg/managedfields/merge.go deleted file mode 100644 index ad58a62..0000000 --- a/pkg/managedfields/merge.go +++ /dev/null @@ -1,313 +0,0 @@ -package managedfields - -import ( - "fmt" - "strings" -) - -var _ (error) = (*Conflict)(nil) - -type Conflict struct { - // Fields is a list of fields that are in conflict. - Fields []FieldsV1 -} - -func (c *Conflict) Error() string { - conflicts := make([]string, len(c.Fields)) - for i, f := range c.Fields { - conflicts[i] = f.Path().String() - } - return fmt.Sprintf( - "conflicting fields: [%s]", - strings.Join(conflicts, ", "), - ) -} - -func MergeManagedFields( - managedFields []FieldManager, - reqFM FieldManager, -) (MergeResult, error) { - mr := MergeResult{} - // Create slice of all field managers except the one we are merging. - otherFields := []FieldsV1{} - for _, mgrs := range managedFields { - if mgrs.Manager == reqFM.Manager { - continue - } - otherFields = append(otherFields, mgrs.FieldsV1) - } - conflicts := Conflict{} - fieldsConflict(&conflicts, reqFM.FieldsV1, otherFields...) - if len(conflicts.Fields) > 0 { - return MergeResult{}, &conflicts - } - fieldsIndex := func() (int, bool) { - for i, mgrs := range managedFields { - if mgrs.Manager == reqFM.Manager { - return i, true - } - } - return -1, false - } - index, ok := fieldsIndex() - if ok { - fieldsDiff(&mr, managedFields[index].FieldsV1, reqFM.FieldsV1) - // Overwrite the existing field manager. - managedFields[index] = reqFM - } else { - managedFields = append(managedFields, reqFM) - } - mr.ManagedFields = managedFields - return mr, nil -} - -type MergeResult struct { - // ManagedFields is the updated list of field managers after the merge. - ManagedFields []FieldManager - // Removed contains a list of fields that were previously owned by the field - // manager and have been removed in this merge request. - Removed []FieldsV1 -} - -func fieldsConflict( - conflicts *Conflict, - fd FieldsV1, - existing ...FieldsV1, -) { - // Object values. - for key, value := range fd.Fields { - subFields := []FieldsV1{} - for _, fields := range existing { - // If the key exists (conflicts) with another manager and this field - // is a leaf (no children), then we have a conflict. - // Otherwise add it to the sub fields to check the children of this - // field. - if subField, ok := fields.Fields[key]; ok { - if value.IsLeaf() { - conflicts.Fields = append(conflicts.Fields, value) - break - } - subFields = append(subFields, subField) - } - } - fieldsConflict(conflicts, value, subFields...) - } - // Array elements. - for key, value := range fd.Elements { - subFields := []FieldsV1{} - for _, fields := range existing { - if subField, ok := fields.Elements[key]; ok { - if value.IsLeaf() { - conflicts.Fields = append(conflicts.Fields, value) - break - } - subFields = append(subFields, subField) - } - } - fieldsConflict(conflicts, value, subFields...) - } -} - -func fieldsDiff(mr *MergeResult, oldFields, newFields FieldsV1) { - for oldKey, oldValue := range oldFields.Fields { - newValue, ok := newFields.Fields[oldKey] - if !ok { - mr.Removed = append(mr.Removed, oldValue) - continue - } - fieldsDiff(mr, oldValue, newValue) - } - for oldKey, oldValue := range oldFields.Elements { - newValue, ok := newFields.Elements[oldKey] - if !ok { - mr.Removed = append(mr.Removed, oldValue) - continue - } - fieldsDiff(mr, oldValue, newValue) - } -} - -func PurgeRemovedFields( - obj map[string]interface{}, - removed []FieldsV1, -) error { - for _, field := range removed { - if err := blaObject(obj, field.Path()); err != nil { - return err - } - } - return nil -} - -func blaObject( - obj map[string]interface{}, - path []FieldsV1Step, -) error { - fmt.Println("blaObject: ", path) - step := path[0] - if step.Key.Type != FieldsV1KeyObject { - return fmt.Errorf( - "expected array but got object at %s", - step.String(), - ) - } - - stepObj, ok := obj[step.Key.Key] - if !ok { - return fmt.Errorf( - "key %q not found at %q", - step.Key.Key, - step.String(), - ) - } - if len(path) == 1 { - delete(obj, step.Key.Key) - return nil - } - nextStep := path[1] - if nextStep.Key.Type == FieldsV1KeyObject { - v, ok := stepObj.(map[string]interface{}) - if !ok { - return fmt.Errorf( - "expected map[string]interface{}, got %T at %q", - stepObj, - step.String(), - ) - } - if err := blaObject(v, path[1:]); err != nil { - return err - } - return nil - } - - if _, ok := stepObj.([]interface{}); !ok { - return fmt.Errorf( - "expected []interface{}, got %T at %q", - stepObj, - step.String(), - ) - } - - arrayVal, err := blaArray(stepObj.([]interface{}), path[1:]) - if err != nil { - return err - } - obj[step.Key.Key] = arrayVal - - return nil -} - -func blaArray( - obj []interface{}, - path []FieldsV1Step, -) ([]interface{}, error) { - step := path[0] - if step.Key.Type == FieldsV1KeyObject { - return nil, fmt.Errorf( - "expected array but got object at %q", - step.String(), - ) - } - index := FindIndexArrayByKey(obj, step.Key) - if index == -1 { - return nil, fmt.Errorf( - "key %q not found at %q", - step.Key.String(), - step.String(), - ) - } - if len(path) == 1 { - // Remove the element from the array. - return append(obj[:index], obj[index+1:]...), nil - } - // If there is still path left, we need to recurse. - nextObj, ok := obj[index].(map[string]interface{}) - if !ok { - return nil, fmt.Errorf( - "expected map[string]interface{}, got %T at %q", - obj[index], - step.String(), - ) - } - if err := blaObject(nextObj, path[1:]); err != nil { - return nil, err - } - return obj, nil -} - -// func MergeObjectsWithFields( -// dst map[string]interface{}, -// src map[string]interface{}, -// fields FieldsV1, -// ) { -// for _, elem := range fields.Elements { -// } -// } - -func MergeObjects( - dst map[string]interface{}, - src map[string]interface{}, -) { - mergeObjects(dst, src) -} - -func mergeObjects(dst, src map[string]interface{}) { - for key, srcValue := range src { - if dstValue, ok := dst[key]; ok { - switch sv := srcValue.(type) { - case map[string]interface{}: - mergeObjects(dstValue.(map[string]interface{}), sv) - case []interface{}: - dst[key] = mergeArrays(dstValue.([]interface{}), srcValue.([]interface{})) - default: - dst[key] = sv - } - continue - } - dst[key] = srcValue - } -} - -func mergeArrays( - dst []interface{}, - src []interface{}, -) []interface{} { - for _, srcObj := range src { - srcObj, ok := srcObj.(map[string]interface{}) - if !ok { - // If not an array of objects, src overwrites dst. - return src - } - srcIDValue, ok := srcObj["id"] - if !ok { - // If src does not have the merge key, src overwrites dst. - return src - } - index := -1 - for i, dstObj := range dst { - dstObj, ok := dstObj.(map[string]interface{}) - if !ok { - // If not an array of objects, src overwrites dst. - // This should not happen, because if dst is an array of objects - // so should src be. - return src - } - if dstObj["id"] == srcIDValue { - index = i - break - } - } - if index == -1 { - // If dst does not have the merge key, add src element to the - // result. - dst = append(dst, srcObj) - continue - } - // If dst does have a matching merge key, merge the objects. - // Then update dstObj in dst. - dstObj := dst[index].(map[string]interface{}) - mergeObjects(dstObj, srcObj) - dst[index] = dstObj - } - return dst -} diff --git a/pkg/store/apply.go b/pkg/store/apply.go index 096789a..7ef23e0 100644 --- a/pkg/store/apply.go +++ b/pkg/store/apply.go @@ -8,7 +8,7 @@ import ( "net/http" "github.com/verifa/horizon/pkg/hz" - "github.com/verifa/horizon/pkg/managedfields" + "github.com/verifa/horizon/pkg/internal/managedfields" ) type ApplyRequest struct { @@ -16,7 +16,9 @@ type ApplyRequest struct { Data []byte // Manager is the name of the field manager for this request. Manager string - Key hz.ObjectKey + // Force will force the apply to happen even if there are conflicts. + Force bool + Key hz.ObjectKey } func (s Store) Apply(ctx context.Context, req ApplyRequest) error { @@ -46,7 +48,7 @@ func (s Store) Apply(ctx context.Context, req ApplyRequest) error { fieldManager := managedfields.FieldManager{ Manager: req.Manager, FieldsV1: fieldsV1, - FieldsType: "FieldsV1", + FieldsType: managedfields.FieldsTypeV1, } // Get the existing object (if it exists). @@ -54,13 +56,7 @@ func (s Store) Apply(ctx context.Context, req ApplyRequest) error { rawObj, err := s.get(ctx, req.Key) if err != nil { if !errors.Is(err, hz.ErrNotFound) { - return &hz.Error{ - Status: http.StatusInternalServerError, - Message: fmt.Sprintf( - "checking existing object: %s", - err.Error(), - ), - } + return err } var generic hz.GenericObject if err := json.Unmarshal(req.Data, &generic); err != nil { @@ -84,13 +80,7 @@ func (s Store) Apply(ctx context.Context, req ApplyRequest) error { } } if err := s.create(ctx, req.Key, bGeneric); err != nil { - return &hz.Error{ - Status: http.StatusInternalServerError, - Message: fmt.Sprintf( - "creating object: %s", - err.Error(), - ), - } + return err } return nil } @@ -112,6 +102,7 @@ func (s Store) Apply(ctx context.Context, req ApplyRequest) error { result, err := managedfields.MergeManagedFields( generic.ManagedFields, fieldManager, + req.Force, ) if err != nil { var conflictErr *managedfields.Conflict @@ -147,7 +138,7 @@ func (s Store) Apply(ctx context.Context, req ApplyRequest) error { // Create map[string]interface{} values for the existing object (dst) and // the request object (src). - // Then purge any removed fields (if any) from the manager in src, in dst. + // Then purge any removed fields (if any) from dst. // Finally merge src into dst. var dst, src map[string]interface{} if err := json.Unmarshal(newObj, &dst); err != nil { @@ -177,7 +168,7 @@ func (s Store) Apply(ctx context.Context, req ApplyRequest) error { ), } } - managedfields.MergeObjects(dst, src) + managedfields.MergeObjects(dst, src, fieldsV1) bDst, err := json.Marshal(dst) if err != nil { return &hz.Error{ diff --git a/pkg/store/apply_test.go b/pkg/store/apply_test.go index ed72050..1f4c3e7 100644 --- a/pkg/store/apply_test.go +++ b/pkg/store/apply_test.go @@ -4,8 +4,7 @@ import ( "context" "encoding/json" "errors" - "strconv" - "strings" + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -35,7 +34,7 @@ func (r DummyApplyObject) ObjectVersion() string { } func (r DummyApplyObject) ObjectGroup() string { - return "DummyApplyGroup" + return "dummy" } func (r DummyApplyObject) ObjectKind() string { @@ -51,28 +50,20 @@ const ( ) type testStep struct { - command testStepCommand - manager string - errStatus *int + Command testStepCommand `json:"cmd,omitempty"` + Manager string `json:"manager,omitempty"` + Status *int `json:"status,omitempty"` + Force bool `json:"force,omitempty"` +} + +func (t testStep) String() string { + return fmt.Sprintf("%s:%s:%v", t.Command, t.Manager, t.Status) } func parseTestFileName(t *testing.T, file string) testStep { - parts := strings.Split(file, ":") ts := testStep{} - for i, part := range parts { - switch i { - case 0: - ts.command = testStepCommand(part) - case 1: - ts.manager = part - case 2: - status, err := strconv.Atoi(part) - tu.AssertNoError(t, err) - ts.errStatus = &status - default: - ts.command = testStepCommandError - } - } + err := json.Unmarshal([]byte(file), &ts) + tu.AssertNoError(t, err) return ts } @@ -94,57 +85,63 @@ func TestApply(t *testing.T) { }) tu.AssertNoError(t, err) - key := hz.ObjectKey{ - Group: "DummyApplyGroup", - Kind: "DummyApplyObject", - Name: "test", - Account: "test", - } ar, err := txtar.ParseFile("./testdata/apply/1.txtar") tu.AssertNoError(t, err) - for _, file := range ar.Files { + for i, file := range ar.Files { ts := parseTestFileName(t, file.Name) - client := hz.NewClient( - ti.Conn, - hz.WithClientInternal(true), - hz.WithClientManager(ts.manager), - ) - switch ts.command { - case testStepCommandApply: - obj, err := yaml.YAMLToJSON(file.Data) - tu.AssertNoError(t, err, "obj yaml to json") - err = client.Apply( - ctx, - hz.WithApplyKey(key), - hz.WithApplyData(obj), + testName := fmt.Sprintf("%d:%s", i, ts.String()) + t.Run(testName, func(t *testing.T) { + client := hz.NewClient( + ti.Conn, + hz.WithClientInternal(true), + hz.WithClientManager(ts.Manager), ) - if ts.errStatus == nil { - tu.AssertNoError(t, err, "client apply") - return - } - var applyErr *hz.Error - if errors.As(err, &applyErr) { - tu.AssertEqual(t, applyErr.Status, *ts.errStatus) - return + switch ts.Command { + case testStepCommandApply: + jsonData, err := yaml.YAMLToJSON(file.Data) + tu.AssertNoError(t, err, "obj yaml to json") + obj := hz.GenericObject{} + err = json.Unmarshal(jsonData, &obj) + tu.AssertNoError(t, err, "unmarshal obj") + + err = client.Apply( + ctx, + hz.WithApplyObject(obj), + hz.WithApplyForce(ts.Force), + ) + if ts.Status == nil { + tu.AssertNoError(t, err, "client apply") + return + } + var applyErr *hz.Error + if errors.As(err, &applyErr) { + tu.AssertEqual(t, applyErr.Status, *ts.Status) + return + } else { + t.Fatal("expected error status") + } + case testStepCommandAssert: + expJSONData, err := yaml.YAMLToJSON(file.Data) + tu.AssertNoError(t, err, "expObj yaml to json") + expObj := hz.GenericObject{} + err = json.Unmarshal(expJSONData, &expObj) + tu.AssertNoError(t, err, "unmarshal obj") + actObj, err := ti.Store.Get(ctx, store.GetRequest{Key: expObj}) + tu.AssertNoError(t, err, "client get") + var exp, act interface{} + err = json.Unmarshal(expJSONData, &exp) + tu.AssertNoError(t, err, "unmarshal exp") + err = json.Unmarshal(actObj, &act) + tu.AssertNoError(t, err, "unmarshal act") + tu.AssertEqual(t, exp, act, cmpOptIgnoreRevision) + + case testStepCommandError: + t.Errorf("invalid test file name: %s", file.Name) + default: + t.Errorf("invalid test file name: %s", file.Name) + } - case testStepCommandAssert: - expObj, err := yaml.YAMLToJSON(file.Data) - tu.AssertNoError(t, err, "expObj yaml to json") - actObj, err := ti.Store.Get(ctx, store.GetRequest{Key: key}) - tu.AssertNoError(t, err, "client get") - var exp, act interface{} - err = json.Unmarshal(expObj, &exp) - tu.AssertNoError(t, err, "unmarshal exp") - err = json.Unmarshal(actObj, &act) - tu.AssertNoError(t, err, "unmarshal act") - tu.AssertEqual(t, exp, act, cmpOptIgnoreRevision) - - case testStepCommandError: - t.Errorf("invalid test file name: %s", file.Name) - default: - t.Errorf("invalid test file name: %s", file.Name) - - } + }) } } diff --git a/pkg/store/create.go b/pkg/store/create.go index 11e4699..b72989d 100644 --- a/pkg/store/create.go +++ b/pkg/store/create.go @@ -55,7 +55,22 @@ func (s Store) create( ) error { _, err := s.kv.Create(ctx, hz.KeyFromObject(key), data) if err != nil { - return fmt.Errorf("creating object: %w", err) + if errors.Is(err, jetstream.ErrKeyExists) { + return &hz.Error{ + Status: http.StatusConflict, + Message: fmt.Sprintf( + "object already exists: %q", + key, + ), + } + } + return &hz.Error{ + Status: http.StatusInternalServerError, + Message: fmt.Sprintf( + "creating object: %s", + err.Error(), + ), + } } return nil } diff --git a/pkg/store/get.go b/pkg/store/get.go index 0575209..1f38e4f 100644 --- a/pkg/store/get.go +++ b/pkg/store/get.go @@ -12,19 +12,25 @@ import ( ) type GetRequest struct { - Key hz.ObjectKey + Key hz.ObjectKeyer } func (s Store) Get(ctx context.Context, req GetRequest) ([]byte, error) { - data, err := s.get(ctx, req.Key) - if err != nil { - return nil, err - } - return data, err + return s.get(ctx, req.Key) } -func (s Store) get(ctx context.Context, key hz.ObjectKey) ([]byte, error) { - kve, err := s.kv.Get(ctx, hz.KeyFromObject(key)) +func (s Store) get(ctx context.Context, key hz.ObjectKeyer) ([]byte, error) { + rawKey, err := hz.KeyFromObjectConcrete(key) + if err != nil { + return nil, &hz.Error{ + Status: http.StatusBadRequest, + Message: fmt.Sprintf( + "invalid key: %s", + err.Error(), + ), + } + } + kve, err := s.kv.Get(ctx, rawKey) if err != nil { if errors.Is(err, jetstream.ErrKeyNotFound) { return nil, hz.ErrNotFound @@ -33,7 +39,7 @@ func (s Store) get(ctx context.Context, key hz.ObjectKey) ([]byte, error) { Status: http.StatusInternalServerError, Message: fmt.Sprintf( "getting key %s: %s", - key, + rawKey, err.Error(), ), } diff --git a/pkg/store/store.go b/pkg/store/store.go index 32852de..9ee4779 100644 --- a/pkg/store/store.go +++ b/pkg/store/store.go @@ -7,6 +7,7 @@ import ( "fmt" "log/slog" "net/http" + "strconv" "strings" "time" @@ -340,7 +341,19 @@ func (s Store) handleInternalMsg(ctx context.Context, msg *nats.Msg) { return } case StoreCommandApply: - manager := msg.Header.Get(hz.HeaderFieldManager) + manager := msg.Header.Get(hz.HeaderApplyFieldManager) + forceStr := msg.Header.Get(hz.HeaderApplyForceConflicts) + force, err := strconv.ParseBool(forceStr) + if err != nil { + _ = hz.RespondError( + msg, + &hz.Error{ + Status: http.StatusBadRequest, + Message: "invalid header " + hz.HeaderApplyForceConflicts, + }, + ) + return + } if manager == "" { _ = hz.RespondError( msg, @@ -355,6 +368,7 @@ func (s Store) handleInternalMsg(ctx context.Context, msg *nats.Msg) { Data: data, Manager: manager, Key: key, + Force: force, } if err := s.Apply(ctx, req); err != nil { diff --git a/pkg/store/testdata/apply/1.txtar b/pkg/store/testdata/apply/1.txtar index be9ec7b..6af8b55 100644 --- a/pkg/store/testdata/apply/1.txtar +++ b/pkg/store/testdata/apply/1.txtar @@ -1,19 +1,23 @@ -# format of filename is: -# [:][:] +# Format of filename is in json. +# Valid fields are: +# - cmd: the command to Run +# - status: the expected status return +# - manager: the name of the manager +# - force: force the apply # # Commands: # - apply: apply the file contents (requires manager) # - assert: checks the current object against the file contents # # Examples: -# - apply:m1 => apply contents as manager "m1" -# - apply:m1:409 => apply contents as manager "m1" and expect error status 409 (conflict) -# - assert => check the current object eqauls file contents +# - {"cmd":"apply","manager":"m1"} => apply contents as manager "m1" +# - {"cmd":"apply","manager":"m2", "status":409} => apply contents as manager "m1" and expect error status 409 (conflict) +# - {"cmd":"assert"} => check the current object eqauls file contents # --- apply:m1 -- +-- {"cmd":"apply","manager":"m1"} -- +apiVersion: dummy/v1 kind: DummyApplyObject -group: DummyApplyGroup metadata: name: test account: test @@ -22,7 +26,8 @@ spec: object: text: test numbers: [1,2,3] --- assert -- +-- {"cmd":"assert"} -- +apiVersion: dummy/v1 kind: DummyApplyObject metadata: name: test @@ -41,9 +46,9 @@ spec: object: text: test numbers: [1,2,3] --- apply:m1 -- +-- {"cmd":"apply","manager":"m1"} -- +apiVersion: dummy/v1 kind: DummyApplyObject -group: DummyApplyGroup metadata: name: test account: test @@ -54,9 +59,9 @@ spec: array: - id: "1" text: test --- assert -- +-- {"cmd":"assert"} -- +apiVersion: dummy/v1 kind: DummyApplyObject -group: DummyApplyGroup metadata: name: test account: test @@ -80,18 +85,18 @@ spec: - id: "1" text: test --- apply:m2:409 -- +-- {"cmd":"apply","manager":"m2", "status":409} -- +apiVersion: dummy/v1 kind: DummyApplyObject -group: DummyApplyGroup metadata: name: test account: test spec: text: test --- apply:m2 -- +-- {"cmd":"apply","manager":"m2"} -- +apiVersion: dummy/v1 kind: DummyApplyObject -group: DummyApplyGroup metadata: name: test account: test @@ -102,9 +107,9 @@ spec: - id: "2" text: test --- assert -- +-- {"cmd":"assert"} -- +apiVersion: dummy/v1 kind: DummyApplyObject -group: DummyApplyGroup metadata: name: test account: test @@ -130,6 +135,59 @@ metadata: k:{"id":"2"}: f:id: {} f:text: {} +spec: + text: test + object: + number: 123 + text: test + array: + - id: "1" + text: test + - id: "2" + text: test +-- {"cmd":"apply","manager":"m3", "force": true} -- +apiVersion: dummy/v1 +kind: DummyApplyObject +metadata: + name: test + account: test +spec: + object: + number: 123 +-- {"cmd":"assert"} -- +apiVersion: dummy/v1 +kind: DummyApplyObject +metadata: + name: test + account: test + managedFields: + - manager: m1 + fieldsType: FieldsV1 + fieldsV1: + f:spec: + f:text: {} + f:object: + f:text: {} + f:array: + k:{"id":"1"}: + f:id: {} + f:text: {} + - manager: m2 + fieldsType: FieldsV1 + fieldsV1: + f:spec: + #f:object: + # f:number: {} + f:array: + k:{"id":"2"}: + f:id: {} + f:text: {} + - manager: m3 + fieldsType: FieldsV1 + fieldsV1: + f:spec: + f:object: + f:number: {} spec: text: test object: diff --git a/pkg/store/update.go b/pkg/store/update.go index 5e45a40..b7ba273 100644 --- a/pkg/store/update.go +++ b/pkg/store/update.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net/http" "github.com/nats-io/nats.go/jetstream" "github.com/verifa/horizon/pkg/hz" @@ -21,14 +22,24 @@ func (s Store) update( data []byte, revision uint64, ) (uint64, error) { - revision, err := s.kv.Update(ctx, hz.KeyFromObject(key), data, revision) + rawKey, err := hz.KeyFromObjectConcrete(key) + if err != nil { + return 0, &hz.Error{ + Status: http.StatusBadRequest, + Message: fmt.Sprintf( + "invalid key: %q", + err.Error(), + ), + } + } + newRevision, err := s.kv.Update(ctx, rawKey, data, revision) if err != nil { if isErrWrongLastSequence(err) { return 0, hz.ErrIncorrectRevision } return 0, fmt.Errorf("update: %w", err) } - return revision, nil + return newRevision, nil } // isErrWrongLastSequence returns true if the error is caused by a write