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.

Sign up for GitHub

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

Jump to bottom

lua: Expose stream info downstreamLocalAddress and downstreamDirectRemoteAddress for Lua filter #13536

Merged
merged 14 commits into from
Oct 20, 2020
Merged

lua: Expose stream info downstreamLocalAddress and downstreamDirectRemoteAddress for Lua filter #13536

merged 14 commits into from
Oct 20, 2020

Conversation

nic-chen
Copy link
Contributor

@nic-chen nic-chen commented Oct 13, 2020 •
edited by dio
Loading

Commit Message:
Client IP should be very commonly used and helpful for users.
Expose downstreamLocalAddress and downstreamDirectRemoteAddress help us get client IP in Lua filters.

Risk Level: Low
Testing: Modify unit test
Docs Changes: updated Lua filter doc
Release Notes: N/A
Fixes #13457

Signed-off-by: nic-chen johz@163.com

@nic-chen nic-chen force-pushed the lua-streaminfo-addresses branch from 3ac10d1 to 76e2050 Compare October 13, 2020 10:22
dio
dio suggested changes Oct 13, 2020
View reviewed changes
Copy link
Member

@dio dio left a comment

Choose a reason for hiding this comment

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

Thanks, looks good.

I think you need to add an entry to docs/root/version_history/current.rst telling that we are adding these two new APIs as "New Features" (there is a couple of examples of Lua changes entries in, e.g. docs/root/version_history/v1.8.0.rst)

test/extensions/filters/http/lua/wrappers_test.cc Outdated Show resolved Hide resolved
@nic-chen nic-chen force-pushed the lua-streaminfo-addresses branch 3 times, most recently from 97101be to 3a6400e Compare October 15, 2020 01:44
@dio
Copy link
Member

dio commented Oct 16, 2020 •
edited
Loading

@nic-chen, sorry, after applying my suggestion here: #13536 (comment) could you merge main? 🙏🏽

@nic-chen nic-chen force-pushed the lua-streaminfo-addresses branch from 3460ce4 to 2bb0682 Compare October 16, 2020 06:21
@johzchen
lua: Expose stream info downstreamLocalAddress and downstreamDirectRe…
951ad1b 
…moteAddress for Lua filter

Signed-off-by: nic-chen <johz@163.com>
@johzchen
doc: add version history for changes
496f885 
Signed-off-by: nic-chen <johz@163.com>
@johzchen
fix: merge test cases for downstreamLocalAddress and `downstreamDir…
84b8a8b 
…ectRemoteAddress` into one

Signed-off-by: nic-chen <johz@163.com>
@johzchen
fix format
d4e8d42 
Signed-off-by: nic-chen <johz@163.com>
@johzchen
fix doc style
668ab42 
Signed-off-by: nic-chen <johz@163.com>
@johzchen
fix test cases
cb0a739 
Signed-off-by: nic-chen <johz@163.com>
@nic-chen
Copy link
Contributor Author

@nic-chen, sorry, after applying my suggestion here: #13536 (comment) could you merge main? 🙏🏽

done.

@nic-chen nic-chen force-pushed the lua-streaminfo-addresses branch from 6e9f90e to dfe3168 Compare October 16, 2020 07:05
@johzchen
fix test case fail
bf4249c 
Signed-off-by: nic-chen <johz@163.com>
@johzchen
fix: doc error
b9d34e7 
Signed-off-by: nic-chen <johz@163.com>
@nic-chen nic-chen force-pushed the lua-streaminfo-addresses branch from dfe3168 to b9d34e7 Compare October 16, 2020 07:15
@dio
Copy link
Member

dio commented Oct 16, 2020

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines, to retry CircleCI checks, use /retest-circle.

🐱

Caused by: a #13536 (comment) was created by @dio.

see: more, trace.

@dio
Copy link
Member

dio commented Oct 16, 2020 •
edited
Loading

@nic-chen I think you can do merge master instead of rebasing your branch with the master (so you don't have to do force-pushing).

I can see from here (#13536 (comment)):

Retrying Azure Pipelines, to retry CircleCI checks, use /retest-circle.

You haven't merged the master branch into yours, since we have removed /retest-circle.

@nic-chen
Copy link
Contributor Author

@dio
OK. thanks for tips.
will pay attention next time.
do I need to do something now ?

dio
dio previously approved these changes Oct 16, 2020
View reviewed changes
Copy link
Member

@dio dio left a comment

Choose a reason for hiding this comment

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

Thanks!

dio
dio approved these changes Oct 17, 2020
View reviewed changes
Copy link
Member

@dio dio left a comment

Choose a reason for hiding this comment

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

Thanks!

@mattklein123 mattklein123 merged commit 108472e into envoyproxy:master Oct 20, 2020
mpuncel added a commit to mpuncel/envoy that referenced this pull request Oct 21, 2020
@mpuncel
Merge branch 'master' into mpuncel/http2-hc-goaway
cd06c28 
* master: (22 commits)
  ci: various improvements (envoyproxy#13660)
  dns: fix defunct fd bug in apple resolver (envoyproxy#13641)
  build: support ppc64le with wasm (envoyproxy#13657)
  [fuzz] Added random load balancer fuzz (envoyproxy#13400)
  dependencies: compute and check release dates via GitHub API. (envoyproxy#13582)
  mac ci: try ignoring update failure (envoyproxy#13658)
  watchdog: Optimize WatchdogImpl::touch in preparation to more frequent petting of the watchdog. (envoyproxy#13103)
  typos: fix a couple 'enovy' mispellings (envoyproxy#13645)
  lua: Expose stream info downstreamLocalAddress and downstreamDirectRemoteAddress for Lua filter (envoyproxy#13536)
  tap: fix upstream streamed transport socket taps (envoyproxy#13638)
  Revert "delay health checks until transport socket secrets are ready. (envoyproxy#13516)" (envoyproxy#13639)
  Watchdog: use abort action as a default if killing is enabled. (envoyproxy#13523)
  [fuzz] Fixed divide by zero bug (envoyproxy#13545)
  wasm: flip the meaning of the "repository" in envoy_wasm_cc_binary(). (envoyproxy#13621)
  fix: record recovered local address (envoyproxy#13581)
  docs: fix incorrect compressor filter doc (envoyproxy#13611)
  docs: clean up docs for azp migration (envoyproxy#13558)
  wasm: fix building Wasm example. (envoyproxy#13619)
  test: Refactor flood tests into a separate test file (envoyproxy#13556)
  wasm: re-enable tests with precompiled modules. (envoyproxy#13583)
  ...

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
@kenjenkins kenjenkins mentioned this pull request Sep 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@dio dio dio approved these changes

Assignees

@dio dio

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

question: How to get client IP and custom headers in Lua filter
4 participants
@nic-chen @dio @mattklein123 @johzchen