fix(ml): anomaly_detection_job import/refresh mismatch #1550

neiljbrookes · 2025-12-15T14:52:10Z

Summary

Fixes elasticstack_elasticsearch_ml_anomaly_detection_job import/refresh mismatch reported in #1549.

Background / repro (from #1549)

terraform import elasticstack_elasticsearch_ml_anomaly_detection_job.example default/<job_id>
Import is sparse, then Terraform immediately runs a refresh (Read).
Provider previously failed during that refresh because the imported state did not yet contain required nested attributes.

Root cause

Import state is intentionally sparse, but the resource model was not able to decode analysis_config = null during the first Read after import.
Additionally, some list fields could become untyped zero values (DynamicPseudoType) when the API returned empty slices and we tried to preserve the pre-existing state value.

What changed

Sparse import state: ImportState now sets only identifiers (id + job_id) and normalizes IDs like default/<job_id> -> <job_id> (the actual Elasticsearch ML job id).
Nullable nested state during import: analysis_config in the state model is now nullable so req.State.Get() can succeed immediately after import, allowing Read to fetch and populate the full object from Elasticsearch.
Typed-empty list defaults: ensure analysis_config.categorization_filters, analysis_config.influencers, and detectors[*].custom_rules always return correctly typed list values (e.g., list(string) / list(object)), avoiding DynamicPseudoType conversion errors when API slices are empty.

Behavior after this change

terraform import ... default/<job_id> succeeds.
The subsequent refresh (Read) successfully populates analysis_config (and other attributes) from Elasticsearch.

Testing

go test ./...

Fixes #1549.

AI assistance

This change was implemented with the help of an AI coding assistant (Cursor + GPT).

Fixes terraform import/refresh failures for elasticstack_elasticsearch_ml_anomaly_detection_job by keeping ImportState sparse (id/job_id only) and allowing analysis_config to be null during import before Read populates it. Also ensures empty nested lists in analysis_config (e.g. categorization_filters/influencers/custom_rules) are always typed to avoid DynamicPseudoType conversion errors. AI assistance: This change was implemented with the help of an AI coding assistant (Cursor + GPT).

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Copilot · 2025-12-15T18:54:17Z

internal/elasticsearch/ml/anomaly_detection_job/resource.go

+	raw := req.ID
+	parts := strings.Split(raw, "/")
+	jobID := parts[len(parts)-1]


The ImportState function does not validate the import ID format before splitting. If req.ID is empty or does not contain the expected format, this could result in setting an empty jobID. Consider adding validation to ensure the ID is not empty and optionally check that it matches the expected format (e.g., 'default/<job_id>' or just '<job_id>').

Suggested change

raw := req.ID

parts := strings.Split(raw, "/")

jobID := parts[len(parts)-1]

raw := req.ID

if strings.TrimSpace(raw) == "" {

resp.Diagnostics.AddError(

"Invalid import ID",

"Import ID cannot be empty. Please provide a valid job ID or 'default/<job_id>'.",

)

return

}

parts := strings.Split(raw, "/")

jobID := parts[len(parts)-1]

if strings.TrimSpace(jobID) == "" {

resp.Diagnostics.AddError(

"Invalid import ID format",

"Could not extract a valid job ID from the import ID. Please use '<job_id>' or 'default/<job_id>'.",

)

return

}

Copilot · 2025-12-15T18:54:18Z

internal/elasticsearch/ml/anomaly_detection_job/models_tf.go

+	if analysisConfigTF.CategorizationFilters.ElementType(ctx) == nil {
+		analysisConfigTF.CategorizationFilters = types.ListNull(types.StringType)
+	} else if _, ok := analysisConfigTF.CategorizationFilters.ElementType(ctx).(basetypes.DynamicType); ok {
+		analysisConfigTF.CategorizationFilters = types.ListNull(types.StringType)
+	}


This logic for handling untyped zero-value lists is duplicated three times in this file (CategorizationFilters, Influencers, and CustomRules). Consider extracting this into a helper function that takes a list value and element type, returning a properly typed list to reduce duplication and improve maintainability.

👍 to making this reusable somewhere (either in this file or in the typeutil package).

Do we actually need this? I'm fairly sure we haven't added this into other resources which makes me a bit sceptical.

Best I can tell is that, yes, it is needed. Without handling this, I see the following errors :

elasticstack_elasticsearch_ml_anomaly_detection_job.example: Importing from ID "default/example-anomaly-detector"... elasticstack_elasticsearch_ml_anomaly_detection_job.example: Import prepared! Prepared elasticstack_elasticsearch_ml_anomaly_detection_job for import elasticstack_elasticsearch_ml_anomaly_detection_job.example: Refreshing state... [id=default/example-anomaly-detector] ╷ │ Error: Value Conversion Error │ │ An unexpected error was encountered while verifying an attribute value matched its expected type to prevent unexpected behavior or panics. This is always an │ error in the provider. Please report the following to the provider developer: │ │ Expected framework type from provider logic: types.ListType[basetypes.StringType] / underlying type: tftypes.List[tftypes.String] │ Received framework type from provider logic: types.ListType[!!! MISSING TYPE !!!] / underlying type: tftypes.List[tftypes.DynamicPseudoType] │ Path: analysis_config.categorization_filters

I'll try and refactor it somewhere more useful !

@copilot open a new pull request to apply changes based on this feedback

0185df6 from CoPilot seems a reasonable attempt.

tobio

Couple of small comments. Can we add an import test step into the existing acceptance tests to verify the import pathway whilst we're here?

internal/elasticsearch/ml/anomaly_detection_job/resource.go

tobio · 2025-12-16T02:36:14Z

internal/elasticsearch/ml/anomaly_detection_job/models_tf.go

+	if analysisConfigTF.CategorizationFilters.ElementType(ctx) == nil {
+		analysisConfigTF.CategorizationFilters = types.ListNull(types.StringType)
+	} else if _, ok := analysisConfigTF.CategorizationFilters.ElementType(ctx).(basetypes.DynamicType); ok {
+		analysisConfigTF.CategorizationFilters = types.ListNull(types.StringType)
+	}


👍 to making this reusable somewhere (either in this file or in the typeutil package).

Do we actually need this? I'm fairly sure we haven't added this into other resources which makes me a bit sceptical.

Co-authored-by: Toby Brain <[email protected]>

Copilot · 2025-12-16T10:51:32Z

@neiljbrookes I've opened a new pull request, #1559, to work on those changes. Once the pull request is ready, I'll request review from you.

This update introduces ImportState testing for the elasticstack_elasticsearch_ml_anomaly_detection_job resource, ensuring that the job can be imported correctly with the necessary configuration variables. The test verifies the import functionality and enhances the robustness of the acceptance tests.

neiljbrookes · 2025-12-16T11:29:05Z

Couple of small comments. Can we add an import test step into the existing acceptance tests to verify the import pathway whilst we're here?

I have attempted an ImportState and ImportStateVerify step in 76d4e4b. 🤞

* Initial plan * Refactor: extract duplicated list type handling into reusable helper function - Created EnsureTypedList helper in typeutils package - Replaced three instances of duplicated logic for handling untyped zero-value lists - Removed unused strings import from resource.go - All three instances (CategorizationFilters, Influencers, CustomRules) now use the centralized helper Co-authored-by: neiljbrookes <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: neiljbrookes <[email protected]>

tobio

LGTM. Are you able to add an entry to CHANGELOG.md for this one (sorry I should have picked this up earlier) before we merge it.

…ml_anomaly_detection_job` import, enhancing resilience to sparse state values.

neiljbrookes requested review from Copilot and tobio December 15, 2025 14:52

Copilot AI reviewed Dec 15, 2025

View reviewed changes

neiljbrookes requested a review from Copilot December 15, 2025 18:53

Copilot AI reviewed Dec 15, 2025

View reviewed changes

tobio reviewed Dec 16, 2025

View reviewed changes

Update internal/elasticsearch/ml/anomaly_detection_job/resource.go

177c6eb

Co-authored-by: Toby Brain <[email protected]>

Copilot AI mentioned this pull request Dec 16, 2025

Extract duplicated list type handling logic into reusable helper #1559

Merged

neiljbrookes added 2 commits December 16, 2025 10:53

Remove unused import in anomaly_detection_job resource file

d07ccda

neiljbrookes requested a review from tobio December 16, 2025 13:34

tobio approved these changes Dec 17, 2025

View reviewed changes

Update CHANGELOG.md to include a fix for `elasticstack_elasticsearch_…

e1dbb86

…ml_anomaly_detection_job` import, enhancing resilience to sparse state values.

-	raw := req.ID
-	parts := strings.Split(raw, "/")
-	jobID := parts[len(parts)-1]
+	raw := req.ID
+	if strings.TrimSpace(raw) == "" {
+		resp.Diagnostics.AddError(
+			"Invalid import ID",
+			"Import ID cannot be empty. Please provide a valid job ID or 'default/<job_id>'.",
+		)
+		return
+	}
+	parts := strings.Split(raw, "/")
+	jobID := parts[len(parts)-1]
+	if strings.TrimSpace(jobID) == "" {
+		resp.Diagnostics.AddError(
+			"Invalid import ID format",
+			"Could not extract a valid job ID from the import ID. Please use '<job_id>' or 'default/<job_id>'.",
+		)
+		return
+	}

fix(ml): anomaly_detection_job import/refresh mismatch #1550

Are you sure you want to change the base?

fix(ml): anomaly_detection_job import/refresh mismatch #1550

Uh oh!

Conversation

neiljbrookes commented Dec 15, 2025

Summary

Background / repro (from #1549)

Root cause

What changed

Behavior after this change

Testing

AI assistance

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

tobio Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

neiljbrookes Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

neiljbrookes Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

neiljbrookes Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

tobio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tobio Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Dec 16, 2025

Uh oh!

neiljbrookes commented Dec 16, 2025

Uh oh!

tobio left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants