Add connection requested server name attribute to TCP read filter #1843
Conversation
istio-testing
added
the
do-not-merge/work-in-progress
googlebot
added
the
cla: yes
| @@ -51,6 +51,9 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { | |||
| builder.AddBool(utils::AttributeName::kConnectionMtls, | |||
| check_data->IsMutualTLS()); | |||
|
|
|||
| builder.AddString(utils::AttributeName::kConnectionRequestedServerName, | |||
There was a problem hiding this comment.
can you please check whether the string is empty first? we normally don't send unset values to mixer.
| @@ -53,6 +53,7 @@ class Filter : public Network::Filter, | |||
| bool GetSourceIpPort(std::string* str_ip, int* port) const override; | |||
| bool GetSourceUser(std::string* user) const override; | |||
| bool IsMutualTLS() const override; | |||
| std::string GetRequestedServerName() const override; | |||
There was a problem hiding this comment.
Make sure to apply the same changes to http filter in src/envoy/http/mixer
|
@kyessenov I have applied your comments. |
vadimeisenbergibm
changed the title
istio-testing
removed
the
do-not-merge/work-in-progress
|
@qiwzhang could you please review this PR? |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hklai, vadimeisenbergibm Assign the PR to them by writing 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 |
| @@ -47,6 +47,9 @@ class CheckData { | |||
| // Returns true if connection is mutual TLS enabled. | |||
| virtual bool IsMutualTLS() const = 0; | |||
|
|
|||
| // Get requested server name, SNI in case of TLS | |||
| virtual std::string GetRequestedServerName() const = 0; | |||
There was a problem hiding this comment.
Since there are cases data is not available, the coding style is to return a bool. It should be:
// return true if data is valid.
bool GetRequestedServerName(std::string* name);
What this PR does / why we need it:
Adds SNI attribute to the TCP read filter, to be used in telemetry reports and policy checks.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): implements #istio/istio#6810Release note: