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

[WIP][Feature] Validate enum values in the plugin framework #4478

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Feb 7, 2025

Changes

Many fields in the various requests and responses of the Databricks REST API are enums. Attributes in the TF provider corresponding to enums are string types, and users enter the string value of the enum value that they want to specify. Today, there is no validation that the specified attribute value is actually one of the allowed enum values. This could result in confusing behavior: in SDKv2-based resources, the enum value is likely passed through to the REST API, where it would be considered "UNKNOWN" and likely rejected; and in plugin framework-based resources, the enum value will fail to be converted from the TFSDK structure to the Go SDK.

To provide a better experience, we can validate upfront that the plan only contains valid enum values for such attributes. This way, the failure is much more user-friendly and it happens earlier in the cycle (plan, as opposed to apply).

One key point is that as the API supports new enum values, existing provider versions will no longer be able to use them without upgrading. This may present friction in adoption of new enum values for users. My hypothesis is that this slightly increased friction may be worth the improved user experience.

One point we will also need to verify is that output-only fields (i.e. computed, non-optional, non-required) are still allowed to take on any value (e.g. a state attribute if a new state is introduced). There is no point in validating these fields, since users cannot specify them in Terraform, and they are not included in any requests to the REST API.

Tests

  • make test run locally
  • relevant change in docs/ folder
  • covered with integration tests in internal/acceptance
  • using Go SDK
  • using TF Plugin Framework

@mgyucht mgyucht requested review from a team as code owners February 7, 2025 09:17
@mgyucht mgyucht requested review from tanmay-db and removed request for a team February 7, 2025 09:17
@mgyucht mgyucht temporarily deployed to test-trigger-is February 7, 2025 09:17 — with GitHub Actions Inactive
@mgyucht mgyucht temporarily deployed to test-trigger-is February 7, 2025 09:17 — with GitHub Actions Inactive
Copy link

github-actions bot commented Feb 7, 2025

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

  • PR number: 4478
  • Commit SHA: 815cdbae484ccf54a5e1b73554d4ba576c3bd9bc

Checks will be approved automatically on success.

Copy link

github-actions bot commented Feb 7, 2025

Please ensure that the NEXT_CHANGELOG.md file is updated with any relevant changes.
If this is not necessary for your PR, please include the following in your PR description:
NO_CHANGELOG=true
and rerun the job.

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.

1 participant