credentials/alts: preserve boundAccessToken in altsTC.Clone

[nileshpatil6]

Summary

altsTC.Clone does not copy boundAccessToken, so a cloned ALTS credential silently loses the token and performs its handshake without it.

Detail

boundAccessToken was added to the altsTC struct in 00be1e1, "[alts] Add plumbing for the bound access token field in the ALTS Start". That commit added the field and consumed it in ClientHandshake:

 type altsTC struct {
-	info      *credentials.ProtocolInfo
-	side      core.Side
-	accounts  []string
-	hsAddress string
+	info             *credentials.ProtocolInfo
+	side             core.Side
+	accounts         []string
+	hsAddress        string
+	boundAccessToken string
 }
...
+	opts.BoundAccessToken = g.boundAccessToken

but it did not update Clone, which still copies only the four original fields:

func (g *altsTC) Clone() credentials.TransportCredentials {
	...
	return &altsTC{
		info:      &info,
		side:      g.side,
		hsAddress: g.hsAddress,
		accounts:  accounts,
	}
}

Clone is part of the exported credentials.TransportCredentials interface, and grpc-go calls it internally, for example when building the RLS control channel in balancer/rls/control_channel.go:

credsOpt = grpc.WithTransportCredentials(bOpts.DialCreds.Clone())

so a clone taken there would hand the handshaker an empty BoundAccessToken.

To be upfront about reachability: there is currently no exported setter for boundAccessToken, so today it is only assigned in tests and the field is always empty in production. This is a latent bug in the plumbing rather than one users can hit right now, but it will bite as soon as a setter lands, and Clone dropping a field it should carry is wrong regardless.

Change

One field added to the struct literal in Clone.

Testing

TestCloneClient already asserts on side, hsAddress and accounts, so I extended it to cover boundAccessToken alongside them. Without the fix it fails:

alts_test.go:121: cc.boundAccessToken = "", want "bound-access-token"

With the fix, go test ./credentials/alts/... passes for the whole tree and go vet ./credentials/alts/... is clean.

RELEASE NOTES: none

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: nileshpatil6 / name: nileshpatil6 (cfe9afc)

[nileshpatil6]

CLA is signed now, and all eight required Testing jobs pass, including -race, arm and i386.

Notes on the three red checks, none of which look related to this change:

• PR Validation / Validate PR (required) — this now fails with This PR must have a label starting with 'Type:'. I added the missing RELEASE NOTES: section that it originally flagged, but external contributors cannot add labels, so this one needs a maintainer to apply Type: Bug. The same check is red on interop: add OpenTelemetry tracing support to interop client and server #9342, xds: allow wildcard listener port on server #9341 and balancer/ringhash: fix flaky TestRingHash_RequestHashKeyConnecting #9329.

• codecov / upload — the failing tests are in stats/opentelemetry and stats/opentelemetry/csm. This job is also failing on extproc : implement flow control for A93 #9343, interop: add OpenTelemetry tracing support to interop client and server #9342 and xds: allow wildcard listener port on server #9341, so it appears repo-wide rather than specific to this PR.

• govulncheck (.) — fails while loading a dependency, not on this diff:

  /home/runner/go/pkg/mod/cloud.google.com/go/auth@v0.23.1/httptransport/transport.go:43:2:
      "google.golang.org/api/googleapi" imported as apierror and not used
  /home/runner/go/pkg/mod/cloud.google.com/go/auth@v0.23.1/httptransport/transport.go:317:34:
      undefined: googleapi
  

  The other four govulncheck shards pass. This PR does not touch go.mod.

Happy to rebase if that would help with any of these.

[eshitachandwani]

Thank you for finding and fixing this.

Adding @easwars for a second review.

[eshitachandwani]

• codecov / upload — the failing tests are in stats/opentelemetry and stats/opentelemetry/csm. This job is also failing on extproc : implement flow control for A93 #9343, interop: add OpenTelemetry tracing support to interop client and server #9342 and xds: allow wildcard listener port on server #9341, so it appears repo-wide rather than specific to this PR.

Happy to rebase if that would help with any of these.

This has been fixed, please rebase on master to get it passing.

[matthewstevenson88]

@rockspore Can you PTAL?

[rockspore]

I think this is fine. Although boundAccessToken has no public use case, it should be preserved in Clone.

[easwars]

@nileshpatil6 : Thanks for your contribution!