Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore(coderd/database): enforce agent name unique within workspace build#18052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
DanielleMaywood merged 13 commits intomainfromdm-workspace-agent-name-uniqueness
May 28, 2025
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
Show all changes
13 commits
Select commitHold shift + click to select a range
fb3cf7b
chore(coderd/database): create db trigger to enforce agent name uniqu…
DanielleMaywoodMay 27, 2025
3c47f69
fix: unique per build not workspace
DanielleMaywoodMay 27, 2025
043005a
fix: unique per resource not build
DanielleMaywoodMay 27, 2025
e51c8be
fix: for child agents, ensure unique across provisioner job
DanielleMaywoodMay 27, 2025
aae7049
chore: fix migration number
DanielleMaywoodMay 27, 2025
58b8a2f
chore: ID -> name
DanielleMaywoodMay 27, 2025
f88f17b
fix: listen to dean
DanielleMaywoodMay 27, 2025
74ab8f2
fix: use unique name per agent
DanielleMaywoodMay 28, 2025
0615eec
Merge branch 'main' into dm-workspace-agent-name-uniqueness
DanielleMaywoodMay 28, 2025
585b311
chore: feedback
DanielleMaywoodMay 28, 2025
5164413
chore: some changes
DanielleMaywoodMay 28, 2025
d6edbb5
chore: improve sql query
DanielleMaywoodMay 28, 2025
479681c
chore: feedback
DanielleMaywoodMay 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletionscoderd/database/dump.sql
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
DROP TRIGGER IF EXISTS workspace_agent_name_unique_trigger ON workspace_agents;
DROP FUNCTION IF EXISTS check_workspace_agent_name_unique();
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
CREATE OR REPLACE FUNCTION check_workspace_agent_name_unique()
RETURNS TRIGGER AS $$
DECLARE
workspace_build_id uuid;
agents_with_name int;
BEGIN
-- Find the workspace build the workspace agent is being inserted into.
SELECT workspace_builds.id INTO workspace_build_id
FROM workspace_resources
JOIN workspace_builds ON workspace_builds.job_id = workspace_resources.job_id
WHERE workspace_resources.id = NEW.resource_id;

-- If the agent doesn't have a workspace build, we'll allow the insert.
IF workspace_build_id IS NULL THEN
RETURN NEW;
END IF;

-- Count how many agents in this workspace build already have the given agent name.
SELECT COUNT(*) INTO agents_with_name
FROM workspace_agents
JOIN workspace_resources ON workspace_resources.id = workspace_agents.resource_id
JOIN workspace_builds ON workspace_builds.job_id = workspace_resources.job_id
WHERE workspace_builds.id = workspace_build_id
AND workspace_agents.name = NEW.name
AND workspace_agents.id != NEW.id;

-- If there's already an agent with this name, raise an error
IF agents_with_name > 0 THEN
RAISE EXCEPTION 'workspace agent name "%" already exists in this workspace build', NEW.name
USING ERRCODE = 'unique_violation';
END IF;

RETURN NEW;
END;
$$ LANGUAGE plpgsql;

CREATE TRIGGER workspace_agent_name_unique_trigger
BEFORE INSERT OR UPDATE OF name, resource_id ON workspace_agents
FOR EACH ROW
EXECUTE FUNCTION check_workspace_agent_name_unique();

COMMENT ON TRIGGER workspace_agent_name_unique_trigger ON workspace_agents IS
'Use a trigger instead of a unique constraint because existing data may violate
the uniqueness requirement. A trigger allows us to enforce uniqueness going
forward without requiring a migration to clean up historical data.';
234 changes: 234 additions & 0 deletionscoderd/database/querier_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,12 +4,14 @@ import (
"context"
"database/sql"
"encoding/json"
"errors"
"fmt"
"sort"
"testing"
"time"

"github.com/google/uuid"
"github.com/lib/pq"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand DownExpand Up@@ -4705,6 +4707,238 @@ func TestGetPresetsAtFailureLimit(t *testing.T) {
})
}

