Skip to content
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

Merged
merged 6 commits into from
Dec 31, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 3 additions & 0 deletions .changelog/2928.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/mongodbatlas_database_user: Avoids import error for database_user when both username and auth database contain hyphens
```
8 changes: 6 additions & 2 deletions docs/resources/database_user.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,14 @@ In addition to all arguments above, the following attributes are exported:

## Import

Database users can be imported using project ID and username, in the format `project_id`-`username`-`auth_database_name`, e.g.
Database users can be imported using project ID, username, and auth database name in the format:

1. `project_id`-`username`-`auth_database_name`
2. `project_id`/`username`/`auth_database_name`

```
$ terraform import mongodbatlas_database_user.my_user 1112222b3bf99403840e8934-my_user-admin
terraform import mongodbatlas_database_user.my_user 1112222b3bf99403840e8934-my_user-admin # (1)
terraform import mongodbatlas_database_user.my_user 1112222b3bf99403840e8934/my-username-dash/my-db-name # (2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we include here when user needs to use one format vs. the other?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point to explain why and when

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in caef9ba

```

~> **NOTE:** Terraform will want to change the password after importing the user if a `password` argument is specified.
8 changes: 8 additions & 0 deletions internal/common/conversion/import_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource"
)

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]
}

Comment on lines +13 to +20
Copy link
Collaborator

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

Copy link
Collaborator

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. Updated in c866b36
So far, this is only used by this resource. Added a ticket here to discuss more usages

Copy link
Collaborator

Choose a reason for hiding this comment

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

nice. Left a small comment on the import doc but this is good for me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This decision does indeed bring a good conversation to have if it should be extended to all resources

func ImportStateProjectIDClusterName(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse, attrNameProjectID, attrNameClusterName string) {
parts := strings.SplitN(req.ID, "-", 2)
if len(parts) != 2 {
Expand Down
49 changes: 49 additions & 0 deletions internal/common/conversion/import_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,52 @@ func TestValidateClusterName(t *testing.T) {
assert.Contains(t, err.Error(), "_invalidClusterName")
assert.Contains(t, err.Error(), "cluster_name must be a string with length between 1 and 64, starting and ending with an alphanumeric character, and containing only alphanumeric characters and hyphens")
}

func TestImportSplit3(t *testing.T) {
tests := map[string]struct {
importRaw string
part1 string
part2 string
part3 string
expected bool
}{
"valid input": {
importRaw: "part1/part2/part3",
expected: true,
part1: "part1",
part2: "part2",
part3: "part3",
},
"invalid input with more parts": {
importRaw: "part1/part2/part3/part4",
expected: false,
part1: "",
part2: "",
part3: "",
},
"invalid input with two parts": {
importRaw: "part1/part2",
expected: false,
part1: "",
part2: "",
part3: "",
},
"invalid input with one part": {
importRaw: "part1",
expected: false,
part1: "",
part2: "",
part3: "",
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
ok, part1, part2, part3 := conversion.ImportSplit3(tc.importRaw)
assert.Equal(t, tc.expected, ok)
assert.Equal(t, tc.part1, part1)
assert.Equal(t, tc.part2, part2)
assert.Equal(t, tc.part3, part3)
})
}
}
66 changes: 66 additions & 0 deletions internal/service/databaseuser/model_database_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/service/databaseuser"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.mongodb.org/atlas-sdk/v20241113004/admin"
)

Expand Down Expand Up @@ -364,3 +365,68 @@ func getDatabaseUserModel(roles, labels, scopes basetypes.SetValue, password typ
Scopes: scopes,
}
}

func TestSplitDatabaseUserImportID(t *testing.T) {
tests := map[string]struct {
importID string
projectID string
username string
authDBName string
errorString string
}{
"valid input": {
importID: "664619d870c247237f4b86a6/my-username-dash/my-db-name",
projectID: "664619d870c247237f4b86a6",
username: "my-username-dash",
authDBName: "my-db-name",
},
"valid input legacy": {
importID: "664619d870c247237f4b86a6-myUsernameCamel-mydbname",
projectID: "664619d870c247237f4b86a6",
username: "myUsernameCamel",
authDBName: "mydbname",
},
"invalid input projectID": {
importID: "part1/part2/part3",
projectID: "part1",
username: "part2",
authDBName: "part3",
errorString: "project_id must be a 24 character hex string: part1",
},
"invalid input with more parts": {
importID: "part1/part2/part3/part4",
projectID: "",
username: "",
authDBName: "",
errorString: databaseuser.ErrorImportFormat,
},
"invalid input with less parts": {
importID: "part1/part2",
projectID: "",
username: "",
authDBName: "",
errorString: databaseuser.ErrorImportFormat,
},
"empty input": {
importID: "",
projectID: "",
username: "",
authDBName: "",
errorString: databaseuser.ErrorImportFormat,
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
part1, part2, part3, err := databaseuser.SplitDatabaseUserImportID(tc.importID)
if tc.errorString != "" {
require.EqualError(t, err, tc.errorString)
} else {
require.NoError(t, err)
}
assert.Equal(t, tc.projectID, part1)
assert.Equal(t, tc.username, part2)
assert.Equal(t, tc.authDBName, part3)
})
}
}
12 changes: 8 additions & 4 deletions internal/service/databaseuser/resource_database_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/config"
)

const (
databaseUserResourceName = "database_user"
ErrorImportFormat = "import format error: to import a Database User, use the format {project_id}-{username}-{auth_database_name} OR {project_id}/{username}/{auth_database_name}"
)

var _ resource.ResourceWithConfigure = &databaseUserRS{}
Expand Down Expand Up @@ -346,17 +348,19 @@ func (r *databaseUserRS) ImportState(ctx context.Context, req resource.ImportSta
}

func SplitDatabaseUserImportID(id string) (projectID, username, authDatabaseName string, err error) {
ok, projectID, username, authDatabaseName := conversion.ImportSplit3(id)
if ok {
err = conversion.ValidateProjectID(projectID)
return
}
var re = regexp.MustCompile(`(?s)^([0-9a-fA-F]{24})-(.*)-([$a-z]{1,15})$`)
parts := re.FindStringSubmatch(id)

if len(parts) != 4 {
err = errors.New("import format error: to import a Database User, use the format {project_id}-{username}-{auth_database_name}")
err = errors.New(ErrorImportFormat)
return
}

projectID = parts[1]
username = parts[2]
authDatabaseName = parts[3]

return
}
Loading