From cdfdb14afe9cd79fbf5adf73e07c09c48fd181b4 Mon Sep 17 00:00:00 2001 From: David Jumani Date: Tue, 1 Aug 2023 17:03:23 -0400 Subject: [PATCH] backport: Expose initial_metadata in GrpcHealthCheck [1.14] (#8530) * feat: Expose initial_metadata in GrpcHealthCheck (#8511) * feat: Expose initial_metadata in GrpcHealthCheck * Trigger CI * Fix uneven whitespace Co-authored-by: Nathan Fudenberg * Adding changelog file to new location * Avoid code duplcation * Removing duplicate changelog * fix test * Add check for forbidden custom headers --------- Co-authored-by: David Jumani Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> Co-authored-by: Nathan Fudenberg Co-authored-by: changelog-bot * Move changelog * Update changelog --------- Co-authored-by: David Jumani Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> Co-authored-by: Nathan Fudenberg --- ...xpose-initialmetadata-grpchealthcheck.yaml | 6 + .../api/v2/core/health_check.proto.sk.md | 2 + .../gloo/crds/gloo.solo.io_v1_Upstream.yaml | 22 ++ .../api_conversion_suite_test.go | 13 ++ pkg/utils/api_conversion/health_check.go | 14 +- pkg/utils/api_conversion/type.go | 14 ++ pkg/utils/api_conversion/type_test.go | 67 ++++++ .../envoy/api/v2/core/health_check.proto | 5 + .../api/v2/core/health_check.pb.equal.go | 17 ++ .../envoy/api/v2/core/health_check.pb.go | 105 +++++---- .../envoy/api/v2/core/health_check.pb.hash.go | 24 +++ .../gloo/pkg/translator/translator_test.go | 202 +++++++++++++---- test/e2e/headers_test.go | 203 +++++++++++------- 13 files changed, 534 insertions(+), 160 deletions(-) create mode 100644 changelog/v1.14.14/expose-initialmetadata-grpchealthcheck.yaml create mode 100644 pkg/utils/api_conversion/api_conversion_suite_test.go create mode 100644 pkg/utils/api_conversion/type_test.go diff --git a/changelog/v1.14.14/expose-initialmetadata-grpchealthcheck.yaml b/changelog/v1.14.14/expose-initialmetadata-grpchealthcheck.yaml new file mode 100644 index 00000000000..f8b0999b19f --- /dev/null +++ b/changelog/v1.14.14/expose-initialmetadata-grpchealthcheck.yaml @@ -0,0 +1,6 @@ +changelog: +- type: FIX + issueLink: https://github.com/solo-io/gloo/issues/8490 + resolvesIssue: true + description: >- + Adds the ability to pass `initial_metadata` while configuring a GrpcHealthCheck. It specifies a list of key-value pairs that should be added to the metadata of each GRPC call that is sent to the health checked cluster. diff --git a/docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/external/envoy/api/v2/core/health_check.proto.sk.md b/docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/external/envoy/api/v2/core/health_check.proto.sk.md index 566d6ce6561..11d4b271847 100644 --- a/docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/external/envoy/api/v2/core/health_check.proto.sk.md +++ b/docs/content/reference/api/github.com/solo-io/gloo/projects/gloo/api/external/envoy/api/v2/core/health_check.proto.sk.md @@ -186,6 +186,7 @@ for details. ```yaml "serviceName": string "authority": string +"initialMetadata": []solo.io.envoy.api.v2.core.HeaderValueOption ``` @@ -193,6 +194,7 @@ for details. | ----- | ---- | ----------- | | `serviceName` | `string` | An optional service name parameter which will be sent to gRPC service in `grpc.health.v1.HealthCheckRequest `_. message. See `gRPC health-checking overview `_ for more information. | | `authority` | `string` | The value of the :authority header in the gRPC health check request. If left empty (default value), the name of the cluster this health check is associated with will be used. | +| `initialMetadata` | [[]solo.io.envoy.api.v2.core.HeaderValueOption](../../../../../../../../../../solo-kit/api/external/envoy/api/v2/core/base.proto.sk/#headervalueoption) | Specifies a list of key-value pairs that should be added to the metadata of each GRPC call that is sent to the health checked cluster. | diff --git a/install/helm/gloo/crds/gloo.solo.io_v1_Upstream.yaml b/install/helm/gloo/crds/gloo.solo.io_v1_Upstream.yaml index 85476608eb1..dabe64e1814 100644 --- a/install/helm/gloo/crds/gloo.solo.io_v1_Upstream.yaml +++ b/install/helm/gloo/crds/gloo.solo.io_v1_Upstream.yaml @@ -557,6 +557,28 @@ spec: properties: authority: type: string + initialMetadata: + items: + properties: + append: + nullable: true + type: boolean + header: + properties: + key: + type: string + value: + type: string + type: object + headerSecretRef: + properties: + name: + type: string + namespace: + type: string + type: object + type: object + type: array serviceName: type: string type: object diff --git a/pkg/utils/api_conversion/api_conversion_suite_test.go b/pkg/utils/api_conversion/api_conversion_suite_test.go new file mode 100644 index 00000000000..24660fc083c --- /dev/null +++ b/pkg/utils/api_conversion/api_conversion_suite_test.go @@ -0,0 +1,13 @@ +package api_conversion_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestChannelutils(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "API Conversion Suite") +} diff --git a/pkg/utils/api_conversion/health_check.go b/pkg/utils/api_conversion/health_check.go index 2f894831bfd..059347040fd 100644 --- a/pkg/utils/api_conversion/health_check.go +++ b/pkg/utils/api_conversion/health_check.go @@ -161,10 +161,15 @@ func ToEnvoyHealthCheck(check *envoycore_gloo.HealthCheck, secrets *v1.SecretLis } case *envoycore_gloo.HealthCheck_GrpcHealthCheck_: + var initialMetadata, err = ToEnvoyHeaderValueOptionList(typed.GrpcHealthCheck.GetInitialMetadata(), secrets, secretOptions) + if err != nil { + return nil, err + } hc.HealthChecker = &envoy_config_core_v3.HealthCheck_GrpcHealthCheck_{ GrpcHealthCheck: &envoy_config_core_v3.HealthCheck_GrpcHealthCheck{ - ServiceName: typed.GrpcHealthCheck.GetServiceName(), - Authority: typed.GrpcHealthCheck.GetAuthority(), + ServiceName: typed.GrpcHealthCheck.GetServiceName(), + Authority: typed.GrpcHealthCheck.GetAuthority(), + InitialMetadata: initialMetadata, }, } case *envoycore_gloo.HealthCheck_CustomHealthCheck_: @@ -253,8 +258,9 @@ func ToGlooHealthCheck(check *envoy_config_core_v3.HealthCheck) (*envoycore_gloo case *envoy_config_core_v3.HealthCheck_GrpcHealthCheck_: hc.HealthChecker = &envoycore_gloo.HealthCheck_GrpcHealthCheck_{ GrpcHealthCheck: &envoycore_gloo.HealthCheck_GrpcHealthCheck{ - ServiceName: typed.GrpcHealthCheck.GetServiceName(), - Authority: typed.GrpcHealthCheck.GetAuthority(), + ServiceName: typed.GrpcHealthCheck.GetServiceName(), + Authority: typed.GrpcHealthCheck.GetAuthority(), + InitialMetadata: ToGlooHeaderValueOptionList(typed.GrpcHealthCheck.GetInitialMetadata()), }, } case *envoy_config_core_v3.HealthCheck_CustomHealthCheck_: diff --git a/pkg/utils/api_conversion/type.go b/pkg/utils/api_conversion/type.go index 07a391135b1..0b614dd451d 100644 --- a/pkg/utils/api_conversion/type.go +++ b/pkg/utils/api_conversion/type.go @@ -1,6 +1,8 @@ package api_conversion import ( + "strings" + envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" envoy_type_v3 "github.com/envoyproxy/go-control-plane/envoy/type/v3" envoytype_gloo "github.com/solo-io/gloo/projects/gloo/pkg/api/external/envoy/type" @@ -69,9 +71,21 @@ func ToEnvoyHeaderValueOptionList(option []*envoycore_sk.HeaderValueOption, secr return result, nil } +// CheckForbiddenCustomHeaders checks whether the custom header is allowed to be modified as per https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers#custom-request-response-headers +func CheckForbiddenCustomHeaders(header envoycore_sk.HeaderValue) error { + key := header.GetKey() + if strings.HasPrefix(key, ":") || strings.ToLower(key) == "host" { + return errors.Errorf(": -prefixed or host headers may not be modified. Received '%s' header", key) + } + return nil +} + func ToEnvoyHeaderValueOptions(option *envoycore_sk.HeaderValueOption, secrets *v1.SecretList, secretOptions HeaderSecretOptions) ([]*envoy_config_core_v3.HeaderValueOption, error) { switch typedOption := option.GetHeaderOption().(type) { case *envoycore_sk.HeaderValueOption_Header: + if err := CheckForbiddenCustomHeaders(*typedOption.Header); err != nil { + return nil, err + } return []*envoy_config_core_v3.HeaderValueOption{ { Header: &envoy_config_core_v3.HeaderValue{ diff --git a/pkg/utils/api_conversion/type_test.go b/pkg/utils/api_conversion/type_test.go new file mode 100644 index 00000000000..7a7f6a1e429 --- /dev/null +++ b/pkg/utils/api_conversion/type_test.go @@ -0,0 +1,67 @@ +package api_conversion_test + +import ( + "github.com/golang/protobuf/ptypes/wrappers" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + . "github.com/solo-io/gloo/pkg/utils/api_conversion" + envoycore_sk "github.com/solo-io/solo-kit/pkg/api/external/envoy/api/v2/core" +) + +var _ = Describe("Type conversion", func() { + Context("ToEnvoyHeaderValueOptionList", func() { + It("should convert allowed headers", func() { + allowedHeaders := []*envoycore_sk.HeaderValueOption{ + { + HeaderOption: &envoycore_sk.HeaderValueOption_Header{ + Header: &envoycore_sk.HeaderValue{ + Key: "allowed", + Value: "header", + }, + }, + Append: &wrappers.BoolValue{ + Value: true, + }, + }, + } + + expectedHeaders := []*envoy_config_core_v3.HeaderValueOption{ + { + Header: &envoy_config_core_v3.HeaderValue{ + Key: "allowed", + Value: "header", + }, + Append: &wrappers.BoolValue{ + Value: true, + }, + }, + } + headers, err := ToEnvoyHeaderValueOptionList(allowedHeaders, nil, HeaderSecretOptions{}) + Expect(err).To(BeNil()) + Expect(headers).To(Equal(expectedHeaders)) + }) + + DescribeTable("should error out forbidden headers", func(key string) { + allowedHeaders := []*envoycore_sk.HeaderValueOption{ + { + HeaderOption: &envoycore_sk.HeaderValueOption_Header{ + Header: &envoycore_sk.HeaderValue{ + Key: key, + Value: "value", + }, + }, + Append: &wrappers.BoolValue{ + Value: true, + }, + }, + } + + _, err := ToEnvoyHeaderValueOptionList(allowedHeaders, nil, HeaderSecretOptions{}) + Expect(err).To(MatchError(ContainSubstring(": -prefixed or host headers may not be modified"))) + }, + Entry("host header", "host"), + Entry(": prefixed header header", ":path")) + }) +}) diff --git a/projects/gloo/api/external/envoy/api/v2/core/health_check.proto b/projects/gloo/api/external/envoy/api/v2/core/health_check.proto index ca66a60f6c2..05c075c8ad4 100644 --- a/projects/gloo/api/external/envoy/api/v2/core/health_check.proto +++ b/projects/gloo/api/external/envoy/api/v2/core/health_check.proto @@ -180,6 +180,11 @@ message HealthCheck { // left empty (default value), the name of the cluster this health check is associated // with will be used. string authority = 2; + + // Specifies a list of key-value pairs that should be added to the metadata of each GRPC call + // that is sent to the health checked cluster. + repeated .solo.io.envoy.api.v2.core.HeaderValueOption initial_metadata = 3 + [(validate.rules).repeated .max_items = 1000]; } diff --git a/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.equal.go b/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.equal.go index fb12bdccaca..80c4d628c8f 100644 --- a/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.equal.go +++ b/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.equal.go @@ -493,6 +493,23 @@ func (m *HealthCheck_GrpcHealthCheck) Equal(that interface{}) bool { return false } + if len(m.GetInitialMetadata()) != len(target.GetInitialMetadata()) { + return false + } + for idx, v := range m.GetInitialMetadata() { + + if h, ok := interface{}(v).(equality.Equalizer); ok { + if !h.Equal(target.GetInitialMetadata()[idx]) { + return false + } + } else { + if !proto.Equal(v, target.GetInitialMetadata()[idx]) { + return false + } + } + + } + return true } diff --git a/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.go b/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.go index 72474930109..de0ac9a233c 100644 --- a/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.go +++ b/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.go @@ -710,6 +710,9 @@ type HealthCheck_GrpcHealthCheck struct { // left empty (default value), the name of the cluster this health check is associated // with will be used. Authority string `protobuf:"bytes,2,opt,name=authority,proto3" json:"authority,omitempty"` + // Specifies a list of key-value pairs that should be added to the metadata of each GRPC call + // that is sent to the health checked cluster. + InitialMetadata []*core.HeaderValueOption `protobuf:"bytes,3,rep,name=initial_metadata,json=initialMetadata,proto3" json:"initial_metadata,omitempty"` } func (x *HealthCheck_GrpcHealthCheck) Reset() { @@ -758,6 +761,13 @@ func (x *HealthCheck_GrpcHealthCheck) GetAuthority() string { return "" } +func (x *HealthCheck_GrpcHealthCheck) GetInitialMetadata() []*core.HeaderValueOption { + if x != nil { + return x.InitialMetadata + } + return nil +} + // Custom health check. type HealthCheck_CustomHealthCheck struct { state protoimpl.MessageState @@ -892,7 +902,7 @@ var file_github_com_solo_io_gloo_projects_gloo_api_external_envoy_api_v2_core_he 0x70, 0x70, 0x65, 0x72, 0x73, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x1a, 0x17, 0x76, 0x61, 0x6c, 0x69, 0x64, 0x61, 0x74, 0x65, 0x2f, 0x76, 0x61, 0x6c, 0x69, 0x64, 0x61, 0x74, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x1a, 0x12, 0x65, 0x78, 0x74, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x65, - 0x78, 0x74, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xf6, 0x13, 0x0a, 0x0b, 0x48, 0x65, 0x61, + 0x78, 0x74, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xdb, 0x14, 0x0a, 0x0b, 0x48, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x43, 0x68, 0x65, 0x63, 0x6b, 0x12, 0x3f, 0x0a, 0x07, 0x74, 0x69, 0x6d, 0x65, 0x6f, 0x75, 0x74, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x19, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x44, 0x75, 0x72, 0x61, @@ -1032,43 +1042,49 @@ var file_github_com_solo_io_gloo_projects_gloo_api_external_envoy_api_v2_core_he 0x43, 0x68, 0x65, 0x63, 0x6b, 0x2e, 0x50, 0x61, 0x79, 0x6c, 0x6f, 0x61, 0x64, 0x52, 0x07, 0x72, 0x65, 0x63, 0x65, 0x69, 0x76, 0x65, 0x1a, 0x24, 0x0a, 0x10, 0x52, 0x65, 0x64, 0x69, 0x73, 0x48, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x43, 0x68, 0x65, 0x63, 0x6b, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, - 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x1a, 0x52, 0x0a, 0x0f, - 0x47, 0x72, 0x70, 0x63, 0x48, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x43, 0x68, 0x65, 0x63, 0x6b, 0x12, - 0x21, 0x0a, 0x0c, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x18, - 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0b, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x4e, 0x61, - 0x6d, 0x65, 0x12, 0x1c, 0x0a, 0x09, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x74, 0x79, 0x18, - 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x09, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x74, 0x79, - 0x1a, 0xad, 0x01, 0x0a, 0x11, 0x43, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x48, 0x65, 0x61, 0x6c, 0x74, - 0x68, 0x43, 0x68, 0x65, 0x63, 0x6b, 0x12, 0x1b, 0x0a, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x01, - 0x20, 0x01, 0x28, 0x09, 0x42, 0x07, 0xfa, 0x42, 0x04, 0x72, 0x02, 0x20, 0x01, 0x52, 0x04, 0x6e, - 0x61, 0x6d, 0x65, 0x12, 0x31, 0x0a, 0x06, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x18, 0x02, 0x20, - 0x01, 0x28, 0x0b, 0x32, 0x17, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, - 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x53, 0x74, 0x72, 0x75, 0x63, 0x74, 0x48, 0x00, 0x52, 0x06, - 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x39, 0x0a, 0x0c, 0x74, 0x79, 0x70, 0x65, 0x64, 0x5f, - 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x14, 0x2e, 0x67, - 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x41, - 0x6e, 0x79, 0x48, 0x00, 0x52, 0x0b, 0x74, 0x79, 0x70, 0x65, 0x64, 0x43, 0x6f, 0x6e, 0x66, 0x69, - 0x67, 0x42, 0x0d, 0x0a, 0x0b, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x5f, 0x74, 0x79, 0x70, 0x65, - 0x42, 0x15, 0x0a, 0x0e, 0x68, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x5f, 0x63, 0x68, 0x65, 0x63, 0x6b, - 0x65, 0x72, 0x12, 0x03, 0xf8, 0x42, 0x01, 0x4a, 0x04, 0x08, 0x0a, 0x10, 0x0b, 0x52, 0x12, 0x72, - 0x65, 0x64, 0x69, 0x73, 0x5f, 0x68, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x5f, 0x63, 0x68, 0x65, 0x63, - 0x6b, 0x2a, 0x60, 0x0a, 0x0c, 0x48, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x53, 0x74, 0x61, 0x74, 0x75, - 0x73, 0x12, 0x0b, 0x0a, 0x07, 0x55, 0x4e, 0x4b, 0x4e, 0x4f, 0x57, 0x4e, 0x10, 0x00, 0x12, 0x0b, - 0x0a, 0x07, 0x48, 0x45, 0x41, 0x4c, 0x54, 0x48, 0x59, 0x10, 0x01, 0x12, 0x0d, 0x0a, 0x09, 0x55, - 0x4e, 0x48, 0x45, 0x41, 0x4c, 0x54, 0x48, 0x59, 0x10, 0x02, 0x12, 0x0c, 0x0a, 0x08, 0x44, 0x52, - 0x41, 0x49, 0x4e, 0x49, 0x4e, 0x47, 0x10, 0x03, 0x12, 0x0b, 0x0a, 0x07, 0x54, 0x49, 0x4d, 0x45, - 0x4f, 0x55, 0x54, 0x10, 0x04, 0x12, 0x0c, 0x0a, 0x08, 0x44, 0x45, 0x47, 0x52, 0x41, 0x44, 0x45, - 0x44, 0x10, 0x05, 0x42, 0x97, 0x01, 0x0a, 0x2f, 0x69, 0x6f, 0x2e, 0x65, 0x6e, 0x76, 0x6f, 0x79, - 0x70, 0x72, 0x6f, 0x78, 0x79, 0x2e, 0x73, 0x6f, 0x6c, 0x6f, 0x2e, 0x69, 0x6f, 0x2e, 0x73, 0x6f, + 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x1a, 0xb6, 0x01, 0x0a, + 0x0f, 0x47, 0x72, 0x70, 0x63, 0x48, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x43, 0x68, 0x65, 0x63, 0x6b, + 0x12, 0x21, 0x0a, 0x0c, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x5f, 0x6e, 0x61, 0x6d, 0x65, + 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0b, 0x73, 0x65, 0x72, 0x76, 0x69, 0x63, 0x65, 0x4e, + 0x61, 0x6d, 0x65, 0x12, 0x1c, 0x0a, 0x09, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x74, 0x79, + 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x09, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x74, + 0x79, 0x12, 0x62, 0x0a, 0x10, 0x69, 0x6e, 0x69, 0x74, 0x69, 0x61, 0x6c, 0x5f, 0x6d, 0x65, 0x74, + 0x61, 0x64, 0x61, 0x74, 0x61, 0x18, 0x03, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x2c, 0x2e, 0x73, 0x6f, 0x6c, 0x6f, 0x2e, 0x69, 0x6f, 0x2e, 0x65, 0x6e, 0x76, 0x6f, 0x79, 0x2e, 0x61, 0x70, 0x69, 0x2e, - 0x76, 0x32, 0x2e, 0x63, 0x6f, 0x72, 0x65, 0x42, 0x10, 0x48, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x43, - 0x68, 0x65, 0x63, 0x6b, 0x50, 0x72, 0x6f, 0x74, 0x6f, 0x50, 0x01, 0x5a, 0x48, 0x67, 0x69, 0x74, - 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x73, 0x6f, 0x6c, 0x6f, 0x2d, 0x69, 0x6f, 0x2f, - 0x67, 0x6c, 0x6f, 0x6f, 0x2f, 0x70, 0x72, 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x73, 0x2f, 0x67, 0x6c, - 0x6f, 0x6f, 0x2f, 0x70, 0x6b, 0x67, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x65, 0x78, 0x74, 0x65, 0x72, - 0x6e, 0x61, 0x6c, 0x2f, 0x65, 0x6e, 0x76, 0x6f, 0x79, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x76, 0x32, - 0x2f, 0x63, 0x6f, 0x72, 0x65, 0xc0, 0xf5, 0x04, 0x01, 0xb8, 0xf5, 0x04, 0x01, 0x62, 0x06, 0x70, - 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x76, 0x32, 0x2e, 0x63, 0x6f, 0x72, 0x65, 0x2e, 0x48, 0x65, 0x61, 0x64, 0x65, 0x72, 0x56, 0x61, + 0x6c, 0x75, 0x65, 0x4f, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x42, 0x09, 0xfa, 0x42, 0x06, 0x92, 0x01, + 0x03, 0x10, 0xe8, 0x07, 0x52, 0x0f, 0x69, 0x6e, 0x69, 0x74, 0x69, 0x61, 0x6c, 0x4d, 0x65, 0x74, + 0x61, 0x64, 0x61, 0x74, 0x61, 0x1a, 0xad, 0x01, 0x0a, 0x11, 0x43, 0x75, 0x73, 0x74, 0x6f, 0x6d, + 0x48, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x43, 0x68, 0x65, 0x63, 0x6b, 0x12, 0x1b, 0x0a, 0x04, 0x6e, + 0x61, 0x6d, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x42, 0x07, 0xfa, 0x42, 0x04, 0x72, 0x02, + 0x20, 0x01, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x31, 0x0a, 0x06, 0x63, 0x6f, 0x6e, 0x66, + 0x69, 0x67, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x17, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, + 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x53, 0x74, 0x72, 0x75, 0x63, + 0x74, 0x48, 0x00, 0x52, 0x06, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x39, 0x0a, 0x0c, 0x74, + 0x79, 0x70, 0x65, 0x64, 0x5f, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x18, 0x03, 0x20, 0x01, 0x28, + 0x0b, 0x32, 0x14, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, + 0x62, 0x75, 0x66, 0x2e, 0x41, 0x6e, 0x79, 0x48, 0x00, 0x52, 0x0b, 0x74, 0x79, 0x70, 0x65, 0x64, + 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x42, 0x0d, 0x0a, 0x0b, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, + 0x5f, 0x74, 0x79, 0x70, 0x65, 0x42, 0x15, 0x0a, 0x0e, 0x68, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x5f, + 0x63, 0x68, 0x65, 0x63, 0x6b, 0x65, 0x72, 0x12, 0x03, 0xf8, 0x42, 0x01, 0x4a, 0x04, 0x08, 0x0a, + 0x10, 0x0b, 0x52, 0x12, 0x72, 0x65, 0x64, 0x69, 0x73, 0x5f, 0x68, 0x65, 0x61, 0x6c, 0x74, 0x68, + 0x5f, 0x63, 0x68, 0x65, 0x63, 0x6b, 0x2a, 0x60, 0x0a, 0x0c, 0x48, 0x65, 0x61, 0x6c, 0x74, 0x68, + 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x0b, 0x0a, 0x07, 0x55, 0x4e, 0x4b, 0x4e, 0x4f, 0x57, + 0x4e, 0x10, 0x00, 0x12, 0x0b, 0x0a, 0x07, 0x48, 0x45, 0x41, 0x4c, 0x54, 0x48, 0x59, 0x10, 0x01, + 0x12, 0x0d, 0x0a, 0x09, 0x55, 0x4e, 0x48, 0x45, 0x41, 0x4c, 0x54, 0x48, 0x59, 0x10, 0x02, 0x12, + 0x0c, 0x0a, 0x08, 0x44, 0x52, 0x41, 0x49, 0x4e, 0x49, 0x4e, 0x47, 0x10, 0x03, 0x12, 0x0b, 0x0a, + 0x07, 0x54, 0x49, 0x4d, 0x45, 0x4f, 0x55, 0x54, 0x10, 0x04, 0x12, 0x0c, 0x0a, 0x08, 0x44, 0x45, + 0x47, 0x52, 0x41, 0x44, 0x45, 0x44, 0x10, 0x05, 0x42, 0x97, 0x01, 0x0a, 0x2f, 0x69, 0x6f, 0x2e, + 0x65, 0x6e, 0x76, 0x6f, 0x79, 0x70, 0x72, 0x6f, 0x78, 0x79, 0x2e, 0x73, 0x6f, 0x6c, 0x6f, 0x2e, + 0x69, 0x6f, 0x2e, 0x73, 0x6f, 0x6c, 0x6f, 0x2e, 0x69, 0x6f, 0x2e, 0x65, 0x6e, 0x76, 0x6f, 0x79, + 0x2e, 0x61, 0x70, 0x69, 0x2e, 0x76, 0x32, 0x2e, 0x63, 0x6f, 0x72, 0x65, 0x42, 0x10, 0x48, 0x65, + 0x61, 0x6c, 0x74, 0x68, 0x43, 0x68, 0x65, 0x63, 0x6b, 0x50, 0x72, 0x6f, 0x74, 0x6f, 0x50, 0x01, + 0x5a, 0x48, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x73, 0x6f, 0x6c, + 0x6f, 0x2d, 0x69, 0x6f, 0x2f, 0x67, 0x6c, 0x6f, 0x6f, 0x2f, 0x70, 0x72, 0x6f, 0x6a, 0x65, 0x63, + 0x74, 0x73, 0x2f, 0x67, 0x6c, 0x6f, 0x6f, 0x2f, 0x70, 0x6b, 0x67, 0x2f, 0x61, 0x70, 0x69, 0x2f, + 0x65, 0x78, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x65, 0x6e, 0x76, 0x6f, 0x79, 0x2f, 0x61, + 0x70, 0x69, 0x2f, 0x76, 0x32, 0x2f, 0x63, 0x6f, 0x72, 0x65, 0xc0, 0xf5, 0x04, 0x01, 0xb8, 0xf5, + 0x04, 0x01, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -1126,13 +1142,14 @@ var file_github_com_solo_io_gloo_projects_gloo_api_external_envoy_api_v2_core_he 14, // 18: solo.io.envoy.api.v2.core.HealthCheck.HttpHealthCheck.method:type_name -> solo.io.envoy.config.core.v3.RequestMethod 2, // 19: solo.io.envoy.api.v2.core.HealthCheck.TcpHealthCheck.send:type_name -> solo.io.envoy.api.v2.core.HealthCheck.Payload 2, // 20: solo.io.envoy.api.v2.core.HealthCheck.TcpHealthCheck.receive:type_name -> solo.io.envoy.api.v2.core.HealthCheck.Payload - 15, // 21: solo.io.envoy.api.v2.core.HealthCheck.CustomHealthCheck.config:type_name -> google.protobuf.Struct - 16, // 22: solo.io.envoy.api.v2.core.HealthCheck.CustomHealthCheck.typed_config:type_name -> google.protobuf.Any - 23, // [23:23] is the sub-list for method output_type - 23, // [23:23] is the sub-list for method input_type - 23, // [23:23] is the sub-list for extension type_name - 23, // [23:23] is the sub-list for extension extendee - 0, // [0:23] is the sub-list for field type_name + 11, // 21: solo.io.envoy.api.v2.core.HealthCheck.GrpcHealthCheck.initial_metadata:type_name -> solo.io.envoy.api.v2.core.HeaderValueOption + 15, // 22: solo.io.envoy.api.v2.core.HealthCheck.CustomHealthCheck.config:type_name -> google.protobuf.Struct + 16, // 23: solo.io.envoy.api.v2.core.HealthCheck.CustomHealthCheck.typed_config:type_name -> google.protobuf.Any + 24, // [24:24] is the sub-list for method output_type + 24, // [24:24] is the sub-list for method input_type + 24, // [24:24] is the sub-list for extension type_name + 24, // [24:24] is the sub-list for extension extendee + 0, // [0:24] is the sub-list for field type_name } func init() { diff --git a/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.hash.go b/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.hash.go index 4e5745dd306..b63d04d23ad 100644 --- a/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.hash.go +++ b/projects/gloo/pkg/api/external/envoy/api/v2/core/health_check.pb.hash.go @@ -612,6 +612,30 @@ func (m *HealthCheck_GrpcHealthCheck) Hash(hasher hash.Hash64) (uint64, error) { return 0, err } + for _, v := range m.GetInitialMetadata() { + + if h, ok := interface{}(v).(safe_hasher.SafeHasher); ok { + if _, err = hasher.Write([]byte("")); err != nil { + return 0, err + } + if _, err = h.Hash(hasher); err != nil { + return 0, err + } + } else { + if fieldValue, err := hashstructure.Hash(v, nil); err != nil { + return 0, err + } else { + if _, err = hasher.Write([]byte("")); err != nil { + return 0, err + } + if err := binary.Write(hasher, binary.LittleEndian, fieldValue); err != nil { + return 0, err + } + } + } + + } + return hasher.Sum64(), nil } diff --git a/projects/gloo/pkg/translator/translator_test.go b/projects/gloo/pkg/translator/translator_test.go index 093fc44d6c4..3d6b5fb669c 100644 --- a/projects/gloo/pkg/translator/translator_test.go +++ b/projects/gloo/pkg/translator/translator_test.go @@ -1117,8 +1117,9 @@ var _ = Describe("Translator", func() { UnhealthyThreshold: DefaultThreshold, HealthChecker: &envoy_config_core_v3.HealthCheck_GrpcHealthCheck_{ GrpcHealthCheck: &envoy_config_core_v3.HealthCheck_GrpcHealthCheck{ - ServiceName: "svc", - Authority: "authority", + ServiceName: "svc", + Authority: "authority", + InitialMetadata: []*envoy_config_core_v3.HeaderValueOption{}, }, }, }, @@ -1173,55 +1174,104 @@ var _ = Describe("Translator", func() { translateWithBuggyHasher() }) - Context("Health checks with secret header", func() { - AfterEach(os.Clearenv) - DescribeTable("can translate health check with secret header", func(enforceMatch, secretNamespace string, expectError bool) { - err := os.Setenv(api_conversion.MatchingNamespaceEnv, enforceMatch) - Expect(err).NotTo(HaveOccurred()) - params.Snapshot.Secrets = v1.SecretList{ + Context("Healthcheck with Forbidden headers", func() { + var healthChecks []*gloo_envoy_core.HealthCheck + + BeforeEach(func() { + healthChecks = []*gloo_envoy_core.HealthCheck{ { - Kind: &v1.Secret_Header{ - Header: &v1.HeaderSecret{ - Headers: map[string]string{ - "Authorization": "basic dXNlcjpwYXNzd29yZA==", + Timeout: DefaultHealthCheckTimeout, + Interval: DefaultHealthCheckInterval, + HealthyThreshold: DefaultThreshold, + UnhealthyThreshold: DefaultThreshold, + }, + } + Expect(healthChecks[0].HealthChecker).To(BeNil()) + }) + + DescribeTable("http health check", func(key string, expectError bool) { + upstream.HealthChecks = healthChecks + healthChecks[0].HealthChecker = &gloo_envoy_core.HealthCheck_HttpHealthCheck_{ + HttpHealthCheck: &gloo_envoy_core.HealthCheck_HttpHealthCheck{ + RequestHeadersToAdd: []*envoycore_sk.HeaderValueOption{ + { + HeaderOption: &envoycore_sk.HeaderValueOption_Header{ + Header: &envoycore_sk.HeaderValue{ + Key: key, + Value: "value", + }, + }, + Append: &wrappers.BoolValue{ + Value: true, }, }, }, - Metadata: &core.Metadata{ - Name: "foo", - Namespace: secretNamespace, - }, }, } + _, errs, _ := translator.Translate(params, proxy) - expectedResult := []*envoy_config_core_v3.HealthCheck{ - { - Timeout: DefaultHealthCheckTimeout, - Interval: DefaultHealthCheckInterval, - HealthyThreshold: DefaultThreshold, - UnhealthyThreshold: DefaultThreshold, - HealthChecker: &envoy_config_core_v3.HealthCheck_HttpHealthCheck_{ - HttpHealthCheck: &envoy_config_core_v3.HealthCheck_HttpHealthCheck{ - Host: "host", - Path: "path", - ServiceNameMatcher: &envoy_type_matcher_v3.StringMatcher{ - MatchPattern: &envoy_type_matcher_v3.StringMatcher_Prefix{ - Prefix: "svc", + if expectError { + Expect(errs.Validate()).To(MatchError(ContainSubstring(": -prefixed or host headers may not be modified"))) + return + } + Expect(errs.Validate()).NotTo(HaveOccurred()) + }, + Entry("Allowed header", "some-header", false), + Entry(":-prefixed header", ":path", true)) + + DescribeTable("grpc health check", func(key string, expectError bool) { + upstream.HealthChecks = healthChecks + healthChecks[0].HealthChecker = &gloo_envoy_core.HealthCheck_GrpcHealthCheck_{ + GrpcHealthCheck: &gloo_envoy_core.HealthCheck_GrpcHealthCheck{ + InitialMetadata: []*envoycore_sk.HeaderValueOption{ + { + HeaderOption: &envoycore_sk.HeaderValueOption_Header{ + Header: &envoycore_sk.HeaderValue{ + Key: key, + Value: "value", }, }, - RequestHeadersToAdd: []*envoy_config_core_v3.HeaderValueOption{}, - RequestHeadersToRemove: []string{}, - CodecClientType: envoy_type_v3.CodecClientType_HTTP2, - ExpectedStatuses: []*envoy_type_v3.Int64Range{}, + Append: &wrappers.BoolValue{ + Value: true, + }, }, }, }, } + _, errs, _ := translator.Translate(params, proxy) - upstream.HealthChecks, err = api_conversion.ToGlooHealthCheckList(expectedResult) - Expect(err).NotTo(HaveOccurred()) + if expectError { + Expect(errs.Validate()).To(MatchError(ContainSubstring(": -prefixed or host headers may not be modified"))) + return + } + Expect(errs.Validate()).NotTo(HaveOccurred()) + }, + Entry("Allowed header", "some-header", false), + Entry("host header", "host", true)) + }) + + Context("Health checks with secret header", func() { + var expectedResult []*envoy_config_core_v3.HealthCheck + var expectedHeaders []*envoy_config_core_v3.HeaderValueOption + var upstreamHeaders []*envoycore_sk.HeaderValueOption + + BeforeEach(func() { + params.Snapshot.Secrets = v1.SecretList{ + { + Kind: &v1.Secret_Header{ + Header: &v1.HeaderSecret{ + Headers: map[string]string{ + "Authorization": "basic dXNlcjpwYXNzd29yZA==", + }, + }, + }, + Metadata: &core.Metadata{ + Name: "foo", + }, + }, + } - expectedResult[0].GetHttpHealthCheck().RequestHeadersToAdd = []*envoy_config_core_v3.HeaderValueOption{ + expectedHeaders = []*envoy_config_core_v3.HeaderValueOption{ { Header: &envoy_config_core_v3.HeaderValue{ Key: "Authorization", @@ -1233,12 +1283,11 @@ var _ = Describe("Translator", func() { }, } - upstream.GetHealthChecks()[0].GetHttpHealthCheck().RequestHeadersToAdd = []*envoycore_sk.HeaderValueOption{ + upstreamHeaders = []*envoycore_sk.HeaderValueOption{ { HeaderOption: &envoycore_sk.HeaderValueOption_HeaderSecretRef{ HeaderSecretRef: &core.ResourceRef{ - Name: "foo", - Namespace: secretNamespace, + Name: "foo", }, }, Append: &wrappers.BoolValue{ @@ -1247,6 +1296,20 @@ var _ = Describe("Translator", func() { }, } + expectedResult = []*envoy_config_core_v3.HealthCheck{ + { + Timeout: DefaultHealthCheckTimeout, + Interval: DefaultHealthCheckInterval, + HealthyThreshold: DefaultThreshold, + UnhealthyThreshold: DefaultThreshold, + }, + } + Expect(expectedResult[0].HealthChecker).To(BeNil()) + }) + + AfterEach(os.Clearenv) + + translate := func(expectError bool) { snap, errs, report := translator.Translate(params, proxy) if expectError { Expect(errs.Validate()).To(MatchError(ContainSubstring("list did not find secret bar.foo"))) @@ -1265,9 +1328,68 @@ var _ = Describe("Translator", func() { msgList = append(msgList, v) } Expect(cluster.HealthChecks).To(ConsistOfProtos(msgList...)) + } + + // Checks to ensure that https://github.com/solo-io/gloo/pull/8505 works as expected. + // It checks whether headerSecretRef and the upstream that the secret is sent to have matching namespaces + // if configured to do so and the code to do so is shared across both the http && grpc healt check. + // The test cases have been split between the http and grpc health checks as it relies on shared code, + // avoids test duplication and to ensure they both work as expected. + DescribeTable("http health check", func(enforceMatch, secretNamespace string, expectError bool) { + err := os.Setenv(api_conversion.MatchingNamespaceEnv, enforceMatch) + Expect(err).NotTo(HaveOccurred()) + + params.Snapshot.Secrets[0].Metadata.Namespace = secretNamespace + expectedResult[0].HealthChecker = &envoy_config_core_v3.HealthCheck_HttpHealthCheck_{ + HttpHealthCheck: &envoy_config_core_v3.HealthCheck_HttpHealthCheck{ + Host: "host", + Path: "path", + ServiceNameMatcher: &envoy_type_matcher_v3.StringMatcher{ + MatchPattern: &envoy_type_matcher_v3.StringMatcher_Prefix{ + Prefix: "svc", + }, + }, + RequestHeadersToAdd: []*envoy_config_core_v3.HeaderValueOption{}, + RequestHeadersToRemove: []string{}, + CodecClientType: envoy_type_v3.CodecClientType_HTTP2, + ExpectedStatuses: []*envoy_type_v3.Int64Range{}, + }, + } + + upstream.HealthChecks, err = api_conversion.ToGlooHealthCheckList(expectedResult) + Expect(err).NotTo(HaveOccurred()) + + expectedResult[0].GetHttpHealthCheck().RequestHeadersToAdd = expectedHeaders + upstream.GetHealthChecks()[0].GetHttpHealthCheck().RequestHeadersToAdd = upstreamHeaders + upstream.GetHealthChecks()[0].GetHttpHealthCheck().RequestHeadersToAdd[0].GetHeaderSecretRef().Namespace = secretNamespace + + translate(expectError) }, Entry("Matching enforced and namespaces match", "true", "gloo-system", false), - Entry("Matching not enforced and namespaces match", "false", "gloo-system", false), + Entry("Matching not enforced and namespaces match", "false", "gloo-system", false)) + + DescribeTable("grpc health check", func(enforceMatch, secretNamespace string, expectError bool) { + err := os.Setenv(api_conversion.MatchingNamespaceEnv, enforceMatch) + Expect(err).NotTo(HaveOccurred()) + + params.Snapshot.Secrets[0].Metadata.Namespace = secretNamespace + expectedResult[0].HealthChecker = &envoy_config_core_v3.HealthCheck_GrpcHealthCheck_{ + GrpcHealthCheck: &envoy_config_core_v3.HealthCheck_GrpcHealthCheck{ + ServiceName: "svc", + Authority: "authority", + InitialMetadata: []*envoy_config_core_v3.HeaderValueOption{}, + }, + } + + upstream.HealthChecks, err = api_conversion.ToGlooHealthCheckList(expectedResult) + Expect(err).NotTo(HaveOccurred()) + + expectedResult[0].GetGrpcHealthCheck().InitialMetadata = expectedHeaders + upstream.GetHealthChecks()[0].GetGrpcHealthCheck().InitialMetadata = upstreamHeaders + upstream.GetHealthChecks()[0].GetGrpcHealthCheck().InitialMetadata[0].GetHeaderSecretRef().Namespace = secretNamespace + + translate(expectError) + }, Entry("Matching not enforced and namespaces don't match", "false", "bar", false), Entry("Matching enforced and namespaces don't match", "true", "bar", true)) }) diff --git a/test/e2e/headers_test.go b/test/e2e/headers_test.go index 7d3ae9c193c..9cb46e5d9a9 100644 --- a/test/e2e/headers_test.go +++ b/test/e2e/headers_test.go @@ -18,7 +18,7 @@ import ( "github.com/solo-io/gloo/test/helpers" ) -var _ = Describe("Test Secrets in HeaderManipulation", func() { +var _ = Describe("HeaderManipulation", func() { var ( testContext *e2e.TestContext @@ -27,64 +27,9 @@ var _ = Describe("Test Secrets in HeaderManipulation", func() { BeforeEach(func() { testContext = testContextFactory.NewTestContext() testContext.BeforeEach() - // put a secret in `writeNamespace` so we have it in the snapshot - // The upstream is in `default` so when we enforce that secrets + upstream namespaces match, it should not be allowed - forbiddenSecret := &gloov1.Secret{ - Kind: &gloov1.Secret_Header{ - Header: &gloov1.HeaderSecret{ - Headers: map[string]string{ - "Authorization": "basic dXNlcjpwYXNzd29yZA==", - }, - }, - }, - Metadata: &coreV1.Metadata{ - Name: "foo", - Namespace: writeNamespace, - }, - } - // Create a secret in the same namespace as the upstream - allowedSecret := &gloov1.Secret{ - Kind: &gloov1.Secret_Header{ - Header: &gloov1.HeaderSecret{ - Headers: map[string]string{ - "Authorization": "basic dXNlcjpwYXNzd29yZA==", - }, - }, - }, - Metadata: &coreV1.Metadata{ - Name: "goodsecret", - Namespace: testContext.TestUpstream().Upstream.GetMetadata().GetNamespace(), - }, - } - headerManipVsBuilder := helpers.NewVirtualServiceBuilder(). - WithNamespace(writeNamespace). - WithRoutePrefixMatcher(e2e.DefaultRouteName, "/endpoint"). - WithRouteActionToUpstream(e2e.DefaultRouteName, testContext.TestUpstream().Upstream) - - goodVS := headerManipVsBuilder.Clone(). - WithName("good"). - WithDomain("custom-domain.com"). - WithVirtualHostOptions(&gloov1.VirtualHostOptions{HeaderManipulation: &headers.HeaderManipulation{ - RequestHeadersToAdd: []*envoycore_sk.HeaderValueOption{{HeaderOption: &envoycore_sk.HeaderValueOption_HeaderSecretRef{HeaderSecretRef: allowedSecret.GetMetadata().Ref()}, - Append: &wrappers.BoolValue{Value: true}}}, - }}). - Build() - badVS := headerManipVsBuilder.Clone(). - WithName("bad"). - WithDomain("another-domain.com"). - WithVirtualHostOptions( - &gloov1.VirtualHostOptions{HeaderManipulation: &headers.HeaderManipulation{ - RequestHeadersToAdd: []*envoycore_sk.HeaderValueOption{{HeaderOption: &envoycore_sk.HeaderValueOption_HeaderSecretRef{HeaderSecretRef: forbiddenSecret.GetMetadata().Ref()}, - Append: &wrappers.BoolValue{Value: true}}}, - }}). - Build() - - testContext.ResourcesToCreate().VirtualServices = v1.VirtualServiceList{goodVS, badVS} - testContext.ResourcesToCreate().Secrets = gloov1.SecretList{forbiddenSecret, allowedSecret} }) AfterEach(func() { - os.Unsetenv(api_conversion.MatchingNamespaceEnv) testContext.AfterEach() }) @@ -96,37 +41,151 @@ var _ = Describe("Test Secrets in HeaderManipulation", func() { testContext.JustAfterEach() }) - Context("With matching not enforced", func() { - + Context("Secrets in HeaderManipulation", func() { BeforeEach(func() { - os.Setenv(api_conversion.MatchingNamespaceEnv, "false") + // put a secret in `writeNamespace` so we have it in the snapshot + // The upstream is in `default` so when we enforce that secrets + upstream namespaces match, it should not be allowed + forbiddenSecret := &gloov1.Secret{ + Kind: &gloov1.Secret_Header{ + Header: &gloov1.HeaderSecret{ + Headers: map[string]string{ + "Authorization": "basic dXNlcjpwYXNzd29yZA==", + }, + }, + }, + Metadata: &coreV1.Metadata{ + Name: "foo", + Namespace: writeNamespace, + }, + } + // Create a secret in the same namespace as the upstream + allowedSecret := &gloov1.Secret{ + Kind: &gloov1.Secret_Header{ + Header: &gloov1.HeaderSecret{ + Headers: map[string]string{ + "Authorization": "basic dXNlcjpwYXNzd29yZA==", + }, + }, + }, + Metadata: &coreV1.Metadata{ + Name: "goodsecret", + Namespace: testContext.TestUpstream().Upstream.GetMetadata().GetNamespace(), + }, + } + headerManipVsBuilder := helpers.NewVirtualServiceBuilder(). + WithNamespace(writeNamespace). + WithRoutePrefixMatcher(e2e.DefaultRouteName, "/endpoint"). + WithRouteActionToUpstream(e2e.DefaultRouteName, testContext.TestUpstream().Upstream) + + goodVS := headerManipVsBuilder.Clone(). + WithName("good"). + WithDomain("custom-domain.com"). + WithVirtualHostOptions(&gloov1.VirtualHostOptions{HeaderManipulation: &headers.HeaderManipulation{ + RequestHeadersToAdd: []*envoycore_sk.HeaderValueOption{{HeaderOption: &envoycore_sk.HeaderValueOption_HeaderSecretRef{HeaderSecretRef: allowedSecret.GetMetadata().Ref()}, + Append: &wrappers.BoolValue{Value: true}}}, + }}). + Build() + badVS := headerManipVsBuilder.Clone(). + WithName("bad"). + WithDomain("another-domain.com"). + WithVirtualHostOptions( + &gloov1.VirtualHostOptions{HeaderManipulation: &headers.HeaderManipulation{ + RequestHeadersToAdd: []*envoycore_sk.HeaderValueOption{{HeaderOption: &envoycore_sk.HeaderValueOption_HeaderSecretRef{HeaderSecretRef: forbiddenSecret.GetMetadata().Ref()}, + Append: &wrappers.BoolValue{Value: true}}}, + }}). + Build() + + testContext.ResourcesToCreate().VirtualServices = v1.VirtualServiceList{goodVS, badVS} + testContext.ResourcesToCreate().Secrets = gloov1.SecretList{forbiddenSecret, allowedSecret} }) - It("Accepts all virtual services", func() { - helpers.EventuallyResourceAccepted(func() (resources.InputResource, error) { - vs, err := testContext.TestClients().VirtualServiceClient.Read(writeNamespace, "bad", clients.ReadOpts{}) - return vs, err + AfterEach(func() { + os.Unsetenv(api_conversion.MatchingNamespaceEnv) + }) + + Context("With matching not enforced", func() { + + BeforeEach(func() { + os.Setenv(api_conversion.MatchingNamespaceEnv, "false") }) - helpers.EventuallyResourceAccepted(func() (resources.InputResource, error) { - return testContext.TestClients().VirtualServiceClient.Read(writeNamespace, "good", clients.ReadOpts{}) + + It("Accepts all virtual services", func() { + helpers.EventuallyResourceAccepted(func() (resources.InputResource, error) { + vs, err := testContext.TestClients().VirtualServiceClient.Read(writeNamespace, "bad", clients.ReadOpts{}) + return vs, err + }) + helpers.EventuallyResourceAccepted(func() (resources.InputResource, error) { + return testContext.TestClients().VirtualServiceClient.Read(writeNamespace, "good", clients.ReadOpts{}) + }) }) + }) + Context("With matching enforced", func() { + + BeforeEach(func() { + os.Setenv(api_conversion.MatchingNamespaceEnv, "true") + }) + + It("rejects the virtual service where the secret is in another namespace and accepts virtual service with a matching namespace", func() { + helpers.EventuallyResourceRejected(func() (resources.InputResource, error) { + return testContext.TestClients().VirtualServiceClient.Read(writeNamespace, "bad", clients.ReadOpts{}) + }) + helpers.EventuallyResourceAccepted(func() (resources.InputResource, error) { + return testContext.TestClients().VirtualServiceClient.Read(writeNamespace, "good", clients.ReadOpts{}) + }) + }) + }) }) - Context("With matching enforced", func() { + + Context("Validates forbidden headers", func() { + var headerManipVsBuilder *helpers.VirtualServiceBuilder BeforeEach(func() { - os.Setenv(api_conversion.MatchingNamespaceEnv, "true") + headerManipVsBuilder = helpers.NewVirtualServiceBuilder(). + WithNamespace(writeNamespace). + WithRoutePrefixMatcher(e2e.DefaultRouteName, "/endpoint"). + WithRouteActionToUpstream(e2e.DefaultRouteName, testContext.TestUpstream().Upstream) + + allowedHeaderManipulationVS := headerManipVsBuilder.Clone(). + WithName("allowed-header-manipulation"). + WithDomain("another-domain.com"). + WithVirtualHostOptions( + &gloov1.VirtualHostOptions{HeaderManipulation: &headers.HeaderManipulation{ + RequestHeadersToAdd: []*envoycore_sk.HeaderValueOption{ + {HeaderOption: &envoycore_sk.HeaderValueOption_Header{ + Header: &envoycore_sk.HeaderValue{Key: "some-header", Value: "some-value"}}, + Append: &wrappers.BoolValue{Value: true}}}, + }}). + Build() + + forbiddenHeaderManipulationVS := headerManipVsBuilder.Clone(). + WithName("forbidden-header-manipulation"). + WithDomain("yet-another-domain.com"). + WithVirtualHostOptions( + &gloov1.VirtualHostOptions{HeaderManipulation: &headers.HeaderManipulation{ + RequestHeadersToAdd: []*envoycore_sk.HeaderValueOption{ + {HeaderOption: &envoycore_sk.HeaderValueOption_Header{ + Header: &envoycore_sk.HeaderValue{Key: ":path", Value: "some-value"}}, + Append: &wrappers.BoolValue{Value: true}}}, + }}). + Build() + + testContext.ResourcesToCreate().VirtualServices = v1.VirtualServiceList{allowedHeaderManipulationVS, forbiddenHeaderManipulationVS} }) - It("rejects the virtual service where the secret is in another namespace and accepts virtual service with a matching namespace", func() { - helpers.EventuallyResourceRejected(func() (resources.InputResource, error) { - return testContext.TestClients().VirtualServiceClient.Read(writeNamespace, "bad", clients.ReadOpts{}) - }) + It("Allows non forbidden headers", func() { helpers.EventuallyResourceAccepted(func() (resources.InputResource, error) { - return testContext.TestClients().VirtualServiceClient.Read(writeNamespace, "good", clients.ReadOpts{}) + vs, err := testContext.TestClients().VirtualServiceClient.Read(writeNamespace, "allowed-header-manipulation", clients.ReadOpts{}) + return vs, err }) }) + It("Does not allow forbidden headers", func() { + helpers.EventuallyResourceRejected(func() (resources.InputResource, error) { + vs, err := testContext.TestClients().VirtualServiceClient.Read(writeNamespace, "forbidden-header-manipulation", clients.ReadOpts{}) + return vs, err + }) + }) }) })