Skip to content

Commit

Permalink
Refactor compat policy (#650)
Browse files Browse the repository at this point in the history
* Remove compatibility policy
* Regenerate stubs
* Remove compatibility policy
* Check fast for plugin version existence and simplify syntax
  • Loading branch information
mostafa authored Dec 28, 2024
1 parent 94397ea commit 788b759
Show file tree
Hide file tree
Showing 19 changed files with 291 additions and 357 deletions.
1 change: 0 additions & 1 deletion .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ jobs:
cd plugin-template-go && make build && cp plugin-template-go ../ptg && cd ..
export SHA256SUM=$(sha256sum ptg | awk '{print $1}')
cat <<EOF > gatewayd_plugins.yaml
compatibilityPolicy: "strict"
metricsMergerPeriod: 1s
healthCheckPeriod: 1s
reloadOnCrash: true
Expand Down
7 changes: 3 additions & 4 deletions api/api_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ func getAPIConfig(httpAddr, grpcAddr string) *API {
Actions: act.BuiltinActions(),
DefaultPolicyName: config.DefaultPolicy,
}),
DevMode: true,
Logger: logger,
Compatibility: config.DefaultCompatibilityPolicy,
StartTimeout: config.DefaultPluginStartTimeout,
DevMode: true,
Logger: logger,
StartTimeout: config.DefaultPluginStartTimeout,
},
)
defaultProxy := network.NewProxy(
Expand Down
21 changes: 9 additions & 12 deletions api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,9 @@ func TestGetPlugins(t *testing.T) {
pluginRegistry := plugin.NewRegistry(
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: zerolog.Logger{},
DevMode: true,
ActRegistry: actRegistry,
Logger: zerolog.Logger{},
DevMode: true,
},
)
pluginRegistry.Add(&plugin.Plugin{
Expand Down Expand Up @@ -192,10 +191,9 @@ func TestGetPluginsWithEmptyPluginRegistry(t *testing.T) {
pluginRegistry := plugin.NewRegistry(
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: zerolog.Logger{},
DevMode: true,
ActRegistry: actRegistry,
Logger: zerolog.Logger{},
DevMode: true,
},
)

Expand Down Expand Up @@ -332,10 +330,9 @@ func TestGetServers(t *testing.T) {
pluginRegistry := plugin.NewRegistry(
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: zerolog.Logger{},
DevMode: true,
ActRegistry: actRegistry,
Logger: zerolog.Logger{},
DevMode: true,
},
)

Expand Down
7 changes: 3 additions & 4 deletions api/healthcheck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,9 @@ func Test_Healthchecker(t *testing.T) {
pluginRegistry := plugin.NewRegistry(
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: zerolog.Logger{},
DevMode: true,
ActRegistry: actRegistry,
Logger: zerolog.Logger{},
DevMode: true,
},
)

Expand Down
463 changes: 228 additions & 235 deletions api/v1/api.pb.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion api/v1/api.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions api/v1/api.swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 2 additions & 8 deletions cmd/gatewayd_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,14 +234,8 @@ func (app *GatewayDApp) createPluginRegistry(runCtx context.Context, logger zero
runCtx,
plugin.Registry{
ActRegistry: app.actRegistry,
Compatibility: config.If(
config.Exists(
config.CompatibilityPolicies, app.conf.Plugin.CompatibilityPolicy,
),
config.CompatibilityPolicies[app.conf.Plugin.CompatibilityPolicy],
config.DefaultCompatibilityPolicy),
Logger: logger,
DevMode: app.DevMode,
Logger: logger,
DevMode: app.DevMode,
},
)
}
Expand Down
1 change: 0 additions & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ func (c *Config) LoadDefaults(ctx context.Context) *gerr.GatewayDError {
}

c.pluginDefaults = PluginConfig{
CompatibilityPolicy: string(Strict),
EnableMetricsMerger: true,
MetricsMergerPeriod: DefaultMetricsMergerPeriod,
HealthCheckPeriod: DefaultPluginHealthCheckPeriod,
Expand Down
14 changes: 2 additions & 12 deletions config/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import (
)

type (
Status uint
CompatibilityPolicy string
LogOutput uint
Status uint
LogOutput uint
)

// Status is the status of the server.
Expand All @@ -16,12 +15,6 @@ const (
Stopped
)

// CompatibilityPolicy is the compatibility policy for plugins.
const (
Strict CompatibilityPolicy = "strict" // Expect all required plugins to be loaded and present
Loose CompatibilityPolicy = "loose" // Load the plugin, even if the requirements are not met
)

// LogOutput is the output type for the logger.
const (
Console LogOutput = iota
Expand Down Expand Up @@ -117,9 +110,6 @@ const (
DefaultGRPCAPIAddress = "localhost:19090"

// Policies.
DefaultCompatibilityPolicy = Strict

// Act.
DefaultPolicy = "passthrough"
DefaultPolicyTimeout = 30 * time.Second
DefaultActionTimeout = 30 * time.Second
Expand Down
4 changes: 0 additions & 4 deletions config/getters.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ import (
)

var (
CompatibilityPolicies = map[string]CompatibilityPolicy{
"strict": Strict,
"loose": Loose,
}
logOutputs = map[string]LogOutput{
"console": Console,
"stdout": Stdout,
Expand Down
1 change: 0 additions & 1 deletion config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ type Policy struct {
}

type PluginConfig struct {
CompatibilityPolicy string `json:"compatibilityPolicy" jsonschema:"enum=strict,enum=loose"`
EnableMetricsMerger bool `json:"enableMetricsMerger"`
MetricsMergerPeriod time.Duration `json:"metricsMergerPeriod" jsonschema:"oneof_type=string;integer"`
HealthCheckPeriod time.Duration `json:"healthCheckPeriod" jsonschema:"oneof_type=string;integer"`
Expand Down
9 changes: 0 additions & 9 deletions gatewayd_plugins.yaml
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
# GatewayD Plugin Configuration

# The compatibility policy controls how GatewayD treats plugins' requirements. If a plugin
# requires a specific version of another plugin, the compatibility policy controls whether to
# allow or reject the plugin.
# - "strict" (default): the plugin is rejected if it requires a specific version of another
# plugin and that version is not the one currently loaded.
# - "loose": the plugin is allowed to run even if it requires a specific version of another
# plugin and that version is not the one currently loaded.
compatibilityPolicy: "strict"

# The metrics policy controls whether to collect and merge metrics from plugins or not.
# The Prometheus metrics are collected from the plugins via a Unix domain socket. The metrics
# are merged and exposed via the GatewayD metrics endpoint via HTTP.
Expand Down
30 changes: 12 additions & 18 deletions network/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ func TestNewProxy(t *testing.T) {
PluginRegistry: plugin.NewRegistry(
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: logger,
ActRegistry: actRegistry,
Logger: logger,
},
),
HealthCheckPeriod: config.DefaultHealthCheckPeriod,
Expand Down Expand Up @@ -124,9 +123,8 @@ func BenchmarkNewProxy(b *testing.B) {
PluginRegistry: plugin.NewRegistry(
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: logger,
ActRegistry: actRegistry,
Logger: logger,
},
),
HealthCheckPeriod: config.DefaultHealthCheckPeriod,
Expand Down Expand Up @@ -182,9 +180,8 @@ func BenchmarkProxyConnectDisconnect(b *testing.B) {
PluginRegistry: plugin.NewRegistry(
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: logger,
ActRegistry: actRegistry,
Logger: logger,
},
),
HealthCheckPeriod: config.DefaultHealthCheckPeriod,
Expand Down Expand Up @@ -248,9 +245,8 @@ func BenchmarkProxyPassThrough(b *testing.B) {
PluginRegistry: plugin.NewRegistry(
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: logger,
ActRegistry: actRegistry,
Logger: logger,
},
),
HealthCheckPeriod: config.DefaultHealthCheckPeriod,
Expand Down Expand Up @@ -319,9 +315,8 @@ func BenchmarkProxyIsHealthyAndIsExhausted(b *testing.B) {
PluginRegistry: plugin.NewRegistry(
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: logger,
ActRegistry: actRegistry,
Logger: logger,
},
),
HealthCheckPeriod: config.DefaultHealthCheckPeriod,
Expand Down Expand Up @@ -388,9 +383,8 @@ func BenchmarkProxyAvailableAndBusyConnectionsString(b *testing.B) {
PluginRegistry: plugin.NewRegistry(
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: logger,
ActRegistry: actRegistry,
Logger: logger,
},
),
HealthCheckPeriod: config.DefaultHealthCheckPeriod,
Expand Down
5 changes: 2 additions & 3 deletions network/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ func TestRunServer(t *testing.T) {
pluginRegistry := plugin.NewRegistry(
ctx,
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: logger,
ActRegistry: actRegistry,
Logger: logger,
})

pluginRegistry.AddHook(v1.HookName_HOOK_NAME_ON_TRAFFIC_FROM_CLIENT, 1, onIncomingTraffic)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# Plugin configuration file for GatewayD

compatibilityPolicy: "strict"
enableMetricsMerger: True
metricsMergerPeriod: 5s
healthCheckPeriod: 5s
Expand Down
39 changes: 15 additions & 24 deletions plugin/plugin_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,8 @@ type Registry struct {
ctx context.Context //nolint:containedctx
DevMode bool

Logger zerolog.Logger
Compatibility config.CompatibilityPolicy
StartTimeout time.Duration
Logger zerolog.Logger
StartTimeout time.Duration
}

var _ IRegistry = (*Registry)(nil)
Expand All @@ -79,13 +78,12 @@ func NewRegistry(
defer span.End()

return &Registry{
plugins: pool.NewPool(regCtx, config.EmptyPoolCapacity),
hooks: map[v1.HookName]map[sdkPlugin.Priority]sdkPlugin.Method{},
ActRegistry: registry.ActRegistry,
ctx: regCtx,
DevMode: registry.DevMode,
Logger: registry.Logger,
Compatibility: registry.Compatibility,
plugins: pool.NewPool(regCtx, config.EmptyPoolCapacity),
hooks: map[v1.HookName]map[sdkPlugin.Priority]sdkPlugin.Method{},
ActRegistry: registry.ActRegistry,
ctx: regCtx,
DevMode: registry.DevMode,
Logger: registry.Logger,
}
}

Expand Down Expand Up @@ -144,6 +142,11 @@ func (reg *Registry) Exists(name, version, remoteURL string) bool {

for _, plugin := range reg.List() {
if plugin.Name == name && plugin.RemoteURL == remoteURL {
// If the version is the same, the plugin exists.
if version == plugin.Version {
return true
}

// Parse the supplied version and the version in the registry.
suppliedVer, err := semver.NewVersion(version)
if err != nil {
Expand All @@ -162,7 +165,7 @@ func (reg *Registry) Exists(name, version, remoteURL string) bool {
// Check if the version of the plugin is less than or equal to
// the version in the registry.
// TODO: Should we check the major version only, or as well?
if suppliedVer.LessThan(registryVer) || suppliedVer.Equal(registryVer) {
if suppliedVer.LessThanEqual(registryVer) {
return true
}

Expand Down Expand Up @@ -559,19 +562,7 @@ func (reg *Registry) LoadPlugins(
"requirement": req.Name,
},
).Msg("The plugin requirement is not met, so it won't work properly")
if reg.Compatibility == config.Strict {
reg.Logger.Debug().Str("name", plugin.ID.Name).Msg(
"Registry is in strict compatibility mode, so the plugin won't be loaded")
plugin.Stop() // Stop the plugin.
continue
}
reg.Logger.Debug().Fields(
map[string]any{
"name": plugin.ID.Name,
"requirement": req.Name,
},
).Msg("Registry is in loose compatibility mode, " +
"so the plugin will be loaded anyway")
plugin.Stop() // Stop the plugin.
}
}

Expand Down
10 changes: 4 additions & 6 deletions plugin/plugin_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ func NewPluginRegistry(t *testing.T) *Registry {
reg := NewRegistry(
context.Background(),
Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: logger,
ActRegistry: actRegistry,
Logger: logger,
})
return reg
}
Expand Down Expand Up @@ -157,9 +156,8 @@ func BenchmarkHookRun(b *testing.B) {
reg := NewRegistry(
context.Background(),
Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Logger: logger,
ActRegistry: actRegistry,
Logger: logger,
})
hookFunction := func(
_ context.Context, args *v1.Struct, _ ...grpc.CallOption,
Expand Down
Loading

0 comments on commit 788b759

Please sign in to comment.