Migrate index resource to plugin framework #698
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tobio
force-pushed
the
zero-replica-index
branch
from
f7b30e3 to
a7f722a
Compare
biscout42
reviewed
Aug 6, 2024
dimuon
reviewed
Aug 26, 2024
* origin/main: Add support for the `alert_delay` param in the Create Rule API (#715) chore: prepare release v0.11.6 (#716) Validate that mappings are a JSON object, not just valid json (#719) fix: move all resources in one namespace for tcp monitor acc tests (#717) Bump github.com/golangci/golangci-lint from 1.59.1 to 1.60.1 in /tools (#714) Bump github.com/docker/docker in /tools (#718) Bump github.com/goreleaser/goreleaser from 1.26.1 to 1.26.2 in /tools (#642) Bump github.com/hashicorp/terraform-plugin-framework (#705) Add kibana synthetics http and tcp monitor resources (#699) Kibana spaces data source (#682) Use ephemeral github token for build. (#712) chore: 8.15.0 is here - lets try it out (#708) Update changelog for 0.11.5 Bump version for 0.11.5 (#706) Bugfix SLO API: Update type for `group_by` to accept either string or array-of-strings (#701) Support `restriction` in `elasticstack_elasticsearch_security_api_key` (#577) chore: follow-up CR changes for synthetics private location resource (#697)
dimuon
reviewed
Aug 27, 2024
dimuon
reviewed
Aug 27, 2024
dimuon
reviewed
Aug 28, 2024
| func setSettingsFromAPI(ctx context.Context, model *tfModel, apiModel models.Index) diag.Diagnostics { | ||
| modelType := reflect.TypeOf(*model) | ||
|
|
||
| for _, key := range dynamicSettingsKeys { |
There was a problem hiding this comment.
Out of curiosity: did you consider using struct instead of map of interfaces for Index.settings? The below code resembles low-level code from the FW. I understand that ES client doesn't provide a model for index but what if we define it instead? Then we can rely on golang types and conversions from the FW.
There was a problem hiding this comment.
I didn't, but probably because I was keeping this analogous to the tf -> api conversion logic. It's an interesting point though.
I had a quick look at this, and I'm concerned that there's a bunch of inconsistencies around how ES handles settings which make this tricky. For example, number_of_shards is defined in the schema as an int64, we pass it to ES as an int, but it gets returned as a string which then breaks decoding (since it's an int in the schema).
I'm sure there's some logic that we could build to make this stuff work, but I'm not sure it will be much nicer than whats in this PR. I can dig in here more if you think it's worthwhile, but I'm not convinced it's worth the time.
There was a problem hiding this comment.
I trust your judgement. Let's keep it as is.
dimuon
reviewed
Aug 28, 2024
dimuon
approved these changes
Sep 9, 2024
tobio
added a commit
to jdesnoes/terraform-provider-elasticstack
that referenced
this pull request
Sep 11, 2024
…-data-source * origin/main: (23 commits) Migrate index resource to plugin framework (elastic#698) Remove duplicate code (elastic#761) fix(deps): update module github.com/hashicorp/terraform-plugin-framework-jsontypes to v0.2.0 (elastic#760) fix(deps): update module github.com/golangci/golangci-lint to v1.61.0 (elastic#759) Add support for the`frequency` field in the Create Rule API (elastic#753) Prevent a provider panic when the referenced snapshot repo does not exist (elastic#758) remove space_id paramter from private locations (elastic#733) chore(deps): update dependency go to v1.23.1 (elastic#755) chore(deps): update docker.elastic.co/elasticsearch/elasticsearch docker tag to v8.15.1 (elastic#756) chore(deps): update golang:latest docker digest to 4a3c2bc (elastic#754) chore(deps): update docker.elastic.co/kibana/kibana docker tag to v8.15.1 (elastic#757) Fixup error handling during saved object imports (elastic#738) fix(deps): update module github.com/goreleaser/goreleaser to v2 (elastic#748) chore(deps): update golangci/golangci-lint-action action to v6 (elastic#746) chore(deps): update codecov/codecov-action action to v4 (elastic#745) fix(deps): update module github.com/go-resty/resty/v2 to v2.14.0 (elastic#741) chore(deps): update actions/setup-go action to v5 (elastic#744) chore(deps): update actions/checkout action to v4 (elastic#743) fix(deps): update module github.com/deepmap/oapi-codegen/v2 to v2.3.0 (elastic#740) chore(deps): update dependency go to v1.23.0 (elastic#739) ...
daemitus
added a commit
to daemitus/terraform-provider-elasticstack
that referenced
this pull request
Sep 11, 2024
This reverts commit de65ccf.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #238
Partly meant as an evaluation of effort in doing these migrations. Partly solving actual customer issues. This PR migrates the existing SDK based index resource to a plugin framework based implementation.
There are no schema changes introduced in the migration, and the change is intended to be fully BWC with current definitions.
It adds an acceptance test explicitly verifying the migration behaviour (i.e creating an index with an SDK based provider version before verifying that the index continues to work with the latest code).
Needs a changelog entry. I can also split this one up more, it's been a thing.