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.

Sign up for GitHub

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

Jump to bottom

[WIP] Terraform plugin framework #3889

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft

[WIP] Terraform plugin framework #3889

wants to merge 21 commits into from

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Aug 13, 2024

Changes

Tests

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

parthban-db and others added 19 commits July 4, 2024 14:27
@parthban-db
Add codegen template for tfsdk struct (#3713)
a6e54e1 
* Add codegen template for tfsdk struct

* updated map and list

* -

* remove tf type for list and map

* comments

* added test again
@edwardfeng-db
update (#3738)
60f0597
@edwardfeng-db
update (#3759)
be7d694
@tanmay-db @nkvuong
@alexmos-db @hectorcast-db @rayalex @dependabot @alexott @karolusz @touchida @mgyucht @pietern @mkubicek @renaudhartert-db @Divyansh-db
[Internal] Bumped terraform-plugin-framework branch to latest main (
1f88025 
#3780)

## Changes
<!-- Summary of your changes that are easy to understand -->
Rebase the Plugin Framework branch to latest main + resolve conflicts:
9e8fd30

This needs to be merged before
#3719
can be merged. Rebasing the PR over main will lead to many
commits/changes so separating them in this PR.

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->
Unit tests 
- [ ] `make test` run locally
- [ ] relevant change in `docs/` folder
- [ ] covered with integration tests in `internal/acceptance`
- [ ] relevant acceptance tests are passing
- [ ] using Go SDK

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: vuong-nguyen <[email protected]>
Co-authored-by: Alex Moschos <[email protected]>
Co-authored-by: hectorcast-db <[email protected]>
Co-authored-by: Aleksandar Dragojević <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Alex Ott <[email protected]>
Co-authored-by: Karol <[email protected]>
Co-authored-by: touchida <[email protected]>
Co-authored-by: Miles Yucht <[email protected]>
Co-authored-by: Pieter Noordhuis <[email protected]>
Co-authored-by: mkubicek <[email protected]>
Co-authored-by: Renaud Hartert <[email protected]>
Co-authored-by: Divyansh Vijayvergia <[email protected]>
@tanmay-db
[Feature] Added support to use protocol version 6 provider server for…
3f3ad23 
… SDK plugin (#3719)

## Changes
<!-- Summary of your changes that are easy to understand -->
Upgrade SDK plugin to use protocol version 6 as this will be used
further for introducing plugin framework. We need to mux them (i.e.
support both sdkv2 and plugin framework) until all resources are
migrated to plugin framework.

Reference for PR on main branch:
#3714,
this PR against the main branch will be used to get the binary for
testing with data team.

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->
All Unit and Integration test passed.
- [x] `make test` run locally
- [ ] relevant change in `docs/` folder
- [ ] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [ ] using Go SDK
@edwardfeng-db
Implement basic functionality for StructToSchema (#3788)
cf5d4bb 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Baisc functionality for `StructToSchema`, everything is `Optional` now
- Need to do:
  - Implement equivalent for `CustomizableSchema`
  - Make tfsdk structs contain information about optional and computed
  - Handle recursive data types

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@edwardfeng-db
Add optional tags to tfsdk struct + Update StructToSchema (#3801)
76b0745 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Added new `tf` tag in the `tfsdk` structs
- Made `StructToSchema` able to parse and handle optional vs required
fields

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@edwardfeng-db
[Fix] Make plugin framework infra functions work for Enum types (#3813)
f94bd5d 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Make the two converter functions + StructToSchema work for Enum types
- We do not need to use `types.String` for Enum types as they do not
have a force send zero value problem
- Adding a unit test test case

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@edwardfeng-db
Make CustomizableSchema work for Plugin Framework (#3819)
1562784 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Added basic supports for CustomizableSchema
- Able to add, remove fields and navigate through the schema
- Feature support matrix:
https://docs.google.com/document/d/1ofoLi9vlu-4L-89kBOYPbpOpPk0GZghZcC5SgnBH4ZM/edit#heading=h.wi3qt4pwk7q4

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@tanmay-db @nkvuong
@alexmos-db @hectorcast-db @rayalex @dependabot @alexott @karolusz @touchida @mgyucht
[Feature] Introduced Terraform Plugin Framework (#3751)
51411f1 
## Changes
<!-- Summary of your changes that are easy to understand -->
This PR introduces plugin framework to the Databricks Terraform
Provider.
- Introduce plugin framework
- Add core support for resource and data source. 
- Add provider schema
- Add support to configure databricks client
- Add support for provider fixture
- Add support to get raw config to be used in provider tests and further
in unit testing framework.
- Add Quality Monitor resource using plugin framework
- Add UC Volumes data source using plugin framework
- Fix bugs related in configuring client
- Fix bugs where provider schema is used instead of resource schema
- Fix miscellaneous bugs 

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->
- Run existing provider tests also for plugin framework. All passes. 
- Manually tested the above 2 resources E2E (apply works for data
source, for resource -- we need customise schema support for
`SetReadOnly`)

Note: We don't have unit and integration test support as of now, so
going ahead with commiting the .tf files for these resources so we can
test the changes in the meantime. Once we have proper support, these can
be converted to example or removed.

<img width="569" alt="image"
src="https://github.com/user-attachments/assets/869cacd7-a471-49db-93de-1a7bbb8b5ea7">

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: vuong-nguyen <[email protected]>
Co-authored-by: Alex Moschos <[email protected]>
Co-authored-by: hectorcast-db <[email protected]>
Co-authored-by: Aleksandar Dragojević <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Alex Ott <[email protected]>
Co-authored-by: Karol <[email protected]>
Co-authored-by: touchida <[email protected]>
Co-authored-by: Miles Yucht <[email protected]>
@edwardfeng-db
Support read only in plugin framework CustomizableSchema (#3842)
1f4a383 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Support `SetReadOnly` in `CustomizableSchema` for terraform plugin
framework

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@edwardfeng-db
Bump up terraform plugin framework version (#3860)
f3f1ef0 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Bump up to the latest released version

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@edwardfeng-db
Update resource quality monitor in plugin framework (#3853)
3407da1 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Part of making acceptance test pass:
#3841
- Update `ResourceQualityMonitor` to make it pass acceptance test
- Commented out adding two new fields temporarily because we need to
have those fields in the tfsdk struct, otherwise `.get()` and `.set()`
don't work
- Updated the target struct for `.get` calls to be `MonitorInfo` because
terraform requires that to be consistent with the schema
- Added logic at the end of `Create` and `Update` to read the latest
`monitorInfo` and set the state

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@edwardfeng-db
Make tfsdk struct enums into types.String (#3852)
4de413d 
## Changes
<!-- Summary of your changes that are easy to understand -->
- These enums have to be strings instead of the indirect types that they
used to be, otherwise `.get` and `.set` does not work properly
- First part of making the acceptance test work as specified here
#3841

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [ ] `make test` run locally
- [ ] relevant change in `docs/` folder
- [ ] covered with integration tests in `internal/acceptance`
- [ ] relevant acceptance tests are passing
- [ ] using Go SDK
@edwardfeng-db
Update reflect_resource_plugin_framework to make acceptance test pass (…
05692c9 
…#3854)

## Changes
<!-- Summary of your changes that are easy to understand -->
- Subpart of
#3841
- Updated reflect_resource_plugin_framework to handle cases
- When a field in the given source struct is nil, the converter function
has to be able to handle it
  - Handle enums in tfsdk structs as types.String
- When a field does not exist on the destination struct, ignore the
field
- Added unit test for the cases mentioned above

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@edwardfeng-db
Remove AddNewField and RemoveField for customizable_schema_plugin_fra…
267b3e8 
…mework (#3868)

## Changes
<!-- Summary of your changes that are easy to understand -->
- Removing these two methods as they will cause the schema and the
struct to be inconsistent

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@edwardfeng-db
Update init test and migrate imports for plugin framework (#3867)
4ad0fbb 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Part of
#3841
- Migrated imports following the documentation
[here](https://developer.hashicorp.com/terraform/plugin/testing/migrating)
  - 
<img width="688" alt="image"
src="https://github.com/user-attachments/assets/f2f14175-f3f8-47b7-a19f-1f0fca13eb47">

- Made `run` in `init_test` able to take both plugin framework resources
as well as sdkv2 resources

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@edwardfeng-db
Add quality monitor acceptance test for plugin framework (#3876)
6ff85a8 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Add two more test cases replicating existing test case for quality
monitor resource, but for the plugin framework
- Manually executed the tests to make sure they are passing
- Taking out the manual read check at the end of each test step as
suggested
[here](#3867 (comment))
because the type casting for clients is causing issue for plugin
framework tests

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@edwardfeng-db
Add acceptance test for volumes data source (#3879)
79121d6 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Make `ids` computed
- Add test case for `data_volumes_pluginframework`

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [ ] `make test` run locally
- [ ] relevant change in `docs/` folder
- [ ] covered with integration tests in `internal/acceptance`
- [ ] relevant acceptance tests are passing
- [ ] using Go SDK
mgyucht
mgyucht commented Aug 13, 2024
View reviewed changes
.codegen/model.go.tmpl Show resolved Hide resolved
.codegen/model.go.tmpl Outdated Show resolved Hide resolved
.codegen/model.go.tmpl Outdated Show resolved Hide resolved
.codegen.json Outdated Show resolved Hide resolved
common/customizable_schema_plugin_framework.go
@@ -0,0 +1,347 @@
package common
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Plugin framework code to its own package.

common/customizable_schema_plugin_framework.go
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
)

type CustomizableSchemaPluginFramework struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

doccoment?

common/customizable_schema_plugin_framework.go Show resolved Hide resolved
common/customizable_schema_plugin_framework.go
cb := func(attr schema.Attribute) schema.Attribute {
val := reflect.ValueOf(attr)

// Make a copy of the existing attr.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may want to fail if the underlying struct is already set properly (e.g. if read-only can be inferred from OpenAPI spec and it is added to TF structs, force us to remove any manual readonly annotations to remove redundant customizations).

edwardfeng-db and others added 2 commits August 15, 2024 14:38
@edwardfeng-db
Update tfsdk struct generation template (#3906)
c4e9c06 
## Changes
<!-- Summary of your changes that are easy to understand -->
- Addressing comments from
databricks/universe#682431
- Specifically
  - Removed unnecessary parts of the codegen template (map, enum)
  - Moved generated package into `internal/services`

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

- [x] `make test` run locally
- [x] relevant change in `docs/` folder
- [x] covered with integration tests in `internal/acceptance`
- [x] relevant acceptance tests are passing
- [x] using Go SDK
@tanmay-db @nkvuong
@alexmos-db @hectorcast-db @rayalex @dependabot @alexott @karolusz @touchida @mgyucht @pietern @mkubicek @renaudhartert-db @Divyansh-db @arpitjasa-db @840
[Internal] Rebase and Fixed merge conflicts with main branch. Also fi…
c5b5e5f 
…xed notification_destination acceptance tests. (#3892)

## Changes
<!-- Summary of your changes that are easy to understand -->
- Rebase to latest main
- Fixed merge conflicts
- Fixed notifications tests as they are incompatible with
terraform-plugin-testing because the test use a different type instead
of directly using the underlying type `func(*terraform.State) error`

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->
Existing unit tests + Verified that no unexpected changes are there by
creating draft PR over main:
#3893.
- [ ] `make test` run locally
- [ ] relevant change in `docs/` folder
- [ ] covered with integration tests in `internal/acceptance`
- [ ] relevant acceptance tests are passing
- [ ] using Go SDK

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: vuong-nguyen <[email protected]>
Co-authored-by: Alex Moschos <[email protected]>
Co-authored-by: hectorcast-db <[email protected]>
Co-authored-by: Aleksandar Dragojević <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Alex Ott <[email protected]>
Co-authored-by: Karol <[email protected]>
Co-authored-by: touchida <[email protected]>
Co-authored-by: Miles Yucht <[email protected]>
Co-authored-by: Pieter Noordhuis <[email protected]>
Co-authored-by: mkubicek <[email protected]>
Co-authored-by: Renaud Hartert <[email protected]>
Co-authored-by: Divyansh Vijayvergia <[email protected]>
Co-authored-by: Arpit Jasapara <[email protected]>
Co-authored-by: Cedric <[email protected]>
Co-authored-by: Alex Ott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edwardfeng-db edwardfeng-db edwardfeng-db left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mgyucht @edwardfeng-db @parthban-db @tanmay-db