api icon indicating copy to clipboard operation
api copied to clipboard

add http header casing option to mesh config

Open su225 opened this issue 3 years ago • 8 comments

Part of https://github.com/istio/istio/issues/32008

Signed-off-by: su225 [email protected]

su225 avatar Apr 22 '21 03:04 su225

😊 Welcome @su225! This is either your first contribution to the Istio api repo, or it's been awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

istio-policy-bot avatar Apr 22 '21 03:04 istio-policy-bot

@su225: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
release-notes_api bf4e2b9d16903512cb45dc8d16e44c7cc83fdb43 link /test release-notes_api

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

istio-testing avatar Apr 22 '21 03:04 istio-testing

I think there are two questions we need to answer here - should this be configurable and should this be default on? This has a lot of similarities to https://istio.io/latest/blog/2021/upcoming-networking-changes/

  • For new users, it seems better it all cases to preserve the case. I don't see a need to allow lowercasing it always
  • For existing users, they might have come to rely on the lowercasing. It seems a bit odd, as most apps probably assume title casing if anything, so hopefully when they adopt Istio they fix to being case insensitive instead of assuming lowercase, but I am sure in practice it may break users

howardjohn avatar Apr 27 '21 22:04 howardjohn

I think it is important to have this configurable. Unfortunately, something like istioctl precheck as mentioned in the blogpost is not applicable here as apps are blackboxes from Istio perspective. If we are turning it on by default, then it is important to notify existing users.

Otherwise, not necessary. This way, if someone is migrating their legacy apps and rely on header casing behavior then they can turn this on. Apps which don't care, don't break anyways.

su225 avatar May 05 '21 14:05 su225

How does this impact Authorization and VirtualService routing policies? Will it break existing user config if casing of header keys is preserved?

nrjpoddar avatar May 12 '21 07:05 nrjpoddar

@su225: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

istio-testing avatar May 12 '21 18:05 istio-testing

Apparently, envoy can route if casing is not correct. Here is a small experiment I did to test that.

docker-compose.yaml

version: '3'
services:
  envoy:
    image: envoyproxy/envoy-dev:ae780e21824bcb947299f5a048797fbf3cac21ba
    ports:
      - "10000:10000"
    volumes:
      - ./envoy.yaml:/etc/envoy/envoy.yaml
  app1:
    image: gcr.io/google-samples/hello-app:1.0
    ports:
      - "8080:8080"
  app2:
    image: gcr.io/google-samples/hello-app:2.0
    ports:
      - "8090:8080"

envoy.yaml

static_resources:
  listeners:
    - address:
        socket_address:
          address: 0.0.0.0
          port_value: 10000
      filter_chains:
        - filters: 
          - name: envoy.filters.http_connection_manager
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
              stat_prefix: ingress_http

              # HTTP header casing options
              http_protocol_options:
                header_key_format:
                  stateful_formatter:
                    name: preserve_case
                    typed_config:
                      "@type": type.googleapis.com/envoy.extensions.http.header_formatters.preserve_case.v3.PreserveCaseFormatterConfig

              http_filters:
                - name: envoy.filters.http.router
              route_config:
                virtual_hosts:
                  - name: default
                    domains: ["*"]
                    routes:
                      - match:
                          prefix: "/"
                          headers:
                            name: X-App-Version
                            exact_match: v2
                        route: { cluster: app_v2 }
                      - match: { prefix: "/" }
                        route: { cluster: app_v1 }
  clusters:
    - name: app_v1
      type: STRICT_DNS
      connect_timeout: 1s
      
      # HTTP header casing options
      typed_extension_protocol_options:
        envoy.extensions.upstreams.http.v3.HttpProtocolOptions:
          "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions
          explicit_http_config:
            http_protocol_options:
              header_key_format:
                stateful_formatter:
                  name: preserve_case
                  typed_config:
                    "@type": type.googleapis.com/envoy.extensions.http.header_formatters.preserve_case.v3.PreserveCaseFormatterConfig
      
      load_assignment:
        cluster_name: app_v1
        endpoints:
          - lb_endpoints:
            - endpoint:
                address:
                  socket_address:
                    address: app1
                    port_value: 8080
    - name: app_v2
      type: STRICT_DNS
      connect_timeout: 1s
      
      # HTTP header casing options
      typed_extension_protocol_options:
        envoy.extensions.upstreams.http.v3.HttpProtocolOptions:
          "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions
          explicit_http_config:
            http_protocol_options:
              header_key_format:
                stateful_formatter:
                  name: preserve_case
                  typed_config:
                    "@type": type.googleapis.com/envoy.extensions.http.header_formatters.preserve_case.v3.PreserveCaseFormatterConfig
      
      load_assignment:
        cluster_name: app_v2
        endpoints:
          - lb_endpoints:
            - endpoint:
                address:
                  socket_address:
                    address: app2
                    port_value: 8080

At least the routing part is fine.

su225 avatar May 16 '21 15:05 su225

I agree with @howardjohn here. I think we should preserve casing for http1.1. by default. I think we may not need an API at mesh config. If at all some customers need other behaviour - we can probably do it vin env var which can be easily removed later.

This seems equivalent to PILOT_HTTP10 to me.

ramaraochavali avatar May 21 '21 08:05 ramaraochavali

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2021-05-21. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.

istio-policy-bot avatar May 15 '24 23:05 istio-policy-bot