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

feat(translator): extension server should fail close #4936

Merged
merged 8 commits into from
Jan 9, 2025
12 changes: 12 additions & 0 deletions api/v1alpha1/envoygateway_types.go
Original file line number Diff line number Diff line change
@@ -500,6 +500,18 @@ type ExtensionManager struct {
//
// +kubebuilder:validation:Required
Service *ExtensionService `json:"service,omitempty"`

// FailOpen defines if Envoy Gateway should ignore errors returned from the Extension Service hooks.
// The default is false, which means Envoy Gateway will fail closed if the Extension Service returns an error.
//
// Fail-close means that if the Extension Service hooks return an error, the relevant route/listener/resource
// will be replaced with a default configuration returning Internal Server Error (HTTP 500).
//
// Fail-open means that if the Extension Service hooks return an error, no changes will be applied to the
// source of the configuration which was sent to the extension server.
//
// +optional
FailOpen bool `json:"failOpen,omitempty"`
}

// ExtensionHooks defines extension hooks across all supported runners
33 changes: 19 additions & 14 deletions internal/extension/registry/extension_manager.go
Original file line number Diff line number Diff line change
@@ -107,6 +107,11 @@
}, c, nil
}

// FailOpen returns true if the extension manager is configured to fail open, and false otherwise.
func (m *Manager) FailOpen() bool {
return m.extension.FailOpen

Check warning on line 112 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L111-L112

Added lines #L111 - L112 were not covered by tests
}

