From 0b1b75fbef0ab1cdb10d8bfccadbb0b4f3f4c043 Mon Sep 17 00:00:00 2001 From: Samuele Date: Tue, 3 Sep 2024 09:08:53 +0200 Subject: [PATCH] fix(diff): support auto fields (#136) * fix(diff): support auto fields call the new utils function FillPluginsDefaultsAutoFields from go-kong to ensure auto fields are considered when doing the diff * fix(crud): detect operation correctly fixes a bug that was introduced in e72f4c2de6609998d5e5671e14a9f7c0cd6a4c3e where noops were still detected as updates. That happened because of a check happening in plugin.go that detected the crud operation based on a diff. When done without considering default/auto fields, this check would see differences where there were none. This resulted in unnecessary updates to the DB and wrong diff strings during `sync` and `diff`. This commit adds defaults/auto fields to the configuration used for that diff. * pr review suggestions --- pkg/diff/diff.go | 14 +++-- pkg/types/core.go | 1 + pkg/types/plugin.go | 13 +++- tests/integration/sync_test.go | 59 +++++++++++++++++++ .../sync/034-fill-auto-oauth2/kong.yaml | 5 ++ 5 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 tests/integration/testdata/sync/034-fill-auto-oauth2/kong.yaml diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index f9b19ac..0fbf690 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -609,10 +609,7 @@ func (sc *Syncer) Solve(ctx context.Context, parallelism int, dry bool, isJSONOu // that will be used for the diff. This is needed to avoid highlighting // default values that were populated by Kong as differences. if plugin, ok := e.Obj.(*state.Plugin); ok { - pluginCopy := &state.Plugin{ - Plugin: *plugin.DeepCopy(), - Meta: plugin.Meta, - } + pluginCopy := &state.Plugin{Plugin: *plugin.DeepCopy()} e.Obj = pluginCopy exists, err := utils.WorkspaceExists(ctx, sc.kongClient) @@ -623,8 +620,13 @@ func (sc *Syncer) Solve(ctx context.Context, parallelism int, dry bool, isJSONOu return nil, err } - if err := kong.FillPluginsDefaults(&pluginCopy.Plugin, schema); err != nil { - return nil, fmt.Errorf("failed filling plugin defaults: %w", err) + var oldPlugin *kong.Plugin + if kongStatePlugin, ok := e.OldObj.(*state.Plugin); ok { + oldPlugin = &kongStatePlugin.Plugin + } + newPlugin := &pluginCopy.Plugin + if err := kong.FillPluginsDefaultsAutoFields(newPlugin, schema, oldPlugin); err != nil { + return nil, fmt.Errorf("failed processing auto fields: %w", err) } } } diff --git a/pkg/types/core.go b/pkg/types/core.go index ed04ce1..912c970 100644 --- a/pkg/types/core.go +++ b/pkg/types/core.go @@ -232,6 +232,7 @@ func NewEntity(t EntityType, opts EntityOpts) (Entity, error) { kind: entityTypeToKind(Plugin), currentState: opts.CurrentState, targetState: opts.TargetState, + kongClient: opts.KongClient, }, }, nil case Consumer: diff --git a/pkg/types/plugin.go b/pkg/types/plugin.go index 8e3b140..ae09493 100644 --- a/pkg/types/plugin.go +++ b/pkg/types/plugin.go @@ -88,6 +88,7 @@ type pluginDiffer struct { kind crud.Kind currentState, targetState *state.KongState + kongClient *kong.Client } func (d *pluginDiffer) Deletes(handler func(crud.Event) error) error { @@ -174,8 +175,18 @@ func (d *pluginDiffer) createUpdatePlugin(plugin *state.Plugin) (*crud.Event, er } currentPlugin = &state.Plugin{Plugin: *currentPlugin.DeepCopy()} // found, check if update needed + // before checking the diff, fill in the defaults + schema, err := d.kongClient.Plugins.GetFullSchema(context.TODO(), plugin.Name) + if err != nil { + return nil, fmt.Errorf("failed getting schema: %w", err) + } + pluginWithDefaults := &state.Plugin{Plugin: *plugin.DeepCopy()} + err = kong.FillPluginsDefaultsAutoFields(&pluginWithDefaults.Plugin, schema, ¤tPlugin.Plugin) + if err != nil { + return nil, fmt.Errorf("failed processing auto fields: %w", err) + } - if !currentPlugin.EqualWithOpts(plugin, false, true, false) { + if !currentPlugin.EqualWithOpts(pluginWithDefaults, false, true, false) { return &crud.Event{ Op: crud.Update, Kind: d.kind, diff --git a/tests/integration/sync_test.go b/tests/integration/sync_test.go index ba5019b..3269e2b 100644 --- a/tests/integration/sync_test.go +++ b/tests/integration/sync_test.go @@ -15,6 +15,7 @@ import ( "net/http/httptest" "net/url" "os" + "regexp" "strings" "testing" "time" @@ -5627,3 +5628,61 @@ func Test_Sync_PluginDoNotFillDefaults(t *testing.T) { }) }) } + +func Test_Sync_PluginAutoFields(t *testing.T) { + client, err := getTestClient() + + require.NoError(t, err) + ctx := context.Background() + t.Run("plugin_with_auto_fields", func(t *testing.T) { + mustResetKongState(ctx, t, client, deckDump.Config{}) + + currentState, err := fetchCurrentState(ctx, client, deckDump.Config{}) + require.NoError(t, err) + targetState := stateFromFile(ctx, t, + "testdata/sync/034-fill-auto-oauth2/kong.yaml", + client, + deckDump.Config{}, + ) + + kongURL, err := url.Parse(client.BaseRootURL()) + require.NoError(t, err) + p := NewRecordRequestProxy(kongURL) + s := httptest.NewServer(p) + c, err := utils.GetKongClient(utils.KongClientConfig{ + Address: s.URL, + }) + require.NoError(t, err) + + syncer, err := deckDiff.NewSyncer(deckDiff.SyncerOpts{ + CurrentState: currentState, + TargetState: targetState, + + KongClient: c, + }) + stats, errs, changes := syncer.Solve(ctx, 1, false, true) + require.Empty(t, errs, "Should have no errors in syncing") + require.NoError(t, err) + + require.Equal(t, int32(1), stats.CreateOps.Count(), "Should create 1 entity") + require.Len(t, changes.Creating, 1, "Should have 1 creating record in changes") + + t.Run("should not override auto values with nils", func(t *testing.T) { + newState, err := fetchCurrentState(ctx, client, deckDump.Config{}) + require.NoError(t, err) + plugins, err := newState.Plugins.GetAll() + require.NoError(t, err) + require.Len(t, plugins, 1) + plugin := plugins[0] + require.Equal(t, "oauth2", *plugin.Name) + provisionKey, ok := plugin.Config["provision_key"] + require.True(t, ok) + + provisionKeyStr, ok := provisionKey.(string) + require.True(t, ok, "provision_key is not a string") + pattern := `^[a-zA-Z0-9]+$` + re := regexp.MustCompile(pattern) + require.True(t, re.MatchString(provisionKeyStr), "provision_key does not match the pattern") + }) + }) +} diff --git a/tests/integration/testdata/sync/034-fill-auto-oauth2/kong.yaml b/tests/integration/testdata/sync/034-fill-auto-oauth2/kong.yaml new file mode 100644 index 0000000..59ba624 --- /dev/null +++ b/tests/integration/testdata/sync/034-fill-auto-oauth2/kong.yaml @@ -0,0 +1,5 @@ +_format_version: "3.0" +plugins: + - name: oauth2 + config: + enable_password_grant: true