worker: add SAMS notifications subscriber by unknwon · Pull Request #63051 · sourcegraph/sourcegraph-public-snapshot

unknwon · 2024-06-03T20:14:08Z

This PR add a new worker for subscribing to SAMS notifications. The current use case is to automatically (hard-)delete users on Sourcegraph.com when the corresponding user is deleted from SAMS.

This worker is only started when running in the Sourcegraph.com mode and the credentials file (service_account.json) is provided, which has been configured since https://github.com/sourcegraph/deploy-sourcegraph-cloud/pull/18591.

Test plan

Tested locally end-to-end using my test GCP project.

Create test service account and download the credentials file
Create test Pub/Sub topic and subscription
Assign "Pub/Sub Subscriber" role to the service account on the subscription

Boot up SAMS locally

export NOTIFICATION_GCP_PUBSUB_PROJECT="joes-playground-425319"
export NOTIFICATION_GCP_PUBSUB_TOPIC="sams-notifications"
overmind start

Boot up SG in dotcom mode locally

# sg.config.overwrite.yaml
commandsets:
  dotcom:
    env:
      SOURCEGRAPHDOTCOM_MODE: true
      SOURCEGRAPH_ACCOUNTS_CREDENTIALS_FILE: './accounts-credentials.json'
      SOURCEGRAPH_ACCOUNTS_NOTIFICATIONS_PROJECT: 'joes-playground-425319'

# dev-private/.../site-config.json
"auth.providers": [
  {
    "allowSignup": true,
    "clientID": "sams_cid_018ea03c-55d2-765c-9074-98acfe0bc520",
    "clientSecret": "REDACTED",
    "displayName": "Sourcegraph Accounts",
    "configID": "sams",
    "issuer": "http://localhost:9991",
    "type": "openidconnect"
  }
]

sg start dotcom

Delete a SAMS user

curl -X DELETE -H "Authorization: Bearer foolmeifyoucan"  http://localhost:9991/api/management/v1/users/018d21f2-10ba-7f83-84b9-615678d6b383

Wait for SG to pick it up (temporarily changed from DEBUG to INFO logging)

bobheadxi · 2024-06-03T21:00:39Z

+	credentialsJSON, err := os.ReadFile(s.config.GCP.CredentialsFile)
+	if err != nil {
+		return nil, errors.Wrap(err, "read GCP credentials file")
+	}
+	credentials, err := google.CredentialsFromJSON(ctx, credentialsJSON, pubsub.ScopePubSub)
+	if err != nil {
+		return nil, errors.Wrap(err, "parse GCP credentials JSON")
+	}


Should this happen up-front in config load instead?

I think all of config init steps here equally crucial, and CredentialsFromJSON doesn't really do any validation other than expecting valid JSON format. Thus prefer init them in the same order as the fields of notificationsv1.SubscriberOptions.

bobheadxi · 2024-06-03T21:01:56Z

+	c.GCP.SubscriptionID = c.Get("SOURCEGRAPH_ACCOUNTS_NOTIFICATIONS_SUBSCRIPTION", "sams-notifications", "GCP Pub/Sub subscription ID to receive SAMS notifications from")
+}
+
+func handleOnUserDeleted(


IMO the handlers deserve their own file for readability and expansion - e.g. in the future, we might have:

user_deletion.go user_rename.go

etc.

I ended up restructuring the files a bit in https://github.com/sourcegraph/sourcegraph/pull/63051/commits/f7ab03a3a8ebcad6e69670bf6e95cfd23ddf87c8, PTAL!

I do want anything notifications-related visually and structurally grouped, since this package is generic "sourcegraphaccounts", who knows if we gonna add more workers here 😁

Much nicer, thanks 😁

bobheadxi · 2024-06-03T21:02:44Z

+- filename: cmd/worker/internal/sourcegraphaccounts/mocks_test.go
+  path: github.com/sourcegraph/sourcegraph/cmd/worker/internal/sourcegraphaccounts
+  interfaces:
+    - notificationsSubscriberStore


…er.go Co-authored-by: Robert Lin <robert@bobheadxi.dev>

cla-bot Bot added the cla-signed label Jun 3, 2024

unknwon force-pushed the jc/sams-notifications branch from 65cad0a to b163d1f Compare June 3, 2024 20:27

unknwon marked this pull request as ready for review June 3, 2024 20:28

unknwon requested a review from a team June 3, 2024 20:28

worker: add SAMS notifications subscriber

a014c8b

unknwon force-pushed the jc/sams-notifications branch from b163d1f to a014c8b Compare June 3, 2024 20:31