diff --git a/cli/templatecreate.go b/cli/templatecreate.go index a0f4014f712f0..de0b8eab8f27e 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -10,6 +10,7 @@ import ( "unicode/utf8" "github.com/briandowns/spinner" + "github.com/google/uuid" "github.com/spf13/cobra" "golang.org/x/xerrors" @@ -91,7 +92,7 @@ func templateCreate() *cobra.Command { Client: client, Organization: organization, Provisioner: database.ProvisionerType(provisioner), - FileHash: resp.Hash, + FileID: resp.ID, ParameterFile: parameterFile, }) if err != nil { @@ -148,7 +149,7 @@ type createValidTemplateVersionArgs struct { Client *codersdk.Client Organization codersdk.Organization Provisioner database.ProvisionerType - FileHash string + FileID uuid.UUID ParameterFile string // Template is only required if updating a template's active version. Template *codersdk.Template @@ -165,7 +166,7 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers req := codersdk.CreateTemplateVersionRequest{ Name: args.Name, StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: args.FileHash, + FileID: args.FileID, Provisioner: codersdk.ProvisionerType(args.Provisioner), ParameterValues: parameters, } diff --git a/cli/templatepull.go b/cli/templatepull.go index 5660261c5158d..09f70c91b8f9e 100644 --- a/cli/templatepull.go +++ b/cli/templatepull.go @@ -66,7 +66,7 @@ func templatePull() *cobra.Command { latest := versions[0] // Download the tar archive. - raw, ctype, err := client.Download(ctx, latest.Job.StorageSource) + raw, ctype, err := client.Download(ctx, latest.Job.FileID) if err != nil { return xerrors.Errorf("download template: %w", err) } diff --git a/cli/templatepush.go b/cli/templatepush.go index f858c6daecb0d..40bafed0ef00c 100644 --- a/cli/templatepush.go +++ b/cli/templatepush.go @@ -80,7 +80,7 @@ func templatePush() *cobra.Command { Client: client, Organization: organization, Provisioner: database.ProvisionerType(provisioner), - FileHash: resp.Hash, + FileID: resp.ID, ParameterFile: parameterFile, Template: &template, ReuseParameters: !alwaysPrompt, diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index 22536583b6d3f..f21fde9a8af7b 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -276,7 +276,7 @@ func build(ctx context.Context, store database.Store, workspace database.Workspa Provisioner: template.Provisioner, Type: database.ProvisionerJobTypeWorkspaceBuild, StorageMethod: priorJob.StorageMethod, - StorageSource: priorJob.StorageSource, + FileID: priorJob.FileID, Input: input, }) if err != nil { diff --git a/coderd/coderd.go b/coderd/coderd.go index 29439a2001a99..b067035b3adb4 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -280,7 +280,7 @@ func New(options *Options) *API { // file content is expensive so it should be small. httpmw.RateLimitPerMinute(12), ) - r.Get("/{hash}", api.fileByHash) + r.Get("/{fileID}", api.fileByID) r.Post("/", api.postFile) }) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index fa778988641f1..a5183f2b6e450 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -142,7 +142,7 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { AssertObject: rbac.ResourceTemplate.InOrg(a.Template.OrganizationID), }, "POST:/api/v2/files": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceFile}, - "GET:/api/v2/files/{hash}": { + "GET:/api/v2/files/{fileID}": { AssertAction: rbac.ActionRead, AssertObject: rbac.ResourceFile.WithOwner(a.Admin.UserID.String()), }, @@ -369,7 +369,7 @@ func NewAuthTester(ctx context.Context, t *testing.T, client *codersdk.Client, a "{workspaceagent}": workspace.LatestBuild.Resources[0].Agents[0].ID.String(), "{buildnumber}": strconv.FormatInt(int64(workspace.LatestBuild.BuildNumber), 10), "{template}": template.ID.String(), - "{hash}": file.Hash, + "{fileID}": file.ID.String(), "{workspaceresource}": workspace.LatestBuild.Resources[0].ID.String(), "{workspaceapp}": workspace.LatestBuild.Resources[0].Agents[0].Apps[0].Name, "{templateversion}": version.ID.String(), diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index d7ac4eb14be97..7ec28668cea91 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -383,7 +383,7 @@ func CreateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID file, err := client.Upload(context.Background(), codersdk.ContentTypeTar, data) require.NoError(t, err) templateVersion, err := client.CreateTemplateVersion(context.Background(), organizationID, codersdk.CreateTemplateVersionRequest{ - StorageSource: file.Hash, + FileID: file.ID, StorageMethod: codersdk.ProvisionerStorageMethodFile, Provisioner: codersdk.ProvisionerTypeEcho, }) @@ -431,7 +431,7 @@ func UpdateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID require.NoError(t, err) templateVersion, err := client.CreateTemplateVersion(context.Background(), organizationID, codersdk.CreateTemplateVersionRequest{ TemplateID: templateID, - StorageSource: file.Hash, + FileID: file.ID, StorageMethod: codersdk.ProvisionerStorageMethodFile, Provisioner: codersdk.ProvisionerTypeEcho, }) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index bdb4c8e0f0e40..80af09a188532 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -316,12 +316,24 @@ func (q *fakeQuerier) DeleteAPIKeyByID(_ context.Context, id string) error { return sql.ErrNoRows } -func (q *fakeQuerier) GetFileByHash(_ context.Context, hash string) (database.File, error) { +func (q *fakeQuerier) GetFileByHashAndCreator(_ context.Context, arg database.GetFileByHashAndCreatorParams) (database.File, error) { q.mutex.RLock() defer q.mutex.RUnlock() for _, file := range q.files { - if file.Hash == hash { + if file.Hash == arg.Hash && file.CreatedBy == arg.CreatedBy { + return file, nil + } + } + return database.File{}, sql.ErrNoRows +} + +func (q *fakeQuerier) GetFileByID(_ context.Context, id uuid.UUID) (database.File, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + for _, file := range q.files { + if file.ID == id { return file, nil } } @@ -1901,6 +1913,7 @@ func (q *fakeQuerier) InsertFile(_ context.Context, arg database.InsertFileParam //nolint:gosimple file := database.File{ + ID: arg.ID, Hash: arg.Hash, CreatedAt: arg.CreatedAt, CreatedBy: arg.CreatedBy, @@ -2085,7 +2098,7 @@ func (q *fakeQuerier) InsertProvisionerJob(_ context.Context, arg database.Inser InitiatorID: arg.InitiatorID, Provisioner: arg.Provisioner, StorageMethod: arg.StorageMethod, - StorageSource: arg.StorageSource, + FileID: arg.FileID, Type: arg.Type, Input: arg.Input, } diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index eb16074e90525..9345907d044c4 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -151,7 +151,8 @@ CREATE TABLE files ( created_at timestamp with time zone NOT NULL, created_by uuid NOT NULL, mimetype character varying(64) NOT NULL, - data bytea NOT NULL + data bytea NOT NULL, + id uuid DEFAULT gen_random_uuid() NOT NULL ); CREATE TABLE gitsshkeys ( @@ -270,10 +271,10 @@ CREATE TABLE provisioner_jobs ( initiator_id uuid NOT NULL, provisioner provisioner_type NOT NULL, storage_method provisioner_storage_method NOT NULL, - storage_source text NOT NULL, type provisioner_job_type NOT NULL, input jsonb NOT NULL, - worker_id uuid + worker_id uuid, + file_id uuid NOT NULL ); CREATE TABLE site_configs ( @@ -432,7 +433,10 @@ ALTER TABLE ONLY audit_logs ADD CONSTRAINT audit_logs_pkey PRIMARY KEY (id); ALTER TABLE ONLY files - ADD CONSTRAINT files_pkey PRIMARY KEY (hash); + ADD CONSTRAINT files_hash_created_by_key UNIQUE (hash, created_by); + +ALTER TABLE ONLY files + ADD CONSTRAINT files_pkey PRIMARY KEY (id); ALTER TABLE ONLY gitsshkeys ADD CONSTRAINT gitsshkeys_pkey PRIMARY KEY (user_id); diff --git a/coderd/database/migrations/000059_file_id.down.sql b/coderd/database/migrations/000059_file_id.down.sql new file mode 100644 index 0000000000000..56dbb13eeb504 --- /dev/null +++ b/coderd/database/migrations/000059_file_id.down.sql @@ -0,0 +1,41 @@ +BEGIN; + +-- Add back the storage_source column. This must be nullable temporarily. +ALTER TABLE provisioner_jobs ADD COLUMN storage_source text; + +-- Set the storage_source to the hash of the files.id reference. +UPDATE + provisioner_jobs +SET + storage_source=files.hash +FROM + files +WHERE + provisioner_jobs.file_id = files.id; + +-- Now that we've populated storage_source drop the file_id column. +ALTER TABLE provisioner_jobs DROP COLUMN file_id; +-- We can set the storage_source column as NOT NULL now. +ALTER TABLE provisioner_jobs ALTER COLUMN storage_source SET NOT NULL; + +-- Delete all the duplicate rows where hashes collide. +-- We filter on 'id' to ensure only 1 unique row. +DELETE FROM + files a +USING + files b +WHERE + a.created_by < b.created_by +AND + a.hash = b.hash; + +-- Drop the primary key on files.id. +ALTER TABLE files DROP CONSTRAINT files_pkey; +-- Drop the id column. +ALTER TABLE files DROP COLUMN id; +-- Drop the unique constraint on hash + owner. +ALTER TABLE files DROP CONSTRAINT files_hash_created_by_key; +-- Set the primary key back to hash. +ALTER TABLE files ADD PRIMARY KEY (hash); + +COMMIT; diff --git a/coderd/database/migrations/000059_file_id.up.sql b/coderd/database/migrations/000059_file_id.up.sql new file mode 100644 index 0000000000000..7e6e919fa9bb3 --- /dev/null +++ b/coderd/database/migrations/000059_file_id.up.sql @@ -0,0 +1,42 @@ +-- This migration updates the files table to move the unique +-- constraint to be hash + created_by. This is necessary to +-- allow regular users who have been granted admin to a specific +-- template to be able to push and read files used for template +-- versions they create. +-- Prior to this collisions on file.hash were not an issue +-- since users who could push files could also read all files. +-- +-- This migration also adds a 'files.id' column as the primary +-- key. As a side effect the provisioner_jobs must now reference +-- the files.id column since the 'hash' column is now ambiguous. +BEGIN; + +-- Drop the primary key on hash. +ALTER TABLE files DROP CONSTRAINT files_pkey; + +-- Add an 'id' column and designate it the primary key. +ALTER TABLE files ADD COLUMN + id uuid NOT NULL PRIMARY KEY DEFAULT gen_random_uuid (); + +-- Update the constraint to include the user who created it. +ALTER TABLE files ADD UNIQUE(hash, created_by); + +-- Update provisioner_jobs to include a file_id column. +-- This must be temporarily nullable. +ALTER TABLE provisioner_jobs ADD COLUMN file_id uuid; + +-- Update all the rows to point to key in the files table. +UPDATE provisioner_jobs +SET + file_id = files.id +FROM + files +WHERE + provisioner_jobs.storage_source = files.hash; + +-- Enforce NOT NULL on file_id now. +ALTER TABLE provisioner_jobs ALTER COLUMN file_id SET NOT NULL; +-- Drop storage_source since it is no longer useful for anything. +ALTER TABLE provisioner_jobs DROP COLUMN storage_source; + +COMMIT; diff --git a/coderd/database/models.go b/coderd/database/models.go index f669b5e618138..2e50d2b647bd7 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -404,6 +404,7 @@ type File struct { CreatedBy uuid.UUID `db:"created_by" json:"created_by"` Mimetype string `db:"mimetype" json:"mimetype"` Data []byte `db:"data" json:"data"` + ID uuid.UUID `db:"id" json:"id"` } type GitSSHKey struct { @@ -501,10 +502,10 @@ type ProvisionerJob struct { InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` Provisioner ProvisionerType `db:"provisioner" json:"provisioner"` StorageMethod ProvisionerStorageMethod `db:"storage_method" json:"storage_method"` - StorageSource string `db:"storage_source" json:"storage_source"` Type ProvisionerJobType `db:"type" json:"type"` Input json.RawMessage `db:"input" json:"input"` WorkerID uuid.NullUUID `db:"worker_id" json:"worker_id"` + FileID uuid.UUID `db:"file_id" json:"file_id"` } type ProvisionerJobLog struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index b58f6abbccfb8..e9c577bcb11a7 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -39,7 +39,8 @@ type sqlcQuerier interface { // are included. GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUID) (GetAuthorizationUserRolesRow, error) GetDeploymentID(ctx context.Context) (string, error) - GetFileByHash(ctx context.Context, hash string) (File, error) + GetFileByHashAndCreator(ctx context.Context, arg GetFileByHashAndCreatorParams) (File, error) + GetFileByID(ctx context.Context, id uuid.UUID) (File, error) GetGitSSHKey(ctx context.Context, userID uuid.UUID) (GitSSHKey, error) GetGroupByID(ctx context.Context, id uuid.UUID) (Group, error) GetGroupByOrgAndName(ctx context.Context, arg GetGroupByOrgAndNameParams) (Group, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2ff1805cd47e1..8673822ca4c55 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -647,19 +647,51 @@ func (q *sqlQuerier) InsertAuditLog(ctx context.Context, arg InsertAuditLogParam return i, err } -const getFileByHash = `-- name: GetFileByHash :one +const getFileByHashAndCreator = `-- name: GetFileByHashAndCreator :one SELECT - hash, created_at, created_by, mimetype, data + hash, created_at, created_by, mimetype, data, id FROM files WHERE hash = $1 +AND + created_by = $2 +LIMIT + 1 +` + +type GetFileByHashAndCreatorParams struct { + Hash string `db:"hash" json:"hash"` + CreatedBy uuid.UUID `db:"created_by" json:"created_by"` +} + +func (q *sqlQuerier) GetFileByHashAndCreator(ctx context.Context, arg GetFileByHashAndCreatorParams) (File, error) { + row := q.db.QueryRowContext(ctx, getFileByHashAndCreator, arg.Hash, arg.CreatedBy) + var i File + err := row.Scan( + &i.Hash, + &i.CreatedAt, + &i.CreatedBy, + &i.Mimetype, + &i.Data, + &i.ID, + ) + return i, err +} + +const getFileByID = `-- name: GetFileByID :one +SELECT + hash, created_at, created_by, mimetype, data, id +FROM + files +WHERE + id = $1 LIMIT 1 ` -func (q *sqlQuerier) GetFileByHash(ctx context.Context, hash string) (File, error) { - row := q.db.QueryRowContext(ctx, getFileByHash, hash) +func (q *sqlQuerier) GetFileByID(ctx context.Context, id uuid.UUID) (File, error) { + row := q.db.QueryRowContext(ctx, getFileByID, id) var i File err := row.Scan( &i.Hash, @@ -667,18 +699,20 @@ func (q *sqlQuerier) GetFileByHash(ctx context.Context, hash string) (File, erro &i.CreatedBy, &i.Mimetype, &i.Data, + &i.ID, ) return i, err } const insertFile = `-- name: InsertFile :one INSERT INTO - files (hash, created_at, created_by, mimetype, "data") + files (id, hash, created_at, created_by, mimetype, "data") VALUES - ($1, $2, $3, $4, $5) RETURNING hash, created_at, created_by, mimetype, data + ($1, $2, $3, $4, $5, $6) RETURNING hash, created_at, created_by, mimetype, data, id ` type InsertFileParams struct { + ID uuid.UUID `db:"id" json:"id"` Hash string `db:"hash" json:"hash"` CreatedAt time.Time `db:"created_at" json:"created_at"` CreatedBy uuid.UUID `db:"created_by" json:"created_by"` @@ -688,6 +722,7 @@ type InsertFileParams struct { func (q *sqlQuerier) InsertFile(ctx context.Context, arg InsertFileParams) (File, error) { row := q.db.QueryRowContext(ctx, insertFile, + arg.ID, arg.Hash, arg.CreatedAt, arg.CreatedBy, @@ -701,6 +736,7 @@ func (q *sqlQuerier) InsertFile(ctx context.Context, arg InsertFileParams) (File &i.CreatedBy, &i.Mimetype, &i.Data, + &i.ID, ) return i, err } @@ -2237,7 +2273,7 @@ WHERE SKIP LOCKED LIMIT 1 - ) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, storage_source, type, input, worker_id + ) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id ` type AcquireProvisionerJobParams struct { @@ -2267,17 +2303,17 @@ func (q *sqlQuerier) AcquireProvisionerJob(ctx context.Context, arg AcquireProvi &i.InitiatorID, &i.Provisioner, &i.StorageMethod, - &i.StorageSource, &i.Type, &i.Input, &i.WorkerID, + &i.FileID, ) return i, err } const getProvisionerJobByID = `-- name: GetProvisionerJobByID :one SELECT - id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, storage_source, type, input, worker_id + id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id FROM provisioner_jobs WHERE @@ -2299,17 +2335,17 @@ func (q *sqlQuerier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (P &i.InitiatorID, &i.Provisioner, &i.StorageMethod, - &i.StorageSource, &i.Type, &i.Input, &i.WorkerID, + &i.FileID, ) return i, err } const getProvisionerJobsByIDs = `-- name: GetProvisionerJobsByIDs :many SELECT - id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, storage_source, type, input, worker_id + id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id FROM provisioner_jobs WHERE @@ -2337,10 +2373,10 @@ func (q *sqlQuerier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUI &i.InitiatorID, &i.Provisioner, &i.StorageMethod, - &i.StorageSource, &i.Type, &i.Input, &i.WorkerID, + &i.FileID, ); err != nil { return nil, err } @@ -2356,7 +2392,7 @@ func (q *sqlQuerier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUI } const getProvisionerJobsCreatedAfter = `-- name: GetProvisionerJobsCreatedAfter :many -SELECT id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, storage_source, type, input, worker_id FROM provisioner_jobs WHERE created_at > $1 +SELECT id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id FROM provisioner_jobs WHERE created_at > $1 ` func (q *sqlQuerier) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt time.Time) ([]ProvisionerJob, error) { @@ -2380,10 +2416,10 @@ func (q *sqlQuerier) GetProvisionerJobsCreatedAfter(ctx context.Context, created &i.InitiatorID, &i.Provisioner, &i.StorageMethod, - &i.StorageSource, &i.Type, &i.Input, &i.WorkerID, + &i.FileID, ); err != nil { return nil, err } @@ -2408,12 +2444,12 @@ INSERT INTO initiator_id, provisioner, storage_method, - storage_source, + file_id, "type", "input" ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, storage_source, type, input, worker_id + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id ` type InsertProvisionerJobParams struct { @@ -2424,7 +2460,7 @@ type InsertProvisionerJobParams struct { InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` Provisioner ProvisionerType `db:"provisioner" json:"provisioner"` StorageMethod ProvisionerStorageMethod `db:"storage_method" json:"storage_method"` - StorageSource string `db:"storage_source" json:"storage_source"` + FileID uuid.UUID `db:"file_id" json:"file_id"` Type ProvisionerJobType `db:"type" json:"type"` Input json.RawMessage `db:"input" json:"input"` } @@ -2438,7 +2474,7 @@ func (q *sqlQuerier) InsertProvisionerJob(ctx context.Context, arg InsertProvisi arg.InitiatorID, arg.Provisioner, arg.StorageMethod, - arg.StorageSource, + arg.FileID, arg.Type, arg.Input, ) @@ -2455,10 +2491,10 @@ func (q *sqlQuerier) InsertProvisionerJob(ctx context.Context, arg InsertProvisi &i.InitiatorID, &i.Provisioner, &i.StorageMethod, - &i.StorageSource, &i.Type, &i.Input, &i.WorkerID, + &i.FileID, ) return i, err } diff --git a/coderd/database/queries/files.sql b/coderd/database/queries/files.sql index a91513d31a345..1f54386bb3ac4 100644 --- a/coderd/database/queries/files.sql +++ b/coderd/database/queries/files.sql @@ -1,15 +1,28 @@ --- name: GetFileByHash :one +-- name: GetFileByID :one +SELECT + * +FROM + files +WHERE + id = $1 +LIMIT + 1; + +-- name: GetFileByHashAndCreator :one SELECT * FROM files WHERE hash = $1 +AND + created_by = $2 LIMIT 1; + -- name: InsertFile :one INSERT INTO - files (hash, created_at, created_by, mimetype, "data") + files (id, hash, created_at, created_by, mimetype, "data") VALUES - ($1, $2, $3, $4, $5) RETURNING *; + ($1, $2, $3, $4, $5, $6) RETURNING *; diff --git a/coderd/database/queries/provisionerjobs.sql b/coderd/database/queries/provisionerjobs.sql index 4775d574e2713..027bd25bc91b3 100644 --- a/coderd/database/queries/provisionerjobs.sql +++ b/coderd/database/queries/provisionerjobs.sql @@ -59,7 +59,7 @@ INSERT INTO initiator_id, provisioner, storage_method, - storage_source, + file_id, "type", "input" ) diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index 44cfd89d43284..b4263c09b4762 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -6,6 +6,7 @@ type UniqueConstraint string // UniqueConstraint enums. const ( + UniqueFilesHashCreatedByKey UniqueConstraint = "files_hash_created_by_key" // ALTER TABLE ONLY files ADD CONSTRAINT files_hash_created_by_key UNIQUE (hash, created_by); UniqueGroupMembersUserIDGroupIDKey UniqueConstraint = "group_members_user_id_group_id_key" // ALTER TABLE ONLY group_members ADD CONSTRAINT group_members_user_id_group_id_key UNIQUE (user_id, group_id); UniqueGroupsNameOrganizationIDKey UniqueConstraint = "groups_name_organization_id_key" // ALTER TABLE ONLY groups ADD CONSTRAINT groups_name_organization_id_key UNIQUE (name, organization_id); UniqueLicensesJWTKey UniqueConstraint = "licenses_jwt_key" // ALTER TABLE ONLY licenses ADD CONSTRAINT licenses_jwt_key UNIQUE (jwt); diff --git a/coderd/files.go b/coderd/files.go index a0b7be2c8be5d..2c304921f8226 100644 --- a/coderd/files.go +++ b/coderd/files.go @@ -10,6 +10,7 @@ import ( "net/http" "github.com/go-chi/chi/v5" + "github.com/google/uuid" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" @@ -50,15 +51,20 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { } hashBytes := sha256.Sum256(data) hash := hex.EncodeToString(hashBytes[:]) - file, err := api.Database.GetFileByHash(ctx, hash) + file, err := api.Database.GetFileByHashAndCreator(ctx, database.GetFileByHashAndCreatorParams{ + Hash: hash, + CreatedBy: apiKey.UserID, + }) if err == nil { // The file already exists! httpapi.Write(ctx, rw, http.StatusOK, codersdk.UploadResponse{ - Hash: file.Hash, + ID: file.ID, }) return } + id := uuid.New() file, err = api.Database.InsertFile(ctx, database.InsertFileParams{ + ID: id, Hash: hash, CreatedBy: apiKey.UserID, CreatedAt: database.Now(), @@ -74,20 +80,30 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) { } httpapi.Write(ctx, rw, http.StatusCreated, codersdk.UploadResponse{ - Hash: file.Hash, + ID: file.ID, }) } -func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) { +func (api *API) fileByID(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - hash := chi.URLParam(r, "hash") - if hash == "" { + + fileID := chi.URLParam(r, "fileID") + if fileID == "" { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "File hash must be provided in url.", + Message: "File id must be provided in url.", }) return } - file, err := api.Database.GetFileByHash(ctx, hash) + + id, err := uuid.Parse(fileID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "File id must be a valid UUID.", + }) + return + } + + file, err := api.Database.GetFileByID(ctx, id) if errors.Is(err, sql.ErrNoRows) { httpapi.ResourceNotFound(rw) return diff --git a/coderd/files_test.go b/coderd/files_test.go index 28d4a33104a5a..b3a3953a43972 100644 --- a/coderd/files_test.go +++ b/coderd/files_test.go @@ -5,6 +5,7 @@ import ( "net/http" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/coderdtest" @@ -64,7 +65,7 @@ func TestDownload(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - _, _, err := client.Download(ctx, "something") + _, _, err := client.Download(ctx, uuid.New()) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) @@ -80,7 +81,7 @@ func TestDownload(t *testing.T) { resp, err := client.Upload(ctx, codersdk.ContentTypeTar, make([]byte, 1024)) require.NoError(t, err) - data, contentType, err := client.Download(ctx, resp.Hash) + data, contentType, err := client.Download(ctx, resp.ID) require.NoError(t, err) require.Len(t, data, 1024) require.Equal(t, codersdk.ContentTypeTar, contentType) diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index bef1110ec3bb0..1863de54c6b98 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -315,7 +315,7 @@ func (server *provisionerdServer) AcquireJob(ctx context.Context, _ *proto.Empty } switch job.StorageMethod { case database.ProvisionerStorageMethodFile: - file, err := server.Database.GetFileByHash(ctx, job.StorageSource) + file, err := server.Database.GetFileByID(ctx, job.FileID) if err != nil { return nil, failJob(fmt.Sprintf("get file by hash: %s", err)) } diff --git a/coderd/provisionerdaemons_test.go b/coderd/provisionerdaemons_test.go index 8fe48314802c5..d3b0be35cd020 100644 --- a/coderd/provisionerdaemons_test.go +++ b/coderd/provisionerdaemons_test.go @@ -33,11 +33,11 @@ func TestProvisionerDaemons(t *testing.T) { resp, err := client.Upload(ctx, codersdk.ContentTypeTar, data) require.NoError(t, err) - t.Log(resp.Hash) + t.Log(resp.ID) version, err := client.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: resp.Hash, + FileID: resp.ID, Provisioner: codersdk.ProvisionerTypeEcho, }) require.NoError(t, err) diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go index e2784112245cf..294b013e00280 100644 --- a/coderd/provisionerjobs.go +++ b/coderd/provisionerjobs.go @@ -316,10 +316,10 @@ func convertProvisionerJobLog(provisionerJobLog database.ProvisionerJobLog) code func convertProvisionerJob(provisionerJob database.ProvisionerJob) codersdk.ProvisionerJob { job := codersdk.ProvisionerJob{ - ID: provisionerJob.ID, - CreatedAt: provisionerJob.CreatedAt, - Error: provisionerJob.Error.String, - StorageSource: provisionerJob.StorageSource, + ID: provisionerJob.ID, + CreatedAt: provisionerJob.CreatedAt, + Error: provisionerJob.Error.String, + FileID: provisionerJob.FileID, } // Applying values optional to the struct. if provisionerJob.StartedAt.Valid { diff --git a/coderd/templates.go b/coderd/templates.go index 3366e02b14216..fbc25a7b5546e 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -600,6 +600,7 @@ func (api *API) autoImportTemplate(ctx context.Context, opts autoImportTemplateO now = database.Now() ) file, err := tx.InsertFile(ctx, database.InsertFileParams{ + ID: uuid.New(), Hash: hex.EncodeToString(hash[:]), CreatedAt: now, CreatedBy: opts.userID, @@ -639,7 +640,7 @@ func (api *API) autoImportTemplate(ctx context.Context, opts autoImportTemplateO InitiatorID: opts.userID, Provisioner: database.ProvisionerTypeTerraform, StorageMethod: database.ProvisionerStorageMethodFile, - StorageSource: file.Hash, + FileID: file.ID, Type: database.ProvisionerJobTypeTemplateVersionImport, Input: []byte{'{', '}'}, }) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 4d5b54b9933cd..ccc1cd976111b 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -285,7 +285,7 @@ func (api *API) postTemplateVersionDryRun(rw http.ResponseWriter, r *http.Reques InitiatorID: apiKey.UserID, Provisioner: job.Provisioner, StorageMethod: job.StorageMethod, - StorageSource: job.StorageSource, + FileID: job.FileID, Type: database.ProvisionerJobTypeTemplateVersionDryRun, Input: input, }) @@ -717,7 +717,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht return } - file, err := api.Database.GetFileByHash(ctx, req.StorageSource) + file, err := api.Database.GetFileByID(ctx, req.FileID) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ Message: "File not found.", @@ -732,12 +732,10 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht return } - // TODO(JonA): Readd this check once we update the unique constraint - // on files to be owner + hash. - // if !api.Authorize(r, rbac.ActionRead, file) { - // httpapi.ResourceNotFound(rw) - // return - // } + if !api.Authorize(r, rbac.ActionRead, file) { + httpapi.ResourceNotFound(rw) + return + } var templateVersion database.TemplateVersion var provisionerJob database.ProvisionerJob @@ -814,7 +812,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht InitiatorID: apiKey.UserID, Provisioner: database.ProvisionerType(req.Provisioner), StorageMethod: database.ProvisionerStorageMethodFile, - StorageSource: file.Hash, + FileID: file.ID, Type: database.ProvisionerJobTypeTemplateVersionImport, Input: []byte{'{', '}'}, }) diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 5521be8ec339c..1bcb5ba8a0aaa 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -66,7 +66,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) { _, err := client.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ TemplateID: templateID, StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: "hash", + FileID: uuid.New(), Provisioner: codersdk.ProvisionerTypeEcho, }) var apiErr *codersdk.Error @@ -84,7 +84,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) { _, err := client.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: "hash", + FileID: uuid.New(), Provisioner: codersdk.ProvisionerTypeEcho, }) var apiErr *codersdk.Error @@ -112,7 +112,7 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) { version, err := client.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ Name: "bananas", StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: file.Hash, + FileID: file.ID, Provisioner: codersdk.ProvisionerTypeEcho, ParameterValues: []codersdk.CreateParameterRequest{{ Name: "example", @@ -842,7 +842,7 @@ func TestPaginatedTemplateVersions(t *testing.T) { eg.Go(func() error { templateVersion, err := client.CreateTemplateVersion(egCtx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ TemplateID: template.ID, - StorageSource: file.Hash, + FileID: file.ID, StorageMethod: codersdk.ProvisionerStorageMethodFile, Provisioner: codersdk.ProvisionerTypeEcho, }) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index b2d8424907df4..6881137cf8727 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -458,7 +458,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { Provisioner: template.Provisioner, Type: database.ProvisionerJobTypeWorkspaceBuild, StorageMethod: templateVersionJob.StorageMethod, - StorageSource: templateVersionJob.StorageSource, + FileID: templateVersionJob.FileID, Input: input, }) if err != nil { diff --git a/coderd/workspaces.go b/coderd/workspaces.go index c96967b1266ed..d847af70328f4 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -435,7 +435,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req Provisioner: template.Provisioner, Type: database.ProvisionerJobTypeWorkspaceBuild, StorageMethod: templateVersionJob.StorageMethod, - StorageSource: templateVersionJob.StorageSource, + FileID: templateVersionJob.FileID, Input: input, }) if err != nil { diff --git a/codersdk/files.go b/codersdk/files.go index 52fcf0215081b..6d728fbfbcab2 100644 --- a/codersdk/files.go +++ b/codersdk/files.go @@ -6,6 +6,8 @@ import ( "fmt" "io" "net/http" + + "github.com/google/uuid" ) const ( @@ -14,7 +16,7 @@ const ( // UploadResponse contains the hash to reference the uploaded file. type UploadResponse struct { - Hash string `json:"hash"` + ID uuid.UUID `json:"hash"` } // Upload uploads an arbitrary file with the content type provided. @@ -35,8 +37,8 @@ func (c *Client) Upload(ctx context.Context, contentType string, content []byte) } // Download fetches a file by uploaded hash. -func (c *Client) Download(ctx context.Context, hash string) ([]byte, string, error) { - res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/files/%s", hash), nil) +func (c *Client) Download(ctx context.Context, id uuid.UUID) ([]byte, string, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/files/%s", id.String()), nil) if err != nil { return nil, "", err } diff --git a/codersdk/organizations.go b/codersdk/organizations.go index 003b9156dd7c6..de5e42122ce28 100644 --- a/codersdk/organizations.go +++ b/codersdk/organizations.go @@ -39,7 +39,7 @@ type CreateTemplateVersionRequest struct { TemplateID uuid.UUID `json:"template_id,omitempty"` StorageMethod ProvisionerStorageMethod `json:"storage_method" validate:"oneof=file,required"` - StorageSource string `json:"storage_source" validate:"required"` + FileID uuid.UUID `json:"file_id" validate:"required"` Provisioner ProvisionerType `json:"provisioner" validate:"oneof=terraform echo,required"` // ParameterValues allows for additional parameters to be provided // during the dry-run provision stage. diff --git a/codersdk/provisionerdaemons.go b/codersdk/provisionerdaemons.go index 5eb8872fe6620..adce0321be691 100644 --- a/codersdk/provisionerdaemons.go +++ b/codersdk/provisionerdaemons.go @@ -64,15 +64,15 @@ const ( ) type ProvisionerJob struct { - ID uuid.UUID `json:"id"` - CreatedAt time.Time `json:"created_at"` - StartedAt *time.Time `json:"started_at,omitempty"` - CompletedAt *time.Time `json:"completed_at,omitempty"` - CanceledAt *time.Time `json:"canceled_at,omitempty"` - Error string `json:"error,omitempty"` - Status ProvisionerJobStatus `json:"status"` - WorkerID *uuid.UUID `json:"worker_id,omitempty"` - StorageSource string `json:"storage_source"` + ID uuid.UUID `json:"id"` + CreatedAt time.Time `json:"created_at"` + StartedAt *time.Time `json:"started_at,omitempty"` + CompletedAt *time.Time `json:"completed_at,omitempty"` + CanceledAt *time.Time `json:"canceled_at,omitempty"` + Error string `json:"error,omitempty"` + Status ProvisionerJobStatus `json:"status"` + WorkerID *uuid.UUID `json:"worker_id,omitempty"` + FileID uuid.UUID `json:"file_id"` } type ProvisionerJobLog struct { diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index b4f96eb61009e..fe6dd6f687f8c 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -286,7 +286,7 @@ func TestTemplateACL(t *testing.T) { _, err = client1.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ Name: "testme", TemplateID: template.ID, - StorageSource: file.Hash, + FileID: file.ID, StorageMethod: codersdk.ProvisionerStorageMethodFile, Provisioner: codersdk.ProvisionerTypeEcho, }) @@ -302,7 +302,7 @@ func TestTemplateACL(t *testing.T) { _, err = client1.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ Name: "testme", TemplateID: template.ID, - StorageSource: file.Hash, + FileID: file.ID, StorageMethod: codersdk.ProvisionerStorageMethodFile, Provisioner: codersdk.ProvisionerTypeEcho, }) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 92088b978f77a..09c2da80fb9e2 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -206,7 +206,7 @@ export interface CreateTemplateVersionRequest { readonly name?: string readonly template_id?: string readonly storage_method: ProvisionerStorageMethod - readonly storage_source: string + readonly file_id: string readonly provisioner: ProvisionerType readonly parameter_values?: CreateParameterRequest[] } @@ -504,7 +504,7 @@ export interface ProvisionerJob { readonly error?: string readonly status: ProvisionerJobStatus readonly worker_id?: string - readonly storage_source: string + readonly file_id: string } // From codersdk/provisionerdaemons.go diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index fe779f3789bdf..b7c7750158130 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -132,7 +132,7 @@ export const MockProvisionerJob: TypesGen.ProvisionerJob = { created_at: "", id: "test-provisioner-job", status: "succeeded", - storage_source: "asdf", + file_id: "fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0", completed_at: "2022-05-17T17:39:01.382927298Z", }