feat/enterpriseportal: db layer for subscription licenses #63792

bobheadxi · 2024-07-11T17:54:35Z

Implements CRUD on the new licenses DB. I had to make significant changes from the initial setup after spending more time working on this.

There's lots of schema changes but that's okay, as we have no data yet.

As in the RPC design, this is intended to accommodate new "types" of licensing in the future, and so the DB is structured as such as well. There's also feedback that context around license management events is very useful - this is encoded in the conditions table, and can be extended to include more types of conditions in the future.

Part of https://linear.app/sourcegraph/issue/CORE-158
Part of https://linear.app/sourcegraph/issue/CORE-100

Test plan

Integration tests

Locally, running sg run enterprise-portal indicates migrations proceed as expected

bobheadxi · 2024-07-11T17:54:53Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @bobheadxi and the rest of your teammates on Graphite

bobheadxi · 2024-07-12T18:22:20Z

cmd/enterprise-portal/internal/database/databasetest/databasetest.go

+	_ pgx.QueryTracer = pgxTestTracer{}
+)
+
+func (t pgxTestTracer) TraceQueryStart(ctx context.Context, conn *pgx.Conn, data pgx.TraceQueryStartData) context.Context {


Handy for dumping DB queries in tests

bobheadxi · 2024-07-12T18:23:25Z

cmd/enterprise-portal/internal/database/internal/utctime/utctime.go

+// Time is a wrapper around time.Time that implements the database/sql.Scanner
+// and database/sql/driver.Valuer interfaces to serialize and deserialize time
+// in UTC time zone.
+type Time time.Time


Primarily avoids scanning times in anything but UTC for the API, but for consistency can also be used to make sure we only ever feed UTC times into the DB

bobheadxi · 2024-07-12T18:24:20Z

cmd/enterprise-portal/internal/database/subscriptions/license_conditions.go

+)`, pgx.NamedArgs{
+		"licenseID": licenseID,
+		// Convert to string representation of EnterpriseSubscriptionLicenseCondition
+		"status":         subscriptionsv1.EnterpriseSubscriptionLicenseCondition_Status_name[int32(opts.Status)],


We "shouldn't" be using API types in storage, but limited sets of enums are okay I think?

Probably - but it would probably also be easy to write a small function that maps status to DB value manually in a switch 😬

Right now the values are pretty simple, like STATUS_REVOKED, STATUS_CREATED, etc, which is pretty much what I would put in the DB right now.

I think if we get to a point where the API enums start to deviate from the DB, we can then introduce a mapper for the values already in there

bobheadxi · 2024-07-12T18:24:48Z

cmd/enterprise-portal/internal/database/subscriptions/licenses.go

+func (s *TableSubscriptionLicense) RunCustomMigrations(migrator gorm.Migrator) error {
+	if migrator.HasColumn(s, "license_kind") {
+		if err := migrator.DropColumn(s, "license_kind"); err != nil {
+			return err
+		}
+	}
+	return nil


Drop renamed columns when gorm refuses to. This is not in use yet so is safe to do

the migration process seems to be quite painful :/

side-note: would be cool if we had the same schema.md generator for this DB so it's easier to diff what gorm does across commits 😬

Would love to have that, though at a cursory search I haven't found a good tool that does this for the gorm schema defs 😬 Maybe we can do something similar to how schema.md is generated, where we apply the schema to a DB and then introspect it with some tool to generate the page

https://linear.app/sourcegraph/issue/CORE-221/enterpriseportal-database-schema-doc-generator

Have you covered https://github.com/gogs/gogs/blob/main/internal/database/schemadoc/main.go 😂 Output https://github.com/gogs/gogs/blob/main/docs/dev/database_schema.md

Also although not recommending (we should migrate to a better migration system), but GORM-based migrations prior art 😂 : https://github.com/gogs/gogs/tree/main/internal/database/migrations

Thanks, added to the issue!

bobheadxi · 2024-07-12T18:25:29Z

cmd/enterprise-portal/internal/database/subscriptions/licenses.go

 	// LicenseData is the license data stored in JSON format. It is read-only
 	// and generally never queried in conditions - properties that are should
 	// be stored at the subscription or license level.
 	//
 	// Value shapes correspond to API types appropriate for each
 	// 'EnterpriseSubscriptionLicenseType'.
-	LicenseData pgtype.JSONB `gorm:"type:jsonb"`
+	LicenseData json.RawMessage `gorm:"type:jsonb"`


Allow different types of licenses to be stored in the same table, persisted as jsonb for query, though most use cases will not require querying over jsonb

bobheadxi · 2024-07-12T18:26:18Z

cmd/enterprise-portal/internal/database/subscriptions/licenses.go

+// LicenseKey corresponds to *subscriptionsv1.EnterpriseSubscriptionLicenseKey
+// and the 'ENTERPRISE_SUBSCRIPTION_LICENSE_TYPE_KEY' license type.
+type LicenseKey struct {
+	Info internallicense.Info


Use internal type in storage layer for consistency

bobheadxi · 2024-07-12T18:28:01Z

@eseliger for optional review if you have time 🙏

bobheadxi · 2024-07-12T18:46:08Z

Actually need a few more fixes, brb

bobheadxi · 2024-07-12T19:38:57Z

cmd/enterprise-portal/internal/database/subscriptions/subscriptions.go

@@ -33,27 +50,23 @@ type Subscription struct {
 	//
 	// TODO: Clean up the database post-deploy and remove the 'Unnamed subscription'
 	// part of the constraint.
-	DisplayName string `gorm:"size:256;not null;uniqueIndex:,where:archived_at IS NULL AND display_name != 'Unnamed subscription' AND display_name != ''"`
+	DisplayName *string `gorm:"size:256;uniqueIndex:,where:archived_at IS NULL AND display_name != 'Unnamed subscription' AND display_name != ''"`


InstanceDomain is nullable, this change makes it explicit

DisplayName is required at API level, but realistically our old data won't have it. Rather than special treatment for empty values or stub values nullable is easier

eseliger · 2024-07-13T11:50:07Z

cmd/enterprise-portal/internal/database/migrate.go

@@ -83,17 +90,20 @@ func maybeMigrate(ctx context.Context, logger log.Logger, contract runtime.Contr
 			span.AddEvent("lock.acquired")

 			versionKey := fmt.Sprintf("%s:db_version", dbName)
+			liveVersion := redisClient.Get(ctx, versionKey).Val()


it seems a bit odd to me that redis would keep the state of the SQL DB :D

Hm, we should probably store it in DB huh

https://linear.app/sourcegraph/issue/CORE-222/enterpriseportal-store-db-migration-state-in-db

Start storing this info in DB somewhat increases the seriousness of the value, that requires us to keep it legit and deal with potential compatibility issues. The current use case of it (and why it's in Redis) is merely a quick optimization about "avoid (if possible) running DB migrations if the version is exactly the same, and it's fine if ran twice, does no harm", it doesn't really care if an upgrade or downgrade has happened.

eseliger · 2024-07-13T11:53:58Z

cmd/enterprise-portal/internal/database/subscriptions/license_conditions.go

+)`, pgx.NamedArgs{
+		"licenseID": licenseID,
+		// Convert to string representation of EnterpriseSubscriptionLicenseCondition
+		"status":         subscriptionsv1.EnterpriseSubscriptionLicenseCondition_Status_name[int32(opts.Status)],


Probably - but it would probably also be easy to write a small function that maps status to DB value manually in a switch 😬

eseliger · 2024-07-13T11:55:17Z

cmd/enterprise-portal/internal/database/subscriptions/licenses.go

+func (s *TableSubscriptionLicense) RunCustomMigrations(migrator gorm.Migrator) error {
+	if migrator.HasColumn(s, "license_kind") {
+		if err := migrator.DropColumn(s, "license_kind"); err != nil {
+			return err
+		}
+	}
+	return nil


the migration process seems to be quite painful :/

side-note: would be cool if we had the same schema.md generator for this DB so it's easier to diff what gorm does across commits 😬

Implements Cody Gateway access in Enterprise Portal DB, such that it replicates the behaviour it has today, retrieving: - Quota overrides - Subscription display name - Active license info - Non-revoked, non-expired license keys as hashes - Revocation + non-expiry replaces the existing mechanism of flagging licenses as `access_token_enabled`. Since we ended up doing zero-config for Cody Gateway, the only license hashes that are valid for Cody Gateway are non-expired licenses - once your license expires you should be switching to a new license key anyway. It's fairly similar to the `dotcomdb` shim we built before, but for our new tables. See https://github.com/sourcegraph/sourcegraph/pull/63792 for the licenses tables. None of this is going live yet. Part of https://linear.app/sourcegraph/issue/CORE-100 Part of https://linear.app/sourcegraph/issue/CORE-160 ## Test plan DB integration tests `sg run enterprise-portal` does the migrations without a hitch

Implements insert and retrieval of subscription conditions, similar to #63792 Unlike for licenses, which have a more limited lifecycle (create and revoke), subscriptions are longer-lived and may be updated frequently. So the storage layer allows the caller to provide the conditions that are of interest for recording. Part of https://linear.app/sourcegraph/issue/CORE-156, but doesn't yet use it from the API. Part of https://linear.app/sourcegraph/issue/CORE-100 ## Test plan Integration tests

cla-bot bot added the cla-signed label Jul 11, 2024

bobheadxi mentioned this pull request Jul 11, 2024

feat/enterpriseportal: db layer for cody gateway access #63737

Merged

bobheadxi changed the title ~~wip~~ feat/enterpriseportal: db layer for subscription licenses Jul 11, 2024

bobheadxi force-pushed the feat/enterprise-portal_db_layer_for_licenses branch 3 times, most recently from 53fdf23 to dc7fed6 Compare July 12, 2024 18:13

bobheadxi commented Jul 12, 2024

View reviewed changes

bobheadxi marked this pull request as ready for review July 12, 2024 18:27

bobheadxi requested a review from a team July 12, 2024 18:27

bobheadxi force-pushed the feat/enterprise-portal_db_layer_for_licenses branch from dc7fed6 to 0ee398b Compare July 12, 2024 18:27

bobheadxi requested a review from eseliger July 12, 2024 18:27

bobheadxi marked this pull request as draft July 12, 2024 18:46

bobheadxi force-pushed the feat/enterprise-portal_db_layer_for_licenses branch from 192a825 to ebe188c Compare July 12, 2024 19:37

bobheadxi commented Jul 12, 2024

View reviewed changes

bobheadxi marked this pull request as ready for review July 12, 2024 19:39

bobheadxi force-pushed the feat/enterprise-portal_db_layer_for_licenses branch from ebe188c to 6a9d351 Compare July 12, 2024 19:53

bobheadxi added 5 commits July 12, 2024 16:36

feat/enterprise-portal: db layer for licenses

73424a2

fix nullable updates

71b3cfe

lift expire_time

0a54a1b

fixup test

ba8a30d

fast-fail

0589692

bobheadxi force-pushed the feat/enterprise-portal_db_layer_for_licenses branch from 785cf80 to 0589692 Compare July 12, 2024 23:36

bobheadxi mentioned this pull request Jul 13, 2024

feat/enterpriseportal: db layer for subscription conditions #63812

Merged

left join instead

9110453

eseliger approved these changes Jul 13, 2024

View reviewed changes

bobheadxi merged commit 795f0bb into main Jul 15, 2024
14 checks passed

bobheadxi deleted the feat/enterprise-portal_db_layer_for_licenses branch July 15, 2024 18:47

This was referenced Jul 15, 2024

chore/enterpriseportal: rewrite subscriptions API tests to be table-driven #63831

Merged

feat/enterpriseportal: support DevOnly in list subscriptions #63837

Merged

feat/enterpriseportal: import data from dotcom #63858

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat/enterpriseportal: db layer for subscription licenses #63792

feat/enterpriseportal: db layer for subscription licenses #63792

bobheadxi commented Jul 11, 2024 •

edited

Loading

bobheadxi commented Jul 11, 2024 •

edited

Loading

bobheadxi Jul 12, 2024

bobheadxi Jul 12, 2024

bobheadxi Jul 12, 2024

eseliger Jul 13, 2024

bobheadxi Jul 15, 2024

bobheadxi Jul 12, 2024

eseliger Jul 13, 2024

bobheadxi Jul 15, 2024

unknwon Aug 14, 2024

bobheadxi Aug 15, 2024

bobheadxi Jul 12, 2024

bobheadxi Jul 12, 2024

bobheadxi commented Jul 12, 2024

bobheadxi commented Jul 12, 2024

bobheadxi Jul 12, 2024

eseliger Jul 13, 2024

bobheadxi Jul 15, 2024

bobheadxi Jul 15, 2024

unknwon Aug 14, 2024 •

edited

Loading

eseliger Jul 13, 2024

eseliger Jul 13, 2024

feat/enterpriseportal: db layer for subscription licenses #63792

feat/enterpriseportal: db layer for subscription licenses #63792

Conversation

bobheadxi commented Jul 11, 2024 • edited Loading

Test plan

bobheadxi commented Jul 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobheadxi commented Jul 12, 2024

bobheadxi commented Jul 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unknwon Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobheadxi commented Jul 11, 2024 •

edited

Loading

bobheadxi commented Jul 11, 2024 •

edited

Loading

unknwon Aug 14, 2024 •

edited

Loading