From 6c2fc74970e0218f5671dd7c286e9922ade8e74b Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 17 Jun 2022 14:49:03 +0300 Subject: [PATCH 1/6] chore: Add failing test --- cli/templatecreate_test.go | 50 ++++++++++++++++++++++++++++++++++++++ cli/templatedelete.go | 2 +- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index bbafcbecb32b1..67b2cab2fb9bc 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -197,6 +197,56 @@ func TestTemplateCreate(t *testing.T) { require.EqualError(t, <-execDone, "Parameter value absent in parameter file for \"region\"!") removeTmpDirUntilSuccess(t, tempDir) }) + + t.Run("Recreate template with same name (create, delete, create)", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + coderdtest.CreateFirstUser(t, client) + + create := func() error { + source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ + Parse: echo.ParseComplete, + Provision: provisionCompleteWithAgent, + }) + args := []string{ + "templates", + "create", + "my-template", + "--yes", + "--directory", source, + "--test.provisioner", string(database.ProvisionerTypeEcho), + } + cmd, root := clitest.New(t, args...) + clitest.SetupConfig(t, client, root) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + cmd.SetErr(pty.Output()) + + return cmd.Execute() + } + del := func() error { + args := []string{ + "templates", + "delete", + "my-template", + } + cmd, root := clitest.New(t, args...) + clitest.SetupConfig(t, client, root) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + + return cmd.Execute() + } + + err := create() + require.NoError(t, err, "Template must be created without error") + err = del() + require.NoError(t, err, "Template must be deleted without error") + err = create() + require.NoError(t, err, "Template must be recreated without error") + }) } func createTestParseResponse() []*proto.Parse_Response { diff --git a/cli/templatedelete.go b/cli/templatedelete.go index 26ba6ca2a01f1..e698335c672a6 100644 --- a/cli/templatedelete.go +++ b/cli/templatedelete.go @@ -76,7 +76,7 @@ func templateDelete() *cobra.Command { return xerrors.Errorf("delete template %q: %w", template.Name, err) } - _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Deleted template "+cliui.Styles.Code.Render(template.Name)+"!") + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Deleted template "+cliui.Styles.Code.Render(template.Name)+"!") } return nil From b1f6ec691861e982b10b3ed045d8b40f278dea40 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 17 Jun 2022 15:08:20 +0300 Subject: [PATCH 2/6] fix: Allow template names to be re-used after deletion Fixes #2152 --- coderd/database/dump.sql | 5 +---- .../migrations/000025_allow_template_name_re_use.down.sql | 4 ++++ .../migrations/000025_allow_template_name_re_use.up.sql | 4 ++++ 3 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 coderd/database/migrations/000025_allow_template_name_re_use.down.sql create mode 100644 coderd/database/migrations/000025_allow_template_name_re_use.up.sql diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index b479dc2315a99..77316f4789ecc 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -392,9 +392,6 @@ ALTER TABLE ONLY template_versions ALTER TABLE ONLY template_versions ADD CONSTRAINT template_versions_template_id_name_key UNIQUE (template_id, name); -ALTER TABLE ONLY templates - ADD CONSTRAINT templates_organization_id_name_key UNIQUE (organization_id, name); - ALTER TABLE ONLY templates ADD CONSTRAINT templates_pkey PRIMARY KEY (id); @@ -446,7 +443,7 @@ 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_templates_name_lower ON templates USING btree (lower((name)::text)); +CREATE UNIQUE INDEX idx_templates_name_lower ON templates USING btree (lower((name)::text)) WHERE (deleted = false); CREATE UNIQUE INDEX idx_users_email ON users USING btree (email); diff --git a/coderd/database/migrations/000025_allow_template_name_re_use.down.sql b/coderd/database/migrations/000025_allow_template_name_re_use.down.sql new file mode 100644 index 0000000000000..3c7e8304be7de --- /dev/null +++ b/coderd/database/migrations/000025_allow_template_name_re_use.down.sql @@ -0,0 +1,4 @@ +DROP INDEX idx_templates_name_lower; +CREATE UNIQUE INDEX idx_templates_name_lower ON templates USING btree (lower(name)); + +ALTER TABLE ONLY templates ADD CONSTRAINT templates_organization_id_name_key UNIQUE (organization_id, name); diff --git a/coderd/database/migrations/000025_allow_template_name_re_use.up.sql b/coderd/database/migrations/000025_allow_template_name_re_use.up.sql new file mode 100644 index 0000000000000..57496868c4add --- /dev/null +++ b/coderd/database/migrations/000025_allow_template_name_re_use.up.sql @@ -0,0 +1,4 @@ +DROP INDEX idx_templates_name_lower; +CREATE UNIQUE INDEX idx_templates_name_lower ON templates USING btree (lower(name)) WHERE deleted = false; + +ALTER TABLE ONLY templates DROP CONSTRAINT templates_organization_id_name_key; From 7b6af6ef2718d130ce49b7d520fb62be4cc3f190 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 17 Jun 2022 16:26:12 +0300 Subject: [PATCH 3/6] fix: Discard ptytest output to avoid issues on macOS --- cli/templatecreate_test.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index 67b2cab2fb9bc..3f1904e5fd6c9 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -1,6 +1,7 @@ package cli_test import ( + "io" "os" "testing" @@ -218,10 +219,8 @@ func TestTemplateCreate(t *testing.T) { } cmd, root := clitest.New(t, args...) clitest.SetupConfig(t, client, root) - pty := ptytest.New(t) - cmd.SetIn(pty.Input()) - cmd.SetOut(pty.Output()) - cmd.SetErr(pty.Output()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) return cmd.Execute() } @@ -233,9 +232,8 @@ func TestTemplateCreate(t *testing.T) { } cmd, root := clitest.New(t, args...) clitest.SetupConfig(t, client, root) - pty := ptytest.New(t) - cmd.SetIn(pty.Input()) - cmd.SetOut(pty.Output()) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) return cmd.Execute() } From 1fa648909e55a98ce547bab26a6e60f04b19d927 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 17 Jun 2022 20:43:53 +0300 Subject: [PATCH 4/6] fix: Consolidate idx_templates_name_lower and templates_organization_id_name_idx --- .../migrations/000025_allow_template_name_re_use.down.sql | 3 ++- .../migrations/000025_allow_template_name_re_use.up.sql | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/coderd/database/migrations/000025_allow_template_name_re_use.down.sql b/coderd/database/migrations/000025_allow_template_name_re_use.down.sql index 3c7e8304be7de..8b58f9fdcac21 100644 --- a/coderd/database/migrations/000025_allow_template_name_re_use.down.sql +++ b/coderd/database/migrations/000025_allow_template_name_re_use.down.sql @@ -1,4 +1,5 @@ -DROP INDEX idx_templates_name_lower; +DROP INDEX templates_organization_id_name_idx; +CREATE UNIQUE INDEX templates_organization_id_name_idx ON templates USING btree (organization_id, name) WHERE deleted = false; CREATE UNIQUE INDEX idx_templates_name_lower ON templates USING btree (lower(name)); ALTER TABLE ONLY templates ADD CONSTRAINT templates_organization_id_name_key UNIQUE (organization_id, name); diff --git a/coderd/database/migrations/000025_allow_template_name_re_use.up.sql b/coderd/database/migrations/000025_allow_template_name_re_use.up.sql index 57496868c4add..7aebd5ca751f3 100644 --- a/coderd/database/migrations/000025_allow_template_name_re_use.up.sql +++ b/coderd/database/migrations/000025_allow_template_name_re_use.up.sql @@ -1,4 +1,5 @@ DROP INDEX idx_templates_name_lower; -CREATE UNIQUE INDEX idx_templates_name_lower ON templates USING btree (lower(name)) WHERE deleted = false; +DROP INDEX templates_organization_id_name_idx; +CREATE UNIQUE INDEX templates_organization_id_name_idx ON templates (organization_id, lower(name)) WHERE deleted = false; ALTER TABLE ONLY templates DROP CONSTRAINT templates_organization_id_name_key; From 3ad2f505831a23707c72e5d17dd4f28cf507722a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 17 Jun 2022 20:57:19 +0300 Subject: [PATCH 5/6] chore: Run make gen --- coderd/database/dump.sql | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 77316f4789ecc..45245bc1e3a3d 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -443,13 +443,11 @@ 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_templates_name_lower ON templates USING btree (lower((name)::text)) WHERE (deleted = false); - CREATE UNIQUE INDEX idx_users_email ON users USING btree (email); CREATE UNIQUE INDEX idx_users_username ON users USING btree (username); -CREATE UNIQUE INDEX templates_organization_id_name_idx ON templates USING btree (organization_id, name) WHERE (deleted = false); +CREATE UNIQUE INDEX templates_organization_id_name_idx ON templates USING btree (organization_id, lower((name)::text)) WHERE (deleted = false); CREATE UNIQUE INDEX users_username_lower_idx ON users USING btree (lower(username)); From cc3f4fb2002c1cdb36970e2821ee8fee5ee19f8d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 17 Jun 2022 21:31:27 +0300 Subject: [PATCH 6/6] chore: Rename conflicting migration --- ...re_use.down.sql => 000026_allow_template_name_re_use.down.sql} | 0 ...ame_re_use.up.sql => 000026_allow_template_name_re_use.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000025_allow_template_name_re_use.down.sql => 000026_allow_template_name_re_use.down.sql} (100%) rename coderd/database/migrations/{000025_allow_template_name_re_use.up.sql => 000026_allow_template_name_re_use.up.sql} (100%) diff --git a/coderd/database/migrations/000025_allow_template_name_re_use.down.sql b/coderd/database/migrations/000026_allow_template_name_re_use.down.sql similarity index 100% rename from coderd/database/migrations/000025_allow_template_name_re_use.down.sql rename to coderd/database/migrations/000026_allow_template_name_re_use.down.sql diff --git a/coderd/database/migrations/000025_allow_template_name_re_use.up.sql b/coderd/database/migrations/000026_allow_template_name_re_use.up.sql similarity index 100% rename from coderd/database/migrations/000025_allow_template_name_re_use.up.sql rename to coderd/database/migrations/000026_allow_template_name_re_use.up.sql