Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement --dedup-plugin-configs flag for dump #581

Closed
wants to merge 4 commits into from
Closed

feat: implement --dedup-plugin-configs flag for dump #581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e1a8fd9
feat: implement --dedup-plugin-configs flag for dump
GGabriele Feb 3, 2022
1962653
rework dedup function and add tests
GGabriele Feb 7, 2022
f9ceccf
refactor
GGabriele Feb 11, 2022
2480cde
lint
GGabriele Apr 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/dump.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,5 +168,7 @@ configure Kong.`,
false, "assume 'yes' to prompts and run non-interactively.")
dumpCmd.Flags().BoolVar(&dumpConfig.SkipCACerts, "skip-ca-certificates",
false, "do not dump CA certificates.")
dumpCmd.Flags().BoolVar(&dumpConfig.DedupPluginsConfig, "dedup-plugin-configs",
false, "deduplicate plugins with same configuration.")
return dumpCmd
}
92 changes: 92 additions & 0 deletions dump/dump.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net/http"
"reflect"

"github.com/kong/deck/utils"
"github.com/kong/go-kong/kong"
Expand All @@ -26,6 +27,10 @@ type Config struct {
// SelectorTags can be used to export entities tagged with only specific
// tags.
SelectorTags []string

// If true, deduplicate plugins with the same configuration into a shared
// config object.
DedupPluginsConfig bool
}

func deduplicate(stringSlice []string) []string {
Expand Down Expand Up @@ -178,6 +183,10 @@ func getProxyConfiguration(ctx context.Context, group *errgroup.Group,
plugins = excludeConsumersPlugins(plugins)
}
state.Plugins = plugins
if config.DedupPluginsConfig {
state.SharedPluginsMap = make(map[string]utils.SharedPlugins)
state.SharedPluginsMap = dedupPluginsConfig(plugins)
}
return nil
})

Expand Down Expand Up @@ -225,6 +234,89 @@ func getProxyConfiguration(ctx context.Context, group *errgroup.Group,
})
}

func addSharedPluginConfig(p *kong.Plugin, sharedPlugin utils.SharedPlugin) utils.SharedPlugin {
if p.Consumer != nil && p.Consumer.ID != nil {
sharedPlugin.Consumers = append(sharedPlugin.Consumers, *p.Consumer.ID)
}
if p.Service != nil && p.Service.ID != nil {
sharedPlugin.Services = append(sharedPlugin.Services, *p.Service.ID)
}
if p.Route != nil && p.Route.ID != nil {
sharedPlugin.Routes = append(sharedPlugin.Routes, *p.Route.ID)
}
return sharedPlugin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tracking here loses the relationship of a plugin with multiple entities.
Consider the following plugin:

  plugins:
    name: rate-limiting
    service:
       id: svc-id
    consumer:
       id: consumer-id
    config:
      day: null
      fault_tolerant: true
      hide_client_headers: false
      hour: null
      limit_by: consumer
      minute: 10
      month: null
      policy: redis
      redis_database: 0
      redis_host: redis.common.svc
      redis_password: null
      redis_port: 6379
      redis_timeout: 2000
      second: null
      year: null
    enabled: true
    protocols:
    - http
    - https

In this case, this data structure would track the plugin as a plugin that is applied to the consumer, and the same plugin is applied to the service, in reality, that is not actually true, the plugin is present only once, on a combination of service and consumer.

The code that uses this data structure, later on, is structured in a way that one never runs into this bug currently. But that is a bit too risky to rely on. Please consider a data structure that captures the entire relationship explicitly.

}

// dedupPluginsConfig populates state.PluginConfigsMap, grouping consumers and services
// sharing the same plugin configs.
//
// Example:
//
// {
// "rate-limiting": {
// "rate-limiting-0": {
// "config": _specific_plugin_config_0,
// "consumers": ["consumer-1", "consumer-2"],
// "services": [],
// "routes": []
// },
// "rate-limiting-1": {
// "config": _specific_plugin_config_1,
// "consumers": ["consumer-3", "consumer-4"],
// "services": [],
// "routes": []
// }
// }
// }
func dedupPluginsConfig(plugins []*kong.Plugin) map[string]utils.SharedPlugins {
sharedPluginMap := make(map[string]utils.SharedPlugins)
for n, p := range plugins {
name := fmt.Sprintf("%s-%d", *p.Name, n)
cfgMap := utils.SharedPlugin{Config: p.Config}
sharedPlugin, ok := sharedPluginMap[*p.Name]
// if plugin is not in sharedPluginMap, add it
if !ok {
sharedPluginMap[*p.Name] = make(utils.SharedPlugins)
sharedPluginMap[*p.Name][name] = addSharedPluginConfig(p, cfgMap)
continue
}

var found bool
// check that plugin config matches any already present in sharedPluginMap
for name, config := range sharedPlugin {
if reflect.DeepEqual(config.Config, p.Config) {
sharedPluginMap[*p.Name][name] = addSharedPluginConfig(p, config)
found = true
break
}
}
// if no match is found, add it
if !found {
sharedPluginMap[*p.Name][name] = addSharedPluginConfig(p, cfgMap)
}
}
return removeSingleEntries(sharedPluginMap)
}

// remove plugins from sharedPlugins that are not reused by different entities
func removeSingleEntries(
sharedPlugins map[string]utils.SharedPlugins,
) map[string]utils.SharedPlugins {
newSharedPlugins := make(map[string]utils.SharedPlugins)
for plugin, content := range sharedPlugins {
for _, entry := range content {
if len(entry.Consumers) > 1 || len(entry.Routes) > 1 || len(entry.Services) > 1 {
if _, ok := newSharedPlugins[plugin]; !ok {
newSharedPlugins[plugin] = make(map[string]utils.SharedPlugin)
newSharedPlugins[plugin] = content
break
}
}
}
}
return newSharedPlugins
}

func getEnterpriseRBACConfiguration(ctx context.Context, group *errgroup.Group,
client *kong.Client, state *utils.KongRawState,
) {
Expand Down
Loading