From fab196043e15d475e5e3d3c81653b49e82c0c573 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Tue, 6 Aug 2024 11:38:55 -0400 Subject: [PATCH] fix: allow tag removal in provisioner upsert (#14187) --- coderd/database/dump.sql | 4 +-- ...provisioner_daemon_org_constraint.down.sql | 5 +++ ...9_provisioner_daemon_org_constraint.up.sql | 5 +++ coderd/database/queries.sql.go | 5 +-- .../database/queries/provisionerdaemons.sql | 5 +-- coderd/database/unique_constraint.go | 2 +- enterprise/coderd/provisionerdaemons_test.go | 35 +++++++++++++++++++ 7 files changed, 50 insertions(+), 11 deletions(-) create mode 100644 coderd/database/migrations/000239_provisioner_daemon_org_constraint.down.sql create mode 100644 coderd/database/migrations/000239_provisioner_daemon_org_constraint.up.sql diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 0bcad08271..527c7df3c7 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1772,9 +1772,9 @@ CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name); CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)); -CREATE UNIQUE INDEX idx_provisioner_daemons_name_owner_key ON provisioner_daemons USING btree (name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); +CREATE UNIQUE INDEX idx_provisioner_daemons_org_name_owner_key ON provisioner_daemons USING btree (organization_id, name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); -COMMENT ON INDEX idx_provisioner_daemons_name_owner_key IS 'Allow unique provisioner daemon names by user'; +COMMENT ON INDEX idx_provisioner_daemons_org_name_owner_key IS 'Allow unique provisioner daemon names by organization and user'; CREATE INDEX idx_tailnet_agents_coordinator ON tailnet_agents USING btree (coordinator_id); diff --git a/coderd/database/migrations/000239_provisioner_daemon_org_constraint.down.sql b/coderd/database/migrations/000239_provisioner_daemon_org_constraint.down.sql new file mode 100644 index 0000000000..1e4266bb84 --- /dev/null +++ b/coderd/database/migrations/000239_provisioner_daemon_org_constraint.down.sql @@ -0,0 +1,5 @@ +CREATE UNIQUE INDEX idx_provisioner_daemons_name_owner_key ON provisioner_daemons USING btree (name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); + +COMMENT ON INDEX idx_provisioner_daemons_name_owner_key IS 'Allow unique provisioner daemon names by user'; + +DROP INDEX idx_provisioner_daemons_org_name_owner_key; diff --git a/coderd/database/migrations/000239_provisioner_daemon_org_constraint.up.sql b/coderd/database/migrations/000239_provisioner_daemon_org_constraint.up.sql new file mode 100644 index 0000000000..dace5136e5 --- /dev/null +++ b/coderd/database/migrations/000239_provisioner_daemon_org_constraint.up.sql @@ -0,0 +1,5 @@ +CREATE UNIQUE INDEX idx_provisioner_daemons_org_name_owner_key ON provisioner_daemons USING btree (organization_id, name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); + +COMMENT ON INDEX idx_provisioner_daemons_org_name_owner_key IS 'Allow unique provisioner daemon names by organization and user'; + +DROP INDEX idx_provisioner_daemons_name_owner_key; diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index d8a6e3a1ab..c99ed1441a 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5011,16 +5011,13 @@ VALUES ( $6, $7, $8 -) ON CONFLICT("name", LOWER(COALESCE(tags ->> 'owner'::text, ''::text))) DO UPDATE SET +) ON CONFLICT("organization_id", "name", LOWER(COALESCE(tags ->> 'owner'::text, ''::text))) DO UPDATE SET provisioners = $3, tags = $4, last_seen_at = $5, "version" = $6, api_version = $8, organization_id = $7 -WHERE - -- Only ones with the same tags are allowed clobber - provisioner_daemons.tags <@ $4 :: jsonb RETURNING id, created_at, name, provisioners, replica_id, tags, last_seen_at, version, api_version, organization_id ` diff --git a/coderd/database/queries/provisionerdaemons.sql b/coderd/database/queries/provisionerdaemons.sql index aa34fb5fff..f7c974b3fb 100644 --- a/coderd/database/queries/provisionerdaemons.sql +++ b/coderd/database/queries/provisionerdaemons.sql @@ -45,16 +45,13 @@ VALUES ( @version, @organization_id, @api_version -) ON CONFLICT("name", LOWER(COALESCE(tags ->> 'owner'::text, ''::text))) DO UPDATE SET +) ON CONFLICT("organization_id", "name", LOWER(COALESCE(tags ->> 'owner'::text, ''::text))) DO UPDATE SET provisioners = @provisioners, tags = @tags, last_seen_at = @last_seen_at, "version" = @version, api_version = @api_version, organization_id = @organization_id -WHERE - -- Only ones with the same tags are allowed clobber - provisioner_daemons.tags <@ @tags :: jsonb RETURNING *; -- name: UpdateProvisionerDaemonLastSeenAt :exec diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index f3f42ea0b7..ab0b3ae90d 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -85,7 +85,7 @@ const ( UniqueIndexCustomRolesNameLower UniqueConstraint = "idx_custom_roles_name_lower" // CREATE UNIQUE INDEX idx_custom_roles_name_lower ON custom_roles USING btree (lower(name)); UniqueIndexOrganizationName UniqueConstraint = "idx_organization_name" // CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name); UniqueIndexOrganizationNameLower UniqueConstraint = "idx_organization_name_lower" // CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)); - UniqueIndexProvisionerDaemonsNameOwnerKey UniqueConstraint = "idx_provisioner_daemons_name_owner_key" // CREATE UNIQUE INDEX idx_provisioner_daemons_name_owner_key ON provisioner_daemons USING btree (name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); + UniqueIndexProvisionerDaemonsOrgNameOwnerKey UniqueConstraint = "idx_provisioner_daemons_org_name_owner_key" // CREATE UNIQUE INDEX idx_provisioner_daemons_org_name_owner_key ON provisioner_daemons USING btree (organization_id, name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); UniqueIndexUsersEmail UniqueConstraint = "idx_users_email" // CREATE UNIQUE INDEX idx_users_email ON users USING btree (email) WHERE (deleted = false); UniqueIndexUsersUsername UniqueConstraint = "idx_users_username" // CREATE UNIQUE INDEX idx_users_username ON users USING btree (username) WHERE (deleted = false); UniqueOrganizationsSingleDefaultOrg UniqueConstraint = "organizations_single_default_org" // CREATE UNIQUE INDEX organizations_single_default_org ON organizations USING btree (is_default) WHERE (is_default = true); diff --git a/enterprise/coderd/provisionerdaemons_test.go b/enterprise/coderd/provisionerdaemons_test.go index d92e278d8e..ff87b1fd4c 100644 --- a/enterprise/coderd/provisionerdaemons_test.go +++ b/enterprise/coderd/provisionerdaemons_test.go @@ -347,6 +347,41 @@ func TestProvisionerDaemonServe(t *testing.T) { } }) + t.Run("ChangeTags", func(t *testing.T) { + t.Parallel() + client, user := coderdenttest.New(t, &coderdenttest.Options{LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureExternalProvisionerDaemons: 1, + }, + }}) + another, _ := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.ScopedRoleOrgAdmin(user.OrganizationID)) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + req := codersdk.ServeProvisionerDaemonRequest{ + ID: uuid.New(), + Name: testutil.MustRandString(t, 63), + Organization: user.OrganizationID, + Provisioners: []codersdk.ProvisionerType{ + codersdk.ProvisionerTypeEcho, + }, + Tags: map[string]string{ + provisionersdk.TagScope: provisionersdk.ScopeOrganization, + }, + } + _, err := another.ServeProvisionerDaemon(ctx, req) + require.NoError(t, err) + + // add tag + req.Tags["new"] = "tag" + _, err = another.ServeProvisionerDaemon(ctx, req) + require.NoError(t, err) + + // remove tag + delete(req.Tags, "new") + _, err = another.ServeProvisionerDaemon(ctx, req) + require.NoError(t, err) + }) + t.Run("PSK_daily_cost", func(t *testing.T) { t.Parallel() const provPSK = `provisionersftw`