-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
azurerm_mongo_cluster
- Supports connection_strings
#28880
base: main
Are you sure you want to change the base?
Conversation
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.
Hey @magodo, thanks for this PR. It looks mostly good but I've left some comments that I'd appreciate if you could address.
return map[string]*schema.Schema{} | ||
func (r MongoClusterResource) Attributes() map[string]*pluginsdk.Schema { | ||
return map[string]*pluginsdk.Schema{ | ||
"connection_strings": { |
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.
We should change this to connection_string
because otherwise it'll look like the following in state as
connection_strings {
//connection string 1 info
}
connection_strings {
//connection string 2 info
}
A plural connection_strings
signifies multiple connection strings in a single block rather than one connection string per block.
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.
@mbfrahry This one is a read only attribute, I assume we shall keep it plural as the user can only reference them like: azurerm_mongo_cluster.example.connection_strings.x
?
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.
Ahhh good call! That makes sense to me! Thank you for walking me through that
Type: pluginsdk.TypeString, | ||
Computed: true, | ||
}, | ||
"connection_string": { |
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.
To avoid confusion around connection_string.x.connection_string
we should change this to value
.
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.
Done
@@ -452,6 +481,14 @@ func (r MongoClusterResource) Read() sdk.ResourceFunc { | |||
state.Tags = pointer.From(model.Tags) | |||
} | |||
|
|||
csResp, err := client.ListConnectionStrings(ctx, *id) | |||
if err != nil { | |||
return fmt.Errorf("listing connection string for %s: %+v", *id, err) |
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.
return fmt.Errorf("listing connection string for %s: %+v", *id, err) | |
return fmt.Errorf("listing connection strings for %s: %+v", *id, err) |
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.
Done
var name string | ||
if cs.Name != nil { | ||
name = *cs.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.
var name string | |
if cs.Name != nil { | |
name = *cs.Name | |
} | |
name := pointer.From(cs.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.
Done
if input == nil { | ||
return nil | ||
} | ||
|
||
results := make([]MongoClusterConnectionString, 0) |
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.
if input == nil { | |
return nil | |
} | |
results := make([]MongoClusterConnectionString, 0) | |
results := make([]MongoClusterConnectionString, 0) | |
if input == nil { | |
return results | |
} |
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.
Done
var description string | ||
if cs.Description != nil { | ||
description = *cs.Description | ||
} |
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.
var description string | |
if cs.Description != nil { | |
description = *cs.Description | |
} | |
description := pointer.From(cs.Description) |
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.
Done
var connectionString string | ||
if cs.ConnectionString != nil { | ||
connectionString = *cs.ConnectionString | ||
} |
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.
var connectionString string | |
if cs.ConnectionString != nil { | |
connectionString = *cs.ConnectionString | |
} | |
connectionString := pointer.From(cs.ConnectionString) |
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.
Done
|
||
* `description` - The description of the connection string. | ||
|
||
* `connection_string` - The Mongo Cluster connection string. The `<user>:<password>` place holder returned from API will be replaced by the real `administrator_username` and `administrator_password` if available in the state. |
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.
* `connection_string` - The Mongo Cluster connection string. The `<user>:<password>` place holder returned from API will be replaced by the real `administrator_username` and `administrator_password` if available in the state. | |
* `connection_string` - The Mongo Cluster connection string. The `<user>:<password>` placeholder returned from API will be replaced by the real `administrator_username` and `administrator_password` if available in the state. |
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.
Done
), | ||
}, | ||
data.ImportStep("administrator_password", "create_mode"), | ||
data.ImportStep("administrator_password", "create_mode", "connection_strings.0.connection_string"), |
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.
We'll need to add "connection_strings.0.connection_string"
to all data.ImportSteps in this test file
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.
Done
Thanks @mbfrahry! I've resolved all the comments except the 1st one, please take another look! |
Community Note
Description
azurerm_mongo_cluster
- Supportsconnection_strings
.Note that the
connection_string
returned from API is a template, with<user>:<password>
place holders. This PR replace them with the real user name and password, to make it directly usable for users. Since the password is only available in the state/config, this replacement won't occur during importing.PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
💢 TF_ACC=1 go test -v -timeout=20h -parallel=20 -run=TestAccMongoCluster_basic ./internal/services/mongocluster === RUN TestAccMongoCluster_basic --- PASS: TestAccMongoCluster_basic (1181.18s) PASS ok github.com/hashicorp/terraform-provider-azurerm/internal/services/mongocluster 1181.195s
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_mongo_cluster
- Supportsconnection_strings
[GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #28841
Note
If this PR changes meaningfully during the course of review please update the title and description as required.