Skip to content

mysql proxy: connection attributes parsing #17209

Merged
mattklein123 merged 4 commits intoenvoyproxy:mainfrom
qinggniq:conn-attribute
Jul 28, 2021
Merged

mysql proxy: connection attributes parsing #17209
mattklein123 merged 4 commits intoenvoyproxy:mainfrom
qinggniq:conn-attribute

Conversation

@qinggniq
Copy link
Contributor

@qinggniq qinggniq commented Jul 1, 2021

Signed-off-by: qinggniq livewithblank@gmail.com

Parse connection attributes when CLIENT_CONNECT_ATTRS is set at the client capability.

Some connector will set connection attributes when connecting. If CLIENT_CONNECT_ATTRS is set but the related field is empty, the server will return bad handshake error.

Commit Message: MySQL proxy codec parsing of connection attributes
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: qinggniq <livewithblank@gmail.com>
@qinggniq qinggniq requested a review from mattklein123 as a code owner July 1, 2021 02:52
@qinggniq
Copy link
Contributor Author

qinggniq commented Jul 1, 2021

@cpakulski PTAL.

@qinggniq
Copy link
Contributor Author

qinggniq commented Jul 1, 2021

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:

🐱

Caused by: a #17209 (comment) was created by @qinggniq.

see: more, trace.

@lizan lizan self-assigned this Jul 1, 2021
@lizan lizan requested a review from cpakulski July 1, 2021 17:34
@lizan
Copy link
Member

lizan commented Jul 13, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@cpakulski cpakulski left a comment

Choose a reason for hiding this comment

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

Overall looks good.
Some spelling mistakes and tests should avoid so much duplicated code.

Signed-off-by: qinggniq <livewithblank@gmail.com>
@qinggniq
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17209 (comment) was created by @qinggniq.

see: more, trace.

@qinggniq
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17209 (comment) was created by @qinggniq.

see: more, trace.

Copy link
Contributor

@cpakulski cpakulski left a comment

Choose a reason for hiding this comment

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

The last small comments.

EXPECT_EQ(login_decode.getAuthResp(), login_encode.getAuthResp());
EXPECT_EQ(login_decode.getDb(), login_encode.getDb());
EXPECT_EQ(login_decode.getAuthPluginName(), login_encode.getAuthPluginName());
if (additional_check) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you compare to nullptr?

qinggniq added 2 commits July 21, 2021 10:56
Signed-off-by: qinggniq <livewithblank@gmail.com>
Signed-off-by: qinggniq <livewithblank@gmail.com>
@qinggniq qinggniq requested a review from cpakulski July 22, 2021 06:03
Copy link
Contributor

@cpakulski cpakulski left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@mattklein123 mattklein123 merged commit 26d288a into envoyproxy:main Jul 28, 2021
baojr added a commit to baojr/envoy that referenced this pull request Jul 29, 2021
…bridge-stream

* upstream/main: (140 commits)
  quiche: remove google quic support (envoyproxy#17465)
  runtime: removing envoy.reloadable_features.check_ocsp_policy (envoyproxy#17524)
  upstream: not trying to do HTTP/3 where not configured (envoyproxy#17454)
  api: Remove confusing line about auto-generation (envoyproxy#17536)
  v2: removing bootstrap (envoyproxy#17523)
  connpool: Fix crash in pool removal if the cluster was already deleted (envoyproxy#17522)
  Enhance the comments clearly (envoyproxy#17517)
  mysql proxy: connection attributes parsing  (envoyproxy#17209)
  [ci] fix false positive CVE scan from node (envoyproxy#17510)
  Fixing Envoy Mobile factory strings (envoyproxy#17509)
  http3: validating codec (envoyproxy#17452)
  quic: add QUIC upstream stream reset error stats (envoyproxy#17496)
  thrift proxy: move UpstreamRequest into its own file (envoyproxy#17498)
  docs: Fixed FaultDelay docs. (envoyproxy#17495)
  updates links to jaegertracing-plugin.tar.gz (envoyproxy#17497)
  http: make custom inline headers bootstrap configurable (envoyproxy#17330)
  deps: update yaml-cpp to latest master (envoyproxy#17489)
  improving tracer coverage (envoyproxy#17493)
  Increase buffer size of `Win32RedirectRecords` (envoyproxy#17471)
  ext_proc: Fix problem with buffered body mode with empty or no body (envoyproxy#17430)
  ...

Signed-off-by: Garrett Bourg <bourg@squareup.com>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Signed-off-by: qinggniq <livewithblank@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants