Skip to content
This repository has been archived by the owner on Jan 16, 2022. It is now read-only.

Introduce new attribute connection.mtls #425

Merged
merged 3 commits into from
Feb 7, 2018

Conversation

JimmyCYJ
Copy link
Member

@JimmyCYJ JimmyCYJ commented Feb 6, 2018

What this PR does / why we need it: Introduce new mixer attribute "connection.mtls" which indicates whether connection is mutual TLS enabled. Add virtual function bool IsMutualTlsEnabledConnection() into http/check_data.h and tcp/check_data.h, and call this function inside AttributesBuilder::ExtractCheckAttributes(). Add another virtual function bool GetDestinationIpPort(std::string* ip, int* port) to report destination IP and port from HTTP filter.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #426

Special notes for your reviewer:

Release note:

@@ -42,6 +42,9 @@ class CheckData {
// Get request HTTP headers
virtual std::map<std::string, std::string> GetRequestHeaders() const = 0;

// Returns true if connection is mutual TLS enabled.
virtual bool IsMutualTlsEnabledConnection() const = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just use "IsMutualTLS"

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@@ -29,6 +29,9 @@ class ReportData {
public:
virtual ~ReportData() {}

// Get upstream tcp connection ip and port.
virtual bool GetDestinationIpPort(std::string* ip, int* port) const = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

please comment on the format of ip in bytes

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@JimmyCYJ
Copy link
Member Author

JimmyCYJ commented Feb 6, 2018

Please take a look. Thanks.

@JimmyCYJ
Copy link
Member Author

JimmyCYJ commented Feb 6, 2018

/test mixerclient-presubmit

@qiwzhang
Copy link
Contributor

qiwzhang commented Feb 6, 2018

/lgtm
/approve

@istio-merge-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiwzhang

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@istio-merge-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@istio-merge-robot
Copy link

Automatic merge from submit-queue.

@istio-merge-robot istio-merge-robot merged commit 5b09518 into istio:master Feb 7, 2018
@JimmyCYJ JimmyCYJ deleted the add-attribute-mtls branch February 7, 2018 00:09
istio-merge-robot pushed a commit to istio/proxy that referenced this pull request Feb 7, 2018
Automatic merge from submit-queue.

Support Mixer filter to send connection.mtls attribute.

**What this PR does / why we need it**: This is a followup PR which implements functions defined in PR #[425](istio/old_mixerclient_repo#425). This support Mixer client to send connection.mtls attribute in Check() and Report() calls.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
```
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send Mixer attribute "connection.mtls" in Check() and Report() calls.
5 participants