fix: fix workspace status filter returning more statuses that requested (#7732)

This commit is contained in:
Steven Masley
2023-06-05 18:12:10 -05:00
committed by GitHub
parent b9e3226612
commit fa8f50a169
6 changed files with 347 additions and 84 deletions

View File

@ -3,6 +3,7 @@ package dbauthz_test
import (
"context"
"database/sql"
"encoding/json"
"time"
"github.com/google/uuid"
@ -240,7 +241,7 @@ func (s *MethodTestSuite) TestSystemFunctions() {
j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{
StartedAt: sql.NullTime{Valid: false},
})
check.Args(database.AcquireProvisionerJobParams{Types: []database.ProvisionerType{j.Provisioner}}).
check.Args(database.AcquireProvisionerJobParams{Types: []database.ProvisionerType{j.Provisioner}, Tags: must(json.Marshal(j.Tags))}).
Asserts( /*rbac.ResourceSystem, rbac.ActionUpdate*/ )
}))
s.Run("UpdateProvisionerJobWithCompleteByID", s.Subtest(func(db database.Store, check *expects) {

View File

@ -1168,82 +1168,68 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.
return nil, xerrors.Errorf("get provisioner job: %w", err)
}
// This logic should match the logic in the workspace.sql file.
var statusMatch bool
switch database.WorkspaceStatus(arg.Status) {
case database.WorkspaceStatusPending:
if !job.StartedAt.Valid {
continue
}
statusMatch = isNull(job.StartedAt)
case database.WorkspaceStatusStarting:
if !job.StartedAt.Valid &&
!job.CanceledAt.Valid &&
job.CompletedAt.Valid &&
time.Since(job.UpdatedAt) > 30*time.Second ||
build.Transition != database.WorkspaceTransitionStart {
continue
}
statusMatch = isNotNull(job.StartedAt) &&
isNull(job.CanceledAt) &&
isNull(job.CompletedAt) &&
time.Since(job.UpdatedAt) < 30*time.Second &&
build.Transition == database.WorkspaceTransitionStart
case database.WorkspaceStatusRunning:
if !job.CompletedAt.Valid &&
job.CanceledAt.Valid &&
job.Error.Valid ||
build.Transition != database.WorkspaceTransitionStart {
continue
}
statusMatch = isNotNull(job.CompletedAt) &&
isNull(job.CanceledAt) &&
isNull(job.Error) &&
build.Transition == database.WorkspaceTransitionStart
case database.WorkspaceStatusStopping:
if !job.StartedAt.Valid &&
!job.CanceledAt.Valid &&
job.CompletedAt.Valid &&
time.Since(job.UpdatedAt) > 30*time.Second ||
build.Transition != database.WorkspaceTransitionStop {
continue
}
statusMatch = isNotNull(job.StartedAt) &&
isNull(job.CanceledAt) &&
isNull(job.CompletedAt) &&
time.Since(job.UpdatedAt) < 30*time.Second &&
build.Transition == database.WorkspaceTransitionStop
case database.WorkspaceStatusStopped:
if !job.CompletedAt.Valid &&
job.CanceledAt.Valid &&
job.Error.Valid ||
build.Transition != database.WorkspaceTransitionStop {
continue
}
statusMatch = isNotNull(job.CompletedAt) &&
isNull(job.CanceledAt) &&
isNull(job.Error) &&
build.Transition == database.WorkspaceTransitionStop
case database.WorkspaceStatusFailed:
if (!job.CanceledAt.Valid && !job.Error.Valid) ||
(!job.CompletedAt.Valid && !job.Error.Valid) {
continue
}
statusMatch = (isNotNull(job.CanceledAt) && isNotNull(job.Error)) ||
(isNotNull(job.CompletedAt) && isNotNull(job.Error))
case database.WorkspaceStatusCanceling:
if !job.CanceledAt.Valid && job.CompletedAt.Valid {
continue
}
statusMatch = isNotNull(job.CanceledAt) &&
isNull(job.CompletedAt)
case database.WorkspaceStatusCanceled:
if !job.CanceledAt.Valid && !job.CompletedAt.Valid {
continue
}
statusMatch = isNotNull(job.CanceledAt) &&
isNotNull(job.CompletedAt)
case database.WorkspaceStatusDeleted:
if !job.StartedAt.Valid &&
job.CanceledAt.Valid &&
!job.CompletedAt.Valid &&
time.Since(job.UpdatedAt) > 30*time.Second ||
build.Transition != database.WorkspaceTransitionDelete {
continue
}
statusMatch = isNotNull(job.StartedAt) &&
isNull(job.CanceledAt) &&
isNotNull(job.CompletedAt) &&
time.Since(job.UpdatedAt) < 30*time.Second &&
build.Transition == database.WorkspaceTransitionDelete &&
isNull(job.Error)
case database.WorkspaceStatusDeleting:
if !job.CompletedAt.Valid &&
job.CanceledAt.Valid &&
job.Error.Valid &&
build.Transition != database.WorkspaceTransitionDelete {
continue
}
statusMatch = isNull(job.CompletedAt) &&
isNull(job.CanceledAt) &&
isNull(job.Error) &&
build.Transition == database.WorkspaceTransitionDelete
default:
return nil, xerrors.Errorf("unknown workspace status in filter: %q", arg.Status)
}
if !statusMatch {
continue
}
}
if arg.HasAgent != "" {
@ -5179,3 +5165,13 @@ func (q *fakeQuerier) UpdateWorkspaceProxyDeleted(_ context.Context, arg databas
}
return sql.ErrNoRows
}
// isNull is only used in dbfake, so reflect is ok. Use this to make the logic
// look more similar to the postgres.
func isNull(v interface{}) bool {
return !isNotNull(v)
}
func isNotNull(v interface{}) bool {
return reflect.ValueOf(v).FieldByName("Valid").Bool()
}

View File

@ -17,14 +17,25 @@ import (
"github.com/tabbed/pqtype"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/database/dbauthz"
"github.com/coder/coder/coderd/database/dbtype"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/cryptorand"
)
// All methods take in a 'seed' object. Any provided fields in the seed will be
// maintained. Any fields omitted will have sensible defaults generated.
// genCtx is to give all generator functions permission if the db is a dbauthz db.
var genCtx = dbauthz.As(context.Background(), rbac.Subject{
ID: "owner",
Roles: rbac.Roles(must(rbac.RoleNames{rbac.RoleOwner()}.Expand())),
Groups: []string{},
Scope: rbac.ExpandableScope(rbac.ScopeAll),
})
func AuditLog(t testing.TB, db database.Store, seed database.AuditLog) database.AuditLog {
log, err := db.InsertAuditLog(context.Background(), database.InsertAuditLogParams{
log, err := db.InsertAuditLog(genCtx, database.InsertAuditLogParams{
ID: takeFirst(seed.ID, uuid.New()),
Time: takeFirst(seed.Time, database.Now()),
UserID: takeFirst(seed.UserID, uuid.New()),
@ -52,7 +63,7 @@ func AuditLog(t testing.TB, db database.Store, seed database.AuditLog) database.
}
func Template(t testing.TB, db database.Store, seed database.Template) database.Template {
template, err := db.InsertTemplate(context.Background(), database.InsertTemplateParams{
template, err := db.InsertTemplate(genCtx, database.InsertTemplateParams{
ID: takeFirst(seed.ID, uuid.New()),
CreatedAt: takeFirst(seed.CreatedAt, database.Now()),
UpdatedAt: takeFirst(seed.UpdatedAt, database.Now()),
@ -88,7 +99,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database
}
}
key, err := db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{
key, err := db.InsertAPIKey(genCtx, database.InsertAPIKeyParams{
ID: takeFirst(seed.ID, id),
// 0 defaults to 86400 at the db layer
LifetimeSeconds: takeFirst(seed.LifetimeSeconds, 0),
@ -108,7 +119,7 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database
}
func WorkspaceAgent(t testing.TB, db database.Store, orig database.WorkspaceAgent) database.WorkspaceAgent {
workspace, err := db.InsertWorkspaceAgent(context.Background(), database.InsertWorkspaceAgentParams{
workspace, err := db.InsertWorkspaceAgent(genCtx, database.InsertWorkspaceAgentParams{
ID: takeFirst(orig.ID, uuid.New()),
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
UpdatedAt: takeFirst(orig.UpdatedAt, database.Now()),
@ -149,7 +160,7 @@ func WorkspaceAgent(t testing.TB, db database.Store, orig database.WorkspaceAgen
}
func Workspace(t testing.TB, db database.Store, orig database.Workspace) database.Workspace {
workspace, err := db.InsertWorkspace(context.Background(), database.InsertWorkspaceParams{
workspace, err := db.InsertWorkspace(genCtx, database.InsertWorkspaceParams{
ID: takeFirst(orig.ID, uuid.New()),
OwnerID: takeFirst(orig.OwnerID, uuid.New()),
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
@ -166,7 +177,7 @@ func Workspace(t testing.TB, db database.Store, orig database.Workspace) databas
}
func WorkspaceBuild(t testing.TB, db database.Store, orig database.WorkspaceBuild) database.WorkspaceBuild {
build, err := db.InsertWorkspaceBuild(context.Background(), database.InsertWorkspaceBuildParams{
build, err := db.InsertWorkspaceBuild(genCtx, database.InsertWorkspaceBuildParams{
ID: takeFirst(orig.ID, uuid.New()),
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
UpdatedAt: takeFirst(orig.UpdatedAt, database.Now()),
@ -185,7 +196,7 @@ func WorkspaceBuild(t testing.TB, db database.Store, orig database.WorkspaceBuil
}
func User(t testing.TB, db database.Store, orig database.User) database.User {
user, err := db.InsertUser(context.Background(), database.InsertUserParams{
user, err := db.InsertUser(genCtx, database.InsertUserParams{
ID: takeFirst(orig.ID, uuid.New()),
Email: takeFirst(orig.Email, namesgenerator.GetRandomName(1)),
Username: takeFirst(orig.Username, namesgenerator.GetRandomName(1)),
@ -200,7 +211,7 @@ func User(t testing.TB, db database.Store, orig database.User) database.User {
}
func GitSSHKey(t testing.TB, db database.Store, orig database.GitSSHKey) database.GitSSHKey {
key, err := db.InsertGitSSHKey(context.Background(), database.InsertGitSSHKeyParams{
key, err := db.InsertGitSSHKey(genCtx, database.InsertGitSSHKeyParams{
UserID: takeFirst(orig.UserID, uuid.New()),
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
UpdatedAt: takeFirst(orig.UpdatedAt, database.Now()),
@ -212,7 +223,7 @@ func GitSSHKey(t testing.TB, db database.Store, orig database.GitSSHKey) databas
}
func Organization(t testing.TB, db database.Store, orig database.Organization) database.Organization {
org, err := db.InsertOrganization(context.Background(), database.InsertOrganizationParams{
org, err := db.InsertOrganization(genCtx, database.InsertOrganizationParams{
ID: takeFirst(orig.ID, uuid.New()),
Name: takeFirst(orig.Name, namesgenerator.GetRandomName(1)),
Description: takeFirst(orig.Description, namesgenerator.GetRandomName(1)),
@ -224,7 +235,7 @@ func Organization(t testing.TB, db database.Store, orig database.Organization) d
}
func OrganizationMember(t testing.TB, db database.Store, orig database.OrganizationMember) database.OrganizationMember {
mem, err := db.InsertOrganizationMember(context.Background(), database.InsertOrganizationMemberParams{
mem, err := db.InsertOrganizationMember(genCtx, database.InsertOrganizationMemberParams{
OrganizationID: takeFirst(orig.OrganizationID, uuid.New()),
UserID: takeFirst(orig.UserID, uuid.New()),
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
@ -236,7 +247,7 @@ func OrganizationMember(t testing.TB, db database.Store, orig database.Organizat
}
func Group(t testing.TB, db database.Store, orig database.Group) database.Group {
group, err := db.InsertGroup(context.Background(), database.InsertGroupParams{
group, err := db.InsertGroup(genCtx, database.InsertGroupParams{
ID: takeFirst(orig.ID, uuid.New()),
Name: takeFirst(orig.Name, namesgenerator.GetRandomName(1)),
OrganizationID: takeFirst(orig.OrganizationID, uuid.New()),
@ -253,7 +264,7 @@ func GroupMember(t testing.TB, db database.Store, orig database.GroupMember) dat
GroupID: takeFirst(orig.GroupID, uuid.New()),
}
//nolint:gosimple
err := db.InsertGroupMember(context.Background(), database.InsertGroupMemberParams{
err := db.InsertGroupMember(genCtx, database.InsertGroupMemberParams{
UserID: member.UserID,
GroupID: member.GroupID,
})
@ -261,8 +272,18 @@ func GroupMember(t testing.TB, db database.Store, orig database.GroupMember) dat
return member
}
// ProvisionerJob is a bit more involved to get the values such as "completedAt", "startedAt", "cancelledAt" set.
func ProvisionerJob(t testing.TB, db database.Store, orig database.ProvisionerJob) database.ProvisionerJob {
job, err := db.InsertProvisionerJob(context.Background(), database.InsertProvisionerJobParams{
id := takeFirst(orig.ID, uuid.New())
// Always set some tags to prevent Acquire from grabbing jobs it should not.
if !orig.StartedAt.Time.IsZero() {
if orig.Tags == nil {
orig.Tags = make(dbtype.StringMap)
}
// Make sure when we acquire the job, we only get this one.
orig.Tags[id.String()] = "true"
}
job, err := db.InsertProvisionerJob(genCtx, database.InsertProvisionerJobParams{
ID: takeFirst(orig.ID, uuid.New()),
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
UpdatedAt: takeFirst(orig.UpdatedAt, database.Now()),
@ -276,11 +297,43 @@ func ProvisionerJob(t testing.TB, db database.Store, orig database.ProvisionerJo
Tags: orig.Tags,
})
require.NoError(t, err, "insert job")
if !orig.StartedAt.Time.IsZero() {
job, err = db.AcquireProvisionerJob(genCtx, database.AcquireProvisionerJobParams{
StartedAt: orig.StartedAt,
Types: []database.ProvisionerType{database.ProvisionerTypeEcho},
Tags: must(json.Marshal(orig.Tags)),
})
require.NoError(t, err)
}
if !orig.CompletedAt.Time.IsZero() || orig.Error.String != "" {
err := db.UpdateProvisionerJobWithCompleteByID(genCtx, database.UpdateProvisionerJobWithCompleteByIDParams{
ID: job.ID,
UpdatedAt: job.UpdatedAt,
CompletedAt: orig.CompletedAt,
Error: orig.Error,
ErrorCode: orig.ErrorCode,
})
require.NoError(t, err)
}
if !orig.CanceledAt.Time.IsZero() {
err := db.UpdateProvisionerJobWithCancelByID(genCtx, database.UpdateProvisionerJobWithCancelByIDParams{
ID: job.ID,
CanceledAt: orig.CanceledAt,
CompletedAt: orig.CompletedAt,
})
require.NoError(t, err)
}
job, err = db.GetProvisionerJobByID(genCtx, job.ID)
require.NoError(t, err)
return job
}
func WorkspaceApp(t testing.TB, db database.Store, orig database.WorkspaceApp) database.WorkspaceApp {
resource, err := db.InsertWorkspaceApp(context.Background(), database.InsertWorkspaceAppParams{
resource, err := db.InsertWorkspaceApp(genCtx, database.InsertWorkspaceAppParams{
ID: takeFirst(orig.ID, uuid.New()),
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
AgentID: takeFirst(orig.AgentID, uuid.New()),
@ -308,7 +361,7 @@ func WorkspaceApp(t testing.TB, db database.Store, orig database.WorkspaceApp) d
}
func WorkspaceResource(t testing.TB, db database.Store, orig database.WorkspaceResource) database.WorkspaceResource {
resource, err := db.InsertWorkspaceResource(context.Background(), database.InsertWorkspaceResourceParams{
resource, err := db.InsertWorkspaceResource(genCtx, database.InsertWorkspaceResourceParams{
ID: takeFirst(orig.ID, uuid.New()),
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
JobID: takeFirst(orig.JobID, uuid.New()),
@ -328,7 +381,7 @@ func WorkspaceResource(t testing.TB, db database.Store, orig database.WorkspaceR
}
func WorkspaceResourceMetadatums(t testing.TB, db database.Store, seed database.WorkspaceResourceMetadatum) []database.WorkspaceResourceMetadatum {
meta, err := db.InsertWorkspaceResourceMetadata(context.Background(), database.InsertWorkspaceResourceMetadataParams{
meta, err := db.InsertWorkspaceResourceMetadata(genCtx, database.InsertWorkspaceResourceMetadataParams{
WorkspaceResourceID: takeFirst(seed.WorkspaceResourceID, uuid.New()),
Key: []string{takeFirst(seed.Key, namesgenerator.GetRandomName(1))},
Value: []string{takeFirst(seed.Value.String, namesgenerator.GetRandomName(1))},
@ -343,7 +396,7 @@ func WorkspaceProxy(t testing.TB, db database.Store, orig database.WorkspaceProx
require.NoError(t, err, "generate secret")
hashedSecret := sha256.Sum256([]byte(secret))
proxy, err := db.InsertWorkspaceProxy(context.Background(), database.InsertWorkspaceProxyParams{
proxy, err := db.InsertWorkspaceProxy(genCtx, database.InsertWorkspaceProxyParams{
ID: takeFirst(orig.ID, uuid.New()),
Name: takeFirst(orig.Name, namesgenerator.GetRandomName(1)),
DisplayName: takeFirst(orig.DisplayName, namesgenerator.GetRandomName(1)),
@ -356,7 +409,7 @@ func WorkspaceProxy(t testing.TB, db database.Store, orig database.WorkspaceProx
// Also set these fields if the caller wants them.
if orig.Url != "" || orig.WildcardHostname != "" {
proxy, err = db.RegisterWorkspaceProxy(context.Background(), database.RegisterWorkspaceProxyParams{
proxy, err = db.RegisterWorkspaceProxy(genCtx, database.RegisterWorkspaceProxyParams{
Url: orig.Url,
WildcardHostname: orig.WildcardHostname,
ID: proxy.ID,
@ -367,7 +420,7 @@ func WorkspaceProxy(t testing.TB, db database.Store, orig database.WorkspaceProx
}
func File(t testing.TB, db database.Store, orig database.File) database.File {
file, err := db.InsertFile(context.Background(), database.InsertFileParams{
file, err := db.InsertFile(genCtx, database.InsertFileParams{
ID: takeFirst(orig.ID, uuid.New()),
Hash: takeFirst(orig.Hash, hex.EncodeToString(make([]byte, 32))),
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
@ -380,7 +433,7 @@ func File(t testing.TB, db database.Store, orig database.File) database.File {
}
func UserLink(t testing.TB, db database.Store, orig database.UserLink) database.UserLink {
link, err := db.InsertUserLink(context.Background(), database.InsertUserLinkParams{
link, err := db.InsertUserLink(genCtx, database.InsertUserLinkParams{
UserID: takeFirst(orig.UserID, uuid.New()),
LoginType: takeFirst(orig.LoginType, database.LoginTypeGithub),
LinkedID: takeFirst(orig.LinkedID),
@ -394,7 +447,7 @@ func UserLink(t testing.TB, db database.Store, orig database.UserLink) database.
}
func GitAuthLink(t testing.TB, db database.Store, orig database.GitAuthLink) database.GitAuthLink {
link, err := db.InsertGitAuthLink(context.Background(), database.InsertGitAuthLinkParams{
link, err := db.InsertGitAuthLink(genCtx, database.InsertGitAuthLinkParams{
ProviderID: takeFirst(orig.ProviderID, uuid.New().String()),
UserID: takeFirst(orig.UserID, uuid.New()),
OAuthAccessToken: takeFirst(orig.OAuthAccessToken, uuid.NewString()),
@ -409,7 +462,7 @@ func GitAuthLink(t testing.TB, db database.Store, orig database.GitAuthLink) dat
}
func TemplateVersion(t testing.TB, db database.Store, orig database.TemplateVersion) database.TemplateVersion {
version, err := db.InsertTemplateVersion(context.Background(), database.InsertTemplateVersionParams{
version, err := db.InsertTemplateVersion(genCtx, database.InsertTemplateVersionParams{
ID: takeFirst(orig.ID, uuid.New()),
TemplateID: orig.TemplateID,
OrganizationID: takeFirst(orig.OrganizationID, uuid.New()),
@ -425,7 +478,7 @@ func TemplateVersion(t testing.TB, db database.Store, orig database.TemplateVers
}
func TemplateVersionVariable(t testing.TB, db database.Store, orig database.TemplateVersionVariable) database.TemplateVersionVariable {
version, err := db.InsertTemplateVersionVariable(context.Background(), database.InsertTemplateVersionVariableParams{
version, err := db.InsertTemplateVersionVariable(genCtx, database.InsertTemplateVersionVariableParams{
TemplateVersionID: takeFirst(orig.TemplateVersionID, uuid.New()),
Name: takeFirst(orig.Name, namesgenerator.GetRandomName(1)),
Description: takeFirst(orig.Description, namesgenerator.GetRandomName(1)),
@ -443,7 +496,7 @@ func WorkspaceAgentStat(t testing.TB, db database.Store, orig database.Workspace
if orig.ConnectionsByProto == nil {
orig.ConnectionsByProto = json.RawMessage([]byte("{}"))
}
scheme, err := db.InsertWorkspaceAgentStat(context.Background(), database.InsertWorkspaceAgentStatParams{
scheme, err := db.InsertWorkspaceAgentStat(genCtx, database.InsertWorkspaceAgentStatParams{
ID: takeFirst(orig.ID, uuid.New()),
CreatedAt: takeFirst(orig.CreatedAt, database.Now()),
UserID: takeFirst(orig.UserID, uuid.New()),
@ -465,3 +518,10 @@ func WorkspaceAgentStat(t testing.TB, db database.Store, orig database.Workspace
require.NoError(t, err, "insert workspace agent stat")
return scheme
}
func must[V any](v V, err error) V {
if err != nil {
panic(err)
}
return v
}

View File

@ -7905,10 +7905,12 @@ WHERE
latest_build.canceled_at IS NULL AND
latest_build.completed_at IS NOT NULL AND
latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND
latest_build.transition = 'delete'::workspace_transition
latest_build.transition = 'delete'::workspace_transition AND
-- If the error field is not null, the status is 'failed'
latest_build.error IS NULL
WHEN $2 = 'deleting' THEN
latest_build.completed_at IS NOT NULL AND
latest_build.completed_at IS NULL AND
latest_build.canceled_at IS NULL AND
latest_build.error IS NULL AND
latest_build.transition = 'delete'::workspace_transition

View File

@ -157,10 +157,12 @@ WHERE
latest_build.canceled_at IS NULL AND
latest_build.completed_at IS NOT NULL AND
latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND
latest_build.transition = 'delete'::workspace_transition
latest_build.transition = 'delete'::workspace_transition AND
-- If the error field is not null, the status is 'failed'
latest_build.error IS NULL
WHEN @status = 'deleting' THEN
latest_build.completed_at IS NOT NULL AND
latest_build.completed_at IS NULL AND
latest_build.canceled_at IS NULL AND
latest_build.error IS NULL AND
latest_build.transition = 'delete'::workspace_transition