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.

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

Refactor compat policy #650

Merged
merged 4 commits into from
Dec 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading