examples/features/csm_observability: Add xDS Credentials#7875
examples/features/csm_observability: Add xDS Credentials#7875zasweq merged 3 commits intogrpc:masterfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7875 +/- ##
==========================================
+ Coverage 81.91% 81.97% +0.05%
==========================================
Files 377 377
Lines 38065 38120 +55
==========================================
+ Hits 31180 31247 +67
+ Misses 5581 5566 -15
- Partials 1304 1307 +3 |
|
please also backport this to 1.68 and push a new image on dockerhub |
dfawley
left a comment
There was a problem hiding this comment.
Please add something to the release notes for this.
| defer cleanup() | ||
|
|
||
| cc, err := grpc.NewClient(*target, grpc.WithTransportCredentials(insecure.NewCredentials())) | ||
| creds, err := xdscreds.NewClientCredentials(xdscreds.ClientOptions{FallbackCreds: insecure.NewCredentials()}) |
There was a problem hiding this comment.
insecure? This doesn't seem proper to include in an example. Maybe we fall back to TLS? Or some kind of "erroring creds"? What do the other languages do for this?
There was a problem hiding this comment.
We probably should at least add comments indicating that the fallback credentials are here for demonstration purposes only and should not be used this way in production as it is insecure. It would be nice if we have a guide to link to.
There was a problem hiding this comment.
Added comments for client and server side.
There was a problem hiding this comment.
Actually this example is for CSM, and the CSM guide recommends setting fallback to insecure, so better to not have this text... But maybe something like:
// Set up xds credentials that fall back to insecure as described in:
// https://cloud.google.com/service-mesh/docs/service-routing/security-proxyless-setup#workloads_are_unable_to_communicate_in_the_security_setup.| log.Fatalf("Failed to listen: %v", err) | ||
| } | ||
| s := grpc.NewServer() | ||
| creds, err := xdscreds.NewServerCredentials(xdscreds.ServerOptions{FallbackCreds: insecure.NewCredentials()}) |
There was a problem hiding this comment.
As above. If xdscreds is unavailable, using insecure seems....insecure.
There was a problem hiding this comment.
Discussed offline; this is part of the CSM requirements.
|
Added release notes |
This PR adds xDS Credentials to the CSM Observability example. It also switches the server to be xDS Enabled.
RELEASE NOTES: