Skip to content

Don't panic on non-existant import #1579

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

Merged
tobio merged 3 commits into from
Dec 23, 2025
+50 −4
Merged

Don't panic on non-existant import #1579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cbff2e2
Don't panic on non-existant import
tobio Dec 21, 2025
c4232d3
Changelog
tobio Dec 21, 2025
b2d2d0a
Merge branch 'main' into import-nil
tobio Dec 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
## [Unreleased]

### Added

- Add `advanced_settings` to `elasticstack_fleet_agent_policy` to configure agent logging, CPU limits, and download settings ([#1545](https://github.com/elastic/terraform-provider-elasticstack/pull/1545))

### Breaking changes

#### `elasticstack_fleet_integration_policy` input block has changed to a map attribute.
Expand Down Expand Up @@ -70,6 +66,8 @@ inputs = {

### Changes

- Add `advanced_settings` to `elasticstack_fleet_agent_policy` to configure agent logging, CPU limits, and download settings ([#1545](https://github.com/elastic/terraform-provider-elasticstack/pull/1545))
Copy link
Contributor

@dimuon dimuon Dec 22, 2025

Choose a reason for hiding this comment

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

Thanks for fixing this.

- Prevent provider panic when importing a non-existant `elasticstack_elasticsearch_ml_datafeed`. ([#1579](https://github.com/elastic/terraform-provider-elasticstack/pull/1579))
- Fix handling of empty `except` attributes in `elasticstack_elasticsearch_security_role` ([#1581](https://github.com/elastic/terraform-provider-elasticstack/pull/1581))
- Fix the enabled property being ignored in `elasticstack_kibana_alerting_rule` ([#1527](https://github.com/elastic/terraform-provider-elasticstack/pull/1527))
- Add `advanced_monitoring_options` to `elasticstack_fleet_agent_policy` to configure HTTP monitoring endpoint and diagnostics settings ([#1537](https://github.com/elastic/terraform-provider-elasticstack/pull/1537))
Expand Down
20 changes: 20 additions & 0 deletions internal/elasticsearch/ml/datafeed/acc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,23 @@ func TestAccResourceDatafeedComprehensive(t *testing.T) {
},
})
}

func TestAccResourceDatafeed_ImportNonExistent(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
Steps: []resource.TestStep{
{
ProtoV6ProviderFactories: acctest.Providers,
ConfigDirectory: acctest.NamedTestCaseDirectory("import_missing"),
ConfigVariables: config.Variables{
"job_id": config.StringVariable("dummy-job"),
"datafeed_id": config.StringVariable("dummy-datafeed"),
},
ResourceName: "elasticstack_elasticsearch_ml_datafeed.test",
ImportState: true,
ImportStateId: "cluster-id/non-existent-datafeed-id",
ImportStateVerify: false,
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The test disables import state verification with ImportStateVerify: false. For a test validating import behavior of non-existent resources, consider adding ExpectError to verify that Terraform properly reports the resource as not found, rather than silently succeeding with verification disabled.

Copilot uses AI. Check for mistakes.
},
},
})
}
5 changes: 5 additions & 0 deletions internal/elasticsearch/ml/datafeed/read.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ func (r *datafeedResource) read(ctx context.Context, model *Datafeed) (bool, fwd
return false, diags
}

if apiModel == nil {
// Datafeed not found
return false, diags
}

// Convert API model to TF model
convertDiags := model.FromAPIModel(ctx, apiModel)
diags.Append(convertDiags...)
Expand Down
23 changes: 23 additions & 0 deletions ...ml/datafeed/testdata/TestAccResourceDatafeed_ImportNonExistent/import_missing/datafeed.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
variable "job_id" {
type = string
}

variable "datafeed_id" {
type = string
}

provider "elasticstack" {
elasticsearch {}
}

resource "elasticstack_elasticsearch_ml_datafeed" "test" {
datafeed_id = var.datafeed_id
job_id = var.job_id
Comment on lines +13 to +15
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The test configuration references a non-existent job via var.job_id set to 'dummy-job'. Since the test is specifically for importing a non-existent datafeed, it should not depend on a non-existent job. Consider creating an actual ML job resource in the test configuration or document why the non-existent job reference is acceptable for this test scenario.

Copilot uses AI. Check for mistakes.
indices = ["test-index-*"]

query = jsonencode({
match_all = {
boost = 1
}
})
}