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

Fixing Case Handling for Volume Resource #1074

Merged

Conversation

T-jegou
Copy link
Contributor

@T-jegou T-jegou commented Nov 10, 2023

Hello, this Pull Request is intended to address the issue #901. However, I've observed that the API V2 accepts, for certain types of resources, the region to be provided in uppercase.

In my Pull Request, I've implemented a solution for the volume resource, involving converting the region string to lowercase when it's added to the opts structure in the resourceDigitalOceanVolumeCreate function, to ensure that the provider calls godo with the region field in lowercase.

However, I find that this approach lacks consistency since the API handles the case of a region field in uppercase in the context of the droplet resource (and probably others).

Perhaps the fix should be applied directly to the API.

Nevertheless, strictly adhering to the API documentation, the region slugs are only in lowercase. In the case where a region is indicated in uppercase, either in the resource schema, we apply a condition to ensure the region is only in lowercase, and the documentation should state "lowercase only" for region slugs, or we accept and handle the formatting to lowercase for storage in the state (with StateFunc) and for sending data to godo in opts.

Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

Hey @T-jegou! Thanks for the PR. I agree that it would be better if the API handled this consistently, and we can share that feedback internally. Though I think this change is helpful for Terraform users in the meantime.

@andrewsomething andrewsomething merged commit c643845 into digitalocean:main Nov 14, 2023
2 checks passed
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.

None yet

2 participants