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 24, 2024
1 parent 6d1bb06 commit 510522f
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 43 deletions.
33 changes: 31 additions & 2 deletions pkg/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,35 @@ func (sc *Syncer) Solve(ctx context.Context, parallelism int, dry bool, isJSONOu
var err error
var result crud.Arg

// holds the original event with the unchanged configuration
// below the config may be updated, to render the diff correctly
originalE := e

// If the event is for a plugin, inject defaults in the plugin's Config
// This is required for computing a diff that takes defaults into account
// Defaults are populated by Kong Gateway when a plugin is created and
// values are not provided; the diff should not display these values as changes
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 @@ -679,8 +708,8 @@ 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)
// fire the request to Kong; Pass the original event, with the unchanged configuration.
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 510522f

Please sign in to comment.