Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add requested server name attribute to release 1 0 #8015

Conversation

vadimeisenbergibm
Copy link
Contributor

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}

@codecov
Copy link

codecov bot commented Aug 17, 2018

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
Contributor

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
Contributor Author

/test istio-pilot-e2e-envoyv2-v1alpha3

@vadimeisenbergibm
Copy link
Contributor Author

@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
Contributor Author

@kyessenov
Copy link
Contributor

/retest
/lgtm
/approve

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

@kyessenov
Copy link
Contributor

/lgtm
/approve

@istio-testing
Copy link
Collaborator

[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

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
Contributor Author

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

@vadimeisenbergibm
Copy link
Contributor Author

/test e2e-bookInfo-envoyv2-v1alpha3

@vadimeisenbergibm
Copy link
Contributor Author

@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
Contributor Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants