Skip to content

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

Merged
merged 3 commits into from
May 19, 2025

Conversation

oyeliseiev-ua
Copy link
Collaborator

…igration

@noprysk-ua
Copy link
Collaborator

It's work in progress, right?

@oyeliseiev-ua
Copy link
Collaborator Author

It's work in progress, right?

@noprysk-ua I'm waiting on management API sorting by regionName is live, because it breaks this resource

@noprysk-ua
Copy link
Collaborator

It's work in progress, right?

@noprysk-ua I'm waiting on management API sorting by regionName is live, because it breaks this resource

  cloud_provider  = data.singlestoredb_regions_v2.all.regions.0.provider
  region_name     = data.singlestoredb_regions_v2.all.regions.0.region_name

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

  cloud_provider  = "AWS"
  region_name     = "us-east-2"

So I'm not 100% sure if we're blocked (even if we're we may temporarily sort here)

@oyeliseiev-ua oyeliseiev-ua requested a review from noprysk-ua May 15, 2025 10:12
@oyeliseiev-ua oyeliseiev-ua marked this pull request as ready for review May 15, 2025 10:18
@oyeliseiev-ua oyeliseiev-ua requested a review from Copilot May 15, 2025 10:18
Copy link

@Copilot Copilot AI left a 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{

@oyeliseiev-ua
Copy link
Collaborator Author

@noprysk-ua Now it's ready. Take a look please.

@noprysk-ua noprysk-ua requested a review from Copilot May 15, 2025 15:12
Copy link

@Copilot Copilot AI left a 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) {

Copy link
Collaborator

@noprysk-ua noprysk-ua left a 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!

@oyeliseiev-ua oyeliseiev-ua merged commit f19b84f into master May 19, 2025
5 checks passed
@oyeliseiev-ua oyeliseiev-ua deleted the PLAT-7481 branch May 19, 2025 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants