Skip to content

Commit

Permalink
feat: emit warnings if regex-like routes are used with prefix path fo…
Browse files Browse the repository at this point in the history
…rmat
  • Loading branch information
GGabriele committed Sep 26, 2022
1 parent 7de14b3 commit 6b73563
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 10 deletions.
49 changes: 41 additions & 8 deletions file/builder.go
Expand Up @@ -4,8 +4,10 @@ import (
"context"
"errors"
"fmt"
"strings"

"github.com/blang/semver/v4"
"github.com/kong/deck/cprint"
"github.com/kong/deck/konnect"
"github.com/kong/deck/state"
"github.com/kong/deck/utils"
Expand All @@ -31,11 +33,11 @@ type stateBuilder struct {

disableDynamicDefaults bool

checkRoutePaths bool

err error
}

var kong140Version = semver.MustParse("1.4.0")

// uuid generates a UUID string and returns a pointer to it.
// It is a variable for testing purpose, to override and supply
// a deterministic UUID generator.
Expand Down Expand Up @@ -63,6 +65,10 @@ func (b *stateBuilder) build() (*utils.KongRawState, *utils.KonnectRawState, err
}
b.defaulter = defaulter

if utils.Kong300Version.LTE(b.kongVersion) {
b.checkRoutePaths = true
}

// build
b.certificates()
if !b.skipCACerts {
Expand Down Expand Up @@ -295,7 +301,7 @@ func (b *stateBuilder) ingestKeyAuths(creds []kong.KeyAuth) error {
cred.ID = kong.String(*existingCred.ID)
}
}
if b.kongVersion.GTE(kong140Version) {
if b.kongVersion.GTE(utils.Kong140Version) {
utils.MustMergeTags(&cred, b.selectTags)
}
b.rawState.KeyAuths = append(b.rawState.KeyAuths, &cred)
Expand All @@ -316,7 +322,7 @@ func (b *stateBuilder) ingestBasicAuths(creds []kong.BasicAuth) error {
cred.ID = kong.String(*existingCred.ID)
}
}
if b.kongVersion.GTE(kong140Version) {
if b.kongVersion.GTE(utils.Kong140Version) {
utils.MustMergeTags(&cred, b.selectTags)
}
b.rawState.BasicAuths = append(b.rawState.BasicAuths, &cred)
Expand All @@ -337,7 +343,7 @@ func (b *stateBuilder) ingestHMACAuths(creds []kong.HMACAuth) error {
cred.ID = kong.String(*existingCred.ID)
}
}
if b.kongVersion.GTE(kong140Version) {
if b.kongVersion.GTE(utils.Kong140Version) {
utils.MustMergeTags(&cred, b.selectTags)
}
b.rawState.HMACAuths = append(b.rawState.HMACAuths, &cred)
Expand All @@ -358,7 +364,7 @@ func (b *stateBuilder) ingestJWTAuths(creds []kong.JWTAuth) error {
cred.ID = kong.String(*existingCred.ID)
}
}
if b.kongVersion.GTE(kong140Version) {
if b.kongVersion.GTE(utils.Kong140Version) {
utils.MustMergeTags(&cred, b.selectTags)
}
b.rawState.JWTAuths = append(b.rawState.JWTAuths, &cred)
Expand All @@ -379,7 +385,7 @@ func (b *stateBuilder) ingestOauth2Creds(creds []kong.Oauth2Credential) error {
cred.ID = kong.String(*existingCred.ID)
}
}
if b.kongVersion.GTE(kong140Version) {
if b.kongVersion.GTE(utils.Kong140Version) {
utils.MustMergeTags(&cred, b.selectTags)
}
b.rawState.Oauth2Creds = append(b.rawState.Oauth2Creds, &cred)
Expand All @@ -402,7 +408,7 @@ func (b *stateBuilder) ingestACLGroups(creds []kong.ACLGroup) error {
cred.ID = kong.String(*existingCred.ID)
}
}
if b.kongVersion.GTE(kong140Version) {
if b.kongVersion.GTE(utils.Kong140Version) {
utils.MustMergeTags(&cred, b.selectTags)
}
b.rawState.ACLGroups = append(b.rawState.ACLGroups, &cred)
Expand Down Expand Up @@ -768,6 +774,29 @@ func getStripPathBasedOnProtocols(route kong.Route) (*bool, error) {
return route.StripPath, nil
}

func checkRoutePaths300AndAbove(route FRoute) {
pathsWarnings := []string{}
for _, p := range route.Paths {
if strings.HasPrefix(*p, "~/") || !utils.IsPathRegexLike(*p) {
continue
}
pathsWarnings = append(pathsWarnings, *p)
}
if len(pathsWarnings) > 0 {
cprint.UpdatePrintf(
"In Route '%s', a path with regular expression was detected.\n"+
"In Kong Gateway versions 3.0 and above, all paths that use regular expressions \n"+
"must be prefixed with a ~ character. Without the ~ prefix, regular expression \n"+
"based paths will not be matched and processed correctly. \n\n"+
"Please run the following command to upgrade your config: \n\n"+
"deck convert --from kong-gateway-2.x --to kong-gateway-3.x "+
"--input-file <config-file> --output-file <new-config-file>\n\n"+
"Please refer to the following document for further details:\n"+
"https://docs.konghq.com/deck/latest/3.0-upgrade.\n\n",
*route.ID)
}
}

func (b *stateBuilder) ingestRoute(r FRoute) error {
if utils.Empty(r.ID) {
route, err := b.currentState.Routes.Get(*r.Name)
Expand Down Expand Up @@ -795,6 +824,10 @@ func (b *stateBuilder) ingestRoute(r FRoute) error {
return err
}

if b.checkRoutePaths {
checkRoutePaths300AndAbove(r)
}

// plugins for the route
var plugins []FPlugin
for _, p := range r.Plugins {
Expand Down
2 changes: 1 addition & 1 deletion file/builder_test.go
Expand Up @@ -1324,7 +1324,7 @@ func Test_stateBuilder_consumers(t *testing.T) {
b := &stateBuilder{
targetContent: tt.fields.targetContent,
currentState: tt.fields.currentState,
kongVersion: kong140Version,
kongVersion: utils.Kong140Version,
}
if tt.fields.kongVersion != nil {
b.kongVersion = *tt.fields.kongVersion
Expand Down
13 changes: 12 additions & 1 deletion utils/utils.go
Expand Up @@ -14,7 +14,18 @@ import (
"github.com/kong/go-kong/kong"
)

var kongVersionRegex = regexp.MustCompile(`^\d+\.\d+`)
var (
kongVersionRegex = regexp.MustCompile(`^\d+\.\d+`)
pathRegexPattern = regexp.MustCompile(`[^a-zA-Z0-9._~/%-]`)

Kong140Version = semver.MustParse("1.4.0")
Kong300Version = semver.MustParse("3.0.0")
)

// IsPathRegexLike checks if a path string contains a regex pattern.
func IsPathRegexLike(path string) bool {
return pathRegexPattern.MatchString(path)
}

// Empty checks if a string referenced by s or s itself is empty.
func Empty(s *string) bool {
Expand Down
39 changes: 39 additions & 0 deletions utils/utils_test.go
Expand Up @@ -287,3 +287,42 @@ func Test_ParseKongVersion(t *testing.T) {
})
}
}

func Test_IsPathRegexLike(t *testing.T) {
tests := []struct {
name string
paths []string
expected bool
}{
{
name: "regex-like paths",
paths: []string{
`/.*`,
`/[fF][oO]{2}`,
`/foo|bar`,
`/blog-\\d+`,
`/bl[ao]go(sphere|web)`,
},
expected: true,
},
{
name: "no regex-like paths",
paths: []string{
"/",
"/foo",
"/foo/",
"/abcd~user~2",
"/abcd%aa%10%ff%AA%FF",
},
expected: false,
},
}

for _, test := range tests {
for _, path := range test.paths {
assert.Equal(
t, test.expected, IsPathRegexLike(path), "test: '%v', path: '%v'", test.name, path,
)
}
}
}

0 comments on commit 6b73563

Please sign in to comment.