// HasExtension checks to see whether a given Group and Kind has an
// associated extension registered for it.
func (m *Manager) HasExtension(g gwapiv1.Group, k gwapiv1.Kind) bool {
@@ -139,15 +144,15 @@
// that are used to generate an xDS resource.
// If the extension makes use of the hook then the XDS Hook Client is returned. If it does not support
// the hook type then nil is returned
func (m *Manager) GetPreXDSHookClient(xdsHookType egv1a1.XDSTranslatorHook) extTypes.XDSHookClient {
func (m *Manager) GetPreXDSHookClient(xdsHookType egv1a1.XDSTranslatorHook) (extTypes.XDSHookClient, error) {

Check warning on line 147 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L147

Added line #L147 was not covered by tests
ctx := context.Background()
ext := m.extension

if ext.Hooks == nil {
return nil
return nil, nil

Check warning on line 152 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L152

Added line #L152 was not covered by tests
}
if ext.Hooks.XDSTranslator == nil {
return nil
return nil, nil

Check warning on line 155 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L155

Added line #L155 was not covered by tests
}

hookUsed := false
@@ -158,20 +163,20 @@
}
}
if !hookUsed {
return nil
return nil, nil

Check warning on line 166 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L166

Added line #L166 was not covered by tests
}

if m.extensionConnCache == nil {
serverAddr := getExtensionServerAddress(ext.Service)

opts, err := setupGRPCOpts(ctx, m.k8sClient, &ext, m.namespace)
if err != nil {
return nil
return nil, err

Check warning on line 174 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L174

Added line #L174 was not covered by tests
}

conn, err := grpc.Dial(serverAddr, opts...)
if err != nil {
return nil
return nil, err

Check warning on line 179 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L179

Added line #L179 was not covered by tests
}

m.extensionConnCache = conn
@@ -181,22 +186,22 @@
xdsHookClient := &XDSHook{
grpcClient: client,
}
return xdsHookClient
return xdsHookClient, nil

Check warning on line 189 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L189

Added line #L189 was not covered by tests
}

// GetPostXDSHookClient checks if the registered extension makes use of a particular hook type that modifies
// xDS resources after they are generated by Envoy Gateway.
// If the extension makes use of the hook then the XDS Hook Client is returned. If it does not support
// the hook type then nil is returned
func (m *Manager) GetPostXDSHookClient(xdsHookType egv1a1.XDSTranslatorHook) extTypes.XDSHookClient {
func (m *Manager) GetPostXDSHookClient(xdsHookType egv1a1.XDSTranslatorHook) (extTypes.XDSHookClient, error) {

Check warning on line 196 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L196

Added line #L196 was not covered by tests
ctx := context.Background()
ext := m.extension

if ext.Hooks == nil {
return nil
return nil, nil

Check warning on line 201 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L201

Added line #L201 was not covered by tests
}
if ext.Hooks.XDSTranslator == nil {
return nil
return nil, nil

Check warning on line 204 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L204

Added line #L204 was not covered by tests
}

hookUsed := false
@@ -207,20 +212,20 @@
}
}
if !hookUsed {
return nil
return nil, nil

Check warning on line 215 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L215

Added line #L215 was not covered by tests
}

if m.extensionConnCache == nil {
serverAddr := getExtensionServerAddress(ext.Service)

opts, err := setupGRPCOpts(ctx, m.k8sClient, &ext, m.namespace)
if err != nil {
return nil
return nil, err

Check warning on line 223 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L223

Added line #L223 was not covered by tests
}

conn, err := grpc.Dial(serverAddr, opts...)
if err != nil {
return nil
return nil, err

Check warning on line 228 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L228

Added line #L228 was not covered by tests
}

m.extensionConnCache = conn
@@ -230,7 +235,7 @@
xdsHookClient := &XDSHook{
grpcClient: client,
}
return xdsHookClient
return xdsHookClient, nil

Check warning on line 238 in internal/extension/registry/extension_manager.go

Codecov / codecov/patch

internal/extension/registry/extension_manager.go#L238

Added line #L238 was not covered by tests
}

func (m *Manager) CleanupHookConns() {
7 changes: 5 additions & 2 deletions internal/extension/types/manager.go
Original file line number Diff line number Diff line change
@@ -25,11 +25,14 @@ type Manager interface {
// that are used to generate an xDS resource.
// If the extension makes use of the hook then the XDS Hook Client is returned. If it does not support
// the hook type then nil is returned
GetPreXDSHookClient(xdsHookType egv1a1.XDSTranslatorHook) XDSHookClient
GetPreXDSHookClient(xdsHookType egv1a1.XDSTranslatorHook) (XDSHookClient, error)

// GetPostXDSHookClient checks if the registered extension makes use of a particular hook type that modifies
// xDS resources after they are generated by Envoy Gateway.
// If the extension makes use of the hook then the XDS Hook Client is returned. If it does not support
// the hook type then nil is returned
GetPostXDSHookClient(xdsHookType egv1a1.XDSTranslatorHook) XDSHookClient
GetPostXDSHookClient(xdsHookType egv1a1.XDSTranslatorHook) (XDSHookClient, error)

// FailOpen returns true if the extension manager is configured to fail open, and false otherwise.
FailOpen() bool
}
20 changes: 16 additions & 4 deletions internal/xds/translator/extension.go
Original file line number Diff line number Diff line change
@@ -35,7 +35,10 @@

// Check if an extension want to modify the route that was just configured/created
extManager := *em
extRouteHookClient := extManager.GetPostXDSHookClient(egv1a1.XDSRoute)
extRouteHookClient, err := extManager.GetPostXDSHookClient(egv1a1.XDSRoute)
if err != nil {
return err
}

Check warning on line 41 in internal/xds/translator/extension.go

Codecov / codecov/patch

internal/xds/translator/extension.go#L40-L41

Added lines #L40 - L41 were not covered by tests
if extRouteHookClient == nil {
return nil
}
@@ -71,7 +74,10 @@

// Check if an extension want to modify the route that was just configured/created
extManager := *em
extVHHookClient := extManager.GetPostXDSHookClient(egv1a1.XDSVirtualHost)
extVHHookClient, err := extManager.GetPostXDSHookClient(egv1a1.XDSVirtualHost)
if err != nil {
return err
}

Check warning on line 80 in internal/xds/translator/extension.go

Codecov / codecov/patch

internal/xds/translator/extension.go#L79-L80

Added lines #L79 - L80 were not covered by tests
if extVHHookClient == nil {
return nil
}
@@ -100,7 +106,10 @@

// Check if an extension want to modify the listener that was just configured/created
extManager := *em
extListenerHookClient := extManager.GetPostXDSHookClient(egv1a1.XDSHTTPListener)
extListenerHookClient, err := extManager.GetPostXDSHookClient(egv1a1.XDSHTTPListener)
if err != nil {
return err
}

Check warning on line 112 in internal/xds/translator/extension.go

Codecov / codecov/patch

internal/xds/translator/extension.go#L111-L112

Added lines #L111 - L112 were not covered by tests
if extListenerHookClient != nil {
unstructuredResources := make([]*unstructured.Unstructured, len(extensionRefs))
for refIdx, ref := range extensionRefs {
@@ -140,7 +149,10 @@
// that is non-static. If a cluster definition is unlikely to change over the course of an extension's lifetime then the custom bootstrap config
// is the preferred way of adding it.
extManager := *em
extensionInsertHookClient := extManager.GetPostXDSHookClient(egv1a1.XDSTranslation)
extensionInsertHookClient, err := extManager.GetPostXDSHookClient(egv1a1.XDSTranslation)
if err != nil {
return err
}

Check warning on line 155 in internal/xds/translator/extension.go

Codecov / codecov/patch

internal/xds/translator/extension.go#L154-L155

Added lines #L154 - L155 were not covered by tests
if extensionInsertHookClient == nil {
return nil
}
12 changes: 12 additions & 0 deletions internal/xds/translator/extensionserver_test.go
Original file line number Diff line number Diff line change
@@ -202,6 +202,18 @@ func (t *testingExtensionServer) PostHTTPListenerModify(_ context.Context, req *

// PostTranslateModifyHook inserts and overrides some clusters/secrets
func (t *testingExtensionServer) PostTranslateModify(_ context.Context, req *pb.PostTranslateModifyRequest) (*pb.PostTranslateModifyResponse, error) {
for _, cluster := range req.Clusters {
// This simulates an extension server that returns an error. It allows verifying that fail-close is working.
if edsConfig := cluster.GetEdsClusterConfig(); edsConfig != nil {
if strings.Contains(edsConfig.ServiceName, "fail-close-error") {
return &pb.PostTranslateModifyResponse{
Clusters: req.Clusters,
Secrets: req.Secrets,
}, fmt.Errorf("cluster hook resource error: %s", edsConfig.ServiceName)
}
}
}

extensionSvcEndpoint := &endpointV3.LbEndpoint_Endpoint{
Endpoint: &endpointV3.Endpoint{
Address: &coreV3.Address{
10 changes: 7 additions & 3 deletions internal/xds/translator/runner/runner_test.go
Original file line number Diff line number Diff line change
@@ -174,12 +174,16 @@ type extManagerMock struct {
types.Manager
}

func (m *extManagerMock) GetPostXDSHookClient(xdsHookType egv1a1.XDSTranslatorHook) types.XDSHookClient {
func (m *extManagerMock) GetPostXDSHookClient(xdsHookType egv1a1.XDSTranslatorHook) (types.XDSHookClient, error) {
if xdsHookType == egv1a1.XDSHTTPListener {
return &xdsHookClientMock{}
return &xdsHookClientMock{}, nil
}

return nil
return nil, nil
}

func (m *extManagerMock) FailOpen() bool {
return false
}

type xdsHookClientMock struct {
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
http:
- name: "extension-post-xdstranslate-hook-error"
address: "0.0.0.0"
port: 10080
hostnames:
- "*"
path:
mergeSlashes: true
escapedSlashesAction: UnescapeAndRedirect
routes:
- name: "first-route"
hostname: "*"
pathMatch:
prefix: "/"
destination:
name: "fail-close-error"
settings:
- endpoints:
- host: "1.2.3.4"
port: 50000
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
- circuitBreakers:
thresholds:
- maxRetries: 1024
commonLbConfig:
localityWeightedLbConfig: {}
connectTimeout: 10s
dnsLookupFamily: V4_PREFERRED
edsClusterConfig:
edsConfig:
ads: {}
resourceApiVersion: V3
serviceName: first-route-dest
ignoreHealthOnHostRemoval: true
lbPolicy: LEAST_REQUEST
name: first-route-dest
perConnectionBufferLimitBytes: 32768
type: EDS
- loadAssignment:
clusterName: mock-extension-injected-cluster
endpoints:
- lbEndpoints:
- endpoint:
address:
socketAddress:
address: exampleservice.examplenamespace.svc.cluster.local
portValue: 5000
name: mock-extension-injected-cluster
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
- clusterName: first-route-dest
endpoints:
- lbEndpoints:
- endpoint:
address:
socketAddress:
address: 1.2.3.4
portValue: 50000
loadBalancingWeight: 1
loadBalancingWeight: 1
locality:
region: first-route-dest/backend/0
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
- address:
socketAddress:
address: 0.0.0.0
portValue: 10080
defaultFilterChain:
filters:
- name: envoy.filters.network.http_connection_manager
typedConfig:
'@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
commonHttpProtocolOptions:
headersWithUnderscoresAction: REJECT_REQUEST
http2ProtocolOptions:
initialConnectionWindowSize: 1048576
initialStreamWindowSize: 65536
maxConcurrentStreams: 100
httpFilters:
- name: envoy.filters.http.router
typedConfig:
'@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
suppressEnvoyHeaders: true
mergeSlashes: true
normalizePath: true
pathWithEscapedSlashesAction: UNESCAPE_AND_REDIRECT
routeConfig:
name: error_route_configuration
virtualHosts:
- domains:
- '*'
name: error_vhost
routes:
- directResponse:
status: 500
match:
prefix: /
name: error_route
serverHeaderTransformation: PASS_THROUGH
statPrefix: http-10080
useRemoteAddress: true
name: extension-post-xdslistener-hook-error
name: extension-post-xdslistener-hook-error
perConnectionBufferLimitBytes: 32768
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
- ignorePortInHostMatching: true
name: extension-post-xdslistener-hook-error
virtualHosts:
- domains:
- '*'
name: extension-post-xdslistener-hook-error/*
routes:
- match:
prefix: /
name: first-route
route:
cluster: first-route-dest
upgradeConfigs:
- upgradeType: websocket
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- genericSecret:
secret:
inlineString: super-secret-extension-secret
name: mock-extension-injected-secret
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
- circuitBreakers:
thresholds:
- maxRetries: 1024
commonLbConfig:
localityWeightedLbConfig: {}
connectTimeout: 10s
dnsLookupFamily: V4_PREFERRED
edsClusterConfig:
edsConfig:
ads: {}
resourceApiVersion: V3
serviceName: extension-post-xdsroute-hook-error-dest
ignoreHealthOnHostRemoval: true
lbPolicy: LEAST_REQUEST
name: extension-post-xdsroute-hook-error-dest
perConnectionBufferLimitBytes: 32768
type: EDS
- loadAssignment:
clusterName: mock-extension-injected-cluster
endpoints:
- lbEndpoints:
- endpoint:
address:
socketAddress:
address: exampleservice.examplenamespace.svc.cluster.local
portValue: 5000
name: mock-extension-injected-cluster
Loading