Skip to content

Commit

Permalink
fix(diff+file): avoid filling defaults in config for kong
Browse files Browse the repository at this point in the history
the previous logic was filling defaults in the configuration that was
passed to Kong. This was  problematic, especially where nils were
populated as defaults, e.g. if a shorthand_field was passed with some
value and the corresponding new field is auto-populated as `nil` by decK
, the auto-populated nil value would take precedence in Kong thus causing
the shorthand_field to be ignored
(https://konghq.atlassian.net/browse/KAG-5157).

This change applies the default values only to configurations used for
diff, the original configuration is always passed to Kong as is.
  • Loading branch information
samugi committed Aug 27, 2024
1 parent 64dd801 commit 592369a
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 42 deletions.
32 changes: 31 additions & 1 deletion pkg/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,36 @@ func (sc *Syncer) Solve(ctx context.Context, parallelism int, dry bool, isJSONOu
var err error
var result crud.Arg

// This variable holds the original event with the unchanged configuration
// 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

// 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
// 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,
}
e.Obj = pluginCopy

exists, err := utils.WorkspaceExists(ctx, sc.kongClient)
if err == nil && exists {
var schema map[string]interface{}
schema, err = sc.kongClient.Plugins.GetFullSchema(ctx, pluginCopy.Plugin.Name)
if err != nil {
return nil, err
}

if err := kong.FillPluginsDefaults(&pluginCopy.Plugin, schema); err != nil {
return nil, fmt.Errorf("failed filling plugin defaults: %w", err)
}
}
}

c := e.Obj.(state.ConsoleString)
objDiff := map[string]interface{}{
"old": e.OldObj,
Expand Down Expand Up @@ -680,7 +710,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, e.Kind, e.Op, e)
result, err = sc.processor.Do(ctx, originalE.Kind, originalE.Op, originalE)
// 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
41 changes: 0 additions & 41 deletions pkg/file/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1443,44 +1443,6 @@ func (b *stateBuilder) ingestRoute(r FRoute) error {
return nil
}

func (b *stateBuilder) getPluginSchema(pluginName string) (map[string]interface{}, error) {
var schema map[string]interface{}

// lookup in cache
if schema, ok := b.schemasCache[pluginName]; ok {
return schema, nil
}

exists, err := utils.WorkspaceExists(b.ctx, b.client)
if err != nil {
return nil, fmt.Errorf("ensure workspace exists: %w", err)
}
if !exists {
return schema, ErrWorkspaceNotFound
}

schema, err = b.client.Plugins.GetFullSchema(b.ctx, &pluginName)
if err != nil {
return schema, err
}
b.schemasCache[pluginName] = schema
return schema, nil
}

func (b *stateBuilder) addPluginDefaults(plugin *FPlugin) error {
if b.client == nil {
return nil
}
schema, err := b.getPluginSchema(*plugin.Name)
if err != nil {
if errors.Is(err, ErrWorkspaceNotFound) {
return nil
}
return fmt.Errorf("retrieve schema for %v from Kong: %w", *plugin.Name, err)
}
return kong.FillPluginsDefaults(&plugin.Plugin, schema)
}

func (b *stateBuilder) ingestPlugins(plugins []FPlugin) error {
for _, p := range plugins {
p := p
Expand All @@ -1504,9 +1466,6 @@ func (b *stateBuilder) ingestPlugins(plugins []FPlugin) error {
if err != nil {
return err
}
if err := b.addPluginDefaults(&p); err != nil {
return fmt.Errorf("add defaults to plugin '%v': %w", *p.Name, err)
}
utils.MustMergeTags(&p, b.selectTags)
if plugin != nil {
p.Plugin.CreatedAt = plugin.CreatedAt
Expand Down

0 comments on commit 592369a

Please sign in to comment.