func TestWorkspaceAgentNameUniqueTrigger(t *testing.T) {
t.Parallel()

if !dbtestutil.WillUsePostgres() {
t.Skip("This test makes use of a database trigger not implemented in dbmem")
}

createWorkspaceWithAgent := func(t *testing.T, db database.Store, org database.Organization, agentName string) (database.WorkspaceBuild, database.WorkspaceResource, database.WorkspaceAgent) {
t.Helper()

user := dbgen.User(t, db, database.User{})
template := dbgen.Template(t, db, database.Template{
OrganizationID: org.ID,
CreatedBy: user.ID,
})
templateVersion := dbgen.TemplateVersion(t, db, database.TemplateVersion{
TemplateID: uuid.NullUUID{Valid: true, UUID: template.ID},
OrganizationID: org.ID,
CreatedBy: user.ID,
})
workspace := dbgen.Workspace(t, db, database.WorkspaceTable{
OrganizationID: org.ID,
TemplateID: template.ID,
OwnerID: user.ID,
})
job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{
Type: database.ProvisionerJobTypeWorkspaceBuild,
OrganizationID: org.ID,
})
build := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{
BuildNumber: 1,
JobID: job.ID,
WorkspaceID: workspace.ID,
TemplateVersionID: templateVersion.ID,
})
resource := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{
JobID: build.JobID,
})
agent := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{
ResourceID: resource.ID,
Name: agentName,
})

return build, resource, agent
}

t.Run("DuplicateNamesInSameWorkspaceResource", func(t *testing.T) {
t.Parallel()

db, _ := dbtestutil.NewDB(t)
org := dbgen.Organization(t, db, database.Organization{})
ctx := testutil.Context(t, testutil.WaitShort)

// Given: A workspace with an agent
_, resource, _ := createWorkspaceWithAgent(t, db, org, "duplicate-agent")

// When: Another agent is created for that workspace with the same name.
_, err := db.InsertWorkspaceAgent(ctx, database.InsertWorkspaceAgentParams{
ID: uuid.New(),
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
Name: "duplicate-agent", // Same name as agent1
ResourceID: resource.ID,
AuthToken: uuid.New(),
Architecture: "amd64",
OperatingSystem: "linux",
APIKeyScope: database.AgentKeyScopeEnumAll,
})

// Then: We expect it to fail.
require.Error(t, err)
var pqErr *pq.Error
require.True(t, errors.As(err, &pqErr))
require.Equal(t, pq.ErrorCode("23505"), pqErr.Code) // unique_violation
require.Contains(t, pqErr.Message, `workspace agent name "duplicate-agent" already exists in this workspace build`)
})

t.Run("DuplicateNamesInSameProvisionerJob", func(t *testing.T) {
t.Parallel()

db, _ := dbtestutil.NewDB(t)
org := dbgen.Organization(t, db, database.Organization{})
ctx := testutil.Context(t, testutil.WaitShort)

// Given: A workspace with an agent
_, resource, agent := createWorkspaceWithAgent(t, db, org, "duplicate-agent")

// When: A child agent is created for that workspace with the same name.
_, err := db.InsertWorkspaceAgent(ctx, database.InsertWorkspaceAgentParams{
ID: uuid.New(),
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
Name: agent.Name,
ResourceID: resource.ID,
AuthToken: uuid.New(),
Architecture: "amd64",
OperatingSystem: "linux",
APIKeyScope: database.AgentKeyScopeEnumAll,
})

// Then: We expect it to fail.
require.Error(t, err)
var pqErr *pq.Error
require.True(t, errors.As(err, &pqErr))
require.Equal(t, pq.ErrorCode("23505"), pqErr.Code) // unique_violation
require.Contains(t, pqErr.Message, `workspace agent name "duplicate-agent" already exists in this workspace build`)
})

t.Run("DuplicateChildNamesOverMultipleResources", func(t *testing.T) {
t.Parallel()

db, _ := dbtestutil.NewDB(t)
org := dbgen.Organization(t, db, database.Organization{})
ctx := testutil.Context(t, testutil.WaitShort)

// Given: A workspace with two agents
_, resource1, agent1 := createWorkspaceWithAgent(t, db, org, "parent-agent-1")

resource2 := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{JobID: resource1.JobID})
agent2 := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{
ResourceID: resource2.ID,
Name: "parent-agent-2",
})

// Given: One agent has a child agent
agent1Child := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{
ParentID: uuid.NullUUID{Valid: true, UUID: agent1.ID},
Name: "child-agent",
ResourceID: resource1.ID,
})

// When: A child agent is inserted for the other parent.
_, err := db.InsertWorkspaceAgent(ctx, database.InsertWorkspaceAgentParams{
ID: uuid.New(),
ParentID: uuid.NullUUID{Valid: true, UUID: agent2.ID},
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
Name: agent1Child.Name,
ResourceID: resource2.ID,
AuthToken: uuid.New(),
Architecture: "amd64",
OperatingSystem: "linux",
APIKeyScope: database.AgentKeyScopeEnumAll,
})

