From 86f5808d150b07c421b820b577fdaa60d7868545 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 2 Sep 2022 19:58:24 +0000 Subject: [PATCH 01/16] feat: add resource orphanage --- provisionersdk/proto/provisioner.pb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provisionersdk/proto/provisioner.pb.go b/provisionersdk/proto/provisioner.pb.go index 836426f24b3f1..bce3f6e7927ad 100644 --- a/provisionersdk/proto/provisioner.pb.go +++ b/provisionersdk/proto/provisioner.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: // protoc-gen-go v1.26.0 -// protoc v3.21.5 +// protoc v3.6.1 // source: provisionersdk/proto/provisioner.proto package proto From 60059ba8fe8041be16d0ea29dd94ef81748e63c4 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 2 Sep 2022 20:32:06 +0000 Subject: [PATCH 02/16] feat: deny custom state in build for regular users --- coderd/workspacebuilds.go | 11 +++++++ coderd/workspacebuilds_test.go | 55 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index ac4d3962e2fe9..d6289a8ab9d36 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -372,6 +372,17 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { return } + // If custom state, deny request since user could be orphaning their + // cloud resources. + if createBuild.ProvisionerState != nil { + if !api.Authorize(r, rbac.ActionUpdate, template.RBACObject()) { + httpapi.Write(rw, http.StatusForbidden, codersdk.Response{ + Message: "Only template managers may provide custom state", + }) + return + } + } + // Store prior build number to compute new build number var priorBuildNum int32 priorHistory, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index 6ce06e12736cc..eecc969152a73 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -2,6 +2,7 @@ package coderd_test import ( "context" + "errors" "fmt" "net/http" "strconv" @@ -164,6 +165,60 @@ func TestWorkspaceBuilds(t *testing.T) { require.NoError(t, err) }) + t.Run("OrphanNotOwner", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + first := coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + + regularUser := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + + workspace := coderdtest.CreateWorkspace(t, regularUser, first.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, regularUser, workspace.LatestBuild.ID) + + _, err := regularUser.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: workspace.LatestBuild.TemplateVersionID, + Transition: workspace.LatestBuild.Transition, + ProvisionerState: []byte(" "), + }) + require.Error(t, err) + + var cerr *codersdk.Error + require.True(t, errors.As(err, &cerr)) + + code := cerr.StatusCode() + require.Equal(t, http.StatusForbidden, code, "unexpected status %s", http.StatusText(code)) + }) + + t.Run("Orphan", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + first := coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + + workspace := coderdtest.CreateWorkspace(t, client, first.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + _, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: workspace.LatestBuild.TemplateVersionID, + Transition: workspace.LatestBuild.Transition, + ProvisionerState: []byte(" "), + }) + require.Nil(t, err) + }) + t.Run("PaginateNonExistentRow", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) From eba7329219eb1a895259f4a6ed8ac90a82043ad7 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 2 Sep 2022 21:25:22 +0000 Subject: [PATCH 03/16] Add orphanage to CLI --- cli/delete.go | 27 +++++++- cli/delete_test.go | 28 ++++++++ coderd/workspacebuilds_test.go | 114 +++++++++++++++++---------------- codersdk/workspaces.go | 20 ++++++ codersdk/workspaces_test.go | 41 ++++++++++++ 5 files changed, 175 insertions(+), 55 deletions(-) create mode 100644 codersdk/workspaces_test.go diff --git a/cli/delete.go b/cli/delete.go index 37134880ce108..ae400bc30c283 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -12,6 +12,7 @@ import ( // nolint func deleteWorkspace() *cobra.Command { + var orphan bool cmd := &cobra.Command{ Annotations: workspaceCommand, Use: "delete ", @@ -36,9 +37,29 @@ func deleteWorkspace() *cobra.Command { if err != nil { return err } + + var state []byte + if orphan { + cliui.Warn(cmd.ErrOrStderr(), "Orphaning workspace", + "Template edit permission is required to orphan workspaces.", + ) + + state, err = client.WorkspaceBuildState(cmd.Context(), workspace.LatestBuild.ID) + if err != nil { + return err + } + + state, err = codersdk.OrphanTerraformState(state) + if err != nil { + return err + } + fmt.Printf("new state: %s\n", state) + } + before := time.Now() build, err := client.CreateWorkspaceBuild(cmd.Context(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{ - Transition: codersdk.WorkspaceTransitionDelete, + Transition: codersdk.WorkspaceTransitionDelete, + ProvisionerState: state, }) if err != nil { return err @@ -53,6 +74,10 @@ func deleteWorkspace() *cobra.Command { return nil }, } + cmd.Flags().BoolVar(&orphan, "orphan", false, + `Delete workspace without deleting its resources. This can delete a +workspace in a broken state, but may also lead to unaccounted cloud resources.`, + ) cliui.AllowSkipPrompt(cmd) return cmd } diff --git a/cli/delete_test.go b/cli/delete_test.go index a2ea15fd18121..d3f6b5cc07756 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -43,6 +43,34 @@ func TestDelete(t *testing.T) { <-doneChan }) + t.Run("Orphan", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + cmd, root := clitest.New(t, "delete", workspace.Name, "-y", "--orphan") + + clitest.SetupConfig(t, client, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + go func() { + defer close(doneChan) + err := cmd.Execute() + // When running with the race detector on, we sometimes get an EOF. + if err != nil { + assert.ErrorIs(t, err, io.EOF) + } + }() + pty.ExpectMatch("Cleaning Up") + <-doneChan + }) + t.Run("DifferentUser", func(t *testing.T) { t.Parallel() adminClient := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index eecc969152a73..c90982294d74f 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -165,60 +165,6 @@ func TestWorkspaceBuilds(t *testing.T) { require.NoError(t, err) }) - t.Run("OrphanNotOwner", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - first := coderdtest.CreateFirstUser(t, client) - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil) - template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - - regularUser := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) - - workspace := coderdtest.CreateWorkspace(t, regularUser, first.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, regularUser, workspace.LatestBuild.ID) - - _, err := regularUser.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ - TemplateVersionID: workspace.LatestBuild.TemplateVersionID, - Transition: workspace.LatestBuild.Transition, - ProvisionerState: []byte(" "), - }) - require.Error(t, err) - - var cerr *codersdk.Error - require.True(t, errors.As(err, &cerr)) - - code := cerr.StatusCode() - require.Equal(t, http.StatusForbidden, code, "unexpected status %s", http.StatusText(code)) - }) - - t.Run("Orphan", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - first := coderdtest.CreateFirstUser(t, client) - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil) - template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - - workspace := coderdtest.CreateWorkspace(t, client, first.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - - _, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ - TemplateVersionID: workspace.LatestBuild.TemplateVersionID, - Transition: workspace.LatestBuild.Transition, - ProvisionerState: []byte(" "), - }) - require.Nil(t, err) - }) - t.Run("PaginateNonExistentRow", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) @@ -285,6 +231,66 @@ func TestWorkspaceBuilds(t *testing.T) { }) } +func TestWorkspaceBuilds_CustomState(t *testing.T) { + t.Parallel() + + t.Run("Forbidden", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + first := coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + + regularUser := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + + workspace := coderdtest.CreateWorkspace(t, regularUser, first.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, regularUser, workspace.LatestBuild.ID) + + _, err := regularUser.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: workspace.LatestBuild.TemplateVersionID, + Transition: workspace.LatestBuild.Transition, + ProvisionerState: []byte(" "), + }) + require.Error(t, err) + + var cerr *codersdk.Error + require.True(t, errors.As(err, &cerr)) + + code := cerr.StatusCode() + require.Equal(t, http.StatusForbidden, code, "unexpected status %s", http.StatusText(code)) + }) + + t.Run("Success", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + first := coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + + workspace := coderdtest.CreateWorkspace(t, client, first.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: workspace.LatestBuild.TemplateVersionID, + Transition: codersdk.WorkspaceTransitionDelete, + ProvisionerState: []byte(" "), + }) + require.Nil(t, err) + + coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID) + }) +} + func TestPatchCancelWorkspaceBuild(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index a4ab1fd54dee9..cc31aad95eeab 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -33,6 +33,26 @@ type Workspace struct { LastUsedAt time.Time `json:"last_used_at"` } +// OrphanTerraformState removes all the resources from the provided Terraform +// state. When the new state is used, Terraform will operate as if none +// of the resources in the original state exist. +func OrphanTerraformState(state []byte) ([]byte, error) { + stateMap := make(map[string]interface{}) + err := json.Unmarshal(state, &stateMap) + if err != nil { + return nil, err + } + + _, ok := stateMap["resources"] + if !ok { + return nil, xerrors.Errorf("no resources detected, is this terraform state?") + } + + stateMap["resources"] = []int{} + + return json.Marshal(stateMap) +} + // CreateWorkspaceBuildRequest provides options to update the latest workspace build. type CreateWorkspaceBuildRequest struct { TemplateVersionID uuid.UUID `json:"template_version_id,omitempty"` diff --git a/codersdk/workspaces_test.go b/codersdk/workspaces_test.go new file mode 100644 index 0000000000000..771b283ec21ef --- /dev/null +++ b/codersdk/workspaces_test.go @@ -0,0 +1,41 @@ +package codersdk_test + +import ( + "reflect" + "testing" + + "github.com/coder/coder/codersdk" +) + +func TestOrphanTerraformState(t *testing.T) { + t.Parallel() + + type args struct { + state []byte + } + tests := []struct { + name string + args args + want []byte + wantErr bool + }{ + {"invalid json", args{[]byte("---")}, nil, true}, + {"no resources", args{[]byte(`{"a":4}`)}, nil, true}, + {"some resources", args{[]byte(`{"a":4, "resources":[1, 2, 3]}`)}, []byte(`{"a":4,"resources":[]}`), false}, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := codersdk.OrphanTerraformState(tt.args.state) + if (err != nil) != tt.wantErr { + t.Errorf("OrphanTerraformState() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("OrphanTerraformState() = %s, want %s", got, tt.want) + } + }) + } +} From 68eb021e2eda14ecdbe00cc532f121aa788c0cf7 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 2 Sep 2022 21:37:54 +0000 Subject: [PATCH 04/16] Fix protoc --- .github/workflows/coder.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/coder.yaml b/.github/workflows/coder.yaml index 9f9e6cd8554a0..36beff91b9a32 100644 --- a/.github/workflows/coder.yaml +++ b/.github/workflows/coder.yaml @@ -200,9 +200,10 @@ jobs: set -x cd dogfood DOCKER_BUILDKIT=1 docker build . --target proto -t protoc - protoc_dir=/usr/local/bin/protoc - docker run --rm --entrypoint cat protoc /tmp/bin/protoc > $protoc_dir - chmod +x $protoc_dir + protoc_path=/usr/local/bin/protoc + docker run --rm --entrypoint cat protoc /tmp/bin/protoc > $protoc_path + chmod +x $protoc_path + protoc --version - uses: actions/setup-go@v3 with: go-version: "~1.19" From 38daa9dedc0b52809e5be787e716297955beeba5 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 2 Sep 2022 21:45:27 +0000 Subject: [PATCH 05/16] Reduce metrics refresh intervals in tests These caused spammy debug logs in most tests. --- coderd/coderdtest/coderdtest.go | 7 +++++-- coderd/templates_test.go | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 76cb4098709c2..d8015be21b092 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -77,6 +77,9 @@ type Options struct { // IncludeProvisionerD when true means to start an in-memory provisionerD IncludeProvisionerD bool APIBuilder func(*coderd.Options) *coderd.API + + MetricsCacheRefreshInterval time.Duration + AgentStatsRefreshInterval time.Duration } // New constructs a codersdk client connected to an in-memory API instance. @@ -235,8 +238,8 @@ func newWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *c }, }, AutoImportTemplates: options.AutoImportTemplates, - MetricsCacheRefreshInterval: time.Millisecond * 100, - AgentStatsRefreshInterval: time.Millisecond * 100, + MetricsCacheRefreshInterval: options.MetricsCacheRefreshInterval, + AgentStatsRefreshInterval: options.AgentStatsRefreshInterval, }) t.Cleanup(func() { _ = coderAPI.Close() diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 3eea3a9387a21..29d8354556f4e 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -550,7 +550,9 @@ func TestTemplateDAUs(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerD: true, + IncludeProvisionerD: true, + AgentStatsRefreshInterval: time.Millisecond * 100, + MetricsCacheRefreshInterval: time.Millisecond * 100, }) user := coderdtest.CreateFirstUser(t, client) From b6c351d315ac8e9031a7361add71844d54efa2aa Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 2 Sep 2022 21:50:28 +0000 Subject: [PATCH 06/16] Protoc --- .github/workflows/coder.yaml | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/.github/workflows/coder.yaml b/.github/workflows/coder.yaml index 36beff91b9a32..669f5989c878f 100644 --- a/.github/workflows/coder.yaml +++ b/.github/workflows/coder.yaml @@ -193,17 +193,6 @@ jobs: - name: Install node_modules run: ./scripts/yarn_install.sh - - name: Install Protoc - run: | - # protoc must be in lockstep with our dogfood Dockerfile - # or the version in the comments will differ. - set -x - cd dogfood - DOCKER_BUILDKIT=1 docker build . --target proto -t protoc - protoc_path=/usr/local/bin/protoc - docker run --rm --entrypoint cat protoc /tmp/bin/protoc > $protoc_path - chmod +x $protoc_path - protoc --version - uses: actions/setup-go@v3 with: go-version: "~1.19" @@ -236,6 +225,18 @@ jobs: - name: Install goimports run: go install golang.org/x/tools/cmd/goimports@latest + - name: Install Protoc + run: | + # protoc must be in lockstep with our dogfood Dockerfile + # or the version in the comments will differ. + set -x + cd dogfood + DOCKER_BUILDKIT=1 docker build . --target proto -t protoc + protoc_path=/usr/local/bin/protoc + docker run --rm --entrypoint cat protoc /tmp/bin/protoc > $protoc_path + chmod +x $protoc_path + protoc --version + - name: make gen run: "make --output-sync -j -B gen" From a1859072e7dd52fb82e6a3d5c4d6db5ef3ad9c15 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 2 Sep 2022 21:56:35 +0000 Subject: [PATCH 07/16] Fix tests! --- cli/delete_test.go | 1 + provisionersdk/proto/provisioner.pb.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/delete_test.go b/cli/delete_test.go index d3f6b5cc07756..7ca73a94fd0ab 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -59,6 +59,7 @@ func TestDelete(t *testing.T) { pty := ptytest.New(t) cmd.SetIn(pty.Input()) cmd.SetOut(pty.Output()) + cmd.SetErr(pty.Output()) go func() { defer close(doneChan) err := cmd.Execute() diff --git a/provisionersdk/proto/provisioner.pb.go b/provisionersdk/proto/provisioner.pb.go index bce3f6e7927ad..836426f24b3f1 100644 --- a/provisionersdk/proto/provisioner.pb.go +++ b/provisionersdk/proto/provisioner.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: // protoc-gen-go v1.26.0 -// protoc v3.6.1 +// protoc v3.21.5 // source: provisionersdk/proto/provisioner.proto package proto From 7506ac4f2dfb8882c5298b777b40eabf141f15d7 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 2 Sep 2022 22:42:30 +0000 Subject: [PATCH 08/16] Tests pass! --- cli/delete.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cli/delete.go b/cli/delete.go index ae400bc30c283..9db35b4fd1f21 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -39,6 +39,7 @@ func deleteWorkspace() *cobra.Command { } var state []byte + if orphan { cliui.Warn(cmd.ErrOrStderr(), "Orphaning workspace", "Template edit permission is required to orphan workspaces.", @@ -48,12 +49,13 @@ func deleteWorkspace() *cobra.Command { if err != nil { return err } - - state, err = codersdk.OrphanTerraformState(state) - if err != nil { - return err + // If there's alreay no state, orphanage makes no sense. + if len(state) > 0 { + state, err = codersdk.OrphanTerraformState(state) + if err != nil { + return err + } } - fmt.Printf("new state: %s\n", state) } before := time.Now() From 9968e2f3fa8a398ca67168fcdc6ccfc9579ea994 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 2 Sep 2022 23:20:49 +0000 Subject: [PATCH 09/16] Move Orphan checks into the backend --- cli/delete.go | 15 +----- coderd/coderdtest/coderdtest.go | 8 ++++ coderd/workspacebuilds.go | 48 ++++++++++++++----- coderd/workspacebuilds_test.go | 43 ++++++++++++++--- codersdk/workspaces.go | 22 +-------- provisionersdk/orphan.go | 34 +++++++++++++ .../orphan_test.go | 12 ++--- 7 files changed, 125 insertions(+), 57 deletions(-) create mode 100644 provisionersdk/orphan.go rename codersdk/workspaces_test.go => provisionersdk/orphan_test.go (66%) diff --git a/cli/delete.go b/cli/delete.go index 9db35b4fd1f21..7a71dc50dca33 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -41,27 +41,16 @@ func deleteWorkspace() *cobra.Command { var state []byte if orphan { - cliui.Warn(cmd.ErrOrStderr(), "Orphaning workspace", + cliui.Warn(cmd.ErrOrStderr(), "Orphaning workspace required template edit permission", "Template edit permission is required to orphan workspaces.", ) - - state, err = client.WorkspaceBuildState(cmd.Context(), workspace.LatestBuild.ID) - if err != nil { - return err - } - // If there's alreay no state, orphanage makes no sense. - if len(state) > 0 { - state, err = codersdk.OrphanTerraformState(state) - if err != nil { - return err - } - } } before := time.Now() build, err := client.CreateWorkspaceBuild(cmd.Context(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{ Transition: codersdk.WorkspaceTransitionDelete, ProvisionerState: state, + Orphan: orphan, }) if err != nil { return err diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index d8015be21b092..55a52b77fde43 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -13,6 +13,7 @@ import ( "encoding/base64" "encoding/json" "encoding/pem" + "errors" "fmt" "io" "math/big" @@ -755,3 +756,10 @@ func (r roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { type nopcloser struct{} func (nopcloser) Close() error { return nil } + +// SDKError coerces err into an SDK error. +func SDKError(t *testing.T, err error) *codersdk.Error { + var cerr *codersdk.Error + require.True(t, errors.As(err, &cerr)) + return cerr +} diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index d6289a8ab9d36..2da80628fda68 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -19,6 +19,7 @@ import ( "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" + "github.com/coder/coder/provisionersdk" ) func (api *API) workspaceBuild(rw http.ResponseWriter, r *http.Request) { @@ -372,17 +373,6 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { return } - // If custom state, deny request since user could be orphaning their - // cloud resources. - if createBuild.ProvisionerState != nil { - if !api.Authorize(r, rbac.ActionUpdate, template.RBACObject()) { - httpapi.Write(rw, http.StatusForbidden, codersdk.Response{ - Message: "Only template managers may provide custom state", - }) - return - } - } - // Store prior build number to compute new build number var priorBuildNum int32 priorHistory, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) @@ -404,6 +394,42 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { return } + if createBuild.Orphan { + if createBuild.Transition != codersdk.WorkspaceTransitionDelete { + httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ + Message: "Orphan is only permitted when deleting a workspace.", + Detail: err.Error(), + }) + return + } + if createBuild.ProvisionerState != nil && createBuild.Orphan { + httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ + Message: "ProvisionerState cannot be set alongside Orphan since state intent is unclear.", + }) + return + } + + createBuild.ProvisionerState, err = provisionersdk.OrphanState(priorHistory.ProvisionerState) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to manipulate state.", + Detail: err.Error(), + }) + return + } + } + + // If custom state, deny request since user could be orphaning their + // cloud resources. + if createBuild.ProvisionerState != nil { + if !api.Authorize(r, rbac.ActionUpdate, template.RBACObject()) { + httpapi.Write(rw, http.StatusForbidden, codersdk.Response{ + Message: "Only template managers may provide custom state", + }) + return + } + } + var workspaceBuild database.WorkspaceBuild var provisionerJob database.ProvisionerJob // This must happen in a transaction to ensure history can be inserted, and diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index c90982294d74f..04f47a3c25846 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -231,10 +231,10 @@ func TestWorkspaceBuilds(t *testing.T) { }) } -func TestWorkspaceBuilds_CustomState(t *testing.T) { +func TestWorkspaceBuilds_State(t *testing.T) { t.Parallel() - t.Run("Forbidden", func(t *testing.T) { + t.Run("Permissions", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) first := coderdtest.CreateFirstUser(t, client) @@ -246,12 +246,26 @@ func TestWorkspaceBuilds_CustomState(t *testing.T) { template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, first.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: workspace.LatestBuild.TemplateVersionID, + Transition: codersdk.WorkspaceTransitionDelete, + ProvisionerState: []byte(" "), + }) + require.Nil(t, err) + + coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID) + + // A regular user on the very same template must not be able to modify the + // state. regularUser := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) - workspace := coderdtest.CreateWorkspace(t, regularUser, first.OrganizationID, template.ID) + workspace = coderdtest.CreateWorkspace(t, regularUser, first.OrganizationID, template.ID) coderdtest.AwaitWorkspaceBuildJob(t, regularUser, workspace.LatestBuild.ID) - _, err := regularUser.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + _, err = regularUser.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ TemplateVersionID: workspace.LatestBuild.TemplateVersionID, Transition: workspace.LatestBuild.Transition, ProvisionerState: []byte(" "), @@ -265,7 +279,7 @@ func TestWorkspaceBuilds_CustomState(t *testing.T) { require.Equal(t, http.StatusForbidden, code, "unexpected status %s", http.StatusText(code)) }) - t.Run("Success", func(t *testing.T) { + t.Run("Orphan", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) first := coderdtest.CreateFirstUser(t, client) @@ -280,14 +294,29 @@ func TestWorkspaceBuilds_CustomState(t *testing.T) { workspace := coderdtest.CreateWorkspace(t, client, first.OrganizationID, template.ID) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + // Providing both state and orphan fails. + _, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ TemplateVersionID: workspace.LatestBuild.TemplateVersionID, Transition: codersdk.WorkspaceTransitionDelete, ProvisionerState: []byte(" "), + Orphan: true, }) - require.Nil(t, err) + require.Error(t, err) + cerr := coderdtest.SDKError(t, err) + require.Equal(t, http.StatusBadRequest, cerr.StatusCode()) + // Regular orphan operation succeeds. + build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: workspace.LatestBuild.TemplateVersionID, + Transition: codersdk.WorkspaceTransitionDelete, + Orphan: true, + }) + require.NoError(t, err) coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID) + + _, err = client.Workspace(ctx, workspace.ID) + require.Error(t, err) + require.Equal(t, http.StatusGone, coderdtest.SDKError(t, err).StatusCode()) }) } diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index cc31aad95eeab..7a4cac76380e0 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -33,32 +33,14 @@ type Workspace struct { LastUsedAt time.Time `json:"last_used_at"` } -// OrphanTerraformState removes all the resources from the provided Terraform -// state. When the new state is used, Terraform will operate as if none -// of the resources in the original state exist. -func OrphanTerraformState(state []byte) ([]byte, error) { - stateMap := make(map[string]interface{}) - err := json.Unmarshal(state, &stateMap) - if err != nil { - return nil, err - } - - _, ok := stateMap["resources"] - if !ok { - return nil, xerrors.Errorf("no resources detected, is this terraform state?") - } - - stateMap["resources"] = []int{} - - return json.Marshal(stateMap) -} - // CreateWorkspaceBuildRequest provides options to update the latest workspace build. type CreateWorkspaceBuildRequest struct { TemplateVersionID uuid.UUID `json:"template_version_id,omitempty"` Transition WorkspaceTransition `json:"transition" validate:"oneof=create start stop delete,required"` DryRun bool `json:"dry_run,omitempty"` ProvisionerState []byte `json:"state,omitempty"` + // Orphan may be set for the Destroy transition. + Orphan bool `json:"orphan,omitempty"` // ParameterValues are optional. It will write params to the 'workspace' scope. // This will overwrite any existing parameters with the same name. // This will not delete old params not included in this list. diff --git a/provisionersdk/orphan.go b/provisionersdk/orphan.go new file mode 100644 index 0000000000000..7ed23f3a5f519 --- /dev/null +++ b/provisionersdk/orphan.go @@ -0,0 +1,34 @@ +package provisionersdk + +import ( + "encoding/json" + + "golang.org/x/xerrors" +) + +// OrphanState removes all the resources from the provided +// state. When the new state is used, the provisioner will operate as if none +// of the resources in the original state exist. +func OrphanState(state []byte) ([]byte, error) { + if len(state) == 0 { + // Presume that state is already orphaned, or we're using + // a no-op provisioner. + return state, nil + } + + stateMap := make(map[string]interface{}) + err := json.Unmarshal(state, &stateMap) + if err != nil { + return nil, err + } + + _, ok := stateMap["resources"] + if !ok { + return nil, xerrors.Errorf("no resources detected, is this terraform state?") + } + + // Terraform wants a resources array. + stateMap["resources"] = []struct{}{} + + return json.Marshal(stateMap) +} diff --git a/codersdk/workspaces_test.go b/provisionersdk/orphan_test.go similarity index 66% rename from codersdk/workspaces_test.go rename to provisionersdk/orphan_test.go index 771b283ec21ef..867f55b6c8a55 100644 --- a/codersdk/workspaces_test.go +++ b/provisionersdk/orphan_test.go @@ -1,13 +1,13 @@ -package codersdk_test +package provisionersdk_test import ( "reflect" "testing" - "github.com/coder/coder/codersdk" + "github.com/coder/coder/provisionersdk" ) -func TestOrphanTerraformState(t *testing.T) { +func TestOrphanState(t *testing.T) { t.Parallel() type args struct { @@ -28,13 +28,13 @@ func TestOrphanTerraformState(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, err := codersdk.OrphanTerraformState(tt.args.state) + got, err := provisionersdk.OrphanState(tt.args.state) if (err != nil) != tt.wantErr { - t.Errorf("OrphanTerraformState() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("OrphanState() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("OrphanTerraformState() = %s, want %s", got, tt.want) + t.Errorf("OrphanState() = %s, want %s", got, tt.want) } }) } From 0ba181915914c2e996f07b93a057f930159c4545 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 2 Sep 2022 23:34:48 +0000 Subject: [PATCH 10/16] Run gen --- site/src/api/typesGenerated.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index c94c8b1c173f8..5454dac7f382f 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -170,6 +170,7 @@ export interface CreateWorkspaceBuildRequest { readonly transition: WorkspaceTransition readonly dry_run?: boolean readonly state?: string + readonly orphan?: boolean readonly parameter_values?: CreateParameterRequest[] } From 0695441d1cb90e344a330af6ad11ac176182e1a2 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sat, 3 Sep 2022 22:05:58 +0000 Subject: [PATCH 11/16] Simplify orphan logic --- coderd/workspacebuilds.go | 92 ++++++++++++++++------------------ coderd/workspacebuilds_test.go | 2 +- provisionersdk/orphan.go | 34 ------------- provisionersdk/orphan_test.go | 41 --------------- 4 files changed, 45 insertions(+), 124 deletions(-) delete mode 100644 provisionersdk/orphan.go delete mode 100644 provisionersdk/orphan_test.go diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 2da80628fda68..764127dd47a23 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -19,7 +19,6 @@ import ( "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" - "github.com/coder/coder/provisionersdk" ) func (api *API) workspaceBuild(rw http.ResponseWriter, r *http.Request) { @@ -319,6 +318,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { } createBuild.TemplateVersionID = latestBuild.TemplateVersionID } + templateVersion, err := api.Database.GetTemplateVersionByID(r.Context(), createBuild.TemplateVersionID) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ @@ -337,6 +337,47 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { }) return } + + template, err := api.Database.GetTemplateByID(r.Context(), templateVersion.TemplateID.UUID) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to get template", + Detail: err.Error(), + }) + return + } + + var state []byte + // If custom state, deny request since user could be corrupting or leaking + // cloud state. + if createBuild.ProvisionerState != nil || createBuild.Orphan { + if !api.Authorize(r, rbac.ActionUpdate, template.RBACObject()) { + httpapi.Write(rw, http.StatusForbidden, codersdk.Response{ + Message: "Only template managers may provide custom state", + }) + return + } + state = createBuild.ProvisionerState + } + + if createBuild.Orphan { + if createBuild.Transition != codersdk.WorkspaceTransitionDelete { + httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ + Message: "Orphan is only permitted when deleting a workspace.", + Detail: err.Error(), + }) + return + } + + if createBuild.ProvisionerState != nil && createBuild.Orphan { + httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ + Message: "ProvisionerState cannot be set alongside Orphan since state intent is unclear.", + }) + return + } + state = []byte{} + } + templateVersionJob, err := api.Database.GetProvisionerJobByID(r.Context(), templateVersion.JobID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ @@ -364,15 +405,6 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { return } - template, err := api.Database.GetTemplateByID(r.Context(), templateVersion.TemplateID.UUID) - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching template job.", - Detail: err.Error(), - }) - return - } - // Store prior build number to compute new build number var priorBuildNum int32 priorHistory, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) @@ -394,40 +426,8 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { return } - if createBuild.Orphan { - if createBuild.Transition != codersdk.WorkspaceTransitionDelete { - httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ - Message: "Orphan is only permitted when deleting a workspace.", - Detail: err.Error(), - }) - return - } - if createBuild.ProvisionerState != nil && createBuild.Orphan { - httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{ - Message: "ProvisionerState cannot be set alongside Orphan since state intent is unclear.", - }) - return - } - - createBuild.ProvisionerState, err = provisionersdk.OrphanState(priorHistory.ProvisionerState) - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Failed to manipulate state.", - Detail: err.Error(), - }) - return - } - } - - // If custom state, deny request since user could be orphaning their - // cloud resources. - if createBuild.ProvisionerState != nil { - if !api.Authorize(r, rbac.ActionUpdate, template.RBACObject()) { - httpapi.Write(rw, http.StatusForbidden, codersdk.Response{ - Message: "Only template managers may provide custom state", - }) - return - } + if state == nil { + state = priorHistory.ProvisionerState } var workspaceBuild database.WorkspaceBuild @@ -494,10 +494,6 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { if err != nil { return xerrors.Errorf("insert provisioner job: %w", err) } - state := createBuild.ProvisionerState - if len(state) == 0 { - state = priorHistory.ProvisionerState - } workspaceBuild, err = db.InsertWorkspaceBuild(r.Context(), database.InsertWorkspaceBuildParams{ ID: workspaceBuildID, diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index 04f47a3c25846..9b0aaa92a1d07 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -231,7 +231,7 @@ func TestWorkspaceBuilds(t *testing.T) { }) } -func TestWorkspaceBuilds_State(t *testing.T) { +func TestWorkspaceBuildsProvisionerState(t *testing.T) { t.Parallel() t.Run("Permissions", func(t *testing.T) { diff --git a/provisionersdk/orphan.go b/provisionersdk/orphan.go deleted file mode 100644 index 7ed23f3a5f519..0000000000000 --- a/provisionersdk/orphan.go +++ /dev/null @@ -1,34 +0,0 @@ -package provisionersdk - -import ( - "encoding/json" - - "golang.org/x/xerrors" -) - -// OrphanState removes all the resources from the provided -// state. When the new state is used, the provisioner will operate as if none -// of the resources in the original state exist. -func OrphanState(state []byte) ([]byte, error) { - if len(state) == 0 { - // Presume that state is already orphaned, or we're using - // a no-op provisioner. - return state, nil - } - - stateMap := make(map[string]interface{}) - err := json.Unmarshal(state, &stateMap) - if err != nil { - return nil, err - } - - _, ok := stateMap["resources"] - if !ok { - return nil, xerrors.Errorf("no resources detected, is this terraform state?") - } - - // Terraform wants a resources array. - stateMap["resources"] = []struct{}{} - - return json.Marshal(stateMap) -} diff --git a/provisionersdk/orphan_test.go b/provisionersdk/orphan_test.go deleted file mode 100644 index 867f55b6c8a55..0000000000000 --- a/provisionersdk/orphan_test.go +++ /dev/null @@ -1,41 +0,0 @@ -package provisionersdk_test - -import ( - "reflect" - "testing" - - "github.com/coder/coder/provisionersdk" -) - -func TestOrphanState(t *testing.T) { - t.Parallel() - - type args struct { - state []byte - } - tests := []struct { - name string - args args - want []byte - wantErr bool - }{ - {"invalid json", args{[]byte("---")}, nil, true}, - {"no resources", args{[]byte(`{"a":4}`)}, nil, true}, - {"some resources", args{[]byte(`{"a":4, "resources":[1, 2, 3]}`)}, []byte(`{"a":4,"resources":[]}`), false}, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - got, err := provisionersdk.OrphanState(tt.args.state) - if (err != nil) != tt.wantErr { - t.Errorf("OrphanState() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("OrphanState() = %s, want %s", got, tt.want) - } - }) - } -} From 98e4365bf1d621a83edfc55e2bb864bd67e0527f Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sat, 3 Sep 2022 22:31:09 +0000 Subject: [PATCH 12/16] Begin experimenting with frontend --- site/src/components/ErrorSummary/ErrorSummary.tsx | 2 +- site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/site/src/components/ErrorSummary/ErrorSummary.tsx b/site/src/components/ErrorSummary/ErrorSummary.tsx index 6e8352a8e9544..931ea1b9fb4bf 100644 --- a/site/src/components/ErrorSummary/ErrorSummary.tsx +++ b/site/src/components/ErrorSummary/ErrorSummary.tsx @@ -17,7 +17,7 @@ export const Language = { } export interface ErrorSummaryProps { - error: ApiError | Error | unknown + error: ApiError | Error | unknown | JSX.Element retry?: () => void dismissible?: boolean defaultMessage?: string diff --git a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx index 874d11b3290a4..90e7654190290 100644 --- a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx +++ b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx @@ -1,3 +1,4 @@ +import { ErrorSummary } from "components/ErrorSummary/ErrorSummary" import { FC } from "react" import { ProvisionerJobLog, WorkspaceBuild } from "../../api/typesGenerated" import { Loader } from "../../components/Loader/Loader" @@ -26,6 +27,7 @@ export const WorkspaceBuildPageView: FC = ({ logs, + {build && } {!logs && } {logs && } From a81a8c79108f119d3fd4aeed895fb5a4fa784dce Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 4 Sep 2022 00:09:31 +0000 Subject: [PATCH 13/16] Complete frontend --- .../components/CodeExample/CodeExample.tsx | 44 ++++++++------- site/src/components/CopyButton/CopyButton.tsx | 1 - .../WorkspaceBuildPageView.stories.tsx | 13 ++++- .../WorkspaceBuildPageView.tsx | 6 ++- .../WorkspaceBuildStateError.tsx | 54 +++++++++++++++++++ site/src/testHelpers/entities.ts | 22 ++++++++ 6 files changed, 117 insertions(+), 23 deletions(-) create mode 100644 site/src/pages/WorkspaceBuildPage/WorkspaceBuildStateError.tsx diff --git a/site/src/components/CodeExample/CodeExample.tsx b/site/src/components/CodeExample/CodeExample.tsx index 8cf51bb815911..869cde43f16da 100644 --- a/site/src/components/CodeExample/CodeExample.tsx +++ b/site/src/components/CodeExample/CodeExample.tsx @@ -1,4 +1,4 @@ -import { makeStyles } from "@material-ui/core/styles" +import { makeStyles, Theme } from "@material-ui/core/styles" import { FC } from "react" import { MONOSPACE_FONT_FAMILY } from "../../theme/constants" import { combineClasses } from "../../util/combineClasses" @@ -9,6 +9,7 @@ export interface CodeExampleProps { className?: string buttonClassName?: string tooltipTitle?: string + inline?: boolean } /** @@ -19,8 +20,9 @@ export const CodeExample: FC> = ({ className, buttonClassName, tooltipTitle, + inline, }) => { - const styles = useStyles() + const styles = useStyles({ inline: inline }) return (
@@ -34,35 +36,41 @@ export const CodeExample: FC> = ({ ) } -const useStyles = makeStyles((theme) => ({ - root: { - display: "flex", +interface styleProps { + inline?: boolean +} + +const useStyles = makeStyles((theme) => ({ + root: (props) => ({ + display: props.inline ? "inline-flex" : "flex", flexDirection: "row", alignItems: "center", - background: "hsl(223, 27%, 3%)", - border: `1px solid ${theme.palette.divider}`, + background: props.inline ? "rgb(0 0 0 / 30%)" : "hsl(223, 27%, 3%)", + border: props.inline ? undefined : `1px solid ${theme.palette.divider}`, color: theme.palette.primary.contrastText, fontFamily: MONOSPACE_FONT_FAMILY, fontSize: 14, borderRadius: theme.shape.borderRadius, - padding: theme.spacing(0.5), - }, - code: { + padding: props.inline ? "0px" : theme.spacing(0.5), + }), + code: (props) => ({ padding: ` - ${theme.spacing(0.5)}px + ${props.inline ? 0 : theme.spacing(0.5)}px ${theme.spacing(0.75)}px - ${theme.spacing(0.5)}px - ${theme.spacing(2)}px + ${props.inline ? 0 : theme.spacing(0.5)}px + ${props.inline ? theme.spacing(1) : theme.spacing(2)}px `, width: "100%", display: "flex", alignItems: "center", wordBreak: "break-all", - }, - button: { + }), + button: (props) => ({ border: 0, - minWidth: 42, - minHeight: 42, + minWidth: props.inline ? 30 : 42, + minHeight: props.inline ? 30 : 42, borderRadius: theme.shape.borderRadius, - }, + padding: props.inline ? theme.spacing(0.4) : undefined, + background: "transparent", + }), })) diff --git a/site/src/components/CopyButton/CopyButton.tsx b/site/src/components/CopyButton/CopyButton.tsx index d02f528cea26b..1b402d1de314a 100644 --- a/site/src/components/CopyButton/CopyButton.tsx +++ b/site/src/components/CopyButton/CopyButton.tsx @@ -86,7 +86,6 @@ export const CopyButton: React.FC> = ({ const useStyles = makeStyles((theme) => ({ copyButtonWrapper: { display: "flex", - marginLeft: theme.spacing(1), }, copyButton: { borderRadius: theme.shape.borderRadius, diff --git a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.stories.tsx b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.stories.tsx index 24e47c97918ce..f5056361d9c4a 100644 --- a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.stories.tsx +++ b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.stories.tsx @@ -1,5 +1,9 @@ import { ComponentMeta, Story } from "@storybook/react" -import { MockWorkspaceBuild, MockWorkspaceBuildLogs } from "../../testHelpers/entities" +import { + MockFailedWorkspaceBuild, + MockWorkspaceBuild, + MockWorkspaceBuildLogs, +} from "../../testHelpers/entities" import { WorkspaceBuildPageView, WorkspaceBuildPageViewProps } from "./WorkspaceBuildPageView" export default { @@ -13,5 +17,10 @@ export const Example = Template.bind({}) Example.args = { build: MockWorkspaceBuild, logs: MockWorkspaceBuildLogs, - isWaitingForLogs: false, +} + +export const FailedDelete = Template.bind({}) +FailedDelete.args = { + build: MockFailedWorkspaceBuild("delete"), + logs: MockWorkspaceBuildLogs, } diff --git a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx index 90e7654190290..2851d3cd00793 100644 --- a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx +++ b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx @@ -1,4 +1,3 @@ -import { ErrorSummary } from "components/ErrorSummary/ErrorSummary" import { FC } from "react" import { ProvisionerJobLog, WorkspaceBuild } from "../../api/typesGenerated" import { Loader } from "../../components/Loader/Loader" @@ -7,6 +6,7 @@ import { PageHeader, PageHeaderTitle } from "../../components/PageHeader/PageHea import { Stack } from "../../components/Stack/Stack" import { WorkspaceBuildLogs } from "../../components/WorkspaceBuildLogs/WorkspaceBuildLogs" import { WorkspaceBuildStats } from "../../components/WorkspaceBuildStats/WorkspaceBuildStats" +import { WorkspaceBuildStateError } from "./WorkspaceBuildStateError" const sortLogsByCreatedAt = (logs: ProvisionerJobLog[]) => { return [...logs].sort( @@ -27,7 +27,9 @@ export const WorkspaceBuildPageView: FC = ({ logs, - + {build && build.transition === "delete" && build.job.status === "failed" && ( + + )} {build && } {!logs && } {logs && } diff --git a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildStateError.tsx b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildStateError.tsx new file mode 100644 index 0000000000000..e75d71b2d5d03 --- /dev/null +++ b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildStateError.tsx @@ -0,0 +1,54 @@ +import { makeStyles } from "@material-ui/core/styles" +import { WorkspaceBuild } from "api/typesGenerated" +import { CodeExample } from "components/CodeExample/CodeExample" +import { Stack } from "components/Stack/Stack" + +const Language = { + stateMessage: "The workspace may have failed to delete due to a Terraform state mismatch.", +} + +export interface WorkspaceBuildStateErrorProps { + build: WorkspaceBuild +} + +export const WorkspaceBuildStateError: React.FC = ({ build }) => { + const styles = useStyles() + + const orphanCommand = `coder rm ${ + build.workspace_owner_name + "/" + build.workspace_name + } --orphan` + return ( + + + + + {Language.stateMessage} A template admin may run{" "} + to delete the workspace skipping resource + destruction. + + + + + ) +} + +const useStyles = makeStyles((theme) => ({ + root: { + background: theme.palette.warning.main, + padding: `${theme.spacing(2)}px`, + borderRadius: theme.shape.borderRadius, + gap: 0, + }, + flex: { + display: "flex", + }, + messageBox: { + justifyContent: "space-between", + }, + errorMessage: { + marginRight: `${theme.spacing(1)}px`, + }, + iconButton: { + padding: 0, + }, +})) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 90e2d7ab8d7f0..ebe56aeeea90b 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -128,6 +128,7 @@ export const MockFailedProvisionerJob: TypesGen.ProvisionerJob = { ...MockProvisionerJob, status: "failed", } + export const MockCancelingProvisionerJob: TypesGen.ProvisionerJob = { ...MockProvisionerJob, status: "canceling", @@ -208,6 +209,27 @@ export const MockWorkspaceBuild: TypesGen.WorkspaceBuild = { reason: "initiator", } +export const MockFailedWorkspaceBuild = ( + transition: TypesGen.WorkspaceTransition = "start", +): TypesGen.WorkspaceBuild => ({ + build_number: 1, + created_at: "2022-05-17T17:39:01.382927298Z", + id: "1", + initiator_id: MockUser.id, + initiator_name: MockUser.username, + job: MockFailedProvisionerJob, + name: "a-workspace-build", + template_version_id: "", + transition: transition, + updated_at: "2022-05-17T17:39:01.382927298Z", + workspace_name: "test-workspace", + workspace_owner_id: MockUser.id, + workspace_owner_name: MockUser.username, + workspace_id: "759f1d46-3174-453d-aa60-980a9c1442f3", + deadline: "2022-05-17T23:39:00.00Z", + reason: "initiator", +}) + export const MockWorkspaceBuildStop: TypesGen.WorkspaceBuild = { ...MockWorkspaceBuild, id: "2", From 2bfe35464b3020e20bbbc4bff74be3160a7047d6 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 4 Sep 2022 19:32:46 +0000 Subject: [PATCH 14/16] Minor fixups --- cli/delete.go | 5 +++-- cli/delete_test.go | 2 +- cli/portforward.go | 3 ++- coderd/workspacebuilds_test.go | 4 ++-- site/src/components/ErrorSummary/ErrorSummary.tsx | 2 +- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cli/delete.go b/cli/delete.go index 7a71dc50dca33..b8a557f48ee35 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -41,8 +41,9 @@ func deleteWorkspace() *cobra.Command { var state []byte if orphan { - cliui.Warn(cmd.ErrOrStderr(), "Orphaning workspace required template edit permission", - "Template edit permission is required to orphan workspaces.", + cliui.Warn( + cmd.ErrOrStderr(), + "Orphaning workspace required template edit permission", ) } diff --git a/cli/delete_test.go b/cli/delete_test.go index 8a0011a47e14d..499c90ef31234 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -45,7 +45,7 @@ func TestDelete(t *testing.T) { t.Run("Orphan", func(t *testing.T) { t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) diff --git a/cli/portforward.go b/cli/portforward.go index 3b78fa8d11a65..ae5daeb9541bd 100644 --- a/cli/portforward.go +++ b/cli/portforward.go @@ -12,11 +12,12 @@ import ( "sync" "syscall" - "cdr.dev/slog" "github.com/pion/udp" "github.com/spf13/cobra" "golang.org/x/xerrors" + "cdr.dev/slog" + "github.com/coder/coder/agent" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/codersdk" diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index 019325876ae66..5ded39bf91c87 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -236,7 +236,7 @@ func TestWorkspaceBuildsProvisionerState(t *testing.T) { t.Run("Permissions", func(t *testing.T) { t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) first := coderdtest.CreateFirstUser(t, client) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) @@ -281,7 +281,7 @@ func TestWorkspaceBuildsProvisionerState(t *testing.T) { t.Run("Orphan", func(t *testing.T) { t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) first := coderdtest.CreateFirstUser(t, client) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) diff --git a/site/src/components/ErrorSummary/ErrorSummary.tsx b/site/src/components/ErrorSummary/ErrorSummary.tsx index 931ea1b9fb4bf..6e8352a8e9544 100644 --- a/site/src/components/ErrorSummary/ErrorSummary.tsx +++ b/site/src/components/ErrorSummary/ErrorSummary.tsx @@ -17,7 +17,7 @@ export const Language = { } export interface ErrorSummaryProps { - error: ApiError | Error | unknown | JSX.Element + error: ApiError | Error | unknown retry?: () => void dismissible?: boolean defaultMessage?: string From 8d460452c6c5cbd604c49408ed55180c3ebb4117 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Tue, 6 Sep 2022 11:57:39 -0500 Subject: [PATCH 15/16] Update cli/delete.go Co-authored-by: Kyle Carberry --- cli/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/delete.go b/cli/delete.go index b8a557f48ee35..5e36314a3300c 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -43,7 +43,7 @@ func deleteWorkspace() *cobra.Command { if orphan { cliui.Warn( cmd.ErrOrStderr(), - "Orphaning workspace required template edit permission", + "Orphaning workspace requires template edit permission", ) } From e9b1edd8e3a76b57431c7a0c76981c8382f20ca9 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Tue, 6 Sep 2022 11:57:45 -0500 Subject: [PATCH 16/16] Update cli/delete.go Co-authored-by: Kyle Carberry --- cli/delete.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/delete.go b/cli/delete.go index 5e36314a3300c..fd30fe95a1c1e 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -67,7 +67,7 @@ func deleteWorkspace() *cobra.Command { }, } cmd.Flags().BoolVar(&orphan, "orphan", false, - `Delete workspace without deleting its resources. This can delete a + `Delete a workspace without deleting its resources. This can delete a workspace in a broken state, but may also lead to unaccounted cloud resources.`, ) cliui.AllowSkipPrompt(cmd)