Make %UPSTREAM_LOCAL_ADDRESS% access-log format work for HTTP requests. by ggreenway · Pull Request #9362 · envoyproxy/envoy

ggreenway · 2019-12-16T17:33:28Z

Make %UPSTREAM_LOCAL_ADDRESS% access-log format work for HTTP requests. It was previously only implemented for tcp-proxy connections.

Risk Level: Low
Testing: Unit and integration tests
Docs Changes: None required; it was never documented to not work
Release Notes: Added

Signed-off-by: Greg Greenway <ggreenway@apple.com>

asraa · 2019-12-16T18:03:08Z

Just a drive-by: would you be able to add coverage for this in our access_log_formatter_fuzz_test? That would involve

adding the field to the StreamInfo information we fuzz here:

envoy/test/fuzz/common.proto

Line 18 in 210acff

message StreamInfo {
setting it in the utility function that constructs the testStreamInfo with that value here:

envoy/test/fuzz/utility.h

Line 114 in 210acff

inline TestStreamInfo fromStreamInfo(const test::fuzz::StreamInfo& stream_info) {
adding a testcase with %UPSTREAM_LOCAL_ADDRESS% and a test value like this one https://github.com/envoyproxy/envoy/blob/master/test/common/access_log/access_log_formatter_corpus/response_code

Signed-off-by: Greg Greenway <ggreenway@apple.com>

ggreenway · 2019-12-16T22:08:55Z

Just a drive-by: would you be able to add coverage for this in our access_log_formatter_fuzz_test?

Good idea. I made an attempt at it.

Signed-off-by: Greg Greenway <ggreenway@apple.com>

asraa · 2019-12-17T13:43:24Z

Just a drive-by: would you be able to add coverage for this in our access_log_formatter_fuzz_test?

Good idea. I made an attempt at it.

Yay! Running the fuzz test as a bazel test target will run it against that corpus entry. Thank you so much!

ggreenway · 2019-12-18T21:39:43Z

/retest

repokitteh-read-only · 2019-12-18T21:39:46Z

🤷‍♀️ nothing to rebuild.

🐱

Caused by: a #9362 (comment) was created by @ggreenway.

see: more, trace.

Signed-off-by: Greg Greenway <ggreenway@apple.com>

asraa

Thank you!! LGTM

ggreenway · 2019-12-19T16:56:58Z

@asraa Any idea why CI isn't completing? I pushed an empty commit yesterday because envoy-linux was still waiting for status to be reported. Now many of the steps are stuck in that state.

asraa · 2019-12-20T14:02:57Z

/retest
/azp run

Hmmm. It might be that envoy-linux is stuck, will get back to you on why. Retesting doesn't work for me either

repokitteh-read-only · 2019-12-20T14:03:00Z

🤷‍♀️ nothing to rebuild.

🐱

Caused by: a #9362 (comment) was created by @asraa.

see: more, trace.

mattklein123 · 2019-12-20T16:26:40Z

/azp run envoy-linux

azure-pipelines · 2019-12-20T16:26:49Z

Azure Pipelines successfully started running 1 pipeline(s).

ggreenway · 2019-12-20T16:46:04Z

/retest

==> Downloading https://github.com/bazelbuild/bazelisk/releases/download/v1.2.0/bazelisk-darwin-amd64
==> Downloading from https://github-production-release-asset-2e65be.s3.amazonaws.com/149661467/54792900-234a-11ea-8134-faf6a597f7d5?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20191220%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20191220T162857Z&X-Amz-Expires=300&X-Amz-Signature=3f10de6b240b718bde337439c5a3e5c165059b77636d553702cc5ca0bc6f8dd6&X-Amz-SignedHeaders=host&actor_id=0&response-content-disposition=attachment%3B%20filename%3Dbazelisk-darwin-amd64&response-content-type=application%2Foctet-stream
Error: An exception occurred within a child process:
  ChecksumMismatchError: SHA256 mismatch
Expected: 6bfca2304cc2258578658d95a6e8dbe43d3ae5dd089076128c726cf14b8c2b8f
  Actual: c9764d559b044a69ec860235d46c5cc7d28e98cb378dd449b7c9fe53f696aa07
 Archive: /Users/runner/Library/Caches/Homebrew/downloads/49bcf3ae6f2484900431af3c21d7d5f7d324dc128ac8be370cbaae117ffa0b4c--bazelisk-darwin-amd64
To retry an incomplete download, remove the file above.

Any other macOS builds seeing this error?

repokitteh-read-only · 2019-12-20T16:46:08Z

🤷‍♀️ nothing to rebuild.

🐱

Caused by: a #9362 (comment) was created by @ggreenway.

see: more, trace.

…dress

ggreenway · 2019-12-20T17:07:59Z

According to https://github.com/bazelbuild/bazelisk/releases/tag/v1.2.0, this version was released 1 hour ago; I think they re-released it, which changed the hash.

* master: (167 commits) stats: Avoid asserts in fuzz-tests by eliminating arbitrary length limits, using the new MemBlock wrapper for memcpy (envoyproxy#8779) Make %UPSTREAM_LOCAL_ADDRESS% access-log format work for HTTP requests. (envoyproxy#9362) tools: API boosting support for using decls and enum constants. (envoyproxy#9418) Fix incorrect cluster InitializePhase type (envoyproxy#9379) build: fix merge race between envoyproxy#9241 and envoyproxy#9413. (envoyproxy#9427) fuzz: fix incorrect evaluator test (envoyproxy#9402) server: fix bogus startup log message (envoyproxy#9404) tools: Add protoxform tests (envoyproxy#9241) api: options after import (envoyproxy#9413) misc: use std::move instead of constructing a copy (envoyproxy#9415) tools: API boosting support for rewriting elaborated types. (envoyproxy#9375) docs: fix invalid transport_socket value (envoyproxy#9403) fix typo in docs (envoyproxy#9394) srds: remove to-de-removed scopes first and then apply additions to avoid scope key conflict. (envoyproxy#9366) api: generate whole directory and sync (envoyproxy#9382) bazel: Add load statements for proto_library (envoyproxy#9367) Fix typo (envoyproxy#9388) Correct test of OptionsImpl argc type (Was: Correct type for std::array size() result) (envoyproxy#9290) http1 encode trailers in chunk encoding (envoyproxy#8667) Add mode to PipeInstance (envoyproxy#8423) ...

…s. (envoyproxy#9362) Signed-off-by: Greg Greenway <ggreenway@apple.com> Signed-off-by: Prakhar <prakhar_au@yahoo.com>

Make %UPSTREAM_LOCAL_ADDRESS% access-log format work for HTTP requests.

c644834

Signed-off-by: Greg Greenway <ggreenway@apple.com>

ggreenway requested review from alyssawilk and mattklein123 as code owners December 16, 2019 17:33

Add fuzz test case

1fe8b17

Signed-off-by: Greg Greenway <ggreenway@apple.com>

fix_format

f043bd0

Signed-off-by: Greg Greenway <ggreenway@apple.com>

jmarantz assigned asraa Dec 18, 2019

empty commit to restart CI that is stuck

55ce16b

Signed-off-by: Greg Greenway <ggreenway@apple.com>

asraa approved these changes Dec 19, 2019

View reviewed changes

mattklein123 closed this Dec 20, 2019

mattklein123 reopened this Dec 20, 2019

mattklein123 self-assigned this Dec 20, 2019

Merge remote-tracking branch 'upstream/master' into upstream-local-ad…

8d2c1d7

…dress

mattklein123 merged commit 9ad469d into envoyproxy:master Dec 20, 2019

prakhag1 pushed a commit to prakhag1/envoy that referenced this pull request Jan 3, 2020

Make %UPSTREAM_LOCAL_ADDRESS% access-log format work for HTTP request…

c14f700

…s. (envoyproxy#9362) Signed-off-by: Greg Greenway <ggreenway@apple.com> Signed-off-by: Prakhar <prakhar_au@yahoo.com>

Conversation

ggreenway commented Dec 16, 2019

Uh oh!

asraa commented Dec 16, 2019

Uh oh!

ggreenway commented Dec 16, 2019

Uh oh!

asraa commented Dec 17, 2019

Uh oh!

ggreenway commented Dec 18, 2019

Uh oh!

repokitteh-read-only bot commented Dec 18, 2019

Uh oh!

asraa left a comment

Choose a reason for hiding this comment

Uh oh!

ggreenway commented Dec 19, 2019

Uh oh!

asraa commented Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Dec 20, 2019

Uh oh!

mattklein123 commented Dec 20, 2019

Uh oh!

azure-pipelines bot commented Dec 20, 2019

Uh oh!

ggreenway commented Dec 20, 2019

Uh oh!

repokitteh-read-only bot commented Dec 20, 2019

Uh oh!

ggreenway commented Dec 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

asraa commented Dec 20, 2019 •

edited

Loading