Implement AutoHostRewrite functionality in request handling #94

Eeshu-Yadav · 2025-09-11T22:41:23Z

fixes : #93

gemini-code-assist

Summary of Changes

Hello @Eeshu-Yadav, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the request handling capabilities by introducing a flexible mechanism for rewriting the authority (host and port) of incoming HTTP requests. This feature allows for dynamic adjustment of the Host header and URI authority based on predefined rules, including an automatic option to match the upstream service, thereby improving routing accuracy and compatibility with various backend configurations.

Highlights

New AutoHostRewrite Functionality: Introduced a new AutoHostRewrite option within the AuthorityRewriteSpecifier to automatically set the request's authority to the upstream service's authority.
Enhanced Authority Rewriting Logic: Implemented a comprehensive apply method for AuthorityRewriteSpecifier that supports rewriting the authority based on a specific value, a request header, or a regular expression, in addition to the new auto-host rewrite.
Integration into Request Handling: Modified the HTTP connection manager's request handling to apply the configured authority rewrite rules, ensuring that the URI's authority and the Host header are correctly updated before forwarding the request upstream.
New Unit Tests: Added dedicated unit tests to validate the correct behavior of the new AuthorityRewriteSpecifier and its various rewrite options, including AutoHostRewrite.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces AutoHostRewrite functionality for request authority rewriting. The implementation is sound and includes good test coverage. My review focuses on a performance improvement to avoid cloning request parts on the hot path, and a minor readability enhancement. The changes are well-contained and address the intended feature.

gemini-code-assist · 2025-09-11T22:43:10Z

orion-configuration/src/config/network_filters/http_connection_manager/route.rs

+    pub fn apply<B>(
+        &self,
+        request: &Request<B>,
+        upstream_authority: &Authority,
+    ) -> Result<Option<Authority>, InvalidUri> {
+        match self {
+            AuthorityRewriteSpecifier::Authority(authority) => Ok(Some(authority.clone())),
+
+            AuthorityRewriteSpecifier::Header(header_name) => {
+                if let Some(header_value) = request.headers().get(header_name.as_str()) {
+                    if let Ok(header_str) = header_value.to_str() {
+                        Authority::from_str(header_str)
+                            .map(Some)
+                            .map_err(|e| e.into())
+                    } else {
+                        Ok(None)
+                    }
+                } else {
+                    Ok(None)
+                }
+            },
+
+            AuthorityRewriteSpecifier::Regex(regex) => {
+                let current_authority = request.uri().authority()
+                    .map(Authority::as_str)
+                    .or_else(|| {
+                        request.headers().get(http::header::HOST)
+                            .and_then(|h| h.to_str().ok())
+                    })
+                    .unwrap_or_default();
+
+                let replacement = regex.pattern.replace_all(current_authority, regex.substitution.as_str());
+                if let std::borrow::Cow::Borrowed(_) = replacement {
+                    Ok(None)
+                } else {
+                    Authority::from_str(&replacement)
+                        .map(Some)
+                        .map_err(|e| e.into())
+                }
+            },
+
+            AuthorityRewriteSpecifier::AutoHostRewrite => Ok(Some(upstream_authority.clone())),
+        }
+    }
+}


The apply method currently takes a &Request<B>, which necessitates an expensive clone of http::request::Parts at the call site in orion-lib. To improve performance, this method can be refactored to accept only the parts it needs: the URI and headers.

This change will also require updating the tests for this function to pass the URI and headers instead of a full request.

I've also taken the opportunity to simplify the logic in the Header match arm for better readability.

pub fn apply( &self, uri: &http::Uri, headers: &http::HeaderMap, upstream_authority: &Authority, ) -> Result<Option<Authority>, InvalidUri> { match self { AuthorityRewriteSpecifier::Authority(authority) => Ok(Some(authority.clone())), AuthorityRewriteSpecifier::Header(header_name) => { let Some(header_value) = headers.get(header_name.as_str()) else { return Ok(None); }; let Ok(header_str) = header_value.to_str() else { return Ok(None); }; Authority::from_str(header_str) .map(Some) .map_err(|e| e.into()) }, AuthorityRewriteSpecifier::Regex(regex) => { let current_authority = uri.authority() .map(Authority::as_str) .or_else(|| { headers.get(http::header::HOST) .and_then(|h| h.to_str().ok()) }) .unwrap_or_default(); let replacement = regex.pattern.replace_all(current_authority, regex.substitution.as_str()); if let std::borrow::Cow::Borrowed(_) = replacement { Ok(None) } else { Authority::from_str(&replacement) .map(Some) .map_err(|e| e.into()) } }, AuthorityRewriteSpecifier::AutoHostRewrite => Ok(Some(upstream_authority.clone())), } } }

