-
Notifications
You must be signed in to change notification settings - Fork 156
Fix DbGate integration to handle resource names with hyphens #1107
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
Conversation
…ith hyphens Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where DbGate integration failed when database resources were named with hyphens (e.g., "mysql-db") because hyphens are not valid characters in Linux environment variable names. The fix introduces a SanitizeConnectionId method that replaces hyphens with underscores for environment variable names while preserving the original resource names in display labels.
Changes:
- Added
SanitizeConnectionIdmethod toDbGateBuilderExtensionsto sanitize resource names for use in environment variables - Updated all DbGate integration implementations (MySQL, PostgreSQL, MongoDB, Redis, SqlServer) to use sanitized connection IDs
- Added test coverage for MySQL integration with hyphenated resource names
- Fixed a typo in Redis integration (
lalbel→label)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/CommunityToolkit.Aspire.Hosting.DbGate/DbGateBuilderExtensions.cs | Added SanitizeConnectionId public static method to replace hyphens with underscores |
| src/CommunityToolkit.Aspire.Hosting.MySql.Extensions/MySqlBuilderExtensions.cs | Updated DbGate configuration to use sanitized connection IDs |
| src/CommunityToolkit.Aspire.Hosting.PostgreSQL.Extensions/PostgresBuilderExtensions.cs | Updated DbGate configuration to use sanitized connection IDs |
| src/CommunityToolkit.Aspire.Hosting.MongoDB.Extensions/MongoDBBuilderExtensions.cs | Updated DbGate configuration to use sanitized connection IDs |
| src/CommunityToolkit.Aspire.Hosting.Redis.Extensions/RedisBuilderExtensions.cs | Updated DbGate configuration to use sanitized connection IDs and fixed typo lalbel → label |
| src/CommunityToolkit.Aspire.Hosting.SqlServer.Extensions/SqlServerBuilderExtensions.cs | Updated DbGate configuration to use sanitized connection IDs |
| tests/CommunityToolkit.Aspire.Hosting.MySql.Extensions.Tests/ResourceCreationTests.cs | Added test verifying hyphenated resource names generate valid environment variables |
| public static string SanitizeConnectionId(string resourceName) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(resourceName); | ||
| return resourceName.Replace('-', '_'); | ||
| } |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SanitizeConnectionId method only replaces hyphens with underscores, but Linux environment variable names have additional constraints beyond just hyphens. Valid environment variable names can only contain letters, numbers, and underscores, and cannot start with a number. Consider validating or sanitizing other invalid characters as well (e.g., dots, spaces, special characters) to prevent future issues. At minimum, add a comment explaining that only hyphens are currently addressed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the documentation to explain that only hyphens are currently addressed and noted the limitations. The remarks section now clarifies that other invalid characters (dots, spaces, special characters) are not handled and resource names should follow standard naming conventions. (commit 46c1820)
tests/CommunityToolkit.Aspire.Hosting.MySql.Extensions.Tests/ResourceCreationTests.cs
Show resolved
Hide resolved
| /// Environment variable names in Linux cannot contain hyphens, so they are replaced with underscores. | ||
| /// This ensures that resource names like "mysql-db" become "mysql_db" when used in environment variable names. | ||
| /// </remarks> | ||
| public static string SanitizeConnectionId(string resourceName) |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new SanitizeConnectionId method lacks direct unit tests. While integration tests verify the sanitization works end-to-end in MySQL, there should be unit tests specifically for this method to cover edge cases like multiple hyphens, leading/trailing hyphens, empty strings (if allowed), and strings without hyphens. Consider adding tests to tests/CommunityToolkit.Aspire.Hosting.DbGate.Tests/DbGatePublicApiTests.cs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comprehensive unit tests for SanitizeConnectionId to DbGatePublicApiTests.cs, covering edge cases including multiple hyphens, leading/trailing hyphens, empty strings, and strings without hyphens. All tests pass. (commit 46c1820)
|
@copilot can you action the two comments |
Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
Minimum allowed line rate is |
Original prompt
This section details on the original issue you should resolve
<issue_title>DbGate does not work with MySql hosting integration</issue_title>
<issue_description>### Describe the bug
Pretty vanilla setup:
DbGate seems to get wrong connection string, here's the console log from DbGate resource:
Adminer with that setup works correctly
Regression
No response
Steps to reproduce
Expected behavior
DbGate is correctly connected to the mysql database
Screenshots
IDE and version
Rider
IDE version
No response
Nuget packages
Additional context
No response
Help us help you
Yes, but only if others can assist</issue_description>
<agent_instructions>Refer to the most recent comment for the instructions on what needs to be done to fix it</agent_instructions>
Comments on the Issue (you are @copilot in this section)
@aaronpowell Ooo nice catch! It turns out that the problem is because the way we map the resources through as connections is to just copy the resource name to the `CONNECTIONS` environment variable, and then create the other environment variables DbGate requires. The result of this is that the resource `mysql-db` generates an environment variable `ENGINE_mysql-db` which is not a valid environment variable name, since they can't have `-` in them.The easiest fix to unblock you right now would be to change the resource name to
mysqlormysqldb(without the-).By we should add some resiliency into the integration so that we clean the resource name up (not just for MySQL, but for any resource that supports DbGate) to ensure that the connection ID that is provided is valid in a Linux environment variable.</comment_new>
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.