From 009db75bb74507b137bc991f9be2d3afd2f5d54d Mon Sep 17 00:00:00 2001 From: David Jumani Date: Thu, 27 Jul 2023 16:14:02 -0400 Subject: [PATCH 1/8] feat: Expose initial_metadata in GrpcHealthCheck --- ...xpose-initialmetadata-grpchealthcheck.yaml | 6 + .../api/v2/core/health_check.proto.sk.md | 2 + .../gloo/crds/gloo.solo.io_v1_Upstream.yaml | 22 ++++ pkg/utils/api_conversion/health_check.go | 14 +- .../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 | 107 ++++++++------- .../envoy/api/v2/core/health_check.pb.hash.go | 24 ++++ .../gloo/pkg/translator/translator_test.go | 123 ++++++++++++------ 9 files changed, 232 insertions(+), 88 deletions(-) create mode 100644 changelog/v1.15.0-beta23/expose-initialmetadata-grpchealthcheck.yaml diff --git a/changelog/v1.15.0-beta23/expose-initialmetadata-grpchealthcheck.yaml b/changelog/v1.15.0-beta23/expose-initialmetadata-grpchealthcheck.yaml new file mode 100644 index 00000000000..f185a3df338 --- /dev/null +++ b/changelog/v1.15.0-beta23/expose-initialmetadata-grpchealthcheck.yaml @@ -0,0 +1,6 @@ +changelog: +- type: NEW_FEATURE + 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 d71c9a81708..5c9901e1c1a 100644 --- a/install/helm/gloo/crds/gloo.solo.io_v1_Upstream.yaml +++ b/install/helm/gloo/crds/gloo.solo.io_v1_Upstream.yaml @@ -559,6 +559,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/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/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..d2606c1bd31 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 463a1793ced..2c98fa7d10b 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, 0xb8, 0xf5, 0x04, 0x01, 0xc0, 0xf5, 0x04, 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, 0x62, 0x06, 0x70, - 0x72, 0x6f, 0x74, 0x6f, 0x33, + 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, 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, 0xb8, 0xf5, 0x04, 0x01, 0xc0, + 0xf5, 0x04, 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, 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..0a3ec0ac8bb 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{}, }, }, }, @@ -1174,10 +1175,11 @@ var _ = Describe("Translator", func() { }) 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()) + 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{ @@ -1188,40 +1190,12 @@ var _ = Describe("Translator", func() { }, }, Metadata: &core.Metadata{ - Name: "foo", - Namespace: secretNamespace, + Name: "foo", }, }, } - 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", - }, - }, - 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 = []*envoy_config_core_v3.HeaderValueOption{ + expectedHeaders = []*envoy_config_core_v3.HeaderValueOption{ { Header: &envoy_config_core_v3.HeaderValue{ Key: "Authorization", @@ -1233,12 +1207,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 +1220,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,6 +1252,64 @@ var _ = Describe("Translator", func() { msgList = append(msgList, v) } Expect(cluster.HealthChecks).To(ConsistOfProtos(msgList...)) + } + + 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 don't match", "false", "bar", false), + Entry("Matching enforced and namespaces don't match", "true", "bar", true)) + + 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 enforced and namespaces match", "true", "gloo-system", false), Entry("Matching not enforced and namespaces match", "false", "gloo-system", false), From 5a76560585b646b7707987ae7e710fffb2e2d9c0 Mon Sep 17 00:00:00 2001 From: David Jumani Date: Fri, 28 Jul 2023 13:14:48 -0400 Subject: [PATCH 2/8] Trigger CI From 375bc6cf0da01a35af08f13170822893057b5aa6 Mon Sep 17 00:00:00 2001 From: David Jumani Date: Fri, 28 Jul 2023 13:44:43 -0400 Subject: [PATCH 3/8] Fix uneven whitespace Co-authored-by: Nathan Fudenberg --- projects/gloo/api/external/envoy/api/v2/core/health_check.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d2606c1bd31..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 @@ -182,7 +182,7 @@ message HealthCheck { 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. + // 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]; } From 6cec1569abe9a109af4069ed46f39e26e52ebd2f Mon Sep 17 00:00:00 2001 From: changelog-bot Date: Fri, 28 Jul 2023 18:43:23 +0000 Subject: [PATCH 4/8] Adding changelog file to new location --- .../expose-initialmetadata-grpchealthcheck.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/v1.15.0-beta24/expose-initialmetadata-grpchealthcheck.yaml diff --git a/changelog/v1.15.0-beta24/expose-initialmetadata-grpchealthcheck.yaml b/changelog/v1.15.0-beta24/expose-initialmetadata-grpchealthcheck.yaml new file mode 100644 index 00000000000..f185a3df338 --- /dev/null +++ b/changelog/v1.15.0-beta24/expose-initialmetadata-grpchealthcheck.yaml @@ -0,0 +1,6 @@ +changelog: +- type: NEW_FEATURE + 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. From e0a844743c81305f8720f64cce852552e0807990 Mon Sep 17 00:00:00 2001 From: David Jumani Date: Fri, 28 Jul 2023 15:17:20 -0400 Subject: [PATCH 5/8] Avoid code duplcation --- projects/gloo/pkg/translator/translator_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/projects/gloo/pkg/translator/translator_test.go b/projects/gloo/pkg/translator/translator_test.go index 0a3ec0ac8bb..011122693aa 100644 --- a/projects/gloo/pkg/translator/translator_test.go +++ b/projects/gloo/pkg/translator/translator_test.go @@ -1254,6 +1254,11 @@ var _ = Describe("Translator", func() { 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()) @@ -1286,8 +1291,6 @@ var _ = Describe("Translator", func() { }, 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 don't match", "false", "bar", false), - Entry("Matching enforced and namespaces don't match", "true", "bar", true)) DescribeTable("grpc health check", func(enforceMatch, secretNamespace string, expectError bool) { err := os.Setenv(api_conversion.MatchingNamespaceEnv, enforceMatch) @@ -1311,8 +1314,6 @@ var _ = Describe("Translator", func() { 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 don't match", "false", "bar", false), Entry("Matching enforced and namespaces don't match", "true", "bar", true)) }) From ae67c443dad280a9fd5c148dd214285f8e0d15c4 Mon Sep 17 00:00:00 2001 From: David Jumani Date: Fri, 28 Jul 2023 15:17:46 -0400 Subject: [PATCH 6/8] Removing duplicate changelog --- .../expose-initialmetadata-grpchealthcheck.yaml | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 changelog/v1.15.0-beta23/expose-initialmetadata-grpchealthcheck.yaml diff --git a/changelog/v1.15.0-beta23/expose-initialmetadata-grpchealthcheck.yaml b/changelog/v1.15.0-beta23/expose-initialmetadata-grpchealthcheck.yaml deleted file mode 100644 index f185a3df338..00000000000 --- a/changelog/v1.15.0-beta23/expose-initialmetadata-grpchealthcheck.yaml +++ /dev/null @@ -1,6 +0,0 @@ -changelog: -- type: NEW_FEATURE - 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. From 1b209b511f03417d7743d31f82e79f79de24fa45 Mon Sep 17 00:00:00 2001 From: David Jumani Date: Fri, 28 Jul 2023 15:45:20 -0400 Subject: [PATCH 7/8] fix test --- projects/gloo/pkg/translator/translator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/gloo/pkg/translator/translator_test.go b/projects/gloo/pkg/translator/translator_test.go index 011122693aa..67784ecbfa6 100644 --- a/projects/gloo/pkg/translator/translator_test.go +++ b/projects/gloo/pkg/translator/translator_test.go @@ -1290,7 +1290,7 @@ var _ = Describe("Translator", func() { 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) From 78932d2d3db94e8f42ee9d446379f64446b790e2 Mon Sep 17 00:00:00 2001 From: David Jumani Date: Mon, 31 Jul 2023 14:40:19 -0400 Subject: [PATCH 8/8] Add check for forbidden custom headers --- .../api_conversion_suite_test.go | 13 ++ pkg/utils/api_conversion/type.go | 14 ++ pkg/utils/api_conversion/type_test.go | 67 ++++++ .../gloo/pkg/translator/translator_test.go | 76 +++++++ test/e2e/headers_test.go | 203 +++++++++++------- 5 files changed, 301 insertions(+), 72 deletions(-) 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/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/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/pkg/translator/translator_test.go b/projects/gloo/pkg/translator/translator_test.go index 67784ecbfa6..3d6b5fb669c 100644 --- a/projects/gloo/pkg/translator/translator_test.go +++ b/projects/gloo/pkg/translator/translator_test.go @@ -1174,6 +1174,82 @@ var _ = Describe("Translator", func() { translateWithBuggyHasher() }) + Context("Healthcheck with Forbidden headers", func() { + var healthChecks []*gloo_envoy_core.HealthCheck + + BeforeEach(func() { + healthChecks = []*gloo_envoy_core.HealthCheck{ + { + 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, + }, + }, + }, + }, + } + _, errs, _ := translator.Translate(params, proxy) + + 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", + }, + }, + Append: &wrappers.BoolValue{ + Value: true, + }, + }, + }, + }, + } + _, errs, _ := translator.Translate(params, proxy) + + 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 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 + }) + }) }) })