// Then: We expect it to fail.
require.Error(t, err)
var pqErr *pq.Error
require.True(t, errors.As(err, &pqErr))
require.Equal(t, pq.ErrorCode("23505"), pqErr.Code) // unique_violation
require.Contains(t, pqErr.Message, `workspace agent name "child-agent" already exists in this workspace build`)
})

t.Run("SameNamesInDifferentWorkspaces", func(t *testing.T) {
t.Parallel()

agentName := "same-name-different-workspace"

db, _ := dbtestutil.NewDB(t)
org := dbgen.Organization(t, db, database.Organization{})

// Given: A workspace with an agent
_, _, agent1 := createWorkspaceWithAgent(t, db, org, agentName)
require.Equal(t, agentName, agent1.Name)

// When: A second workspace is created with an agent having the same name
_, _, agent2 := createWorkspaceWithAgent(t, db, org, agentName)
require.Equal(t, agentName, agent2.Name)

// Then: We expect there to be different agents with the same name.
require.NotEqual(t, agent1.ID, agent2.ID)
require.Equal(t, agent1.Name, agent2.Name)
})

t.Run("NullWorkspaceID", func(t *testing.T) {
t.Parallel()

db, _ := dbtestutil.NewDB(t)
org := dbgen.Organization(t, db, database.Organization{})
ctx := testutil.Context(t, testutil.WaitShort)

// Given: A resource that does not belong to a workspace build (simulating template import)
orphanJob := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{
Type: database.ProvisionerJobTypeTemplateVersionImport,
OrganizationID: org.ID,
})
orphanResource := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{
JobID: orphanJob.ID,
})

// And this resource has a workspace agent.
agent1, err := db.InsertWorkspaceAgent(ctx, database.InsertWorkspaceAgentParams{
ID: uuid.New(),
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
Name: "orphan-agent",
ResourceID: orphanResource.ID,
AuthToken: uuid.New(),
Architecture: "amd64",
OperatingSystem: "linux",
APIKeyScope: database.AgentKeyScopeEnumAll,
})
require.NoError(t, err)
require.Equal(t, "orphan-agent", agent1.Name)

// When: We created another resource that does not belong to a workspace build.
orphanJob2 := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{
Type: database.ProvisionerJobTypeTemplateVersionImport,
OrganizationID: org.ID,
})
orphanResource2 := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{
JobID: orphanJob2.ID,
})

// Then: We expect to be able to create an agent in this new resource that has the same name.
agent2, err := db.InsertWorkspaceAgent(ctx, database.InsertWorkspaceAgentParams{
ID: uuid.New(),
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
Name: "orphan-agent", // Same name as agent1
ResourceID: orphanResource2.ID,
AuthToken: uuid.New(),
Architecture: "amd64",
OperatingSystem: "linux",
APIKeyScope: database.AgentKeyScopeEnumAll,
})
require.NoError(t, err)
require.Equal(t, "orphan-agent", agent2.Name)
require.NotEqual(t, agent1.ID, agent2.ID)
})
}

func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) {
t.Helper()
require.ElementsMatch(t, expected, database.ConvertUserRows(found), msg)
Expand Down
8 changes: 4 additions & 4 deletionscoderd/insights_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -609,8 +609,8 @@ func TestTemplateInsights_Golden(t *testing.T) {
Name: "example",
Type: "aws_instance",
Agents: []*proto.Agent{{
Id: uuid.NewString(), // Doesn't matter, not used in DB.
Name: "dev",
Id: uuid.NewString(),// Doesn't matter, not used in DB.
Name:fmt.Sprintf("dev-%d", len(resources)), // Ensure unique name per agent
Auth: &proto.Agent_Token{
Token: authToken.String(),
},
Expand DownExpand Up@@ -1525,8 +1525,8 @@ func TestUserActivityInsights_Golden(t *testing.T) {
Name: "example",
Type: "aws_instance",
Agents: []*proto.Agent{{
Id: uuid.NewString(), // Doesn't matter, not used in DB.
Name: "dev",
Id: uuid.NewString(),// Doesn't matter, not used in DB.
Name:fmt.Sprintf("dev-%d", len(resources)), // Ensure unique name per agent
Auth: &proto.Agent_Token{
Token: authToken.String(),
},
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp