-
Notifications
You must be signed in to change notification settings - Fork 451
feat: support dynamic modules #5669
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// Copyright Envoy Gateway Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// The full text of the Apache license is available in the LICENSE file at | ||
// the root of the repo. | ||
|
||
package v1alpha1 | ||
|
||
import ( | ||
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" | ||
) | ||
|
||
// DynamicModule defines a Dynamic Module extension. | ||
type DynamicModule struct { | ||
// Name is a unique name for this Dynamic Module extension. It is used to identify the | ||
// Dynamic Module extension if multiple extensions are loaded. | ||
// It's also used for logging/debugging. | ||
// If not specified, EG will generate a unique name for the Dynamic Module extension. | ||
// | ||
// +optional | ||
Name *string `json:"name,omitempty"` | ||
|
||
// Module is the name of the dynamic module to load. | ||
// The module name is used to search for the shared library file in the search path. | ||
// The search path is configured by the environment variable ENVOY_DYNAMIC_MODULES_SEARCH_PATH. | ||
// The actual search path is ${ENVOY_DYNAMIC_MODULES_SEARCH_PATH}/lib${name}.so. | ||
// | ||
// +kubebuilder:validation:Required | ||
Module string `json:"module"` | ||
|
||
// Config is the configuration for the Dynamic Module extension. | ||
// This configuration will be passed to the Dynamic Module extension. | ||
// +optional | ||
Config *apiextensionsv1.JSON `json:"config,omitempty"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i am not sure if we want to force JSON all the time. The rationale behind why the Envoy API is Any is that we don't want to force dynamic modules to pull in dependencies to parse the input. Simply making this to string should work i guess? For example, let's say i develop a module that allows javascript to run inside envoy to modify headers. Then this config will likely be a javascrip string instead of json There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Related to the comment above, I would make this |
||
|
||
// DoNotClose prevents the module from being unloaded with dlclose. | ||
// This is useful for modules that have global state that should not be unloaded. | ||
// A module is closed when no more references to it exist in the process. | ||
// For example, no HTTP filters are using the module (e.g. after configuration update). | ||
// | ||
// +optional | ||
DoNotClose *bool `json:"doNotClose,omitempty"` | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -65,6 +65,13 @@ type EnvoyExtensionPolicySpec struct { | |||||
// +kubebuilder:validation:MaxItems=16 | ||||||
// +optional | ||||||
Lua []Lua `json:"lua,omitempty"` | ||||||
|
||||||
// DynamicModule is an ordered list of Dynamic Module filters | ||||||
// that should be added to the envoy filter chain | ||||||
// | ||||||
// +kubebuilder:validation:MaxItems=16 | ||||||
// +optional | ||||||
DynamicModule []DynamicModule `json:"dynamicModule,omitempty"` | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess plural ?
Suggested change
|
||||||
} | ||||||
|
||||||
//+kubebuilder:object:root=true | ||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
apiVersion: gateway.envoyproxy.io/v1alpha1 | ||
kind: EnvoyExtensionPolicy | ||
metadata: | ||
name: dynamicmodule-example | ||
spec: | ||
targetRef: | ||
group: gateway.networking.k8s.io | ||
kind: HTTPRoute | ||
name: example | ||
dynamicModule: | ||
- name: my-dynamic-module | ||
module: my_module | ||
config: | ||
key: value | ||
# Prevent the module from being unloaded | ||
doNotClose: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
// Copyright Envoy Gateway Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// The full text of the Apache license is available in the LICENSE file at | ||
// the root of the repo. | ||
|
||
package gatewayapi | ||
|
||
import ( | ||
"fmt" | ||
"strconv" | ||
|
||
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" | ||
"github.com/envoyproxy/gateway/internal/gatewayapi/resource" | ||
"github.com/envoyproxy/gateway/internal/ir" | ||
) | ||
|
||
func (t *Translator) buildDynamicModules( | ||
policy *egv1a1.EnvoyExtensionPolicy, | ||
_ *resource.Resources, | ||
) ([]ir.DynamicModule, error) { | ||
var dynamicModuleIRList []ir.DynamicModule | ||
|
||
if policy == nil { | ||
return nil, nil | ||
} | ||
|
||
for idx, dm := range policy.Spec.DynamicModule { | ||
name := irConfigNameForDynamicModule(policy, idx) | ||
dynamicModuleIR, err := t.buildDynamicModule(name, dm) | ||
if err != nil { | ||
return nil, err | ||
} | ||
dynamicModuleIRList = append(dynamicModuleIRList, *dynamicModuleIR) | ||
} | ||
return dynamicModuleIRList, nil | ||
} | ||
|
||
func (t *Translator) buildDynamicModule( | ||
name string, | ||
config egv1a1.DynamicModule, | ||
) (*ir.DynamicModule, error) { | ||
// Validate required fields | ||
if config.Module == "" { | ||
return nil, fmt.Errorf("module is required") | ||
} | ||
|
||
dynamicModuleName := name | ||
if config.Name != nil { | ||
dynamicModuleName = *config.Name | ||
} | ||
|
||
// Set DoNotClose if specified | ||
doNotClose := false | ||
if config.DoNotClose != nil { | ||
doNotClose = *config.DoNotClose | ||
} | ||
|
||
dynamicModuleIR := &ir.DynamicModule{ | ||
Name: dynamicModuleName, | ||
Module: config.Module, | ||
Config: config.Config, | ||
DoNotClose: doNotClose, | ||
} | ||
|
||
return dynamicModuleIR, nil | ||
} | ||
|
||
func irConfigNameForDynamicModule(policy *egv1a1.EnvoyExtensionPolicy, index int) string { | ||
return fmt.Sprintf( | ||
"%s/dynamicmodule/%s", | ||
irConfigName(policy), | ||
strconv.Itoa(index)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would make it
ExtensionName
to clarify what is this for. I would also want to add the comments about one module can contain multiple extensions so this name is used to specify one out of many