Migrate index resource to plugin framework #698
Conversation
tobio
force-pushed
the
zero-replica-index
branch
from
f7b30e3 to
a7f722a
Compare
* 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)
| 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.
This reverts commit de65ccf.
3 participants
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.