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

Mutation rules do not allow specific header modifications from external processor #29573

Closed
DimitarHKostov opened this issue Sep 12, 2023 · 7 comments
Labels
area/ext_proc question Questions that are neither investigations, bugs, nor enhancements stale stalebot believes this issue/PR has not been touched recently

Comments

@DimitarHKostov
Copy link

Hi Envoy team,

we found the following issue: using external processor(with GRPC) we want to be able to modify the 'host' header, internal 'x-envoy-* headers etc. The two properties that we are interested in are 'allow-envoy'(for the internal envoy headers) and 'allow-all-routing'. After changing the values of the two properties to true we still can not add/override the mentioned headers from the external processor. Here is a code snippet with the configurations we use for the external processor:

externalProcessingFilterConfig, err := anypb.New(&extProc.ExternalProcessor{
ProcessingMode: processingMode,
MessageTimeout: durationpb.New(DefaultTimeoutInSeconds * time.Second),
GrpcService: &grpcService,
MutationRules: &mutation_rulesv3.HeaderMutationRules{
AllowEnvoy: &wrapperspb.BoolValue{Value: true},
AllowAllRouting: &wrapperspb.BoolValue{Value: true},
},
})

In the external processor server in the HeaderResponse -> CommonResponse -> HeaderMutation -> SetHeaders it says in the documentation: "Add or replace HTTP headers. Attempts to set the value of any “x-envoy“ header, and attempts to set the “:method“, “:authority“, “:scheme“, or “host“ headers will be ignored.". It seems that this contradicts with the MutationRules for the external processing filter. Here is a snippet of the configuration:

headersResponse := &pb.HeadersResponse{
Response: &pb.CommonResponse{
HeaderMutation: &pb.HeaderMutation{
SetHeaders: headersToSet, //in the docu for this field it says we can not override the desired headers
RemoveHeaders: headersToRemove,
},
ClearRouteCache: true,
},
}
processingResponse := &pb.ProcessingResponse{
Response: &pb.ProcessingResponse_RequestHeaders{
RequestHeaders: headersResponse,
},
}

return processingResponse

Do we miss something? Any help will be highly appreciated.

@DimitarHKostov DimitarHKostov added the triage Issue requires triage label Sep 12, 2023
@adisuissa adisuissa added question Questions that are neither investigations, bugs, nor enhancements area/ext_proc and removed triage Issue requires triage labels Sep 12, 2023
@adisuissa
Copy link
Contributor

I think these types of headers are intentionally immutable.
cc @yanjunxiang-google @tyxia to confirm.

@yanjunxiang-google
Copy link
Contributor

yanjunxiang-google commented Sep 21, 2023

@DimitarHKostov
Do you have the envoy Ext_proc filter configured with allow_all_routing and allow_envoy?

google.protobuf.BoolValue allow_all_routing = 1;

@DimitarHKostov
Copy link
Author

Yes, both are set to true. Is there a known issue in any version of envoy when it comes to this properties?

@yanjunxiang-google
Copy link
Contributor

Let me try to reproduce the issue.

@yanjunxiang-google
Copy link
Contributor

yanjunxiang-google commented Sep 30, 2023

I created an test and it is ok to modify the host header and add x-envoy-xxx headers. Please check this PR:

#29886

Could you please let me know the ext_proc response you are sending back which contains the header mutation? I am thinking the header mutation in the ext_proc server response maybe not right.

BTW, if you want to modify the "host" header, you should just send back the "host" header with a new value, and leave the "append" to be false. That way the "host" header will be modified. You can not remove the "host" header, then add a new "host" header. "remove" operation is not allowed for "host" header.

zuercher pushed a commit that referenced this issue Oct 2, 2023
…9886)

Adding an ext_proc integration test to verify with allow_envoy and allow_all_routing enabled in the ext_proc filter configure, the host header can be modified and x-envoy-xxxx header can be added.

This is to address: #29573

Risk Level: low, test only
Testing: new unit test
Docs Changes: n/a
Release Notes: n/a
Signed-off-by: Yanjun Xiang <[email protected]>
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Oct 30, 2023
Copy link

github-actions bot commented Nov 6, 2023

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ext_proc question Questions that are neither investigations, bugs, nor enhancements stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

No branches or pull requests

3 participants