selfhosted
/
coder
mirror of https://github.com/coder/coder.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix: only allow promoting successful template versions (#9998)

This commit is contained in:
Kayla Washburn 2023-10-05 10:49:25 -06:00 committed by GitHub
parent 48ee80a559
commit f001a57614
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 201 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
coderd/coderdtest/coderdtest.go
View File

@ -760,21 +760,53 @@ func UpdateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID
return templateVersion
}
// AwaitTemplateVersionJobCompleted awaits for an import job to reach completed status.
// AwaitTemplateVersionJobRunning waits for the build to be picked up by a provisioner.
func AwaitTemplateVersionJobRunning(t *testing.T, client *codersdk.Client, version uuid.UUID) codersdk.TemplateVersion {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
defer cancel()
t.Logf("waiting for template version %s build job to start", version)
var templateVersion codersdk.TemplateVersion
require.Eventually(t, func() bool {
var err error
templateVersion, err = client.TemplateVersion(ctx, version)
if err != nil {
return false
}
t.Logf("template version job status: %s", templateVersion.Job.Status)
switch templateVersion.Job.Status {
case codersdk.ProvisionerJobPending:
return false
case codersdk.ProvisionerJobRunning:
return true
default:
t.FailNow()
return false
}
}, testutil.WaitShort, testutil.IntervalFast, "make sure you set `IncludeProvisionerDaemon`!")
t.Logf("template version %s job has started", version)
return templateVersion
}
// AwaitTemplateVersionJobCompleted waits for the build to be completed. This may result
// from cancelation, an error, or from completing successfully.
func AwaitTemplateVersionJobCompleted(t *testing.T, client *codersdk.Client, version uuid.UUID) codersdk.TemplateVersion {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
t.Logf("waiting for template version job %s", version)
t.Logf("waiting for template version %s build job to complete", version)
var templateVersion codersdk.TemplateVersion
require.Eventually(t, func() bool {
var err error
templateVersion, err = client.TemplateVersion(ctx, version)
t.Logf("template version job status: %s", templateVersion.Job.Status)
return assert.NoError(t, err) && templateVersion.Job.CompletedAt != nil
}, testutil.WaitLong, testutil.IntervalMedium)
t.Logf("got template version job %s", version)
}, testutil.WaitLong, testutil.IntervalMedium, "make sure you set `IncludeProvisionerDaemon`!")
t.Logf("template version %s job has completed", version)
return templateVersion
}

12
coderd/insights_test.go
View File

@ -2110,7 +2110,7 @@ func TestTemplateInsights_RBAC(t *testing.T) {
t.Run("AsOwner", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{})
client := coderdtest.New(t, nil)
admin := coderdtest.CreateFirstUser(t, client)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
@ -2134,7 +2134,7 @@ func TestTemplateInsights_RBAC(t *testing.T) {
t.Run("AsTemplateAdmin", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{})
client := coderdtest.New(t, nil)
admin := coderdtest.CreateFirstUser(t, client)
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleTemplateAdmin())
@ -2160,7 +2160,7 @@ func TestTemplateInsights_RBAC(t *testing.T) {
t.Run("AsRegularUser", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{})
client := coderdtest.New(t, nil)
admin := coderdtest.CreateFirstUser(t, client)
regular, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
@ -2239,7 +2239,7 @@ func TestGenericInsights_RBAC(t *testing.T) {
t.Run("AsOwner", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{})
client := coderdtest.New(t, nil)
admin := coderdtest.CreateFirstUser(t, client)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
@ -2261,7 +2261,7 @@ func TestGenericInsights_RBAC(t *testing.T) {
t.Run("AsTemplateAdmin", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{})
client := coderdtest.New(t, nil)
admin := coderdtest.CreateFirstUser(t, client)
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleTemplateAdmin())
@ -2285,7 +2285,7 @@ func TestGenericInsights_RBAC(t *testing.T) {
t.Run("AsRegularUser", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{})
client := coderdtest.New(t, nil)
admin := coderdtest.CreateFirstUser(t, client)
regular, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)

21
coderd/templateversions.go
View File

@ -20,6 +20,7 @@ import (
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
"github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/coderd/database/provisionerjobs"
"github.com/coder/coder/v2/coderd/externalauth"
@ -248,7 +249,7 @@ func (api *API) templateVersionRichParameters(rw http.ResponseWriter, r *http.Re
return
}
if !job.CompletedAt.Valid {
httpapi.Write(ctx, rw, http.StatusPreconditionFailed, codersdk.Response{
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
Message: "Job hasn't completed!",
})
return
@ -383,7 +384,7 @@ func (api *API) templateVersionVariables(rw http.ResponseWriter, r *http.Request
return
}
if !job.CompletedAt.Valid {
httpapi.Write(ctx, rw, http.StatusPreconditionFailed, codersdk.Response{
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
Message: "Job hasn't completed!",
})
return
@ -1040,6 +1041,22 @@ func (api *API) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Reque
})
return
}
job, err := api.Database.GetProvisionerJobByID(ctx, version.JobID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching template version job status.",
Detail: err.Error(),
})
return
}
jobStatus := db2sdk.ProvisionerJobStatus(job)
if jobStatus != codersdk.ProvisionerJobSucceeded {
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
Message: "Only versions that have been built successfully can be promoted.",
Detail: fmt.Sprintf("Attempted to promote a version with a %s build", jobStatus),
})
return
}
err = api.Database.InTx(func(store database.Store) error {
err = store.UpdateTemplateActiveVersionByID(ctx, database.UpdateTemplateActiveVersionByIDParams{

80
coderd/templateversions_test.go
View File

@ -241,7 +241,9 @@ func TestPatchCancelTemplateVersion(t *testing.T) {
})
t.Run("AlreadyCanceled", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
client := coderdtest.New(t, &coderdtest.Options{
IncludeProvisionerDaemon: true,
})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
Parse: echo.ParseComplete,
@ -255,15 +257,7 @@ func TestPatchCancelTemplateVersion(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
require.Eventually(t, func() bool {
var err error
version, err = client.TemplateVersion(ctx, version.ID)
if !assert.NoError(t, err) {
return false
}
t.Logf("Status: %s", version.Job.Status)
return version.Job.Status == codersdk.ProvisionerJobRunning
}, testutil.WaitShort, testutil.IntervalFast)
coderdtest.AwaitTemplateVersionJobRunning(t, client, version.ID)
err := client.CancelTemplateVersion(ctx, version.ID)
require.NoError(t, err)
err = client.CancelTemplateVersion(ctx, version.ID)
@ -280,7 +274,9 @@ func TestPatchCancelTemplateVersion(t *testing.T) {
// Running -> Canceling is the best we can do for now.
t.Run("Canceling", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
client := coderdtest.New(t, &coderdtest.Options{
IncludeProvisionerDaemon: true,
})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
Parse: echo.ParseComplete,
@ -557,13 +553,60 @@ func TestPatchActiveTemplateVersion(t *testing.T) {
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
})
t.Run("DoesNotBelong", func(t *testing.T) {
t.Run("CanceledBuild", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
version = coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, nil, template.ID)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
err := client.CancelTemplateVersion(ctx, version.ID)
require.NoError(t, err)
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
err = client.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{
ID: version.ID,
})
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
require.Contains(t, apiErr.Detail, "canceled")
})
t.Run("PendingBuild", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
version = coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, nil, template.ID)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
err := client.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{
ID: version.ID,
})
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
require.Contains(t, apiErr.Detail, "pending")
})
t.Run("DoesNotBelong", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{
IncludeProvisionerDaemon: true,
})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
_ = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
@ -576,13 +619,18 @@ func TestPatchActiveTemplateVersion(t *testing.T) {
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
})
t.Run("Found", func(t *testing.T) {
t.Run("SuccessfulBuild", func(t *testing.T) {
t.Parallel()
auditor := audit.NewMock()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, Auditor: auditor})
client := coderdtest.New(t, &coderdtest.Options{
IncludeProvisionerDaemon: true,
Auditor: auditor,
})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
version = coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, nil, template.ID)
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
@ -592,8 +640,8 @@ func TestPatchActiveTemplateVersion(t *testing.T) {
})
require.NoError(t, err)
require.Len(t, auditor.AuditLogs(), 5)
assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs()[4].Action)
require.Len(t, auditor.AuditLogs(), 6)
assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs()[5].Action)
})
}

2
coderd/workspaceagents_test.go
View File

@ -255,11 +255,11 @@ func TestWorkspaceAgent(t *testing.T) {
req.TemplateID = template.ID
})
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
err = client.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{
ID: version.ID,
})
require.NoError(t, err)
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
// Creating another workspace is just easier.
workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)

1
coderd/workspaces_test.go
View File

@ -428,7 +428,6 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
first := coderdtest.CreateFirstUser(t, client)
other, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleMember(), rbac.RoleOwner())
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)

14
site/src/pages/TemplatePage/TemplateVersionsPage/VersionRow.tsx
View File

@ -32,6 +32,8 @@ export const VersionRow: React.FC<VersionRowProps> = ({
onClick: () => navigate(version.name),
});
const jobStatus = version.job.status;
return (
<TimelineEntry
data-testid={`version-${version.id}`}
@ -78,10 +80,20 @@ export const VersionRow: React.FC<VersionRowProps> = ({
<Stack direction="row" alignItems="center" spacing={2}>
{isActive && <Pill text="Active" type="success" />}
{isLatest && <Pill text="Newest" type="info" />}
{jobStatus === "pending" && (
<Pill text={<>Pending&hellip;</>} type="warning" lightBorder />
)}
{jobStatus === "running" && (
<Pill text={<>Building&hellip;</>} type="warning" lightBorder />
)}
{(jobStatus === "canceling" || jobStatus === "canceled") && (
<Pill text="Canceled" type="neutral" lightBorder />
)}
{jobStatus === "failed" && <Pill text="Failed" type="error" />}
{onPromoteClick && (
<Button
className={styles.promoteButton}
disabled={isActive}
disabled={isActive || jobStatus !== "succeeded"}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();

56
site/src/pages/TemplatePage/TemplateVersionsPage/VersionsTable.stories.tsx
View File

@ -1,10 +1,17 @@
import { action } from "@storybook/addon-actions";
import { MockTemplateVersion } from "testHelpers/entities";
import {
MockCanceledProvisionerJob,
MockCancelingProvisionerJob,
MockFailedProvisionerJob,
MockPendingProvisionerJob,
MockRunningProvisionerJob,
MockTemplateVersion,
} from "testHelpers/entities";
import { VersionsTable } from "./VersionsTable";
import type { Meta, StoryObj } from "@storybook/react";
const meta: Meta<typeof VersionsTable> = {
title: "components/VersionsTable",
title: "pages/TemplatePage/VersionsTable",
component: VersionsTable,
};
@ -43,6 +50,51 @@ export const CanPromote: Story = {
},
};
export const BuildStatuses: Story = {
args: {
activeVersionId: MockTemplateVersion.id,
onPromoteClick: action("onPromoteClick"),
versions: [
{
...MockTemplateVersion,
id: "6",
name: "test-version-6",
created_at: "2022-05-18T18:39:01.382927298Z",
job: MockCancelingProvisionerJob,
},
{
...MockTemplateVersion,
id: "5",
name: "test-version-5",
created_at: "2022-05-18T18:39:01.382927298Z",
job: MockCanceledProvisionerJob,
},
{
...MockTemplateVersion,
id: "4",
name: "test-version-4",
created_at: "2022-05-18T18:39:01.382927298Z",
job: MockRunningProvisionerJob,
},
{
...MockTemplateVersion,
id: "3",
name: "test-version-3",
created_at: "2022-05-18T18:39:01.382927298Z",
job: MockPendingProvisionerJob,
},
{
...MockTemplateVersion,
id: "2",
name: "test-version-2",
created_at: "2022-05-18T18:39:01.382927298Z",
job: MockFailedProvisionerJob,
},
MockTemplateVersion,
],
},
};
export const Empty: Story = {
args: {
versions: [],

14
site/src/pages/TemplatePage/TemplateVersionsPage/VersionsTable.tsx
View File

@ -5,7 +5,7 @@ import TableCell from "@mui/material/TableCell";
import TableContainer from "@mui/material/TableContainer";
import TableRow from "@mui/material/TableRow";
import { Timeline } from "components/Timeline/Timeline";
import { FC } from "react";
import { type FC } from "react";
import * as TypesGen from "api/typesGenerated";
import { EmptyState } from "components/EmptyState/EmptyState";
import { TableLoader } from "components/TableLoader/TableLoader";
@ -24,13 +24,15 @@ export interface VersionsTableProps {
versions?: TypesGen.TemplateVersion[];
}
export const VersionsTable: FC<React.PropsWithChildren<VersionsTableProps>> = ({
versions,
onPromoteClick,
activeVersionId,
}) => {
export const VersionsTable: FC<VersionsTableProps> = (props) => {
const { versions, onPromoteClick, activeVersionId } = props;
const latestVersionId = versions?.reduce(
(latestSoFar, against) => {
if (against.job.status !== "succeeded") {
return latestSoFar;
}
if (!latestSoFar) {
return against;
}