Skip to content

fix!: enforce agent names be case-insensitive-unique per-workspace - #16614

Merged
ethanndickson merged 1 commit into
mainfrom
ethan/case-insensitive-unique-agent-names
Feb 20, 2025
Merged

fix!: enforce agent names be case-insensitive-unique per-workspace#16614
ethanndickson merged 1 commit into
mainfrom
ethan/case-insensitive-unique-agent-names

Conversation

@ethanndickson

@ethanndickson ethanndickson commented Feb 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Relates to coder/coder-desktop-macos#54

Currently, it's possible to have two agents within the same workspace whose names only differ in capitalization:
This leads to an ambiguity in two cases:

  • For CoderVPN, we'd like to allow support to workspaces with a hostname of the form: agent.workspace.username.coder.
  • Workspace apps (coder_apps) currently use subdomains of the form: <app>--<agent>--<workspace>--<username>(--<suffix>)?.

Of note is that DNS hosts must be strictly lower case, hence the ambiguity.

This fix is technically a breaking change, but only for the incredibly rare use case where a user has:

  • A workspace with two agents
  • Those agent names differ only in capitalization.

Those templates & workspaces will now fail to build. This can be fixed by choosing wholly unique names for the agents.

@ethanndickson ethanndickson mentioned this pull request Feb 19, 2025
Merged
@ethanndickson Graphite App

ethanndickson commented Feb 19, 2025

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ethanndickson
ethanndickson force-pushed the ethan/case-insensitive-unique-agent-names branch from ea0c978 to 5241805 Compare February 19, 2025 05:30
@ethanndickson
ethanndickson force-pushed the ethan/case-insensitive-unique-agent-names branch from 5241805 to 566e3de Compare February 19, 2025 05:50
@ethanndickson
ethanndickson marked this pull request as ready for review February 19, 2025 05:57
@ethanndickson ethanndickson added the release/breaking This label is applied to PRs to detect breaking changes as part of the release process label Feb 19, 2025
mafredri
mafredri approved these changes Feb 19, 2025
View reviewed changes

@mafredri mafredri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had no idea this was allowed 🤯, nice find!

Should we add this guard on the provider as well? I'm thinking we can surface the error faster/better over there. I absolutely think it's worth guarding the input here as well, though.

@ethanndickson

ethanndickson commented Feb 19, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Should we add this guard on the provider as well? I'm thinking we can surface the error faster/better over there. I absolutely think it's worth guarding the input here as well, though.

Unfortunately not possible, since we derive the agent name from the name of the resource, which the provider SDK purposefully doesn't expose to us :( [1]

i.e. for resource "coder_agent" "agent_name" there's no way in the provider Go code to read agent_name.

For the same reason, you can't use the for_each meta-argument with agents.

@ethanndickson Graphite App

ethanndickson commented Feb 20, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Merge activity

  • Feb 19, 8:51 PM EST: A user started a stack merge that includes this pull request via Graphite.
  • Feb 19, 8:51 PM EST: A user merged this pull request with Graphite.

@ethanndickson
ethanndickson merged commit 3fddfef into main Feb 20, 2025
@ethanndickson
ethanndickson deleted the ethan/case-insensitive-unique-agent-names branch February 20, 2025 01:51
@ethanndickson ethanndickson mentioned this pull request Feb 20, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mafredri mafredri mafredri approved these changes
@deansheather deansheather deansheather approved these changes

Assignees

@ethanndickson ethanndickson

Labels

release/breaking This label is applied to PRs to detect breaking changes as part of the release process

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ethanndickson @mafredri @deansheather