Skip to content

Add database access for GCP Spanner#40859

Merged
GavinFrazar merged 1 commit intomasterfrom
gavinfrazar/gcloud-spanner
May 7, 2024
Merged

Add database access for GCP Spanner#40859
GavinFrazar merged 1 commit intomasterfrom
gavinfrazar/gcloud-spanner

Conversation

@GavinFrazar
Copy link
Copy Markdown
Contributor

Changelog: Added support for GCP Spanner to Teleport Database Service.

This PR adds support for GCP Spanner to database access.

Implements https://github.com/gravitational/customer-sensitive-requests/issues/170

IAM Setup

  1. You need a GCP service account with permissions to impersonate other identities. This can be an identity with the predefined role roles/iam.serviceAccountTokenCreator bound to it. It just needs iam.serviceAccounts.getAccessToken permission though.
  2. Then you need another service account that has permissions to do things in GCP Spanner.
    There are some predefined roles you can bind to your service account like roles/spanner.databaseAdmin or roles/spanner.databaseUser.
    The latter should be sufficient to do whatever you'd like to test manually.
    See: https://cloud.google.com/spanner/docs/iam#roles

If it helps, this is the terraform module I made to setup a service account for teleport (the controller) and 3 service accounts that the controller can impersonate to access Spanner: https://github.com/GavinFrazar/infra/blob/master/terraform/modules/gcp-iam-spanner/main.tf

Teleport DB Setup

Create a database with the "spanner" protocol and the gcp project/instance ID spec populated, for example:

kind: db
version: v3
metadata:
  name: "teleport-spanner"
  description: "gcp spanner db"
  labels:
    env: "dev"
spec:
  protocol: "spanner"
 # this will default to google api endpoint "spanner.googleapis.com:443".
 # it's required to be that endpoint too.
  uri: ""
  gcp:
    project_id: "example-proj123"
    instance_id: "example-spanner"

Access

A spanner instance can contain multiple databases.
Teleport will enforce db_names for these databases for each RPC.

The --db-user you use corresponds to the name of a service account you wish to access the database as.
Note that this does not include the full "name@.iam.gservice.account.com", it's only the name, e.g.
if the service account is gavin-spanner@teleport-dev-123456.iam.gserviceaccount.com then I would specify --db-user=gavin-spanner

CLI

There's a cli I found that works pretty nicely, and you can install it with go install github.com/cloudspannerecosystem/spanner-cli@latest
Then just tsh db connect ... and it should work too. It also uses a local proxy tunnel under the hood.

GUI

Start a local proxy tunnel: tsh proxy db --tunnel --port 8080 --db-user=<user> --db-name=example-db teleport-spanner

Configure your client to connect to the local proxy, and ensure it also has the right project, instance, and database id, for example you can use this jdbc string in DataGrip: jdbc:cloudspanner://localhost:8080/projects/example-proj123/instances/example-spanner/databases/example-db
It follows the template "projects/<project>/instances/<instance>/databases/<database>"

You also need to configure your client to connect to the local proxy tunnel without tls and without GCP credentials. DataGrip and even the driver in go do not allow you to use any credentials without TLS, and DataGrip also will not use any client certs even if you use the custom ssl settings, unfortunately.
So tsh proxy db --tunnel is pretty much the only way to go here.

image

image

TODO

I will write a full docs guide in a follow up PR.

@GavinFrazar GavinFrazar added database-access Database access related issues and PRs gcp labels Apr 24, 2024
@GavinFrazar GavinFrazar requested a review from greedy52 April 24, 2024 02:17
@github-actions github-actions Bot requested review from avatus and ibeckermayer April 24, 2024 02:18
@public-teleport-github-review-bot
Copy link
Copy Markdown

@GavinFrazar - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes.

@github-actions github-actions Bot added audit-log Issues related to Teleports Audit Log size/xl tsh tsh - Teleport's command line tool for logging into nodes running Teleport. ui labels Apr 24, 2024
@GavinFrazar GavinFrazar requested review from Tener, gabrielcorado and smallinsky and removed request for avatus and ibeckermayer April 24, 2024 02:18
Copy link
Copy Markdown
Contributor

@Tener Tener left a comment

Choose a reason for hiding this comment

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

A few comments from my first round. It was a lot to take in at once, is there a meaningful way to split this PR into smaller chunks?

Comment thread api/utils/gcp/endpoint.go Outdated
Comment thread lib/events/eventstest/channel.go Outdated
Comment thread lib/srv/db/ca.go Outdated
Comment thread lib/srv/db/spanner/engine.go Outdated
Comment thread lib/srv/db/spanner/engine.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Copy link
Copy Markdown
Contributor

@smallinsky smallinsky left a comment

Choose a reason for hiding this comment

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

First quick pass.

Comment thread web/packages/teleport/src/services/audit/makeEvent.ts Outdated
Comment thread lib/srv/db/spanner/test.go Outdated
Comment thread lib/srv/db/spanner/listener.go Outdated
Comment thread lib/srv/db/spanner/listener.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
@smallinsky smallinsky requested review from Tener and smallinsky April 24, 2024 14:27
@GavinFrazar
Copy link
Copy Markdown
Contributor Author

GavinFrazar commented Apr 25, 2024

A few comments from my first round. It was a lot to take in at once, is there a meaningful way to split this PR into smaller chunks?

I will split the PR into:

  1. configuration and client side changes (new db protocol, db object validation, tsh changes)
  2. engine
  3. web ui

we've already got some good discussion focused mostly on the engine. Let's treat this PR as just engine review. I'll change its merge base to get all the stuff from PR 1 out of the diff.

@Tener @smallinsky @greedy52

Edit: actually I wonder if that will be easier to review or just more tedious and difficult for reviewers to see the full picture of these changes. I kind of like that you can try out the full implementation with tsh and audit events in the web ui currently.
If you guys do want it split up, then please let me know whether the above outline sounds good.

@GavinFrazar GavinFrazar force-pushed the gavinfrazar/gcloud-spanner branch from 5c3ef8d to 5280081 Compare April 25, 2024 04:43
@Tener
Copy link
Copy Markdown
Contributor

Tener commented Apr 25, 2024

Edit: actually I wonder if that will be easier to review or just more tedious and difficult for reviewers to see the full picture of these changes. I kind of like that you can try out the full implementation with tsh and audit events in the web ui currently.
If you guys do want it split up, then please let me know whether the above outline sounds good.

I'm personally in favour of splitting up. It is true that getting the full picture is harder once this is split, but I find it easier to provide quality feedback with these smaller PRs.

Managing a set of PRs is a bit of extra work too, especially if the chain is long, but I believe it leads to superior quality in the long run.

Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Copy link
Copy Markdown
Contributor

@greedy52 greedy52 left a comment

Choose a reason for hiding this comment

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

This is amazing! Good job.
First quick pass. Will test out next round.
I think splitting would certainly make review easier and smaller PRs can go in a lot quicker. That said I don't mind reviewing PR of this size either.

Comment thread api/types/events/oneof.go Outdated
Comment thread lib/srv/db/spanner/engine.go Outdated
Comment thread lib/srv/db/spanner/grpcserver.go Outdated
Comment thread lib/srv/db/spanner/interceptors.go Outdated
Comment thread tool/tsh/common/proxy.go Outdated
Comment thread web/packages/teleport/src/services/audit/makeEvent.ts Outdated
@GavinFrazar GavinFrazar force-pushed the gavinfrazar/gcloud-spanner branch from ed1c657 to 958597d Compare April 29, 2024 07:05
@GavinFrazar
Copy link
Copy Markdown
Contributor Author

I pulled out the tsh, lib/client, teleterm and webui stuff from this PR to make it smaller

@GavinFrazar GavinFrazar force-pushed the gavinfrazar/gcloud-spanner branch from 6632fd2 to 8e534ed Compare April 29, 2024 15:49
@GavinFrazar GavinFrazar requested a review from Tener April 29, 2024 16:19
Comment thread lib/srv/db/spanner/engine.go Outdated
Comment thread lib/srv/db/spanner/listener.go Outdated
@Tener
Copy link
Copy Markdown
Contributor

Tener commented May 6, 2024

That said, do we count common.GetMessagesFromServerMetric or common.GetMessagesFromClientMetric in spanner?

This should be easy to add via interceptor.

@GavinFrazar
Copy link
Copy Markdown
Contributor Author

GavinFrazar commented May 6, 2024

That said, do we count common.GetMessagesFromServerMetric or common.GetMessagesFromClientMetric in spanner?

This should be easy to add via interceptor.

I've added these metrics in a grpc.StatsHandler in spanner/grpcserver.go

@smallinsky friendly ping - this needs group 1

@GavinFrazar GavinFrazar force-pushed the gavinfrazar/gcloud-spanner branch from f0a4cbc to f58c976 Compare May 7, 2024 20:39
@GavinFrazar GavinFrazar removed ui tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels May 7, 2024
@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented May 7, 2024

/excludeflake *

@GavinFrazar GavinFrazar enabled auto-merge May 7, 2024 22:13
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

:shipit:

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from gabrielcorado May 7, 2024 22:15
@GavinFrazar GavinFrazar added this pull request to the merge queue May 7, 2024
Merged via the queue into master with commit 12c113b May 7, 2024
@GavinFrazar GavinFrazar deleted the gavinfrazar/gcloud-spanner branch May 7, 2024 22:39
@public-teleport-github-review-bot
Copy link
Copy Markdown

@GavinFrazar See the table below for backport results.

Branch Result
branch/v15 Failed

GavinFrazar added a commit that referenced this pull request May 9, 2024
GavinFrazar added a commit that referenced this pull request May 27, 2024
github-merge-queue Bot pushed a commit that referenced this pull request May 29, 2024
* add GCP Spanner (#40859)

* support gcloud spanner in tsh and web UI (#41129)

* add spanner to tsh and teleterm

* handle spanner events in web ui

* update web audit events test snapshot

* customize web ui connect for spanner

* update ConnectDialog snapshot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

audit-log Issues related to Teleports Audit Log database-access Database access related issues and PRs gcp size/xl

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants