Skip to content

Commit

Permalink
fix(sync): fill auto fields with nil
Browse files Browse the repository at this point in the history
Fill auto fields with nil in the config sent to the CP.
The rest of the default values are still not filled in the configuration
that is sent out, as they are meant to be filled by the CP itself, but
auto fields should not be generated, to ensure the configuration remains
truly "declarative".
  • Loading branch information
samugi committed Sep 4, 2024
1 parent a78866a commit fdbf4dc
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 32 deletions.
21 changes: 14 additions & 7 deletions pkg/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ func (sc *Syncer) Solve(ctx context.Context, parallelism int, dry bool, isJSONOu
// Below the configuration in `e` may be modified. This is done solely for
// the purpose of displaying a correct diff and should not affect the
// configuration that is sent to Kong.
originalE := e
eventForKong := e

// If the event is for a plugin, inject defaults in the plugin's config
// that will be used for the diff. This is needed to avoid highlighting
Expand All @@ -620,12 +620,19 @@ func (sc *Syncer) Solve(ctx context.Context, parallelism int, dry bool, isJSONOu
return nil, err
}

var oldPlugin *kong.Plugin
if kongStatePlugin, ok := e.OldObj.(*state.Plugin); ok {
oldPlugin = &kongStatePlugin.Plugin
}
// fill defaults and auto fields for the configuration that will be used for the diff
newPlugin := &pluginCopy.Plugin
if err := kong.FillPluginsDefaultsAutoFields(newPlugin, schema, oldPlugin); err != nil {
if err := kong.FillPluginsDefaults(newPlugin, schema); err != nil {
return nil, fmt.Errorf("failed processing auto fields: %w", err)
}

// only fill auto fields for the configuration sent to Kong
// this is done because we want to avoid Kong to auto generate fields, which
// would make decK's configuration no longer fully "declarative"
if err := kong.FillPluginsDefaultsWithOpts(&plugin.Plugin, schema, kong.FillRecordOptions{
FillDefaults: false,
FillAuto: true,
}); err != nil {
return nil, fmt.Errorf("failed processing auto fields: %w", err)
}
}
Expand Down Expand Up @@ -712,7 +719,7 @@ func (sc *Syncer) Solve(ctx context.Context, parallelism int, dry bool, isJSONOu
if !dry {
// sync mode
// fire the request to Kong
result, err = sc.processor.Do(ctx, originalE.Kind, originalE.Op, originalE)
result, err = sc.processor.Do(ctx, eventForKong.Kind, eventForKong.Op, eventForKong)
// TODO https://github.com/Kong/go-database-reconciler/issues/22 this does not print, but is switched on
// sc.enableEntityActions because the existing behavior returns a result from the anon Run function.
// Refactoring should use only the channel and simplify the return, probably to just an error (all the other
Expand Down
2 changes: 1 addition & 1 deletion pkg/types/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func (d *pluginDiffer) createUpdatePlugin(plugin *state.Plugin) (*crud.Event, er
return nil, fmt.Errorf("failed getting schema: %w", err)
}
pluginWithDefaults := &state.Plugin{Plugin: *plugin.DeepCopy()}
err = kong.FillPluginsDefaultsAutoFields(&pluginWithDefaults.Plugin, schema, &currentPlugin.Plugin)
err = kong.FillPluginsDefaults(&pluginWithDefaults.Plugin, schema)
if err != nil {
return nil, fmt.Errorf("failed processing auto fields: %w", err)
}
Expand Down
29 changes: 5 additions & 24 deletions tests/integration/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"net/http/httptest"
"net/url"
"os"
"regexp"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -5660,29 +5659,11 @@ func Test_Sync_PluginAutoFields(t *testing.T) {

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")
_, errs, _ := syncer.Solve(ctx, 1, false, true)

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")
})
require.NotNil(t, errs)
require.Len(t, errs, 1)
require.Contains(t, errs[0].Error(), "provision_key: required field missing",
"Should error out due to missing provision_key")
})
}

0 comments on commit fdbf4dc

Please sign in to comment.