Skip to content
Open
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
14 changes: 8 additions & 6 deletions api/v1alpha1/kgateway/backend_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,17 @@ const (
)

// BackendSpec defines the desired state of Backend.
// +kubebuilder:validation:XValidation:message="aws backend must be specified when type is 'AWS'",rule="self.type == 'AWS' ? has(self.aws) : true"
// +kubebuilder:validation:XValidation:message="static backend must be specified when type is 'Static'",rule="self.type == 'Static' ? has(self.static) : true"
// +kubebuilder:validation:XValidation:message="dynamicForwardProxy backend must be specified when type is 'DynamicForwardProxy'",rule="self.type == 'DynamicForwardProxy' ? has(self.dynamicForwardProxy) : true"
Comment on lines -44 to -46
Copy link
Member

Choose a reason for hiding this comment

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

I think we still want these rules in place even if the Type field becomes optional.

// +kubebuilder:validation:ExactlyOneOf=aws;static;dynamicForwardProxy
// +kubebuilder:validation:XValidation:message="exactly one of aws, static, or dynamicForwardProxy must be specified",rule="[has(self.aws), has(self.static), has(self.dynamicForwardProxy)].filter(v, v).size() == 1"
Copy link
Member

Choose a reason for hiding this comment

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

Let's revert back to // +kubebuilder:validation:ExactlyOneOf=aws;static;dynamicForwardProxy. That ExactlyOneOf marker is the preferred pattern for these one-of based APIs.

// +kubebuilder:validation:XValidation:message="backend configuration must match the specified type",rule="!has(self.type) || (self.type == 'AWS' && has(self.aws)) || (self.type == 'Static' && has(self.static)) || (self.type == 'DynamicForwardProxy' && has(self.dynamicForwardProxy))"
type BackendSpec struct {
// Type indicates the type of the backend to be used.
//
// Deprecated: The Type field is deprecated and will be removed in a future release.
// The backend type is inferred from the configuration.
//
// +kubebuilder:validation:Enum=AWS;Static;DynamicForwardProxy
// +required
Type BackendType `json:"type"`
// +optional
Type BackendType `json:"type,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Surprised the linter didn't catch this, but this should be a pointer type now. See https://github.com/kgateway-dev/kgateway/pull/12923/files for some inspiration.

// Aws is the AWS backend configuration.
// The Aws backend type is only supported with envoy-based gateways, it is not supported in agentgateway.
// +optional
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,28 +213,26 @@ spec:
- hosts
type: object
type:
description: Type indicates the type of the backend to be used.
description: |-
Type indicates the type of the backend to be used.

Deprecated: The Type field is deprecated and will be removed in a future release.
The backend type is inferred from the configuration.
enum:
- AWS
- Static
- DynamicForwardProxy
type: string
required:
- type
type: object
x-kubernetes-validations:
- message: aws backend must be specified when type is 'AWS'
rule: 'self.type == ''AWS'' ? has(self.aws) : true'
- message: static backend must be specified when type is 'Static'
rule: 'self.type == ''Static'' ? has(self.static) : true'
- message: dynamicForwardProxy backend must be specified when type is
'DynamicForwardProxy'
rule: 'self.type == ''DynamicForwardProxy'' ? has(self.dynamicForwardProxy)
: true'
- message: exactly one of the fields in [aws static dynamicForwardProxy]
must be set
rule: '[has(self.aws),has(self.static),has(self.dynamicForwardProxy)].filter(x,x==true).size()
== 1'
- message: exactly one of aws, static, or dynamicForwardProxy must be
specified
rule: '[has(self.aws), has(self.static), has(self.dynamicForwardProxy)].filter(v,
v).size() == 1'
- message: backend configuration must match the specified type
rule: '!has(self.type) || (self.type == ''AWS'' && has(self.aws)) ||
(self.type == ''Static'' && has(self.static)) || (self.type == ''DynamicForwardProxy''
&& has(self.dynamicForwardProxy))'
status:
description: BackendStatus defines the observed state of Backend.
properties:
Expand Down
36 changes: 29 additions & 7 deletions pkg/kgateway/extensions2/plugins/backend/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ func buildTranslateFunc(
) func(krtctx krt.HandlerContext, i *kgateway.Backend) *backendIr {
return func(krtctx krt.HandlerContext, i *kgateway.Backend) *backendIr {
var beIr backendIr
switch i.Spec.Type {
backendType := getBackendType(i.Spec)
switch backendType {
case kgateway.BackendTypeStatic:
staticIr, err := buildStaticIr(i.Spec.Static)
if err != nil {
Expand Down Expand Up @@ -212,7 +213,8 @@ func processBackendForEnvoy(ctx context.Context, in ir.BackendObjectIR, out *env
// TODO: propagated error to CRD #11558.
// TODO(tim): do we need to do anything here for AI backends?
spec := be.Spec
switch spec.Type {
backendType := getBackendType(spec)
switch backendType {
case kgateway.BackendTypeStatic:
processStatic(beIr.staticIr, out)
case kgateway.BackendTypeAWS:
Expand All @@ -227,7 +229,8 @@ func processBackendForEnvoy(ctx context.Context, in ir.BackendObjectIR, out *env
}

func parseAppProtocol(b *kgateway.Backend) ir.AppProtocol {
switch b.Spec.Type {
backendType := getBackendType(b.Spec)
switch backendType {
case kgateway.BackendTypeStatic:
appProtocol := b.Spec.Static.AppProtocol
if appProtocol != nil {
Expand All @@ -239,7 +242,7 @@ func parseAppProtocol(b *kgateway.Backend) ir.AppProtocol {

// hostname returns the hostname for the backend. Only static backends are supported.
func hostname(in *kgateway.Backend) string {
if in.Spec.Type != kgateway.BackendTypeStatic {
if getBackendType(in.Spec) != kgateway.BackendTypeStatic {
return ""
}
if len(in.Spec.Static.Hosts) == 0 {
Expand All @@ -250,15 +253,33 @@ func hostname(in *kgateway.Backend) string {

func processEndpoints(be *kgateway.Backend) *ir.EndpointsForBackend {
spec := be.Spec
backendType := getBackendType(spec)
switch {
case spec.Type == kgateway.BackendTypeStatic:
case backendType == kgateway.BackendTypeStatic:
return processEndpointsStatic(spec.Static)
case spec.Type == kgateway.BackendTypeAWS:
case backendType == kgateway.BackendTypeAWS:
return processEndpointsAws(spec.Aws)
}
return nil
}

func getBackendType(spec kgateway.BackendSpec) kgateway.BackendType {
// nolint:staticcheck // Intentional: we still read the deprecated Type field for backward compatibility
if spec.Type != "" {
return spec.Type
}
if spec.Aws != nil {
return kgateway.BackendTypeAWS
}
if spec.Static != nil {
return kgateway.BackendTypeStatic
}
if spec.DynamicForwardProxy != nil {
return kgateway.BackendTypeDynamicForwardProxy
}
return ""
}

type backendPlugin struct {
ir.UnimplementedProxyTranslationPass
needsDfpFilter map[string]bool
Expand All @@ -276,7 +297,8 @@ func (p *backendPlugin) Name() string {

func (p *backendPlugin) ApplyForBackend(pCtx *ir.RouteBackendContext, in ir.HttpBackend, out *envoyroutev3.Route) error {
backend := pCtx.Backend.Obj.(*kgateway.Backend)
switch backend.Spec.Type {
backendType := getBackendType(backend.Spec)
switch backendType {
case kgateway.BackendTypeDynamicForwardProxy:
if p.needsDfpFilter == nil {
p.needsDfpFilter = make(map[string]bool)
Expand Down
14 changes: 14 additions & 0 deletions test/e2e/tests/api_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@ spec:
`,
wantErrors: []string{"exactly one of the fields in [aws static dynamicForwardProxy] must be set"},
},
{
name: "Backend: inferred type from configuration",
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to test this, but if we do, let's migrate it over to the https://github.com/kgateway-dev/kgateway/tree/af11f9bab3cfae7d2d2d9f61df70fc97325c7348/api/tests suite.

input: `---
apiVersion: gateway.kgateway.dev/v1alpha1
kind: Backend
metadata:
name: backend-inferred-type
spec:
static:
hosts:
- host: example.com
port: 80
`,
},
{
name: "Backend: empty lambda qualifier does not match pattern",
input: `---
Expand Down
Loading