Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter state: Add missing api to get filterstate format working #35559

Merged
merged 6 commits into from
Aug 30, 2024

Conversation

vikaschoudhary16
Copy link
Contributor

@vikaschoudhary16 vikaschoudhary16 commented Aug 2, 2024

Commit Message: Filter state: Add missing api to get filterstate format working
Additional Description:
Consider the case where filter state is being set using the set_filter_state filter:

- name: "envoy.filters.http.set_filter_state"
            typed_config:
              "@type": "type.googleapis.com/envoy.extensions.filters.http.set_filter_state.v3.Config"
              on_request_headers:
              - object_key: "envoy.network.upstream_server_name"
                format_string:
                  text_format_source:
                    inline_string: "%REQ(:AUTHORITY)%"
                shared_with_upstream: "TRANSITIVE"

Now if one try to use the value from the filter state, for example in the tunneling config hostname:

- name: encap
    internal_listener: {}
    filter_chains:
    - filters:
      - name: tcp
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
          stat_prefix: tcp_stats
          cluster: cluster_0
          tunneling_config:
            hostname: "%FILTER_STATE(envoy.network.upstream_server_name:PLAIN)%"

Then filter state formatter fails to find the value because here, it invokes serializeAsString() on the stored filter state object, which is UpstreamServerName instance in this case and it is missing this function.

I think similar change would be required for other well-known name objects as well like Upstream subject alt names and alpn application protocols.

related #30674

@vikaschoudhary16
Copy link
Contributor Author

ping @kyessenov to validate my understanding

kyessenov
kyessenov previously approved these changes Aug 14, 2024
Copy link
Contributor

@kyessenov kyessenov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this makes sense. Not sure why serializeAsString was not implemented.

Signed-off-by: Vikas Choudhary <[email protected]>
Signed-off-by: Vikas Choudhary <[email protected]>
Signed-off-by: Vikas Choudhary (vikasc) <[email protected]>
Signed-off-by: Vikas Choudhary (vikasc) <[email protected]>
Signed-off-by: Vikas Choudhary (vikasc) <[email protected]>
@kyessenov kyessenov merged commit b4d6c9a into envoyproxy:main Aug 30, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants