VTGate: Set immediate caller id from gRPC static auth username

[brendar]

Signed-off-by: Brendan Dougherty brendan.dougherty@shopify.com

Description

Currently, when using gRPC static auth on VTGate, the immediate caller id is always set to unsecure_grpc_client, rather than the username provided by the client. This means that table ACLs on VTTablet will reject the client's queries.

The problem appears to have been discussed when static auth was originally added here, but was not changed in a followup.

This PR takes a similar approach to what was suggested and sets the authenticated username in the Context. This allows the VTGate server to retrieve the username in a similar manner to how it retrieves the certificate common name as the immediate caller id when using mTLS (source).

Related Issue(s)

Fixes #12049

Checklist

• "Backport to:" labels have been added if this change should be back-ported
• Tests were added or are not required
• Documentation was added or is not required

Deployment Notes

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• If this is a change that users need to know about, please apply the release notes (needs details) label so that merging is blocked unless the summary release notes document is included.
• If a test is added or modified, there should be a documentation on top of the test to explain what the expected behavior is what the test does.

If a new flag is being introduced:

• Is it really necessary to add this flag?
• Flag names should be clear and intuitive (as far as possible)
• Help text should be descriptive.
• Flag names should use dashes (-) as word separators rather than underscores (_).

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow should be required, the maintainer team should be notified.

Bug fixes

• There should be at least one unit or end-to-end test.
• The Pull Request description should include a link to an issue that describes the bug.

Non-trivial changes

• There should be some code comments as to why things are implemented the way they are.

New/Existing features

• Should be documented, either by modifying the existing documentation or creating new documentation.
• New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• vtctl command output order should be stable and awk-able.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from VTop, if used there.

[brendar]

I'm not sure if this is the right shard to use. Any guidance would be appreciated.

[GuptaManan100]

This shard looks as good as any. I checked the run of this shard as well. It takes about 13 minutes with this new test, so there is no risk of flakiness because of timeouts. It should be okay.

[GuptaManan100]

Other than this, I like the changes and the tests too.

[GuptaManan100]

To me the tests feel self-explanatory, but could you add a couple of lines to each describing what its testing.

[brendar]

Done

[Phanatic]

👋🏾 I work at PlanetScale and built out our Service-to-service authentication story and custom ACLs for PlanetScale databases.

PlanetScale's usage of vtgate static auth and ACLs system is a bit unique in that we don't handle authentication for user queries in vtgate. We do that in our own query front-end service.
VTGate static authentication is configured for service-to-service authentication between the query front-end and vtgate, ACL system is configured to pass through user roles from our credential store to vtgate.

We do this by setting the effectiveCallerID on requests made to the vtgate gRPC service and having the same names reflected in the acl config file for a given vttablet.

vitess/go/vt/vtgate/grpcvtgateservice/server.go

Lines 107 to 113 in c0b5df6

	immediate, securityGroups := immediateCallerID(ctx)
	if immediate == "" && useEffective && effectiveCallerID != nil {
	immediate = effectiveCallerID.Principal
	if useEffectiveGroups && len(effectiveCallerID.Groups) > 0 {
	securityGroups = effectiveCallerID.Groups
	}
	}

Given ☝🏾 , this change looks good to me. Since the caller identity is still the EffectiveCallerId from the context and not request identity used for service-to-service communication.

[brendar]

Thank you both for the reviews. Is there anything else needed or would it be possible for you to merge the PR?

[deepthi]

@brendar we are planning to effectively revert this PR and backport the change to release-16.0 branch as well. See discussion #12961 (comment)