Skip to content

Migrate ILM resource to the plugin framework#2002

Open
tobio wants to merge 7 commits intomainfrom
ilm-plugin-framework
Open

Migrate ILM resource to the plugin framework#2002
tobio wants to merge 7 commits intomainfrom
ilm-plugin-framework

Conversation

@tobio
Copy link
Copy Markdown
Member

@tobio tobio commented Mar 24, 2026

Related to #482

Note

Migrate elasticstack_elasticsearch_index_lifecycle resource to the Plugin Framework

  • Removes the SDK-based ILM resource from provider.go and registers a new Plugin Framework implementation via provider/plugin_framework.go.
  • Replaces list-nested blocks (e.g. hot.0.rollover.*) with SingleNestedBlock object-shaped attributes (e.g. hot.rollover.*) across all ILM phases and actions in the new schema.go.
  • Implements a v0→v1 state upgrader in state_upgrade.go that unwraps prior list-wrapped phase and action blocks into the new object form.
  • Converts PutIlm, GetIlm, and DeleteIlm in index.go from SDK diagnostics to framework-style fwdiags.Diagnostics.
  • Adds cross-version acceptance tests (TestAccResourceILMFromSDK) verifying state compatibility between the legacy SDK provider (0.14.3) and the new framework-based provider.
  • Behavioral Change: phase configuration now uses single nested blocks instead of lists; existing state is automatically upgraded, but any external tooling or modules that reference list-indexed paths like hot.0.* will break.

Macroscope summarized 1712951.

@tobio tobio self-assigned this Mar 24, 2026
Copilot AI review requested due to automatic review settings March 24, 2026 09:00

This comment was marked as resolved.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 24, 2026

⚠️ Terraform acceptance tests failed for snapshot stack version 9.4.0-SNAPSHOT.
This failure is non-blocking because snapshot builds are allowed to fail.

Run: https://github.com/elastic/terraform-provider-elasticstack/actions/runs/23486141485

tobio and others added 3 commits March 24, 2026 21:16
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment on lines +58 to +59
for actionName, action := range p {
if a := action.([]any); len(a) > 0 {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Low ilm/expand.go:58

The type assertion action.([]any) on line 59 panics at runtime if action has any other type, such as nil or a different concrete type. This happens before the len(a) > 0 check, so malformed input causes a crash rather than a handled error. Consider using the two-return form a, ok := action.([]any) and skipping the action if !ok, or returning a diagnostic error.

-		if a := action.([]any); len(a) > 0 {
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file internal/elasticsearch/index/ilm/expand.go around lines 58-59:

The type assertion `action.([]any)` on line 59 panics at runtime if `action` has any other type, such as `nil` or a different concrete type. This happens before the `len(a) > 0` check, so malformed input causes a crash rather than a handled error. Consider using the two-return form `a, ok := action.([]any)` and skipping the action if `!ok`, or returning a diagnostic error.

Evidence trail:
internal/elasticsearch/index/ilm/expand.go lines 48-59 (REVIEWED_COMMIT) - shows `expandPhase(p map[string]any, ...)` and line 59 `if a := action.([]any); len(a) > 0 {`
internal/elasticsearch/index/ilm/model_expand.go lines 83-127 (REVIEWED_COMMIT) - `attrValueToExpandRaw` returns `string`, `int64`, `bool`, or `[]any` depending on type
internal/elasticsearch/index/ilm/model_expand.go lines 49-51 (REVIEWED_COMMIT) - `applyAllocateJSONDefaults` uses defensive comma-ok form: `allocList, ok := allocRaw.([]any); if !ok || len(allocList) == 0 { return }`

if v, ok := action.(map[string]any)[setting]; ok && v != nil {
options := ilmActionSettingOptions[setting]

if options.minVersion != nil && options.minVersion.GreaterThan(serverVersion) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Low ilm/expand.go:138

When serverVersion is nil, the call to options.minVersion.GreaterThan(serverVersion) at line 138 panics with a nil pointer dereference. This can occur when the Elasticsearch server version cannot be determined. Consider adding a nil check before the version comparison, or handling the nil serverVersion case explicitly.

-			if options.minVersion != nil && options.minVersion.GreaterThan(serverVersion) {
+			if options.minVersion != nil && serverVersion != nil && options.minVersion.GreaterThan(serverVersion) {
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file internal/elasticsearch/index/ilm/expand.go around line 138:

When `serverVersion` is `nil`, the call to `options.minVersion.GreaterThan(serverVersion)` at line 138 panics with a nil pointer dereference. This can occur when the Elasticsearch server version cannot be determined. Consider adding a nil check before the version comparison, or handling the nil `serverVersion` case explicitly.

Evidence trail:
internal/elasticsearch/index/ilm/expand.go:138 (REVIEWED_COMMIT) - shows code `if options.minVersion != nil && options.minVersion.GreaterThan(serverVersion)` which only checks `options.minVersion` for nil, not `serverVersion`. github.com/hashicorp/go-version version.go:110-118 shows `Compare` method immediately calls `other.String()` without nil check. github.com/hashicorp/go-version version.go:286-288 shows `GreaterThan` simply returns `v.Compare(o) > 0` with no nil handling. Compare with `Equal` method (lines 279-284) which does have nil handling: `if v == nil || o == nil { return v == o }`.

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