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.

Sign up for GitHub

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

examples/features/csm_observability: Add xDS Credentials #7875

Merged
merged 3 commits into from
Dec 2, 2024
Merged

examples/features/csm_observability: Add xDS Credentials #7875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fcf91a5
Add xDS Credentials
zasweq Nov 26, 2024
27f1e9e
Responded to Doug's comment
zasweq Nov 26, 2024
adab125
Responded to Doug's comment
zasweq Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion examples/features/csm_observability/client/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
xdscreds "google.golang.org/grpc/credentials/xds"
"google.golang.org/grpc/examples/features/proto/echo"
"google.golang.org/grpc/stats/opentelemetry"
"google.golang.org/grpc/stats/opentelemetry/csm"
Expand Down Expand Up @@ -56,7 +57,13 @@ func main() {
cleanup := csm.EnableObservability(context.Background(), opentelemetry.Options{MetricsOptions: opentelemetry.MetricsOptions{MeterProvider: provider}})
defer cleanup()

cc, err := grpc.NewClient(*target, grpc.WithTransportCredentials(insecure.NewCredentials()))
// 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.
creds, err := xdscreds.NewClientCredentials(xdscreds.ClientOptions{FallbackCreds: insecure.NewCredentials()})
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comments for client and server side.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

if err != nil {
log.Fatalf("Failed to create xDS credentials: %v", err)
}
cc, err := grpc.NewClient(*target, grpc.WithTransportCredentials(creds))
if err != nil {
log.Fatalf("Failed to start NewClient: %v", err)
}
Expand Down
14 changes: 13 additions & 1 deletion examples/features/csm_observability/server/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@ import (
"net/http"

"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
xdscreds "google.golang.org/grpc/credentials/xds"
pb "google.golang.org/grpc/examples/features/proto/echo"
"google.golang.org/grpc/stats/opentelemetry"
"google.golang.org/grpc/stats/opentelemetry/csm"
"google.golang.org/grpc/xds"

"github.com/prometheus/client_golang/prometheus/promhttp"
"go.opentelemetry.io/otel/exporters/prometheus"
Expand Down Expand Up @@ -67,7 +70,16 @@ func main() {
if err != nil {
log.Fatalf("Failed to listen: %v", err)
}
s := grpc.NewServer()
// 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.
creds, err := xdscreds.NewServerCredentials(xdscreds.ServerOptions{FallbackCreds: insecure.NewCredentials()})
Copy link
Member

Choose a reason for hiding this comment

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

As above. If xdscreds is unavailable, using insecure seems....insecure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed offline; this is part of the CSM requirements.

if err != nil {
log.Fatalf("Failed to create xDS credentials: %v", err)
}
s, err := xds.NewGRPCServer(grpc.Creds(creds))
if err != nil {
log.Fatalf("Failed to start xDS Server: %v", err)
}
pb.RegisterEchoServer(s, &echoServer{addr: ":" + *port})

log.Printf("Serving on %s\n", *port)
Expand Down