Skip to content

Commit

Permalink
Rewrite options to fix validation and overriding (#119)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tarick authored Nov 26, 2021
1 parent 585b7e2 commit 8abc127
Show file tree
Hide file tree
Showing 12 changed files with 176 additions and 121 deletions.
5 changes: 2 additions & 3 deletions api/v1alpha1/api_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,8 @@ func (r *API) validate() error {
if err != nil {
return fmt.Errorf("spec: x-kusk should be a valid set of options: %w", err)
}

err = opts.FillDefaultsAndValidate()
if err != nil {
opts.FillDefaults()
if err = opts.Validate(); err != nil {
return fmt.Errorf("spec: x-kusk should be a valid set of options: %w", err)
}

Expand Down
9 changes: 5 additions & 4 deletions config/crd/bases/gateway.kusk.io_staticroutes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,11 @@ spec:
description: Rewrite is the pattern (regex) and a
substitution string that will change URL when request
is being forwarded to the upstream service. e.g.
given that Base is set to "/petstore/api/v3", and
with Rewrite.Pattern is set to "^/petstore", Rewrite.Substitution
is set to "" path that would be generated is "/petstore/api/v3/pets",
URL that the upstream service would receive is "/api/v3/pets".
given that Prefix is set to "/petstore/api/v3",
and with Rewrite.Pattern is set to "^/petstore",
Rewrite.Substitution is set to "" path that would
be generated is "/petstore/api/v3/pets", URL that
the upstream service would receive is "/api/v3/pets".
properties:
pattern:
type: string
Expand Down
4 changes: 2 additions & 2 deletions controllers/config_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ func (c *KubeEnvoyConfigManager) UpdateConfiguration(ctx context.Context) error
if err != nil {
return fmt.Errorf("failed to parse options: %w", err)
}

if err := opts.FillDefaultsAndValidate(); err != nil {
opts.FillDefaults()
if err := opts.Validate(); err != nil {
return fmt.Errorf("failed to validate options: %w", err)
}

Expand Down
8 changes: 4 additions & 4 deletions envoy/config/config_routines.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package config

import (
"fmt"
"reflect"
"regexp"
"strconv"
"strings"
Expand All @@ -20,9 +19,10 @@ import (
"github.com/getkin/kin-openapi/openapi3"
"github.com/golang/protobuf/ptypes"
"github.com/golang/protobuf/ptypes/wrappers"
"github.com/kubeshop/kusk-gateway/options"
"google.golang.org/protobuf/types/known/durationpb"
"google.golang.org/protobuf/types/known/wrapperspb"

"github.com/kubeshop/kusk-gateway/options"
)

var (
Expand Down Expand Up @@ -347,7 +347,7 @@ func generateRoute(
}

func generateRedirect(redirectOpts *options.RedirectOptions) (*route.Route_Redirect, error) {
if reflect.DeepEqual(&options.RedirectOptions{}, redirectOpts) {
if redirectOpts == nil {
return nil, nil
}
redirectAction := &route.RedirectAction{
Expand Down Expand Up @@ -391,7 +391,7 @@ func generateRedirect(redirectOpts *options.RedirectOptions) (*route.Route_Redir
}

func generateCORSPolicy(corsOpts *options.CORSOptions) (*route.CorsPolicy, error) {
if corsOpts == nil || reflect.DeepEqual(&options.CORSOptions{}, corsOpts) {
if corsOpts == nil {
return nil, nil
}
allowOriginsMatcher := []*envoytypematcher.StringMatcher{}
Expand Down
56 changes: 14 additions & 42 deletions envoy/config/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"

"github.com/getkin/kin-openapi/openapi3"
"github.com/jinzhu/copier"

route "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
envoytypematcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
Expand Down Expand Up @@ -42,59 +41,29 @@ func (e *envoyConfiguration) UpdateConfigFromAPIOpts(opts *options.Options, spec
}
// Iterate on all paths and build routes
// The overriding works in the following way:
// 1. For each path create a copy of top x-kusk SubOpts struct as new pathOpts var. For that path override it with pathSubOpts.
// 2. For each method create a copy of previously created pathOpts (finalOpts) and override it with opSubOpts.
// Copier will skip override of undefined (nul) fields with IgnoreEmpty option.
// 1. For each path we get SubOptions from the opts map and merge in top level SubOpts
// 2. For each method we get SubOptions for that method from the opts map and merge in path SubOpts
for path, pathItem := range spec.Paths {
// x-kusk options per path
// This var is reused during following methods merges,
// we do this merge once per path since it is expensive to do it for every method
var pathOpts options.SubOptions
if err := copier.CopyWithOption(&pathOpts, &opts.SubOptions, copier.Option{IgnoreEmpty: true, DeepCopy: false}); err != nil {
return err
}

if pathSubOpts, ok := opts.PathSubOptions[path]; ok {
if err := copier.CopyWithOption(&pathOpts, &pathSubOpts, copier.Option{IgnoreEmpty: true, DeepCopy: false}); err != nil {
return err
}
}

// x-kusk options per operation (http method)
for method, operation := range pathItem.Operations() {
opSubOpts, ok := opts.OperationSubOptions[method+path]
if ok {
// Exit early if disabled per method, don't do further copies
if *opSubOpts.Disabled {
continue
}
}

var finalOpts options.SubOptions
finalOpts := opts.OperationFinalSubOptions[method+path]

// copy into new var already merged path opts
if err := copier.CopyWithOption(&finalOpts, &pathOpts, copier.Option{IgnoreEmpty: true, DeepCopy: false}); err != nil {
return err
}

// finally override with opSubOpts, if there are any
if ok {
if err := copier.CopyWithOption(&finalOpts, &opSubOpts, copier.Option{IgnoreEmpty: true, DeepCopy: false}); err != nil {
return err
}
}

// Once we have final merged Options, skip if disabled either on top, path or method level.
if finalOpts.Disabled != nil && *finalOpts.Disabled {
continue
}

routePath := generateRoutePath(finalOpts.Path.Prefix, path)
var routePath string
if finalOpts.Path != nil {
routePath = generateRoutePath(finalOpts.Path.Prefix, path)
} else {
routePath = generateRoutePath("", path)
}
routeName := generateRouteName(routePath, method)

params := extractParams(operation.Parameters)

corsPolicy, err := generateCORSPolicy(&finalOpts.CORS)
corsPolicy, err := generateCORSPolicy(finalOpts.CORS)
if err != nil {
return err
}
Expand All @@ -119,7 +88,10 @@ func (e *envoyConfiguration) UpdateConfigFromAPIOpts(opts *options.Options, spec
if !e.ClusterExist(clusterName) {
e.AddCluster(clusterName, upstreamHostname, upstreamPort)
}
rewritePathRegex := generateRewriteRegex(finalOpts.Path.Rewrite.Pattern, finalOpts.Path.Rewrite.Substitution)
var rewritePathRegex *envoytypematcher.RegexMatchAndSubstitute
if finalOpts.Path != nil {
rewritePathRegex = generateRewriteRegex(finalOpts.Path.Rewrite.Pattern, finalOpts.Path.Rewrite.Substitution)
}

var (
requestTimeout, requestIdleTimeout int64 = 0, 0
Expand Down
47 changes: 47 additions & 0 deletions examples/httpbin/httpbin_v1_api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,21 @@ spec:
name: httpbin
namespace: default
port: 8080
cors:
origins:
- "*"
methods:
- POST
- GET
- OPTIONS
- PUT
headers:
- Content-Type
credentials: true
expose_headers:
- X-Custom-TopLevel1
- X-Custom-TopLevel2
max_age: 86200
paths:
"/status/{code}":
x-kusk:
Expand Down Expand Up @@ -220,12 +235,44 @@ spec:
# Old API stream, rewrite to new
"/oldAPI/stream/{number}":
x-kusk:
cors:
origins:
- "*"
methods:
- POST
- GET
- OPTIONS
headers:
- Content-Type
credentials: true
expose_headers:
- X-Custom-PathLevel1
- X-Custom-PathLevel2
max_age: 86200
path:
# /oldAPI/stream/{number} -> to upstream: /stream/{number}
rewrite:
pattern: "^/oldAPI"
substitution: ""
# Since at the top we specify upstream.service, this one should replace it as upstream.host
upstream:
host:
hostname: httpbin
port: 8080
get:
x-kusk:
cors:
origins:
- "*"
methods:
- GET
headers:
- Content-Type
credentials: true
expose_headers:
- X-Custom-OperationLevel1
- X-Custom-OperationLevel2
max_age: 86200
description: Old stream, rewrite to new.
parameters:
- name: number
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ require (
github.com/go-ozzo/ozzo-validation/v4 v4.3.0
github.com/gofrs/uuid v4.0.0+incompatible
github.com/golang/protobuf v1.5.2
github.com/jinzhu/copier v0.3.2
github.com/onsi/ginkgo v1.16.4
github.com/onsi/gomega v1.15.0
github.com/stretchr/testify v1.7.0
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,6 @@ github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
github.com/jinzhu/copier v0.3.2 h1:QdBOCbaouLDYaIPFfi1bKv5F5tPpeTwXe4sD0jqtz5w=
github.com/jinzhu/copier v0.3.2/go.mod h1:24xnZezI2Yqac9J61UC6/dG/k76ttpq0DdJI3QmUvro=
github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo=
github.com/jonboulle/clockwork v0.2.2/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8=
github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
Expand Down
3 changes: 2 additions & 1 deletion local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ func parseAndApply(apiSpecPath string, envoyMgr *envoyConfigManager.EnvoyConfigM
if err != nil {
return err
}
if err = kuskExtensionOpts.FillDefaultsAndValidate(); err != nil {
kuskExtensionOpts.FillDefaults()
if err = kuskExtensionOpts.Validate(); err != nil {
return err
}
envoyConfig := envoyConfig.New()
Expand Down
Loading

0 comments on commit 8abc127

Please sign in to comment.