From be565188881498da8f7a871ea22ecc73db25b7d8 Mon Sep 17 00:00:00 2001 From: Gabriele Gerbino Date: Thu, 21 Jul 2022 11:52:13 +0200 Subject: [PATCH] feat: add version compatibility logic for upstreams --- .../ws/config/compat/compatibility_table.go | 120 ++++++ .../kong/ws/config/version_compatibility.go | 72 +++- .../ws/config/version_compatibility_test.go | 374 ++++++++++++++++++ .../test/e2e/version_compatibility_test.go | 149 ++++++- 4 files changed, 709 insertions(+), 6 deletions(-) diff --git a/internal/server/kong/ws/config/compat/compatibility_table.go b/internal/server/kong/ws/config/compat/compatibility_table.go index 85c59b887..1c6d9abcc 100644 --- a/internal/server/kong/ws/config/compat/compatibility_table.go +++ b/internal/server/kong/ws/config/compat/compatibility_table.go @@ -42,6 +42,17 @@ func standardPluginNotAvailableMessage(pluginName string, versionWithFeatureSupp ) } +func standardCoreEntityFieldsMessage(entityName string, fields []string, versionWithFeatureSupport string) string { + quotedFields := "'" + strings.Join(fields, "', '") + "'" + return fmt.Sprintf("For the '%s' entity, "+ + "one or more of the following schema fields are set: %s "+ + "but Kong gateway versions < %s do not support these fields. "+ + entityName, + quotedFields, + versionWithFeatureSupport, + ) +} + const ( versionsPre260 = "< 2.6.0" versionsPre270 = "< 2.7.0" @@ -437,6 +448,115 @@ var ( }, }, }, + { + Metadata: config.ChangeMetadata{ + ID: config.ChangeID("P121"), + Severity: config.ChangeSeverityError, + Description: standardCoreEntityFieldsMessage(config.Upstream.String(), + []string{ + "hash_on_query_arg", + "hash_fallback_query_arg", + "hash_on_uri_capture", + "hash_fallback_uri_capture", + }, + "3.0"), + Resolution: standardUpgradeMessage("3.0"), + }, + SemverRange: versionsPre300, + Update: config.ConfigTableUpdates{ + Name: config.Upstream.String(), + Type: config.Upstream, + RemoveFields: []string{ + "hash_on_query_arg", + "hash_fallback_query_arg", + "hash_on_uri_capture", + "hash_fallback_uri_capture", + }, + }, + }, + { + Metadata: config.ChangeMetadata{ + ID: config.ChangeID("P122"), + Severity: config.ChangeSeverityError, + Description: fmt.Sprintf("For the 'upstreams' entity, "+ + "one or more of the '%s' schema fields are set with one "+ + "of the following values: %s, but Kong gateway versions < 3.0 "+ + "do not support these values. Because of this, 'hash_on' and 'hash_fallback'"+ + "have been changed in the data-plane to 'none' and hashing is "+ + "not working as expected.", + strings.Join([]string{"hash_on", "hash_fallback"}, ", "), + strings.Join([]string{"path", "query_arg", "uri_capture"}, ", "), + ), + Resolution: standardUpgradeMessage("3.0"), + }, + SemverRange: versionsPre300, + Update: config.ConfigTableUpdates{ + Name: config.Upstream.String(), + Type: config.Upstream, + FieldUpdates: []config.ConfigTableFieldCondition{ + { + Field: "hash_on", + Condition: "hash_on=path", + Updates: []config.ConfigTableFieldUpdate{ + { + Field: "hash_on", + Value: "none", + }, + }, + }, + { + Field: "hash_on", + Condition: "hash_on=query_arg", + Updates: []config.ConfigTableFieldUpdate{ + { + Field: "hash_on", + Value: "none", + }, + }, + }, + { + Field: "hash_on", + Condition: "hash_on=uri_capture", + Updates: []config.ConfigTableFieldUpdate{ + { + Field: "hash_on", + Value: "none", + }, + }, + }, + { + Field: "hash_fallback", + Condition: "hash_fallback=path", + Updates: []config.ConfigTableFieldUpdate{ + { + Field: "hash_fallback", + Value: "none", + }, + }, + }, + { + Field: "hash_fallback", + Condition: "hash_fallback=query_arg", + Updates: []config.ConfigTableFieldUpdate{ + { + Field: "hash_fallback", + Value: "none", + }, + }, + }, + { + Field: "hash_fallback", + Condition: "hash_fallback=uri_capture", + Updates: []config.ConfigTableFieldUpdate{ + { + Field: "hash_fallback", + Value: "none", + }, + }, + }, + }, + }, + }, } ) diff --git a/internal/server/kong/ws/config/version_compatibility.go b/internal/server/kong/ws/config/version_compatibility.go index a4634b6ed..5447c857b 100644 --- a/internal/server/kong/ws/config/version_compatibility.go +++ b/internal/server/kong/ws/config/version_compatibility.go @@ -45,10 +45,11 @@ type UpdateType uint8 const ( Plugin UpdateType = iota Service + Upstream ) func (u UpdateType) String() string { - return [...]string{"plugins", "services"}[u] + return [...]string{"plugins", "services", "upstreams"}[u] } //nolint:revive @@ -246,6 +247,8 @@ func (vc *WSVersionCompatibility) processConfigTableUpdates(uncompressedPayload processedPayload = vc.processPluginUpdates(processedPayload, configTableUpdate, dataPlaneVersion) case Service: + fallthrough + case Upstream: processedPayload = vc.processCoreEntityUpdates(processedPayload, configTableUpdate, dataPlaneVersion) default: @@ -552,6 +555,73 @@ func (vc *WSVersionCompatibility) processCoreEntityUpdates(payload string, } } } + + // Field updates + for _, update := range configTableUpdate.FieldUpdates { + if gjson.Get(updatedRaw, update.Field).Exists() { + conditionField := fmt.Sprintf("[@this].#(%s)", update.Condition) + if gjson.Get(updatedRaw, conditionField).Exists() { + for _, fieldUpdate := range update.Updates { + conditionUpdate := fieldUpdate.Field + if fieldUpdate.Value == nil && len(fieldUpdate.ValueFromField) == 0 { + // Handle field removal + if updatedRaw, err = sjson.Delete(updatedRaw, conditionUpdate); err != nil { + vc.logger.With(zap.String("entity", entityName)). + With(zap.String("field", update.Field)). + With(zap.String("data-plane", dataPlaneVersion)). + With(zap.Error(err)). + Error("entity item was not removed from configuration") + } else { + vc.logger.With(zap.String("entity", entityName)). + With(zap.String("field", update.Field)). + With(zap.String("data-plane", dataPlaneVersion)). + Warn("removing entity field which is incompatible with data plane") + } + } else { + // Get the field value if "Value" is a field + var value interface{} + if fieldUpdate.Value != nil { + value = fieldUpdate.Value + } else { + valueFromField := fmt.Sprintf("%v", fieldUpdate.ValueFromField) + res := gjson.Get(updatedRaw, valueFromField) + if res.Exists() { + value = res.Value() + } else { + vc.logger.With(zap.String("entity", entityName)). + With(zap.String("field", update.Field)). + With(zap.String("condition", update.Condition)). + With(zap.Any("new-value", fieldUpdate.Value)). + With(zap.String("data-plane", dataPlaneVersion)). + With(zap.Error(err)). + Error("entity configuration does not contain field value") + break + } + } + + // Handle field update from value of value of field + if updatedRaw, err = sjson.Set(updatedRaw, conditionUpdate, value); err != nil { + vc.logger.With(zap.String("entity", entityName)). + With(zap.String("field", update.Field)). + With(zap.String("condition", update.Condition)). + With(zap.Any("new-value", fieldUpdate.Value)). + With(zap.String("data-plane", dataPlaneVersion)). + With(zap.Error(err)). + Error("entity configuration field was not updated int configuration") + } else { + vc.logger.With(zap.String("entity", entityName)). + With(zap.String("field", update.Field)). + With(zap.String("condition", update.Condition)). + With(zap.Any("new-value", fieldUpdate.Value)). + With(zap.String("data-plane", dataPlaneVersion)). + Warn("removing entity field which is incompatible with data plane") + } + } + } + } + } + } + if err = json.Unmarshal([]byte(updatedRaw), &entityJSON); err != nil { vc.logger.With(zap.String("entity", entityName)). With(zap.String("name", name)). diff --git a/internal/server/kong/ws/config/version_compatibility_test.go b/internal/server/kong/ws/config/version_compatibility_test.go index dc3fba6de..8e8b9baf2 100644 --- a/internal/server/kong/ws/config/version_compatibility_test.go +++ b/internal/server/kong/ws/config/version_compatibility_test.go @@ -2629,6 +2629,380 @@ func TestVersionCompatibility_ProcessConfigTableUpdates(t *testing.T) { } }`, }, + { + name: "drop single upstream field", + configTableUpdates: map[string][]ConfigTableUpdates{ + "< 3.0.0": { + { + Type: Upstream, + RemoveFields: []string{ + "upstream_field_1", + }, + }, + }, + }, + uncompressedPayload: `{ + "config_table": { + "upstreams": [ + { + "name": "upstream_1", + "upstream_field_1": "element", + "upstream_field_2": "element" + } + ] + } + }`, + dataPlaneVersion: "2.7.0", + expectedPayload: `{ + "config_table": { + "upstreams": [ + { + "name": "upstream_1", + "upstream_field_2": "element" + } + ] + } + }`, + }, + { + name: "drop multiple upstream fields", + configTableUpdates: map[string][]ConfigTableUpdates{ + "< 3.0.0": { + { + Type: Upstream, + RemoveFields: []string{ + "upstream_field_1", + "upstream_field_2", + }, + }, + }, + }, + uncompressedPayload: `{ + "config_table": { + "upstreams": [ + { + "name": "upstream_1", + "upstream_field_1": "element", + "upstream_field_2": "element", + "upstream_field_3": "element" + } + ] + } + }`, + dataPlaneVersion: "2.7.0", + expectedPayload: `{ + "config_table": { + "upstreams": [ + { + "name": "upstream_1", + "upstream_field_3": "element" + } + ] + } + }`, + }, + { + name: "drop multiple upstream fields from multiple upstreams", + configTableUpdates: map[string][]ConfigTableUpdates{ + "< 3.0.0": { + { + Type: Upstream, + RemoveFields: []string{ + "upstream_field_1", + "upstream_field_2", + "upstream_field_4", + }, + }, + }, + }, + uncompressedPayload: `{ + "config_table": { + "upstreams": [ + { + "name": "upstream_1", + "upstream_field_1": "element", + "upstream_field_2": "element", + "upstream_field_3": "element" + }, + { + "name": "upstream_2", + "upstream_field_1": "element", + "upstream_field_3": "element", + "upstream_field_4": "element" + } + ] + } + }`, + dataPlaneVersion: "2.7.0", + expectedPayload: `{ + "config_table": { + "upstreams": [ + { + "name": "upstream_1", + "upstream_field_3": "element" + }, + { + "name": "upstream_2", + "upstream_field_3": "element" + } + ] + } + }`, + }, + { + name: "drop upstreams and plugins' fields", + configTableUpdates: map[string][]ConfigTableUpdates{ + "< 3.0.0": { + { + Type: Upstream, + RemoveFields: []string{ + "upstream_field_1", + "upstream_field_2", + "upstream_field_4", + }, + }, + { + Name: "plugin_1", + Type: Plugin, + Remove: true, + }, + }, + }, + uncompressedPayload: `{ + "config_table": { + "plugins": [ + { + "name": "plugin_2", + "config": { + "plugin_2_field_1": "element" + } + }, + { + "name": "plugin_1", + "config": { + "plugin_1_field_1": "element" + } + }, + { + "name": "plugin_3", + "config": { + "plugin_3_field_1": "element" + } + }, + { + "name": "plugin_1", + "config": { + "plugin_1_field_1": "element" + } + } + ], + "upstreams": [ + { + "name": "upstream_1", + "upstream_field_1": "element", + "upstream_field_2": "element", + "upstream_field_3": "element" + }, + { + "name": "upstream_2", + "upstream_field_1": "element", + "upstream_field_3": "element", + "upstream_field_4": "element" + } + ] + } + }`, + dataPlaneVersion: "2.7.0", + expectedPayload: `{ + "config_table": { + "plugins": [ + { + "name": "plugin_2", + "config": { + "plugin_2_field_1": "element" + } + }, + { + "name": "plugin_3", + "config": { + "plugin_3_field_1": "element" + } + } + ], + "upstreams": [ + { + "name": "upstream_1", + "upstream_field_3": "element" + }, + { + "name": "upstream_2", + "upstream_field_3": "element" + } + ] + } + }`, + }, + { + name: "ensure unsupported upstreams fields values are changed to 'none'", + configTableUpdates: map[string][]ConfigTableUpdates{ + "< 3.0.0": { + { + Type: Upstream, + FieldUpdates: []ConfigTableFieldCondition{ + { + Field: "hash_on", + Condition: "hash_on=path", + Updates: []ConfigTableFieldUpdate{ + { + Field: "hash_on", + Value: "none", + }, + }, + }, + { + Field: "hash_on", + Condition: "hash_on=query_arg", + Updates: []ConfigTableFieldUpdate{ + { + Field: "hash_on", + Value: "none", + }, + }, + }, + { + Field: "hash_on", + Condition: "hash_on=uri_capture", + Updates: []ConfigTableFieldUpdate{ + { + Field: "hash_on", + Value: "none", + }, + }, + }, + { + Field: "hash_fallback", + Condition: "hash_fallback=path", + Updates: []ConfigTableFieldUpdate{ + { + Field: "hash_fallback", + Value: "none", + }, + }, + }, + { + Field: "hash_fallback", + Condition: "hash_fallback=query_arg", + Updates: []ConfigTableFieldUpdate{ + { + Field: "hash_fallback", + Value: "none", + }, + }, + }, + { + Field: "hash_fallback", + Condition: "hash_fallback=uri_capture", + Updates: []ConfigTableFieldUpdate{ + { + Field: "hash_fallback", + Value: "none", + }, + }, + }, + }, + }, + }, + }, + uncompressedPayload: `{ + "config_table": { + "upstreams": [ + { + "name": "upstream_1", + "hash_on": "path", + "hash_fallback": "path" + }, + { + "name": "upstream_2", + "hash_on": "query_arg", + "hash_fallback": "query_arg" + }, + { + "name": "upstream_3", + "hash_on": "uri_capture", + "hash_fallback": "uri_capture" + } + ] + } + }`, + dataPlaneVersion: "2.7.0", + expectedPayload: `{ + "config_table": { + "upstreams": [ + { + "name": "upstream_1", + "hash_on": "none", + "hash_fallback": "none" + }, + { + "name": "upstream_2", + "hash_on": "none", + "hash_fallback": "none" + }, + { + "name": "upstream_3", + "hash_on": "none", + "hash_fallback": "none" + } + ] + } + }`, + }, + { + name: "ensure unsupported upstreams fields are replaced and dropped", + configTableUpdates: map[string][]ConfigTableUpdates{ + "< 3.0.0": { + { + Type: Upstream, + FieldUpdates: []ConfigTableFieldCondition{ + { + Field: "upstream_unsupported_1", + Condition: "upstream_unsupported_1", + Updates: []ConfigTableFieldUpdate{ + { + Field: "upstream_supported_2", + ValueFromField: "upstream_unsupported_1", + }, + { + Field: "upstream_unsupported_1", + }, + }, + }, + }, + }, + }, + }, + uncompressedPayload: `{ + "config_table": { + "upstreams": [ + { + "name": "upstream_1", + "upstream_unsupported_1": "foo" + } + ] + } + }`, + dataPlaneVersion: "2.7.0", + expectedPayload: `{ + "config_table": { + "upstreams": [ + { + "name": "upstream_1", + "upstream_supported_2": "foo" + } + ] + } + }`, + }, } for _, test := range tests { diff --git a/internal/test/e2e/version_compatibility_test.go b/internal/test/e2e/version_compatibility_test.go index 15af6098d..5384bb219 100644 --- a/internal/test/e2e/version_compatibility_test.go +++ b/internal/test/e2e/version_compatibility_test.go @@ -38,14 +38,20 @@ type vcPlugins struct { expectedConfig string } -func TestVersionCompatibility(t *testing.T) { +type vcUpstreamsTC struct { + name string + upstream *v1.Upstream + versionedExpected map[string]*v1.Upstream +} + +func TestPluginsVersionCompatibility(t *testing.T) { cleanup := run.Koko(t) defer cleanup() dpCleanup := run.KongDP(kong.GetKongConfForShared()) defer dpCleanup() - util.WaitForKong(t) - util.WaitForKongAdminAPI(t) + require.NoError(t, util.WaitForKong(t)) + require.NoError(t, util.WaitForKongAdminAPI(t)) admin := httpexpect.New(t, "http://localhost:3000") @@ -381,6 +387,139 @@ func TestVersionCompatibility(t *testing.T) { }) } +func TestUpstreamsVersionCompatibility(t *testing.T) { + cleanup := run.Koko(t) + defer cleanup() + + dpCleanup := run.KongDP(kong.GetKongConfForShared()) + defer dpCleanup() + require.NoError(t, util.WaitForKong(t)) + require.NoError(t, util.WaitForKongAdminAPI(t)) + + admin := httpexpect.New(t, "http://localhost:3000") + + tests := []vcUpstreamsTC{ + { + name: "ensure hash_on_query_arg is dropped for DP < 3.0", + upstream: &v1.Upstream{ + Id: uuid.NewString(), + Name: "foo-with-hash_on_query_arg", + HashOn: "ip", + HashOnQueryArg: "test", + }, + versionedExpected: map[string]*v1.Upstream{ + "< 3.0.0": { + Name: "foo-with-hash_on_query_arg", + HashOn: "ip", + HashOnQueryArg: "", + }, + ">= 3.0.0": { + Name: "foo-with-hash_on_query_arg", + HashOn: "ip", + HashOnQueryArg: "test", + }, + }, + }, + { + name: "ensure hash_on is reverted to 'none' when configured to incompatible values for DP < 3.0", + upstream: &v1.Upstream{ + Id: uuid.NewString(), + Name: "foo-with-hash_on", + HashOn: "path", + HashOnQueryArg: "test", + }, + versionedExpected: map[string]*v1.Upstream{ + "< 3.0.0": { + Name: "foo-with-hash_on", + HashOn: "none", + HashOnQueryArg: "", + }, + ">= 3.0.0": { + Name: "foo-with-hash_on", + HashOn: "path", + HashOnQueryArg: "test", + }, + }, + }, + { + name: "ensure hash_fallback is reverted to 'none' when configured to incompatible values for DP < 3.0", + upstream: &v1.Upstream{ + Id: uuid.NewString(), + Name: "foo-with-hash_fallback", + HashFallback: "path", + HashOn: "ip", + HashOnQueryArg: "test", + }, + versionedExpected: map[string]*v1.Upstream{ + "< 3.0.0": { + Name: "foo-with-hash_fallback", + HashFallback: "none", + HashOn: "ip", + HashOnQueryArg: "", + }, + ">= 3.0.0": { + Name: "foo-with-hash_fallback", + HashFallback: "path", + HashOn: "ip", + HashOnQueryArg: "test", + }, + }, + }, + } + for _, test := range tests { + res := admin.POST("/v1/upstreams").WithJSON(test.upstream).Expect() + res.Status(http.StatusCreated) + } + + util.WaitFunc(t, func() error { + err := ensureUpstreams(tests) + t.Log("upstreams validation failed", err) + return err + }) +} + +func ensureUpstreams(upstreams []vcUpstreamsTC) error { + kongAdmin, err := kongClient.NewClient(util.BasedKongAdminAPIAddr, nil) + if err != nil { + return fmt.Errorf("create go client for kong: %v", err) + } + ctx := context.Background() + info, err := kongAdmin.Root(ctx) + if err != nil { + return fmt.Errorf("fetching Kong Gateway info: %v", err) + } + dataPlaneVersion, err := kongClient.ParseSemanticVersion(kongClient.VersionFromInfo(info)) + if err != nil { + return fmt.Errorf("parsing Kong Gateway version: %v", err) + } + dataPlaneUpstreams, err := kongAdmin.Upstreams.ListAll(ctx) + if err != nil { + return fmt.Errorf("fetching upstreams: %v", err) + } + + if len(upstreams) != len(dataPlaneUpstreams) { + return fmt.Errorf("upstreams configured count does not match [%d != %d]", len(upstreams), len(dataPlaneUpstreams)) + } + + expectedConfig := &v1.TestingConfig{ + Upstreams: []*v1.Upstream{}, + } + for _, u := range upstreams { + for _, dataPlaneUpstream := range dataPlaneUpstreams { + if u.upstream.Name == *dataPlaneUpstream.Name && u.upstream.Id == *dataPlaneUpstream.ID { + for version, expectedUpstream := range u.versionedExpected { + version := semver.MustParseRange(version) + if version(dataPlaneVersion) { + expectedConfig.Upstreams = append(expectedConfig.Upstreams, expectedUpstream) + } + } + } + } + } + + return util.EnsureConfig(expectedConfig) +} + func ensurePlugins(plugins []vcPlugins) error { kongAdmin, err := kongClient.NewClient(util.BasedKongAdminAPIAddr, nil) if err != nil { @@ -474,8 +613,8 @@ func TestVersionCompatibilitySyslogFacilityField(t *testing.T) { dpCleanup := run.KongDP(kong.GetKongConfForShared()) defer dpCleanup() - util.WaitForKong(t) - util.WaitForKongAdminAPI(t) + require.NoError(t, util.WaitForKong(t)) + require.NoError(t, util.WaitForKongAdminAPI(t)) admin := httpexpect.New(t, "http://localhost:3000")