gemini-code-assist · 2025-09-11T22:43:11Z

orion-lib/src/listeners/http_connection_manager/route.rs

+                    let authority_replacement = if let Some(authority_rewrite) = &self.authority_rewrite {
+                        let original_request = Request::from_parts(parts.clone(), ());
+                        authority_rewrite
+                            .apply(&original_request, &svc_channel.upstream_authority)
+                            .with_context_msg("invalid authority after rewrite")?
+                    } else {
+                        None
+                    };


Cloning http::request::Parts via parts.clone() on every request with an authority rewrite can be a performance bottleneck, as it involves cloning the entire set of headers. This is done to construct a temporary Request object for AuthorityRewriteSpecifier::apply.

To improve performance, you can refactor AuthorityRewriteSpecifier::apply to accept &http::Uri and &http::HeaderMap instead of &Request<B>, as it only needs access to these parts. This avoids the expensive clone.

I've added a suggestion on orion-configuration/src/config/network_filters/http_connection_manager/route.rs to refactor apply. With that change, you can update this block to avoid the clone.

let authority_replacement = if let Some(authority_rewrite) = &self.authority_rewrite { authority_rewrite .apply(&parts.uri, &parts.headers, &svc_channel.upstream_authority) .with_context_msg("invalid authority after rewrite")? } else { None };

This commit implements the missing AutoHostRewrite functionality that was configured but not actually used during request processing. The implementation adds support for all AuthorityRewriteSpecifier variants: - AutoHostRewrite: Uses upstream cluster's authority - Authority: Uses a specific authority - Header: Uses value from a request header - Regex: Applies regex transformation to current authority Both the request URI authority and Host header are updated when authority rewriting is applied, ensuring consistent behavior across the request. Fixes the issue mentioned in PR kmesh-net#86 review where autorewrite configuration was parsed but not executed during request handling. Signed-off-by: Eeshu-Yadav <eeshuyadav123@gmail.com>

YaoZengzeng · 2025-09-16T12:08:58Z

orion-configuration/src/config/network_filters/http_connection_manager/route.rs

+
+        let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap();
+        assert_eq!(result, Some(Authority::from_str("api.rewritten.com").unwrap()));
+    }


Add a error case that we don't host in headers.

The rest is LGTM

This commit adds a test case to cover the edge case where there's no Host header in the request headers. The test verifies that the regex-based authority rewrite handles gracefully when both the URI has no authority and there's no Host header in the headers, ensuring robust behavior in all scenarios. Test: test_authority_rewrite_regex_no_host_header Signed-off-by: Eeshu-Yadav <eeshuyadav123@gmail.com>

Copilot

Pull Request Overview

This PR implements AutoHostRewrite functionality in request handling, allowing automatic rewriting of the authority (host) portion of HTTP requests to match upstream targets.

Adds authority rewriting capability with support for multiple rewrite strategies including AutoHostRewrite
Modifies request handling to apply both path and authority rewrites when processing HTTP requests
Updates Host header to reflect authority changes when authority rewriting occurs

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
orion-lib/src/listeners/http_connection_manager/route.rs	Integrates authority rewriting into request processing pipeline
orion-configuration/src/config/network_filters/http_connection_manager/route.rs	Implements AuthorityRewriteSpecifier with apply method and comprehensive test coverage

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-17T03:22:49Z

orion-lib/src/listeners/http_connection_manager/route.rs

