From 655edf8f80f3d65e629f56b86f3fa3415c1200be Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Fri, 16 Aug 2019 15:30:58 -0700 Subject: [PATCH] disable unions, since they're still experimental --- internal/fixture/state.go | 16 ++++++++++++++++ merge/union_test.go | 7 +++++++ merge/update.go | 26 ++++++++++++++++++++------ typed/typed.go | 4 ++++ 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/internal/fixture/state.go b/internal/fixture/state.go index 3a74860c..1cf956f8 100644 --- a/internal/fixture/state.go +++ b/internal/fixture/state.go @@ -261,6 +261,8 @@ type TestCase struct { // Managed, if not nil, is the ManagedFields as expected // after all operations are run. Managed fieldpath.ManagedFields + // Set to true if the test case needs the union behavior enabled. + RequiresUnions bool } // Test runs the test-case using the given parser and a dummy converter. @@ -283,6 +285,9 @@ func (tc TestCase) BenchWithConverter(parser typed.ParseableType, converter merg Updater: &merge.Updater{Converter: converter}, Parser: parser, } + if tc.RequiresUnions { + state.Updater.EnableUnionFeature() + } // We currently don't have any test that converts, we can take // care of that later. for i, ops := range tc.Ops { @@ -300,6 +305,17 @@ func (tc TestCase) TestWithConverter(parser typed.ParseableType, converter merge Updater: &merge.Updater{Converter: converter}, Parser: parser, } + if tc.RequiresUnions { + state.Updater.EnableUnionFeature() + } else { + // Also test it with unions on. + tc2 := tc + tc2.RequiresUnions = true + err := tc2.TestWithConverter(parser, converter) + if err != nil { + return fmt.Errorf("fails if unions are on: %v", err) + } + } // We currently don't have any test that converts, we can take // care of that later. for i, ops := range tc.Ops { diff --git a/merge/union_test.go b/merge/union_test.go index 1f54e822..1c933cc7 100644 --- a/merge/union_test.go +++ b/merge/union_test.go @@ -67,6 +67,7 @@ var unionFieldsParser = func() typed.ParseableType { func TestUnion(t *testing.T) { tests := map[string]TestCase{ "union_apply_owns_discriminator": { + RequiresUnions: true, Ops: []Operation{ Apply{ Manager: "default", @@ -91,6 +92,7 @@ func TestUnion(t *testing.T) { }, }, "union_apply_without_discriminator_conflict": { + RequiresUnions: true, Ops: []Operation{ Update{ Manager: "controller", @@ -125,6 +127,7 @@ func TestUnion(t *testing.T) { }, }, "union_apply_with_null_value": { + RequiresUnions: true, Ops: []Operation{ Apply{ Manager: "default", @@ -138,6 +141,7 @@ func TestUnion(t *testing.T) { }, }, "union_apply_multiple_unions": { + RequiresUnions: true, Ops: []Operation{ Apply{ Manager: "default", @@ -176,6 +180,7 @@ func TestUnion(t *testing.T) { func TestUnionErrors(t *testing.T) { tests := map[string]TestCase{ "union_apply_two": { + RequiresUnions: true, Ops: []Operation{ Apply{ Manager: "default", @@ -188,6 +193,7 @@ func TestUnionErrors(t *testing.T) { }, }, "union_apply_two_and_discriminator": { + RequiresUnions: true, Ops: []Operation{ Apply{ Manager: "default", @@ -201,6 +207,7 @@ func TestUnionErrors(t *testing.T) { }, }, "union_apply_wrong_discriminator": { + RequiresUnions: true, Ops: []Operation{ Apply{ Manager: "default", diff --git a/merge/update.go b/merge/update.go index 731ec4d7..a279db58 100644 --- a/merge/update.go +++ b/merge/update.go @@ -31,6 +31,14 @@ type Converter interface { // merge the object on Apply. type Updater struct { Converter Converter + + enableUnions bool +} + +// EnableUnionFeature turns on union handling. It is disabled by default until the +// feature is complete. +func (s *Updater) EnableUnionFeature() { + s.enableUnions = true } func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, workflow string, force bool) (fieldpath.ManagedFields, error) { @@ -112,9 +120,12 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa // PATCH call), and liveObject must be the original object (empty if // this is a CREATE call). func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string) (*typed.TypedValue, fieldpath.ManagedFields, error) { - newObject, err := liveObject.NormalizeUnions(newObject) - if err != nil { - return nil, fieldpath.ManagedFields{}, err + var err error + if s.enableUnions { + newObject, err = liveObject.NormalizeUnions(newObject) + if err != nil { + return nil, fieldpath.ManagedFields{}, err + } } managers = shallowCopyManagers(managers) managers, err = s.update(liveObject, newObject, version, managers, manager, true) @@ -144,9 +155,12 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp // and return it. func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string, force bool) (*typed.TypedValue, fieldpath.ManagedFields, error) { managers = shallowCopyManagers(managers) - configObject, err := configObject.NormalizeUnionsApply(configObject) - if err != nil { - return nil, fieldpath.ManagedFields{}, err + var err error + if s.enableUnions { + configObject, err = configObject.NormalizeUnionsApply(configObject) + if err != nil { + return nil, fieldpath.ManagedFields{}, err + } } newObject, err := liveObject.Merge(configObject) if err != nil { diff --git a/typed/typed.go b/typed/typed.go index 6eaab261..956fed0d 100644 --- a/typed/typed.go +++ b/typed/typed.go @@ -153,6 +153,8 @@ func (tv TypedValue) RemoveItems(items *fieldpath.Set) *TypedValue { // - If discriminator changed to non-nil, all other fields but the // discriminated one will be cleared, // - Otherwise, If only one field is left, update discriminator to that value. +// +// Please note: union behavior isn't finalized yet and this is still experimental. func (tv TypedValue) NormalizeUnions(new *TypedValue) (*TypedValue, error) { var errs ValidationErrors var normalizeFn = func(w *mergingWalker) { @@ -177,6 +179,8 @@ func (tv TypedValue) NormalizeUnions(new *TypedValue) (*TypedValue, error) { // NormalizeUnionsApply specifically normalize unions on apply. It // validates that the applied union is correct (there should be no // ambiguity there), and clear the fields according to the sent intent. +// +// Please note: union behavior isn't finalized yet and this is still experimental. func (tv TypedValue) NormalizeUnionsApply(new *TypedValue) (*TypedValue, error) { var errs ValidationErrors var normalizeFn = func(w *mergingWalker) {