Skip to content

Commit

Permalink
fix(diff): support auto fields (#136)
Browse files Browse the repository at this point in the history
* 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
e72f4c2 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
  • Loading branch information
samugi authored Sep 3, 2024
1 parent 25ccc04 commit 0b1b75f
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 7 deletions.
14 changes: 8 additions & 6 deletions pkg/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}
}
Expand Down
1 change: 1 addition & 0 deletions pkg/types/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
13 changes: 12 additions & 1 deletion pkg/types/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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, &currentPlugin.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,
Expand Down
59 changes: 59 additions & 0 deletions tests/integration/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"net/http/httptest"
"net/url"
"os"
"regexp"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -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")
})
})
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
_format_version: "3.0"
plugins:
- name: oauth2
config:
enable_password_grant: true

0 comments on commit 0b1b75f

Please sign in to comment.