Skip to content

fix!: enforce regex for agent names #16641

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 20, 2025
Merged

fix!: enforce regex for agent names #16641

merged 2 commits into from
Feb 20, 2025

Conversation

deansheather
Copy link
Member

@deansheather deansheather commented Feb 20, 2025

Underscores and trailing, leading and double hyphens are now blocked. The regex is almost the exact same as the coder_app slug regex, but uppercase characters are still permitted.

Underscores and double hyphens are now blocked. The regex is almost the
exact same as the `coder_app` `slug` regex, but uppercase characters are
still permitted.
@deansheather deansheather added the release/breaking This label is applied to PRs to detect breaking changes as part of the release process label Feb 20, 2025
@deansheather deansheather self-assigned this Feb 20, 2025
Copy link
Member

@ethanndickson ethanndickson left a comment

Choose a reason for hiding this comment

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

lgtm after a lint!
I like that we have a clear error message on what changed.

Comment on lines +219 to +221
if tfResource.Name == "" {
return nil, xerrors.Errorf("agent name cannot be empty")
}
Copy link
Member

Choose a reason for hiding this comment

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

This is technically already checked for us by Terraform, but I think it's fine to check again?

Copy link
Member

@ethanndickson ethanndickson Feb 20, 2025

Choose a reason for hiding this comment

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

I guess in case we ever change how we extract the name it's good to have

@deansheather deansheather enabled auto-merge (squash) February 20, 2025 04:56
@deansheather deansheather merged commit 9469b78 into main Feb 20, 2025
33 checks passed
@deansheather deansheather deleted the dean/agent-name-regex branch February 20, 2025 05:09
@github-actions github-actions bot locked and limited conversation to collaborators Feb 20, 2025
@matifali
Copy link
Member

@ethanndickson @stirby, we need to call this out in the changelog.

@ethanndickson
Copy link
Member

Yep, this and #16614 are both annotated with the breaking tag

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release/breaking This label is applied to PRs to detect breaking changes as part of the release process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants