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

POC: Standardize and verify ID formats #1380

Merged
merged 3 commits into from
Feb 29, 2024
Merged

Conversation

julienduchesne
Copy link
Member

@julienduchesne julienduchesne commented Feb 26, 2024

Currently, resource ID formats are all over the place. The separator is mostly :, but sometimes it's _, / or ;

In this PR, I create a helper struct to manage IDs:

  • It defines a default separator with retrocompatibility with the old formats. The old separators should be deprecated on the next major version
  • It provides validation for the number of fields provided. Currently, it panics in most resources if you provide not enough fields
  • It does auto-generation of the import.sh files

If this looks good, I can expand this to all resources and then validate that the ID is set this way for all resources. Providing consistent import and ID management for all resources!

Currently, resource ID formats are all over the place.
The separator is mostly `:`, but sometimes it's `_`, `/` or `;`

In this PR, I create a helper struct to manage IDs:
- It defines a default separator (with retrocompatibility with the old formats
- It provides validation for the number of fields provided. Currently, it panics in most resources if you provide not enough fields
- It does auto-generation of the `import.sh` files

If this looks good, I can expand this to all resources and then validate that the ID is set this way for all resources. Providing consistent import and ID management for all resources!
@julienduchesne julienduchesne requested a review from a team February 26, 2024 19:38
@julienduchesne julienduchesne requested review from a team as code owners February 26, 2024 19:38
Copy link

In order to lower resource usage and have a faster runtime, PRs will not run Cloud tests automatically.
To do so, a Grafana Labs employee must trigger the cloud acceptance tests workflow manually.

Comment on lines +44 to +64
// Import with different ID formats (Legacy and current)
{
ResourceName: "grafana_cloud_plugin_installation.test-installation",
ImportState: true,
ImportStateVerify: true,
ImportStateId: fmt.Sprintf("%s_%s", stackSlug, pluginSlug),
},
{
ResourceName: "grafana_cloud_plugin_installation.test-installation",
ImportState: true,
ImportStateVerify: true,
ImportStateId: fmt.Sprintf("%s:%s", stackSlug, pluginSlug),
},
// Test import with invalid ID
{
ResourceName: "grafana_cloud_plugin_installation.test-installation",
ImportState: true,
ImportStateVerify: true,
ImportStateId: "noseparator",
ExpectError: regexp.MustCompile("Error: id \"noseparator\" does not match expected format. Should be in the format: stackSlug:pluginSlug"),
},
Copy link
Member Author

Choose a reason for hiding this comment

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

The helper is essentially tested here

@julienduchesne julienduchesne merged commit d960a6d into main Feb 29, 2024
25 checks passed
@julienduchesne julienduchesne deleted the julienduchesne/gen-imports branch February 29, 2024 14:22
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