-
Notifications
You must be signed in to change notification settings - Fork 178
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
fix: Avoids import error for database_user when both username and auth database contain hyphens #2928
base: master
Are you sure you want to change the base?
Conversation
APIx bot: a message has been sent to Docs Slack channel |
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.
LGTM!
func ImportSplit3(importRaw string) (ok bool, part1, part2, part3 string) { | ||
parts := strings.Split(importRaw, "/") | ||
if len(parts) != 3 { | ||
return false, "", "", "" | ||
} | ||
return true, parts[0], parts[1], parts[2] | ||
} | ||
|
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.
[Curious] Is the use of /
as a separator for import applied to all the resources? Besides this question, I think you should add additional documentation to highlight this change in the TF doc
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.
+1 would update the resource documentation import section.
@@ -364,3 +365,69 @@ func getDatabaseUserModel(roles, labels, scopes basetypes.SetValue, password typ | |||
Scopes: scopes, | |||
} | |||
} | |||
|
|||
func TestSplitDatabaseUserImportID(t *testing.T) { | |||
const invalidDefaultString = "import format error: to import a Database User, use the format {project_id}-{username}-{auth_database_name} OR {project_id}/{username}/{auth_database_name}" |
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.
can we export this error message to a common const and re-use here?
Description
id
by/
. See details on why/
in issueLink to any related issue(s): CLOUDP-289266 #2876
Type of change:
Required Checklist:
Further comments