+                    if path_and_query_replacement.is_some() || authority_replacement.is_some() {
                        parts.uri = {
-                            let UriParts { scheme, authority, path_and_query: _, .. } = parts.uri.into_parts();
+                            let UriParts { scheme, authority, path_and_query, .. } = parts.uri.into_parts();


The destructuring pattern now ignores the original path_and_query value but still extracts it. Since path_and_query is used in the fallback logic on line 115, removing the underscore suffix makes the code clearer about its intended use.

Copilot · 2025-09-17T03:22:50Z

orion-configuration/src/config/network_filters/http_connection_manager/route.rs

+                let Ok(header_str) = header_value.to_str() else {
+                    return Ok(None);
+                };
+                Authority::from_str(header_str).map(Some).map_err(|e| e.into())


The error conversion |e| e.into() is redundant. The ? operator can handle the conversion automatically: Authority::from_str(header_str).map(Some)

Suggested change

Authority::from_str(header_str).map(Some).map_err(|e| e.into())

Authority::from_str(header_str).map(Some)

Copilot · 2025-09-17T03:22:50Z

orion-configuration/src/config/network_filters/http_connection_manager/route.rs

+                if let std::borrow::Cow::Borrowed(_) = replacement {
+                    Ok(None)
+                } else {
+                    Authority::from_str(&replacement).map(Some).map_err(|e| e.into())


The error conversion |e| e.into() is redundant. The ? operator can handle the conversion automatically: Authority::from_str(&replacement).map(Some)

Suggested change

Authority::from_str(&replacement).map(Some).map_err(|e| e.into())

Authority::from_str(&replacement).map(Some)

hzxuzhonghu

Overall LGTM

hzxuzhonghu · 2025-09-17T03:40:35Z

orion-configuration/src/config/network_filters/http_connection_manager/route.rs

+        uri: &http::Uri,
+        headers: &http::HeaderMap,
+        upstream_authority: &Authority,
+    ) -> Result<Option<Authority>, InvalidUri> {


nit: InvalidUri seems redudant

- Remove unnecessary .map_err(|e| e.into()) calls - The ? operator handles error conversion automatically for Authority::from_str - Remove redundant InvalidUri error type Signed-off-by: Eeshu-Yadav <eeshuyadav123@gmail.com>

hzxuzhonghu

/lgtm

kmesh-bot · 2025-09-22T06:21:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [hzxuzhonghu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kmesh-bot requested review from dawid-nowak and hzxuzhonghu September 11, 2025 22:41

gemini-code-assist bot reviewed Sep 11, 2025

View reviewed changes

kmesh-bot added the size/L label Sep 11, 2025

gemini-code-assist bot reviewed Sep 11, 2025

View reviewed changes

Eeshu-Yadav force-pushed the fix/implement-auto-host-rewrite branch from f71c2e3 to 5ad97fc Compare September 11, 2025 22:50

Eeshu-Yadav force-pushed the fix/implement-auto-host-rewrite branch from 5ad97fc to 9121161 Compare September 11, 2025 22:53

YaoZengzeng reviewed Sep 16, 2025

View reviewed changes

kmesh-bot added the do-not-merge/invalid-commit-message label Sep 16, 2025

Eeshu-Yadav force-pushed the fix/implement-auto-host-rewrite branch from 096ca70 to eed95b8 Compare September 16, 2025 14:49

kmesh-bot removed the do-not-merge/invalid-commit-message label Sep 16, 2025

hzxuzhonghu requested a review from Copilot September 17, 2025 03:22

Copilot AI reviewed Sep 17, 2025

View reviewed changes

hzxuzhonghu approved these changes Sep 17, 2025

View reviewed changes

kmesh-bot added the approved label Sep 17, 2025

hzxuzhonghu reviewed Sep 17, 2025

View reviewed changes

Address review comments: remove redundant error conversions

2b301f6

- Remove unnecessary .map_err(|e| e.into()) calls - The ? operator handles error conversion automatically for Authority::from_str - Remove redundant InvalidUri error type Signed-off-by: Eeshu-Yadav <eeshuyadav123@gmail.com>

hzxuzhonghu approved these changes Sep 22, 2025

View reviewed changes

kmesh-bot assigned hzxuzhonghu Sep 22, 2025

kmesh-bot added the lgtm label Sep 22, 2025

kmesh-bot merged commit 3e9820e into kmesh-net:main Sep 22, 2025
4 checks passed

	Authority::from_str(header_str).map(Some).map_err(\|e\| e.into())
	Authority::from_str(header_str).map(Some)

	Authority::from_str(&replacement).map(Some).map_err(\|e\| e.into())
	Authority::from_str(&replacement).map(Some)

Implement AutoHostRewrite functionality in request handling #94

Implement AutoHostRewrite functionality in request handling #94

Uh oh!

Conversation

Eeshu-Yadav commented Sep 11, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

YaoZengzeng Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

YaoZengzeng Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Eeshu-Yadav Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu left a comment

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Eeshu-Yadav Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu left a comment

Choose a reason for hiding this comment

Uh oh!

kmesh-bot commented Sep 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants