-
Notifications
You must be signed in to change notification settings - Fork 6
PLAT-7481 Support Terraform Provider region_id to provider and name m… #63
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
It's work in progress, right? |
@noprysk-ua I'm waiting on management API sorting by regionName is live, because it breaks this resource |
I expect terraform users not to do it. They may read from regions yet not use the first item in the list. Instead, I expect them to hardcode the values
So I'm not 100% sure if we're blocked (even if we're we may temporarily sort here) |
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 supports the migration from using the legacy region_id configuration to the new cloud_provider and region_name parameters for the Terraform provider. Key changes include updating test cases to use the new region representation (regionsv2), renaming validation functions to better reflect the required parameters, and updating example and documentation files for consistent configuration.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
internal/provider/workspacegroups/resource_test.go | Updated tests to use regionsv2 and to check for cloud_provider/region_name rather than region_id. |
internal/provider/workspacegroups/resource.go | Renamed and refactored validation functions to support the new region migration logic. |
internal/provider/config/const.go | Updated mock admin password value for consistency in tests. |
examples/resources/singlestoredb_workspace_group_advanced/resource.tf | Replaced region_id with cloud_provider and region_name parameters. |
examples/resources/singlestoredb_workspace_group/resource.tf | Updated data source and parameters to use regions_v2. |
docs/resources/workspace_group.md | Documentation updated to reflect the migration changes in configuration. |
Comments suppressed due to low confidence (1)
internal/provider/workspacegroups/resource_test.go:32
- [nitpick] The variable 'regionsv2' clearly distinguishes the new region representation; consider adding a brief comment above this declaration to explain its purpose in the migration from region_id to cloud_provider/region_name for future maintainers.
regionsv2 := []management.RegionV2{
@noprysk-ua Now it's ready. Take a look please. |
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 migrates the Terraform provider to use the new region configuration parameters (cloud_provider and region_name) instead of the legacy region_id. Key changes include updating test cases to use regionsv2, revising resource validation and error messaging in the provider implementation, and updating related examples and documentation.
- Updated test files to use regionsv2 with cloud_provider and region_name.
- Modified resource validation functions and error messages to enforce the new region configuration.
- Updated examples and configuration constants to align with the new migration model.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
internal/provider/workspacegroups/resource_test.go | Updated tests to reference regionsv2, cloud_provider, and region_name. |
internal/provider/workspacegroups/resource.go | Revised validation functions and error messages for region migration. |
internal/provider/config/const.go | Updated admin password constant to a mock value. |
examples/resources/singlestoredb_workspace_group_advanced/resource.tf | Updated resource configuration to use cloud_provider and region_name. |
examples/resources/singlestoredb_workspace_group/resource.tf | Replaced legacy data source and region_id with regions_v2 and new parameters. |
docs/resources/workspace_group.md | Revised documentation to reflect the new region configuration. |
Comments suppressed due to low confidence (1)
internal/provider/workspacegroups/resource.go:482
- Verify that the validation logic for 'cloud_provider' aligns with the intended migration behavior and does not inadvertently conflict with any legacy usage of 'region_id'.
if !plan.CloudProvider.IsNull() && !state.CloudProvider.IsNull() && !plan.CloudProvider.Equal(state.CloudProvider) {
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.
Thank you for picking it up!
…igration