coder · sreya · Oct 13, 2022 · Oct 12, 2022 · Oct 12, 2022 · Oct 12, 2022
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/coderd/database/migrations/000059_file_id.down.sql 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
@@ -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
diff --git a/coderd/database/querier.go b/coderd/database/querier.go