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

Inject requestHeaderModifier to LuaData #223

Merged

Conversation

lujiajing1126
Copy link
Contributor

@lujiajing1126 lujiajing1126 commented Jun 27, 2024

Ⅰ. Describe what this PR does

In TrafficRoutingStrategy struct, RequestHeaderModifier is allowed to be used for request header modification.
But this field was not actually injected to the LuaData. Thus it cannot be used in the script.

Istio also supports HeaderOperations,

https://istio.io/latest/docs/reference/config/networking/virtual-service/#Headers-HeaderOperations

Ⅱ. Does this pull request fix one issue?

Ⅲ. Special notes for reviews

@kruise-bot kruise-bot requested review from FillZpp and veophi June 27, 2024 02:59
@lujiajing1126
Copy link
Contributor Author

PTAL @furykerry

Copy link
Member

@furykerry furykerry left a comment

Choose a reason for hiding this comment

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

/lgtm

@furykerry
Copy link
Member

@zmberg PTAL

@furykerry
Copy link
Member

@lujiajing1126 please squash the commits into one

* fix lua script and test case
* use ptr instead of struct
* add requestHeaderModifier to testcase debugging toolkit

Signed-off-by: Megrez Lu <[email protected]>
@lujiajing1126 lujiajing1126 force-pushed the inject-request-header-modifier branch from 008562a to 34a1848 Compare July 22, 2024 02:00
@kruise-bot kruise-bot removed the lgtm label Jul 22, 2024
@lujiajing1126
Copy link
Contributor Author

@lujiajing1126 please squash the commits into one

Squashed. But I suppose maintainers can always merge and squash while merging the Pull Request on GitHub

@zmberg
Copy link
Member

zmberg commented Jul 22, 2024

/lgtm
/approve

@kruise-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zmberg

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:

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

@kruise-bot kruise-bot merged commit 6fae708 into openkruise:master Jul 22, 2024
21 checks passed
@lujiajing1126 lujiajing1126 deleted the inject-request-header-modifier branch August 16, 2024 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants