Skip to content

Add requested server name attribute to release 1 0#8015

Merged
costinm merged 9 commits intoistio:release-1.0from
vadimeisenbergibm:add_requested_server_name_to_release_1_0
Aug 21, 2018
Merged

Add requested server name attribute to release 1 0#8015
costinm merged 9 commits intoistio:release-1.0from
vadimeisenbergibm:add_requested_server_name_to_release_1_0

Conversation

@vadimeisenbergibm
Copy link
Copy Markdown
Contributor

@vadimeisenbergibm vadimeisenbergibm commented Aug 17, 2018

Checked according to https://istio.io/docs/examples/advanced-egress/egress-gateway/#direct-https-traffic-through-an-egress-gateway.

kubectl -n istio-system logs $(kubectl -n istio-system get pods -l istio-mixer-type=telemetry -o jsonpath='{.items[0].metadata.name}') mixer | grep tcpaccesslog.logentry.istio-system | grep '"connectionEvent":"open"'

{"level":"info","time":"2018-08-17T02:30:32.692709Z","instance":"tcpaccesslog.logentry.istio-system","connectionDuration":"0s","connectionEvent":"open","connection_security_policy":"unknown","destinationApp":"istio-egressgateway","destinationIp":"172.30.146.115","destinationName":"istio-egressgateway-7fb9959565-6pdxk","destinationNamespace":"istio-system","destinationOwner":"kubernetes://apis/extensions/v1beta1/namespaces/istio-system/deployments/istio-egressgateway","destinationPrincipal":"","destinationServiceHost":"edition.cnn.com","destinationWorkload":"istio-egressgateway","protocol":"tcp","receivedBytes":0,"reporter":"source","requestedServerName":"edition.cnn.com","sentBytes":0,"sourceApp":"sleep","sourceIp":"172.30.109.111","sourceName":"sleep-776b7bcdcd-f2dsb","sourceNamespace":"default","sourceOwner":"kubernetes://apis/extensions/v1beta1/namespaces/default/deployments/sleep","sourcePrincipal":"","sourceWorkload":"sleep","totalReceivedBytes":0,"totalSentBytes":0}
{"level":"info","time":"2018-08-17T02:30:32.696204Z","instance":"tcpaccesslog.logentry.istio-system","connectionDuration":"0s","connectionEvent":"open","connection_security_policy":"unknown","destinationApp":"","destinationIp":"151.101.1.67","destinationName":"unknown","destinationNamespace":"default","destinationOwner":"unknown","destinationPrincipal":"cluster.local/ns/istio-system/sa/istio-egressgateway-service-account","destinationServiceHost":"","destinationWorkload":"unknown","protocol":"tcp","receivedBytes":297,"reporter":"source","requestedServerName":"edition.cnn.com","sentBytes":0,"sourceApp":"istio-egressgateway","sourceIp":"172.30.109.111","sourceName":"istio-egressgateway-7fb9959565-6pdxk","sourceNamespace":"istio-system","sourceOwner":"kubernetes://apis/extensions/v1beta1/namespaces/istio-system/deployments/istio-egressgateway","sourcePrincipal":"cluster.local/ns/default/sa/default","sourceWorkload":"istio-egressgateway","totalReceivedBytes":297,"totalSentBytes":0}

@vadimeisenbergibm vadimeisenbergibm mentioned this pull request Aug 17, 2018
Closed
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 17, 2018 •
edited
Loading

Codecov Report

Merging #8015 into release-1.0 will decrease coverage by 1%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           release-1.0   #8015    +/-   ##
============================================
- Coverage           72%     72%   -<1%     
============================================
  Files              358     358            
  Lines            31226   31149    -77     
============================================
- Hits             22290   22206    -84     
- Misses            7985    7992     +7     
  Partials           951     951
Impacted Files Coverage Δ
pilot/pkg/config/memory/monitor.go 82% <0%> (-9%) ⬇️
mixer/adapter/solarwinds/log_handler.go 58% <0%> (-8%) ⬇️
mixer/adapter/redisquota/redisquota.go 87% <0%> (-3%) ⬇️
mixer/adapter/statsd/statsd.go 96% <0%> (-2%) ⬇️
mixer/adapter/bypass/bypass.go 62% <0%> (-1%) ⬇️
mixer/adapter/solarwinds/solarwinds.go 0% <0%> (ø) ⬆️
mixer/adapter/stdio/stdio.go 100% <0%> (ø) ⬆️
mixer/adapter/memquota/dedup.go 100% <0%> (ø) ⬆️
mixer/adapter/list/regexList.go 100% <0%> (ø) ⬆️
mixer/adapter/cloudwatch/client.go 0% <0%> (ø) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ecffa8d...f87e1fc. Read the comment docs.

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Aug 17, 2018

we may need to bump the proxy to pick up istio/proxy#1909

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Aug 18, 2018
@istio-testing istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Aug 19, 2018
@vadimeisenbergibm
Copy link
Copy Markdown
Contributor Author

vadimeisenbergibm commented Aug 19, 2018

/test istio-pilot-e2e-envoyv2-v1alpha3

@vadimeisenbergibm
Copy link
Copy Markdown
Contributor Author

vadimeisenbergibm commented Aug 19, 2018

@kyessenov Would it be OK to merge this one without istio/proxy#1924? Now this one passes the tests, so istio/proxy#1924 could be merged later to 1.1 or 1.0.2. If yes, could you please approve this PR?

@vadimeisenbergibm
Copy link
Copy Markdown
Contributor Author

vadimeisenbergibm commented Aug 20, 2018

@kyessenov
istio/proxy#1924 has been merged, I have updated this PR. Waiting for https://storage.googleapis.com/istio-build/proxy/envoy-debug-cc83ed8fe87d854f6a3f14a06d874879a78076d1.tar.gz to be uploaded.

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Aug 20, 2018

/retest
/lgtm
/approve

Thanks for doing this @vadimeisenbergibm . Changes look good to me.

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Aug 20, 2018

/lgtm
/approve

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Aug 20, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kyessenov

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@vadimeisenbergibm
Copy link
Copy Markdown
Contributor Author

vadimeisenbergibm commented Aug 20, 2018

Thanks @kyessenov, sorry I missed additional changes, hope the tests will pass now.

@vadimeisenbergibm
Copy link
Copy Markdown
Contributor Author

vadimeisenbergibm commented Aug 20, 2018

/test e2e-bookInfo-envoyv2-v1alpha3

@vadimeisenbergibm
Copy link
Copy Markdown
Contributor Author

vadimeisenbergibm commented Aug 21, 2018

@costinm @rkpagadala Could you please merge this one? It is a PR that depends on
istio/proxy#1924. The PR is a minor one - adds a Mixer attribute to two log entries, updates istio/proxy according to istio/proxy#1924 and fixes the tests accordingly. It is a customer issue, I would like to have it in 1.0.1.

@costinm costinm merged commit 698263a into istio:release-1.0 Aug 21, 2018
@vadimeisenbergibm
Copy link
Copy Markdown
Contributor Author

vadimeisenbergibm commented Aug 21, 2018

Thanks, @costinm ! This is SNI reporting to Mixer, BTW.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kyessenov kyessenov Awaiting requested review from kyessenov

@mandarjog mandarjog Awaiting requested review from mandarjog

@sbezverk sbezverk Awaiting requested review from sbezverk

Assignees

@kyessenov kyessenov

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vadimeisenbergibm @kyessenov @istio-testing @costinm @googlebot