google-cloud-go icon indicating copy to clipboard operation
google-cloud-go copied to clipboard

feat(auth): configure ALTS bound tokens in Options's validate()

Open rockspore opened this issue 10 months ago • 3 comments

This is a different approach compared with #11665. The unexported field altsCredentials will be resolved during the validate() at most one time and used in subsequent dial() calls.

@quartzmo Could you PTAL at this? If this approach makes more sense to you, I feel we should do the same thing for the existing DetectDefault call especially with the introduction of MTLS_S2A feature. Let me know if there are any caveats I am missing here. Thanks.

rockspore avatar Mar 12 '25 20:03 rockspore

@rockspore Thank you for exploring the problems I raised in #11665. I think this PR suggests a good beginning for a larger cleanup of grpctransport. I would continue it as follows:

  1. Figure out why DetectDefault is potentially repeatedly called in dial instead of just once in Dial.
  2. An instance of grpctransport.Options should be considered mutable until its resolveDetectOptions method is called. After that, it should be considered an immutable record of the configuration that produced the credentials.DetectOptions. (This options object is subsequently used to call DetectDefault.)
  3. If another call to DetectDefault is needed, with a modified configuration, a new grpctransport.Options must be created. It can be a copy of the original. Its usage should also follow 2. above.
  4. Move all mutation of grpctransport.Options as well as all calls to DetectDefault out of dial and into Dial. This will improve efficiency as well as the readability of dial.
  5. Move the resolution of auth.Credentials out of dial and into Dial.

I think that these changes go far beyond your changes in #11665, which at the moment are consistent with the current design as well as nicely encapsulated in the configureDirectPath func. At this point, I feel we should merge #11665, as there is nothing in it that cannot be changed within the plan sketched out above. Also, I don't want step 1. above to delay you.

quartzmo avatar Mar 14 '25 17:03 quartzmo

I'd like to keep this PR open until it can be expanded or replaced. However, I am converting it to a Draft since it is incomplete.

quartzmo avatar Mar 14 '25 17:03 quartzmo

Thank you, @quartzmo! It makes a lot of sense.

rockspore avatar Mar 14 '25 17:03 rockspore