Dec 18, 2023 · Dec 14, 2023 · Dec 14, 2023 · Dec 14, 2023 · Dec 14, 2023 · Dec 14, 2023
diff --git a/cli/testdata/coder_list_--output_json.golden b/cli/testdata/coder_list_--output_json.golden
        "worker_id": "[workspace build worker ID]",
        "file_id": "[workspace build file ID]",
        "tags": {
          "owner": "",
          "scope": "organization"
        },
        "queue_position": 0,
diff --git a/coderd/coderd.go b/coderd/coderd.go
 "context"
 "crypto/tls"
 "crypto/x509"
 "database/sql"
 "flag"
 "fmt"
 "io"
 "github.com/coder/coder/v2/coderd/batchstats"
 "github.com/coder/coder/v2/coderd/database"
 "github.com/coder/coder/v2/coderd/database/dbauthz"
 "github.com/coder/coder/v2/coderd/database/dbtime"
 "github.com/coder/coder/v2/coderd/database/pubsub"
 "github.com/coder/coder/v2/coderd/externalauth"
 "github.com/coder/coder/v2/coderd/gitsshkey"
 }
 }()

 tags := provisionerdserver.Tags{
 provisionersdk.TagScope: provisionersdk.ScopeOrganization,
 //nolint:gocritic // in-memory provisioners are owned by system
 daemon, err := api.Database.UpsertProvisionerDaemon(dbauthz.AsSystemRestricted(ctx), database.UpsertProvisionerDaemonParams{
 Name:      name,
 CreatedAt: dbtime.Now(),
 Provisioners: []database.ProvisionerType{
 database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform,
 },
 Tags:       provisionersdk.MutateTags(uuid.Nil, nil),
 LastSeenAt: sql.NullTime{Time: dbtime.Now(), Valid: true},
 Version:    buildinfo.Version(),
 APIVersion: "1.0",
 })
 if err != nil {
 return nil, xerrors.Errorf("failed to create in-memory provisioner daemon: %w", err)
 }

 mux := drpcmux.New()
 api.Logger.Info(ctx, "starting in-memory provisioner daemon", slog.F("name", name))
 logger := api.Logger.Named(fmt.Sprintf("inmem-provisionerd-%s", name))
 srv, err := provisionerdserver.NewServer(
 api.ctx,
 api.ctx, // use the same ctx as the API
 api.AccessURL,
 uuid.New(),
 daemon.ID,
 logger,
 []database.ProvisionerType{
 database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform,
 },
 tags,
 daemon.Provisioners,
 provisionerdserver.Tags(daemon.Tags),
 api.Database,
 api.Pubsub,
 api.Acquirer,
diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
 }()

 daemon := provisionerd.New(func(ctx context.Context) (provisionerdproto.DRPCProvisionerDaemonClient, error) {
 return coderAPI.CreateInMemoryProvisionerDaemon(ctx,t.Name())
 return coderAPI.CreateInMemoryProvisionerDaemon(ctx,"test")
 }, &provisionerd.Options{
 Logger:              coderAPI.Logger.Named("provisionerd").Leveled(slog.LevelDebug),
 UpdateInterval:      250 * time.Millisecond,
diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
 rbac.ResourceOrganization.Type:       {rbac.ActionCreate},
 rbac.ResourceOrganizationMember.Type: {rbac.ActionCreate},
 rbac.ResourceOrgRoleAssignment.Type:  {rbac.ActionCreate},
 rbac.ResourceProvisionerDaemon.Type:  {rbac.ActionCreate, rbac.ActionUpdate},
 rbac.ResourceUser.Type:               {rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
 rbac.ResourceUserData.Type:           {rbac.ActionCreate, rbac.ActionUpdate},
 rbac.ResourceWorkspace.Type:          {rbac.ActionUpdate},
 return q.db.UpdateMemberRoles(ctx, arg)
 }

 func (q *querier) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error {
 if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceProvisionerDaemon); err != nil {
 return err
 }
 return q.db.UpdateProvisionerDaemonLastSeenAt(ctx, arg)
 }

 // TODO: We need to create a ProvisionerJob resource type
 func (q *querier) UpdateProvisionerJobByID(ctx context.Context, arg database.UpdateProvisionerJobByIDParams) error {
 // if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil {
diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
 s.NoError(err, "insert provisioner daemon")
 check.Args().Asserts(rbac.ResourceSystem, rbac.ActionDelete)
 }))
 s.Run("UpdateProvisionerDaemonLastSeenAt", s.Subtest(func(db database.Store, check *expects) {
 d, err := db.UpsertProvisionerDaemon(context.Background(), database.UpsertProvisionerDaemonParams{
 Tags: database.StringMap(map[string]string{
 provisionersdk.TagScope: provisionersdk.ScopeOrganization,
 }),
 })
 s.NoError(err, "insert provisioner daemon")
 check.Args(database.UpdateProvisionerDaemonLastSeenAtParams{
 ID:         d.ID,
 LastSeenAt: sql.NullTime{Time: dbtime.Now(), Valid: true},
 }).Asserts(rbac.ResourceProvisionerDaemon, rbac.ActionUpdate)
 }))
 }

 // All functions in this method test suite are not implemented in dbmem, but
diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go
 "github.com/coder/coder/v2/coderd/util/slice"
 )

 var errMatchAny =errors.New("match any error")
 var errMatchAny =xerrors.New("match any error")

 var skipMethods = map[string]string{
 "InTx":           "Not relevant",
diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
 return database.OrganizationMember{}, sql.ErrNoRows
 }

 func (q *FakeQuerier) UpdateProvisionerDaemonLastSeenAt(_ context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error {
 err := validateDatabaseType(arg)
 if err != nil {
 return err
 }

 q.mutex.Lock()
 defer q.mutex.Unlock()

 for idx := range q.provisionerDaemons {
 if q.provisionerDaemons[idx].ID != arg.ID {
 continue
 }
 if q.provisionerDaemons[idx].LastSeenAt.Time.After(arg.LastSeenAt.Time) {
 continue
 }
 q.provisionerDaemons[idx].LastSeenAt = arg.LastSeenAt
 return nil
 }
 return sql.ErrNoRows
 }

 func (q *FakeQuerier) UpdateProvisionerJobByID(_ context.Context, arg database.UpdateProvisionerJobByIDParams) error {
 if err := validateDatabaseType(arg); err != nil {
 return err
diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go
diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000181_coalesce_provisioner_daemon_idx_owner.down.sql b/coderd/database/migrations/000181_coalesce_provisioner_daemon_idx_owner.down.sql
 DROP INDEX IF EXISTS idx_provisioner_daemons_name_owner_key;

 CREATE UNIQUE INDEX IF NOT EXISTS idx_provisioner_daemons_name_owner_key
 ON provisioner_daemons
 USING btree (name, lower((tags->>'owner')::text));

 COMMENT ON INDEX idx_provisioner_daemons_name_owner_key
 IS 'Relax uniqueness constraint for provisioner daemon names';
diff --git a/coderd/database/migrations/000181_coalesce_provisioner_daemon_idx_owner.up.sql b/coderd/database/migrations/000181_coalesce_provisioner_daemon_idx_owner.up.sql
 DROP INDEX IF EXISTS idx_provisioner_daemons_name_owner_key;

 CREATE UNIQUE INDEX IF NOT EXISTS idx_provisioner_daemons_name_owner_key
 ON provisioner_daemons
 USING btree (name, LOWER(COALESCE(tags->>'owner', '')::text));

 COMMENT ON INDEX idx_provisioner_daemons_name_owner_key
 IS 'Allow unique provisioner daemon names by user';
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/provisionerdaemons.sql b/coderd/database/queries/provisionerdaemons.sql
 @last_seen_at,
 @version,
 @api_version
 ) ON CONFLICT("name",lower((tags ->> 'owner'::text))) DO UPDATE SET
 ) ON CONFLICT("name",LOWER(COALESCE(tags ->> 'owner'::text, ''::text))) DO UPDATE SET
 provisioners = @provisioners,
 tags = @tags,
 last_seen_at = @last_seen_at,
 -- Only ones with the same tags are allowed clobber
 provisioner_daemons.tags <@ @tags :: jsonb
 RETURNING *;

 -- name: UpdateProvisionerDaemonLastSeenAt :exec
 UPDATE provisioner_daemons
 SET
 last_seen_at = @last_seen_at
 WHERE
 id = @id
 AND
 last_seen_at <= @last_seen_at;
diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go
Original file line number	Diff line number	Diff line change
Expand Up		@@ -36,6 +36,7 @@
		"worker_id": "[workspace build worker ID]",
		"file_id": "[workspace build file ID]",
		"tags": {
		"owner": "",
		"scope": "organization"
		},
		"queue_position": 0,
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -4,6 +4,7 @@ import (
		"context"
		"crypto/tls"
		"crypto/x509"
		"database/sql"
		"flag"
		"fmt"
		"io"
Expand DownExpand Up		@@ -49,6 +50,7 @@ import (
		"github.com/coder/coder/v2/coderd/batchstats"
		"github.com/coder/coder/v2/coderd/database"
		"github.com/coder/coder/v2/coderd/database/dbauthz"
		"github.com/coder/coder/v2/coderd/database/dbtime"
		"github.com/coder/coder/v2/coderd/database/pubsub"
		"github.com/coder/coder/v2/coderd/externalauth"
		"github.com/coder/coder/v2/coderd/gitsshkey"
Expand DownExpand Up		@@ -1178,22 +1180,32 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, name string
		}
		}()

		tags := provisionerdserver.Tags{
		provisionersdk.TagScope: provisionersdk.ScopeOrganization,
		//nolint:gocritic // in-memory provisioners are owned by system
		daemon, err := api.Database.UpsertProvisionerDaemon(dbauthz.AsSystemRestricted(ctx), database.UpsertProvisionerDaemonParams{
		Name: name,
		CreatedAt: dbtime.Now(),
		Provisioners: []database.ProvisionerType{
		database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform,
		},
		Tags: provisionersdk.MutateTags(uuid.Nil, nil),
		LastSeenAt: sql.NullTime{Time: dbtime.Now(), Valid: true},
		Version: buildinfo.Version(),
		APIVersion: "1.0",
		})
		if err != nil {
		return nil, xerrors.Errorf("failed to create in-memory provisioner daemon: %w", err)
		}

		mux := drpcmux.New()
		api.Logger.Info(ctx, "starting in-memory provisioner daemon", slog.F("name", name))
		logger := api.Logger.Named(fmt.Sprintf("inmem-provisionerd-%s", name))
		srv, err := provisionerdserver.NewServer(
		api.ctx,
		api.ctx, // use the same ctx as the API
		api.AccessURL,
		uuid.New(),
		daemon.ID,
		logger,
		[]database.ProvisionerType{
		database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform,
		},
		tags,
		daemon.Provisioners,
		provisionerdserver.Tags(daemon.Tags),
		api.Database,
		api.Pubsub,
		api.Acquirer,
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -533,7 +533,7 @@ func NewProvisionerDaemon(t testing.TB, coderAPI *coderd.API) io.Closer {
		}()

		daemon := provisionerd.New(func(ctx context.Context) (provisionerdproto.DRPCProvisionerDaemonClient, error) {
		return coderAPI.CreateInMemoryProvisionerDaemon(ctx,t.Name())
		return coderAPI.CreateInMemoryProvisionerDaemon(ctx,"test")
johnstcn marked this conversation as resolved. Show resolvedHide resolved
		}, &provisionerd.Options{
		Logger: coderAPI.Logger.Named("provisionerd").Leveled(slog.LevelDebug),
		UpdateInterval: 250 * time.Millisecond,
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -232,6 +232,7 @@ var (
		rbac.ResourceOrganization.Type: {rbac.ActionCreate},
		rbac.ResourceOrganizationMember.Type: {rbac.ActionCreate},
		rbac.ResourceOrgRoleAssignment.Type: {rbac.ActionCreate},
		rbac.ResourceProvisionerDaemon.Type: {rbac.ActionCreate, rbac.ActionUpdate},
		rbac.ResourceUser.Type: {rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete},
		rbac.ResourceUserData.Type: {rbac.ActionCreate, rbac.ActionUpdate},
		rbac.ResourceWorkspace.Type: {rbac.ActionUpdate},
Expand DownExpand Up		@@ -2499,6 +2500,13 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb
		return q.db.UpdateMemberRoles(ctx, arg)
		}

		func (q *querier) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error {
		if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceProvisionerDaemon); err != nil {
		return err
		}
		return q.db.UpdateProvisionerDaemonLastSeenAt(ctx, arg)
		}

		// TODO: We need to create a ProvisionerJob resource type
		func (q *querier) UpdateProvisionerJobByID(ctx context.Context, arg database.UpdateProvisionerJobByIDParams) error {
		// if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil {
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -1592,6 +1592,18 @@ func (s *MethodTestSuite) TestExtraMethods() {
		s.NoError(err, "insert provisioner daemon")
		check.Args().Asserts(rbac.ResourceSystem, rbac.ActionDelete)
		}))
		s.Run("UpdateProvisionerDaemonLastSeenAt", s.Subtest(func(db database.Store, check *expects) {
		d, err := db.UpsertProvisionerDaemon(context.Background(), database.UpsertProvisionerDaemonParams{
		Tags: database.StringMap(map[string]string{
		provisionersdk.TagScope: provisionersdk.ScopeOrganization,
		}),
		})
		s.NoError(err, "insert provisioner daemon")
		check.Args(database.UpdateProvisionerDaemonLastSeenAtParams{
		ID: d.ID,
		LastSeenAt: sql.NullTime{Time: dbtime.Now(), Valid: true},
		}).Asserts(rbac.ResourceProvisionerDaemon, rbac.ActionUpdate)
		}))
		}

		// All functions in this method test suite are not implemented in dbmem, but
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -28,7 +28,7 @@ import (
		"github.com/coder/coder/v2/coderd/util/slice"
		)

		var errMatchAny =errors.New("match any error")
		var errMatchAny =xerrors.New("match any error")

		var skipMethods = map[string]string{
		"InTx": "Not relevant",
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -5945,6 +5945,28 @@ func (q *FakeQuerier) UpdateMemberRoles(_ context.Context, arg database.UpdateMe
		return database.OrganizationMember{}, sql.ErrNoRows
		}

		func (q *FakeQuerier) UpdateProvisionerDaemonLastSeenAt(_ context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error {
		err := validateDatabaseType(arg)
		if err != nil {
		return err
		}

		q.mutex.Lock()
		defer q.mutex.Unlock()

		for idx := range q.provisionerDaemons {
		if q.provisionerDaemons[idx].ID != arg.ID {
		continue
		}
		if q.provisionerDaemons[idx].LastSeenAt.Time.After(arg.LastSeenAt.Time) {
		continue
		}
		q.provisionerDaemons[idx].LastSeenAt = arg.LastSeenAt
johnstcn marked this conversation as resolved. Show resolvedHide resolved
		return nil
		}
		return sql.ErrNoRows
Copy link Member mafredriDec 15, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Is this really the same return signature the DB gives? I believe you have to check the affected rows of the`Exec` as it will not return an error, just zero affected rows. I wonder if we rely on similar assumptions elsewhere in the code, seeing as the generated code doesn't check affected rows 🤔 Copy link MemberAuthor johnstcnDec 15, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Other dbmem queries have very similar logic; I just copied it. mtojek reacted with thumbs up emoji Copy link Member mafredriDec 18, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Ideally we should change it so that our dbmem logic matches reality. Otherwise a developer may rely on this behavior and wonder why it doesn't work in the real world. johnstcn reacted with thumbs up emoji Copy link MemberAuthor johnstcnDec 18, 2023• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I'll file a follow-up issue 👍 EDIT:#11263 mafredri reacted with heart emoji
		}

		func (q *FakeQuerier) UpdateProvisionerJobByID(_ context.Context, arg database.UpdateProvisionerJobByIDParams) error {
		if err := validateDatabaseType(arg); err != nil {
		return err
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,8 @@
		DROP INDEX IF EXISTS idx_provisioner_daemons_name_owner_key;

		CREATE UNIQUE INDEX IF NOT EXISTS idx_provisioner_daemons_name_owner_key
		ON provisioner_daemons
		USING btree (name, lower((tags->>'owner')::text));

		COMMENT ON INDEX idx_provisioner_daemons_name_owner_key
		IS 'Relax uniqueness constraint for provisioner daemon names';
Original file line number	Diff line number	Diff line change
Expand Up		@@ -35,7 +35,7 @@ VALUES (
		@last_seen_at,
		@version,
		@api_version
		) ON CONFLICT("name",lower((tags ->> 'owner'::text))) DO UPDATE SET
		) ON CONFLICT("name",LOWER(COALESCE(tags ->> 'owner'::text, ''::text))) DO UPDATE SET
		provisioners = @provisioners,
		tags = @tags,
		last_seen_at = @last_seen_at,
Expand All		@@ -45,3 +45,12 @@ WHERE
		-- Only ones with the same tags are allowed clobber
		provisioner_daemons.tags <@ @tags :: jsonb
		RETURNING *;

		-- name: UpdateProvisionerDaemonLastSeenAt :exec
		UPDATE provisioner_daemons
		SET
		last_seen_at = @last_seen_at
		WHERE
		id = @id
		AND
		last_seen_at <= @last_seen_at;