From db07e3e279b8e2010590fbd8f99c186cd603d58e Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Tue, 1 Mar 2022 15:54:18 -0800 Subject: [PATCH 1/7] feat(httproute) add support for regex header match --- internal/dataplane/parser/translate_utils.go | 18 ++++++++++++------ .../dataplane/parser/translate_utils_test.go | 6 +++--- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/internal/dataplane/parser/translate_utils.go b/internal/dataplane/parser/translate_utils.go index 8211541b2a..1b7e178b9b 100644 --- a/internal/dataplane/parser/translate_utils.go +++ b/internal/dataplane/parser/translate_utils.go @@ -7,6 +7,10 @@ import ( gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) +// kongHeaderRegexPrefix is a reserved prefix string that Kong uses to determine if it should parse a header value +// as a regex +const kongHeaderRegexPrefix = "~*" + // ----------------------------------------------------------------------------- // Translate Utilities - Gateway // ----------------------------------------------------------------------------- @@ -18,14 +22,16 @@ func convertGatewayMatchHeadersToKongRouteMatchHeaders(headers []gatewayv1alpha2 // options and otherwise converting to kong type format. convertedHeaders := make(map[string][]string) for _, header := range headers { - // TODO: implement regex header matching if header.Type != nil && *header.Type == gatewayv1alpha2.HeaderMatchRegularExpression { - return nil, fmt.Errorf("regular expression header matches are not yet supported") - } + convertedHeaders[string(header.Name)] = []string{kongHeaderRegexPrefix + header.Value} + } else if header.Type == nil || *header.Type == gatewayv1alpha2.HeaderMatchExact { - // split the header values by comma - values := strings.Split(header.Value, ",") - convertedHeaders[string(header.Name)] = values + // split the header values by comma + values := strings.Split(header.Value, ",") + convertedHeaders[string(header.Name)] = values + } else { + return nil, fmt.Errorf("unknown/unsupported header match type: %s", string(*header.Type)) + } } return convertedHeaders, nil diff --git a/internal/dataplane/parser/translate_utils_test.go b/internal/dataplane/parser/translate_utils_test.go index 47f92c3be5..bda7e7a08a 100644 --- a/internal/dataplane/parser/translate_utils_test.go +++ b/internal/dataplane/parser/translate_utils_test.go @@ -1,7 +1,6 @@ package parser import ( - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -26,8 +25,9 @@ func Test_convertGatewayMatchHeadersToKongRouteMatchHeaders(t *testing.T) { Name: "Content-Type", Value: "^audio/*", }}, - output: nil, - err: fmt.Errorf("regular expression header matches are not yet supported"), + output: map[string][]string{ + "Content-Type": {kongHeaderRegexPrefix + "^audio/*"}, + }, }, { msg: "a single exact header match with no type defaults to exact type and converts properly", From 63631600b84d127b714bb6303432056ded98cfb4 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Tue, 1 Mar 2022 16:02:29 -0800 Subject: [PATCH 2/7] feat(httproute) reject conflicting headers If an HTTPRoute includes multiple header match rules with the same header name, reject the route and log the reason for rejection. --- internal/dataplane/parser/translate_utils.go | 4 ++++ .../dataplane/parser/translate_utils_test.go | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/internal/dataplane/parser/translate_utils.go b/internal/dataplane/parser/translate_utils.go index 1b7e178b9b..45e2aa4f8b 100644 --- a/internal/dataplane/parser/translate_utils.go +++ b/internal/dataplane/parser/translate_utils.go @@ -22,6 +22,10 @@ func convertGatewayMatchHeadersToKongRouteMatchHeaders(headers []gatewayv1alpha2 // options and otherwise converting to kong type format. convertedHeaders := make(map[string][]string) for _, header := range headers { + if _, exists := convertedHeaders[string(header.Name)]; exists { + return nil, fmt.Errorf("multiple header matches for the same header are not allowed: %s", + string(header.Name)) + } if header.Type != nil && *header.Type == gatewayv1alpha2.HeaderMatchRegularExpression { convertedHeaders[string(header.Name)] = []string{kongHeaderRegexPrefix + header.Value} } else if header.Type == nil || *header.Type == gatewayv1alpha2.HeaderMatchExact { diff --git a/internal/dataplane/parser/translate_utils_test.go b/internal/dataplane/parser/translate_utils_test.go index bda7e7a08a..67b7c06e58 100644 --- a/internal/dataplane/parser/translate_utils_test.go +++ b/internal/dataplane/parser/translate_utils_test.go @@ -1,6 +1,7 @@ package parser import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -61,6 +62,21 @@ func Test_convertGatewayMatchHeadersToKongRouteMatchHeaders(t *testing.T) { "Content-Type": {"audio/vorbis", "audio/mpeg"}, }, }, + { + msg: "multiple header matches for the same header are rejected", + input: []gatewayv1alpha2.HTTPHeaderMatch{ + { + Name: "Content-Type", + Value: "audio/vorbis", + }, + { + Name: "Content-Type", + Value: "audio/flac", + }, + }, + output: nil, + err: fmt.Errorf("multiple header matches for the same header are not allowed: Content-Type"), + }, { msg: "multiple header matches with a mixture of value counts convert properly", input: []gatewayv1alpha2.HTTPHeaderMatch{ From 92cfd882c6c28850aa85265a6ae6328ebbfc73ba Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Wed, 2 Mar 2022 12:12:18 -0800 Subject: [PATCH 3/7] fix(httproute) remove CSV header splits Remove support for CSVs to set multiple header match values. Although supported by Kong, the Gateway spec does not indicate this behavior, and does not allow specifying multiple values by including multiple matches for the same header name. The expected configuration to accept multiple values is a regex header match. --- internal/dataplane/parser/translate_utils.go | 6 +----- .../dataplane/parser/translate_utils_test.go | 17 +++-------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/internal/dataplane/parser/translate_utils.go b/internal/dataplane/parser/translate_utils.go index 45e2aa4f8b..17a7ba862b 100644 --- a/internal/dataplane/parser/translate_utils.go +++ b/internal/dataplane/parser/translate_utils.go @@ -2,7 +2,6 @@ package parser import ( "fmt" - "strings" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -29,10 +28,7 @@ func convertGatewayMatchHeadersToKongRouteMatchHeaders(headers []gatewayv1alpha2 if header.Type != nil && *header.Type == gatewayv1alpha2.HeaderMatchRegularExpression { convertedHeaders[string(header.Name)] = []string{kongHeaderRegexPrefix + header.Value} } else if header.Type == nil || *header.Type == gatewayv1alpha2.HeaderMatchExact { - - // split the header values by comma - values := strings.Split(header.Value, ",") - convertedHeaders[string(header.Name)] = values + convertedHeaders[string(header.Name)] = []string{header.Value} } else { return nil, fmt.Errorf("unknown/unsupported header match type: %s", string(*header.Type)) } diff --git a/internal/dataplane/parser/translate_utils_test.go b/internal/dataplane/parser/translate_utils_test.go index 67b7c06e58..4796f1b086 100644 --- a/internal/dataplane/parser/translate_utils_test.go +++ b/internal/dataplane/parser/translate_utils_test.go @@ -51,17 +51,6 @@ func Test_convertGatewayMatchHeadersToKongRouteMatchHeaders(t *testing.T) { "Content-Type": {"audio/vorbis"}, }, }, - { - msg: "a single exact header match with multiple values converts properly", - input: []gatewayv1alpha2.HTTPHeaderMatch{{ - Type: &exactType, - Name: "Content-Type", - Value: "audio/vorbis,audio/mpeg", - }}, - output: map[string][]string{ - "Content-Type": {"audio/vorbis", "audio/mpeg"}, - }, - }, { msg: "multiple header matches for the same header are rejected", input: []gatewayv1alpha2.HTTPHeaderMatch{ @@ -78,12 +67,12 @@ func Test_convertGatewayMatchHeadersToKongRouteMatchHeaders(t *testing.T) { err: fmt.Errorf("multiple header matches for the same header are not allowed: Content-Type"), }, { - msg: "multiple header matches with a mixture of value counts convert properly", + msg: "multiple header matches convert properly", input: []gatewayv1alpha2.HTTPHeaderMatch{ { Type: &exactType, Name: "Content-Type", - Value: "audio/vorbis,audio/mpeg", + Value: "audio/vorbis", }, { Name: "Content-Length", @@ -91,7 +80,7 @@ func Test_convertGatewayMatchHeadersToKongRouteMatchHeaders(t *testing.T) { }, }, output: map[string][]string{ - "Content-Type": {"audio/vorbis", "audio/mpeg"}, + "Content-Type": {"audio/vorbis"}, "Content-Length": {"999999999"}, }, }, From 1c7ffccb6f7cf935d6cbe58526cd7dfed8b3c94d Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Wed, 2 Mar 2022 12:50:38 -0800 Subject: [PATCH 4/7] feat(util) add Kong version utility Add utility functions to set the Kong version once at startup and retrieve it from elsewhere without passing the version down through the call chain. --- internal/dataplane/parser/translate_utils.go | 12 ++- .../dataplane/parser/translate_utils_test.go | 73 ++++++++++++++++++- internal/manager/run.go | 7 ++ internal/util/kongversion.go | 25 +++++++ internal/util/logging.go | 3 + 5 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 internal/util/kongversion.go diff --git a/internal/dataplane/parser/translate_utils.go b/internal/dataplane/parser/translate_utils.go index 17a7ba862b..f65de15363 100644 --- a/internal/dataplane/parser/translate_utils.go +++ b/internal/dataplane/parser/translate_utils.go @@ -3,13 +3,18 @@ package parser import ( "fmt" + "github.com/blang/semver/v4" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" ) // kongHeaderRegexPrefix is a reserved prefix string that Kong uses to determine if it should parse a header value // as a regex const kongHeaderRegexPrefix = "~*" +var minRegexHeaderKongVersion = semver.MustParse("2.8.0") + // ----------------------------------------------------------------------------- // Translate Utilities - Gateway // ----------------------------------------------------------------------------- @@ -26,7 +31,12 @@ func convertGatewayMatchHeadersToKongRouteMatchHeaders(headers []gatewayv1alpha2 string(header.Name)) } if header.Type != nil && *header.Type == gatewayv1alpha2.HeaderMatchRegularExpression { - convertedHeaders[string(header.Name)] = []string{kongHeaderRegexPrefix + header.Value} + if util.GetKongVersion().LT(minRegexHeaderKongVersion) { + return nil, fmt.Errorf("Kong version %s does not support HeaderMatchRegularExpression", + util.GetKongVersion().String()) + } else { + convertedHeaders[string(header.Name)] = []string{kongHeaderRegexPrefix + header.Value} + } } else if header.Type == nil || *header.Type == gatewayv1alpha2.HeaderMatchExact { convertedHeaders[string(header.Name)] = []string{header.Value} } else { diff --git a/internal/dataplane/parser/translate_utils_test.go b/internal/dataplane/parser/translate_utils_test.go index 4796f1b086..232aa016e9 100644 --- a/internal/dataplane/parser/translate_utils_test.go +++ b/internal/dataplane/parser/translate_utils_test.go @@ -4,13 +4,84 @@ import ( "fmt" "testing" + "github.com/blang/semver/v4" "github.com/stretchr/testify/assert" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" ) +func Test_convertGatewayMatchHeadersToKongRouteMatchHeadersVersionBehavior(t *testing.T) { + regexType := gatewayv1alpha2.HeaderMatchRegularExpression + + type Case struct { + msg string + input []gatewayv1alpha2.HTTPHeaderMatch + output map[string][]string + err error + } + + // util reports Kong version 0.0.0 when not initialized + belowThresholdTests := []Case{ + { + msg: "regex header matches fail on unsupported versions", + input: []gatewayv1alpha2.HTTPHeaderMatch{{ + Type: ®exType, + Name: "Content-Type", + Value: "^audio/*", + }}, + output: nil, + err: fmt.Errorf("Kong version %s does not support HeaderMatchRegularExpression", semver.MustParse("0.0.0")), + }, + { + msg: "a single exact header match succeeds on any version", + input: []gatewayv1alpha2.HTTPHeaderMatch{{ + Name: "Content-Type", + Value: "audio/vorbis", + }}, + output: map[string][]string{ + "Content-Type": {"audio/vorbis"}, + }, + }, + } + + for _, tt := range belowThresholdTests { + t.Run(tt.msg, func(t *testing.T) { + output, err := convertGatewayMatchHeadersToKongRouteMatchHeaders(tt.input) + assert.Equal(t, tt.err, err) + assert.Equal(t, tt.output, output) + }) + } + + util.SetKongVersion(semver.MustParse("2.8.0")) + + aboveThresholdTests := []Case{ + { + msg: "regex header matches succeed on supported versions", + input: []gatewayv1alpha2.HTTPHeaderMatch{{ + Type: ®exType, + Name: "Content-Type", + Value: "^audio/*", + }}, + output: map[string][]string{ + "Content-Type": {kongHeaderRegexPrefix + "^audio/*"}, + }, + }, + } + + for _, tt := range aboveThresholdTests { + t.Run(tt.msg, func(t *testing.T) { + output, err := convertGatewayMatchHeadersToKongRouteMatchHeaders(tt.input) + assert.Equal(t, tt.err, err) + assert.Equal(t, tt.output, output) + }) + } +} + func Test_convertGatewayMatchHeadersToKongRouteMatchHeaders(t *testing.T) { regexType := gatewayv1alpha2.HeaderMatchRegularExpression exactType := gatewayv1alpha2.HeaderMatchExact + util.SetKongVersion(semver.MustParse("2.8.0")) t.Log("generating several gateway header matches") tests := []struct { @@ -20,7 +91,7 @@ func Test_convertGatewayMatchHeadersToKongRouteMatchHeaders(t *testing.T) { err error }{ { - msg: "regex header matches will fail due to lack of support", + msg: "regex header matches convert correctly", input: []gatewayv1alpha2.HTTPHeaderMatch{{ Type: ®exType, Name: "Content-Type", diff --git a/internal/manager/run.go b/internal/manager/run.go index 63a6e9b9eb..dda4308c39 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -16,6 +16,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + "github.com/kong/go-kong/kong" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane" "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/metadata" mgrutils "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/utils" @@ -71,6 +72,12 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic) e if err != nil { return fmt.Errorf("could not retrieve Kong admin root: %w", err) } + kongVersion, err := kong.ParseSemanticVersion(kong.VersionFromInfo(kongRoot)) + if err != nil { + setupLog.V(util.WarnLevel).Info("could not parse Kong version, version-specific behavior disabled", "error", err) + } else { + util.SetKongVersion(kongVersion) + } kongRootConfig, ok := kongRoot["configuration"].(map[string]interface{}) if !ok { return fmt.Errorf("invalid root configuration, expected a map[string]interface{} got %T", diff --git a/internal/util/kongversion.go b/internal/util/kongversion.go new file mode 100644 index 0000000000..6ba0776150 --- /dev/null +++ b/internal/util/kongversion.go @@ -0,0 +1,25 @@ +package util + +import ( + "sync" + + "github.com/blang/semver/v4" +) + +var ( + kongVersion = semver.MustParse("0.0.0") + kongVersionOnce sync.Once +) + +// SetKongVersion sets the Kong version. It can only be used once. Repeated calls will not update the Kong +// version +func SetKongVersion(version semver.Version) { + kongVersionOnce.Do(func() { + kongVersion = version + }) +} + +// GetKongVersion retrieves the Kong version. If the version is not set, it returns the lowest possible version +func GetKongVersion() semver.Version { + return kongVersion +} diff --git a/internal/util/logging.go b/internal/util/logging.go index a3e70ea46a..b8073d3e73 100644 --- a/internal/util/logging.go +++ b/internal/util/logging.go @@ -26,6 +26,9 @@ const ( // DebugLevel is the converted logging level from logrus to go-logr for // debug level logging. DebugLevel = int(logrus.DebugLevel) - logrusrDiff + + // WarnLevel is the converted logrus level to go-logr for warnings + WarnLevel = int(logrus.WarnLevel) - logrusrDiff ) var ( From dc3219e66916d02d17ea52ce16a38e433bf2588d Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Wed, 2 Mar 2022 13:02:27 -0800 Subject: [PATCH 5/7] chore(parser) refactor mtls check for util version Refactor the mtls-auth version check to use the Kong version getter from the util package. --- internal/dataplane/kongstate/consumer.go | 5 +- internal/dataplane/kongstate/consumer_test.go | 76 ++++++++----------- internal/dataplane/kongstate/kongstate.go | 2 +- internal/manager/run.go | 2 +- 4 files changed, 36 insertions(+), 49 deletions(-) diff --git a/internal/dataplane/kongstate/consumer.go b/internal/dataplane/kongstate/consumer.go index 7577044f82..1075de95ed 100644 --- a/internal/dataplane/kongstate/consumer.go +++ b/internal/dataplane/kongstate/consumer.go @@ -6,6 +6,7 @@ import ( "github.com/blang/semver/v4" "github.com/kong/go-kong/kong" + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" configurationv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1" ) @@ -70,7 +71,7 @@ func (c *Consumer) SanitizedCopy() *Consumer { } } -func (c *Consumer) SetCredential(credType string, credConfig interface{}, version semver.Version) error { +func (c *Consumer) SetCredential(credType string, credConfig interface{}) error { switch credType { case "key-auth", "keyauth_credential": cred, err := NewKeyAuth(credConfig) @@ -109,7 +110,7 @@ func (c *Consumer) SetCredential(credType string, credConfig interface{}, versio } c.ACLGroups = append(c.ACLGroups, cred) case "mtls-auth": - if version.LT(minMTLSCredentialVersion) { + if util.GetKongVersion().LT(minMTLSCredentialVersion) { return fmt.Errorf("controller cannot support mtls-auth below version %v", minMTLSCredentialVersion) } cred, err := NewMTLSAuth(credConfig) diff --git a/internal/dataplane/kongstate/consumer_test.go b/internal/dataplane/kongstate/consumer_test.go index 6ddb57fb6f..ade7eb3b1a 100644 --- a/internal/dataplane/kongstate/consumer_test.go +++ b/internal/dataplane/kongstate/consumer_test.go @@ -7,6 +7,7 @@ import ( "github.com/kong/go-kong/kong" "github.com/stretchr/testify/assert" + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" configurationv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1" ) @@ -73,27 +74,24 @@ func TestConsumer_SanitizedCopy(t *testing.T) { func TestConsumer_SetCredential(t *testing.T) { username := "example" - standardVersion := semver.MustParse("2.3.2") - mtlsUnsupportedVersion := semver.MustParse("1.3.2") type args struct { credType string consumer *Consumer credConfig interface{} - version semver.Version } - tests := []struct { + type Case struct { name string args args result *Consumer wantErr bool - }{ + } + tests := []Case{ { name: "invalid cred type errors", args: args{ credType: "invalid-type", consumer: &Consumer{}, credConfig: nil, - version: standardVersion, }, result: &Consumer{}, wantErr: true, @@ -104,7 +102,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "key-auth", consumer: &Consumer{}, credConfig: map[string]string{"key": "foo"}, - version: standardVersion, }, result: &Consumer{ KeyAuths: []*KeyAuth{ @@ -121,7 +118,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "key-auth", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]string{}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -132,7 +128,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "key-auth", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]interface{}{"key": true}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -143,7 +138,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "keyauth_credential", consumer: &Consumer{}, credConfig: map[string]string{"key": "foo"}, - version: standardVersion, }, result: &Consumer{ KeyAuths: []*KeyAuth{ @@ -163,7 +157,6 @@ func TestConsumer_SetCredential(t *testing.T) { "username": "foo", "password": "bar", }, - version: standardVersion, }, result: &Consumer{ BasicAuths: []*BasicAuth{ @@ -181,7 +174,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "basic-auth", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]string{}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -192,7 +184,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "basic-auth", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]interface{}{"username": true}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -206,7 +197,6 @@ func TestConsumer_SetCredential(t *testing.T) { "username": "foo", "password": "bar", }, - version: standardVersion, }, result: &Consumer{ BasicAuths: []*BasicAuth{ @@ -227,7 +217,6 @@ func TestConsumer_SetCredential(t *testing.T) { "username": "foo", "secret": "bar", }, - version: standardVersion, }, result: &Consumer{ HMACAuths: []*HMACAuth{ @@ -245,7 +234,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "hmac-auth", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]string{}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -256,7 +244,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "hmac-auth", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]interface{}{"username": true}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -270,7 +257,6 @@ func TestConsumer_SetCredential(t *testing.T) { "username": "foo", "secret": "bar", }, - version: standardVersion, }, result: &Consumer{ HMACAuths: []*HMACAuth{ @@ -293,7 +279,6 @@ func TestConsumer_SetCredential(t *testing.T) { "client_secret": "baz", "redirect_uris": []string{"example.com"}, }, - version: standardVersion, }, result: &Consumer{ Oauth2Creds: []*Oauth2Credential{ @@ -315,7 +300,6 @@ func TestConsumer_SetCredential(t *testing.T) { credConfig: map[string]interface{}{ "client_id": "bar", }, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -328,7 +312,6 @@ func TestConsumer_SetCredential(t *testing.T) { credConfig: map[string]interface{}{ "name": "bar", }, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -339,7 +322,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "oauth2", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]interface{}{"client_id": true}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -354,7 +336,6 @@ func TestConsumer_SetCredential(t *testing.T) { "rsa_public_key": "bar", "secret": "baz", }, - version: standardVersion, }, result: &Consumer{ JWTAuths: []*JWTAuth{ @@ -375,7 +356,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "jwt", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]string{}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -386,7 +366,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "jwt", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]interface{}{"key": true}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -401,7 +380,6 @@ func TestConsumer_SetCredential(t *testing.T) { "rsa_public_key": "bar", "secret": "baz", }, - version: standardVersion, }, result: &Consumer{ JWTAuths: []*JWTAuth{ @@ -422,7 +400,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "acl", consumer: &Consumer{}, credConfig: map[string]string{"group": "group-foo"}, - version: standardVersion, }, result: &Consumer{ ACLGroups: []*ACLGroup{ @@ -439,7 +416,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "acl", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]string{}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -450,18 +426,40 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "acl", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]interface{}{"group": true}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, }, + { + name: "mtls-auth on unsupported version", + args: args{ + credType: "mtls-auth", + consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, + credConfig: map[string]string{"subject_name": "foo@example.com"}, + }, + result: &Consumer{Consumer: kong.Consumer{Username: &username}}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := tt.args.consumer.SetCredential(tt.args.credType, + tt.args.credConfig); (err != nil) != tt.wantErr { + t.Errorf("processCredential() error = %v, wantErr %v", + err, tt.wantErr) + } + assert.Equal(t, tt.result, tt.args.consumer) + }) + } + + util.SetKongVersion(semver.MustParse("2.3.2")) // minimum version for mtls-auths with tags + mtlsSupportedTests := []Case{ { name: "mtls-auth", args: args{ credType: "mtls-auth", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]string{"subject_name": "foo@example.com"}, - version: standardVersion, }, result: &Consumer{ Consumer: kong.Consumer{Username: &username}, @@ -479,18 +477,6 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "mtls-auth", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]string{}, - version: standardVersion, - }, - result: &Consumer{Consumer: kong.Consumer{Username: &username}}, - wantErr: true, - }, - { - name: "mtls-auth on unsupported version", - args: args{ - credType: "mtls-auth", - consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, - credConfig: map[string]string{"subject_name": "foo@example.com"}, - version: mtlsUnsupportedVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, @@ -501,16 +487,16 @@ func TestConsumer_SetCredential(t *testing.T) { credType: "mtls-auth", consumer: &Consumer{Consumer: kong.Consumer{Username: &username}}, credConfig: map[string]interface{}{"subject_name": true}, - version: standardVersion, }, result: &Consumer{Consumer: kong.Consumer{Username: &username}}, wantErr: true, }, } - for _, tt := range tests { + + for _, tt := range mtlsSupportedTests { t.Run(tt.name, func(t *testing.T) { if err := tt.args.consumer.SetCredential(tt.args.credType, - tt.args.credConfig, tt.args.version); (err != nil) != tt.wantErr { + tt.args.credConfig); (err != nil) != tt.wantErr { t.Errorf("processCredential() error = %v, wantErr %v", err, tt.wantErr) } diff --git a/internal/dataplane/kongstate/kongstate.go b/internal/dataplane/kongstate/kongstate.go index cf47cd4cba..d88b0f896c 100644 --- a/internal/dataplane/kongstate/kongstate.go +++ b/internal/dataplane/kongstate/kongstate.go @@ -115,7 +115,7 @@ func (ks *KongState) FillConsumersAndCredentials(log logrus.FieldLogger, s store log.Errorf("failed to provision credential: empty secret") continue } - err = c.SetCredential(credType, credConfig, ks.Version) + err = c.SetCredential(credType, credConfig) if err != nil { log.Errorf("failed to provision credential: %v", err) continue diff --git a/internal/manager/run.go b/internal/manager/run.go index dda4308c39..59475e1965 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -8,6 +8,7 @@ import ( "net/http" "time" + "github.com/kong/go-kong/kong" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -16,7 +17,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - "github.com/kong/go-kong/kong" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane" "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/metadata" mgrutils "github.com/kong/kubernetes-ingress-controller/v2/internal/manager/utils" From f59404336ee8632fcc34b7a408be9b38634d5207 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Wed, 2 Mar 2022 15:27:26 -0800 Subject: [PATCH 6/7] test(httproute) add header integration test --- internal/dataplane/parser/translate_utils.go | 8 ++-- test/integration/httproute_test.go | 45 ++++++++++++++------ test/integration/utils_test.go | 9 +++- 3 files changed, 44 insertions(+), 18 deletions(-) diff --git a/internal/dataplane/parser/translate_utils.go b/internal/dataplane/parser/translate_utils.go index f65de15363..1c3811cdeb 100644 --- a/internal/dataplane/parser/translate_utils.go +++ b/internal/dataplane/parser/translate_utils.go @@ -13,7 +13,8 @@ import ( // as a regex const kongHeaderRegexPrefix = "~*" -var minRegexHeaderKongVersion = semver.MustParse("2.8.0") +// MinRegexHeaderKongVersion is the minimum Kong version that supports regex header matches +var MinRegexHeaderKongVersion = semver.MustParse("2.8.0") // ----------------------------------------------------------------------------- // Translate Utilities - Gateway @@ -31,12 +32,11 @@ func convertGatewayMatchHeadersToKongRouteMatchHeaders(headers []gatewayv1alpha2 string(header.Name)) } if header.Type != nil && *header.Type == gatewayv1alpha2.HeaderMatchRegularExpression { - if util.GetKongVersion().LT(minRegexHeaderKongVersion) { + if util.GetKongVersion().LT(MinRegexHeaderKongVersion) { return nil, fmt.Errorf("Kong version %s does not support HeaderMatchRegularExpression", util.GetKongVersion().String()) - } else { - convertedHeaders[string(header.Name)] = []string{kongHeaderRegexPrefix + header.Value} } + convertedHeaders[string(header.Name)] = []string{kongHeaderRegexPrefix + header.Value} } else if header.Type == nil || *header.Type == gatewayv1alpha2.HeaderMatchExact { convertedHeaders[string(header.Name)] = []string{header.Value} } else { diff --git a/test/integration/httproute_test.go b/test/integration/httproute_test.go index cc1ffcefbf..ed7cb27e59 100644 --- a/test/integration/httproute_test.go +++ b/test/integration/httproute_test.go @@ -21,8 +21,12 @@ import ( "github.com/kong/kubernetes-ingress-controller/v2/internal/annotations" "github.com/kong/kubernetes-ingress-controller/v2/internal/controllers/gateway" + "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser" + "github.com/kong/kubernetes-ingress-controller/v2/internal/util" ) +var emptyHeaderSet = make(map[string]string) + func TestHTTPRouteEssentials(t *testing.T) { ns, cleanup := namespace(t) defer cleanup() @@ -113,6 +117,7 @@ func TestHTTPRouteEssentials(t *testing.T) { pathMatchPrefix := gatewayv1alpha2.PathMatchPathPrefix pathMatchRegularExpression := gatewayv1alpha2.PathMatchRegularExpression pathMatchExact := gatewayv1alpha2.PathMatchExact + headerMatchRegex := gatewayv1alpha2.HeaderMatchRegularExpression httproute := &gatewayv1alpha2.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: uuid.NewString(), @@ -158,6 +163,17 @@ func TestHTTPRouteEssentials(t *testing.T) { }}, }, } + if util.GetKongVersion().GTE(parser.MinRegexHeaderKongVersion) { + httproute.Spec.Rules[0].Matches = append(httproute.Spec.Rules[0].Matches, gatewayv1alpha2.HTTPRouteMatch{ + Headers: []gatewayv1alpha2.HTTPHeaderMatch{ + { + Type: &headerMatchRegex, + Value: "^audio/*", + Name: "Content-Type", + }, + }, + }) + } httproute, err = c.GatewayV1alpha2().HTTPRoutes(ns.Name).Create(ctx, httproute, metav1.CreateOptions{}) require.NoError(t, err) @@ -174,12 +190,17 @@ func TestHTTPRouteEssentials(t *testing.T) { eventuallyGatewayIsLinkedInStatus(t, c, ns.Name, httproute.Name) t.Log("waiting for routes from HTTPRoute to become operational") - eventuallyGETPath(t, "httpbin", http.StatusOK, "httpbin.org") + eventuallyGETPath(t, "httpbin", http.StatusOK, "httpbin.org", emptyHeaderSet) eventuallyGETPath(t, "httpbin/base64/wqt5b8q7ccK7IGRhbiBib3NocWEgYmlyIGphdm9iaW1peiB5b8q7cWRpci4K", - http.StatusOK, "«yoʻq» dan boshqa bir javobimiz yoʻqdir.") - eventuallyGETPath(t, "regex-123-httpbin", http.StatusOK, "httpbin.org") - eventuallyGETPath(t, "exact-httpbin", http.StatusOK, "httpbin.org") - eventuallyGETPath(t, "exact-httpbina", http.StatusNotFound, "no Route matched") + http.StatusOK, "«yoʻq» dan boshqa bir javobimiz yoʻqdir.", emptyHeaderSet) + eventuallyGETPath(t, "regex-123-httpbin", http.StatusOK, "httpbin.org", emptyHeaderSet) + eventuallyGETPath(t, "exact-httpbin", http.StatusOK, "httpbin.org", emptyHeaderSet) + eventuallyGETPath(t, "exact-httpbina", http.StatusNotFound, "no Route matched", emptyHeaderSet) + + if util.GetKongVersion().GTE(parser.MinRegexHeaderKongVersion) { + t.Log("verifying HTTPRoute header match") + eventuallyGETPath(t, "", http.StatusOK, "httpbin.org", map[string]string{"Content-Type": "audio/mp3"}) + } t.Log("removing the parentrefs from the HTTPRoute") oldParentRefs := httproute.Spec.ParentRefs @@ -195,7 +216,7 @@ func TestHTTPRouteEssentials(t *testing.T) { eventuallyGatewayIsUnlinkedInStatus(t, c, ns.Name, httproute.Name) t.Log("verifying that the data-plane configuration from the HTTPRoute gets dropped with the parentRefs now removed") - eventuallyGETPath(t, "httpbin", http.StatusNotFound, "") + eventuallyGETPath(t, "httpbin", http.StatusNotFound, "", emptyHeaderSet) t.Log("putting the parentRefs back") require.Eventually(t, func() bool { @@ -210,7 +231,7 @@ func TestHTTPRouteEssentials(t *testing.T) { eventuallyGatewayIsLinkedInStatus(t, c, ns.Name, httproute.Name) t.Log("verifying that putting the parentRefs back results in the routes becoming available again") - eventuallyGETPath(t, "httpbin", http.StatusOK, "httpbin.org") + eventuallyGETPath(t, "httpbin", http.StatusOK, "httpbin.org", emptyHeaderSet) t.Log("deleting the GatewayClass") oldGWCName := gwc.Name @@ -220,7 +241,7 @@ func TestHTTPRouteEssentials(t *testing.T) { eventuallyGatewayIsUnlinkedInStatus(t, c, ns.Name, httproute.Name) t.Log("verifying that the data-plane configuration from the HTTPRoute gets dropped with the GatewayClass now removed") - eventuallyGETPath(t, "httpbin", http.StatusNotFound, "") + eventuallyGETPath(t, "httpbin", http.StatusNotFound, "", emptyHeaderSet) t.Log("putting the GatewayClass back") gwc = &gatewayv1alpha2.GatewayClass{ @@ -238,7 +259,7 @@ func TestHTTPRouteEssentials(t *testing.T) { eventuallyGatewayIsLinkedInStatus(t, c, ns.Name, httproute.Name) t.Log("verifying that creating the GatewayClass again triggers reconciliation of HTTPRoutes and the route becomes available again") - eventuallyGETPath(t, "httpbin", http.StatusOK, "httpbin.org") + eventuallyGETPath(t, "httpbin", http.StatusOK, "httpbin.org", emptyHeaderSet) t.Log("deleting the Gateway") oldGWName := gw.Name @@ -248,7 +269,7 @@ func TestHTTPRouteEssentials(t *testing.T) { eventuallyGatewayIsUnlinkedInStatus(t, c, ns.Name, httproute.Name) t.Log("verifying that the data-plane configuration from the HTTPRoute gets dropped with the Gateway now removed") - eventuallyGETPath(t, "httpbin", http.StatusNotFound, "") + eventuallyGETPath(t, "httpbin", http.StatusNotFound, "", emptyHeaderSet) t.Log("putting the Gateway back") gw = &gatewayv1alpha2.Gateway{ @@ -274,7 +295,7 @@ func TestHTTPRouteEssentials(t *testing.T) { eventuallyGatewayIsLinkedInStatus(t, c, ns.Name, httproute.Name) t.Log("verifying that creating the Gateway again triggers reconciliation of HTTPRoutes and the route becomes available again") - eventuallyGETPath(t, "httpbin", http.StatusOK, "httpbin.org") + eventuallyGETPath(t, "httpbin", http.StatusOK, "httpbin.org", emptyHeaderSet) t.Log("deleting both GatewayClass and Gateway rapidly") require.NoError(t, c.GatewayV1alpha2().GatewayClasses().Delete(ctx, gwc.Name, metav1.DeleteOptions{})) @@ -284,7 +305,7 @@ func TestHTTPRouteEssentials(t *testing.T) { eventuallyGatewayIsUnlinkedInStatus(t, c, ns.Name, httproute.Name) t.Log("verifying that the data-plane configuration from the HTTPRoute does not get orphaned with the GatewayClass and Gateway gone") - eventuallyGETPath(t, "httpbin", http.StatusNotFound, "") + eventuallyGETPath(t, "httpbin", http.StatusNotFound, "", emptyHeaderSet) } // ----------------------------------------------------------------------------- diff --git a/test/integration/utils_test.go b/test/integration/utils_test.go index 3801e35ab7..3066ad5eb8 100644 --- a/test/integration/utils_test.go +++ b/test/integration/utils_test.go @@ -289,9 +289,14 @@ func identifyTestCasesForFile(filePath string) ([]string, error) { // not pay attention to hostname or other routing rules. This uses a "require" // for the desired conditions so if this request doesn't eventually succeed the // calling test will fail and stop. -func eventuallyGETPath(t *testing.T, path string, statusCode int, bodyContents string) { +func eventuallyGETPath(t *testing.T, path string, statusCode int, bodyContents string, headers map[string]string) { + req, err := http.NewRequest("GET", fmt.Sprintf("%s/%s", proxyURL, path), nil) + require.NoError(t, err) + for header, value := range headers { + req.Header.Set(header, value) + } require.Eventually(t, func() bool { - resp, err := httpc.Get(fmt.Sprintf("%s/%s", proxyURL, path)) + resp, err := httpc.Do(req) if err != nil { t.Logf("WARNING: http request failed for GET %s/%s: %v", proxyURL, path, err) return false From a07daab311696de15a576df0b705a5a934c94680 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 3 Mar 2022 10:46:20 -0800 Subject: [PATCH 7/7] docs(changelog) add HTTPRoute regex headers --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0248db64c2..4a862fb943 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,11 +48,23 @@ > Release date: TBD +#### Breaking changes + +- HTTPRoute header matches no longer interpret CSV values as multiple match + values, as this was not part of the HTTPRoute specification. Multiple values + should use regular expressions instead. + [#2302](https://github.com/Kong/kubernetes-ingress-controller/pull/2302) + #### Added - Deployment manifests now include an IngressClass resource and permissions to read IngressClass resources. [#2292](https://github.com/Kong/kubernetes-ingress-controller/pull/2292) +- HTTPRoute header matches now support regular expressions. + [#2302](https://github.com/Kong/kubernetes-ingress-controller/pull/2302) +- HTTPRoutes that define multiple matches for the same header are rejected to + comply with the HTTPRoute specification. + [#2302](https://github.com/Kong/kubernetes-ingress-controller/pull/2302) #### Fixed