Skip to content

Commit

Permalink
data-source/external: Omit query attribute map elements with null values
Browse files Browse the repository at this point in the history
Reference: #194
Reference: #208

The migration to terraform-plugin-framework exposes map values wholly and v2.3.0 handled that transition properly. However, v2.3.1 introduced a subtle change where `query` attribute map elements with null values are now sent with an empty string value, rather than being omitted like previously. Since certain receiving `program` may be dependent on the nuance of `query` values, null element values should continue to omit the element entirely to prevent the introduction of a breaking change.

Previously before logic update:

```
=== RUN   TestDataSource_Query_NullElementValue
    data_source_test.go:367: Step 1/1 error: Error running pre-apply refresh: exit status 1

        Error: External Program Execution Failed

          with data.external.test,
          on terraform_plugin_test.tf line 3, in data "external" "test":
           3:                             program = ["/Users/bflad/go/bin/tf-acc-external-data-source"]

        The data source received an unexpected error while attempting to execute the
        program.

        Program: /Users/bflad/go/bin/tf-acc-external-data-source
        Error Message: I was asked to fail

        State: exit status 1
--- FAIL: TestDataSource_Query_NullElementValue (0.26s)
```
  • Loading branch information
bflad committed Apr 10, 2023
1 parent 57528ad commit 3892046
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 13 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/BUG FIXES-20230410-130443.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: BUG FIXES
body: 'data-source/external: Prevent regression since v2.3.1 where null `query` element
values would be sent to the program as an empty string'
time: 2023-04-10T13:04:43.615508-04:00
custom:
Issue: "208"
11 changes: 11 additions & 0 deletions internal/provider/data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,17 @@ func (n *externalDataSource) Read(ctx context.Context, req datasource.ReadReques

filteredQuery := make(map[string]string)
for key, value := range query {
// Preserve v2.2.3 and earlier behavior of filtering whole map elements
// with null values.
// Reference: https://github.com/hashicorp/terraform-provider-external/issues/208
//
// The external program protocol could be updated to support null values
// as a breaking change by marshaling map[string]*string to JSON.
// Reference: https://github.com/hashicorp/terraform-provider-external/issues/209
if value.IsNull() {
continue
}

filteredQuery[key] = value.ValueString()
}

Expand Down
42 changes: 34 additions & 8 deletions internal/provider/data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func TestDataSource_Program_EmptyStringAndNullValues(t *testing.T) {
})
}

func TestDataSource_Query_NullAndEmptyValue(t *testing.T) {
func TestDataSource_Query_EmptyElementValue(t *testing.T) {
programPath, err := buildDataSourceTestProgram()
if err != nil {
t.Fatal(err)
Expand All @@ -206,14 +206,42 @@ func TestDataSource_Query_NullAndEmptyValue(t *testing.T) {
program = [%[1]q]
query = {
value = null,
value2 = ""
value = "",
}
}
`, programPath),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("data.external.test", "result.value", ""),
resource.TestCheckResourceAttr("data.external.test", "result.value2", ""),
),
},
},
})
}

// Reference: https://github.com/hashicorp/terraform-provider-external/issues/208
func TestDataSource_Query_NullElementValue(t *testing.T) {
programPath, err := buildDataSourceTestProgram()
if err != nil {
t.Fatal(err)
return
}

resource.UnitTest(t, resource.TestCase{
ProtoV5ProviderFactories: protoV5ProviderFactories(),
Steps: []resource.TestStep{
{
Config: fmt.Sprintf(`
data "external" "test" {
program = [%q]
query = {
# Program will return exit status 1 if the "fail" key is present.
fail = null
}
}
`, programPath),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckNoResourceAttr("data.external.test", "result.fail"),
),
},
},
Expand Down Expand Up @@ -256,14 +284,12 @@ func TestDataSource_CurrentDir(t *testing.T) {
program = [%[1]q]
query = {
value = null,
value2 = ""
value = "test",
}
}
`, "tf-acc-external-data-source"),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("data.external.test", "result.value", ""),
resource.TestCheckResourceAttr("data.external.test", "result.value2", ""),
resource.TestCheckResourceAttr("data.external.test", "result.value", "test"),
),
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,33 @@ func main() {
panic(err)
}

var query map[string]string
var query map[string]*string
err = json.Unmarshal(queryBytes, &query)
if err != nil {
panic(err)
}

if query["fail"] != "" {
if _, ok := query["fail"]; ok {
fmt.Fprintf(os.Stderr, "I was asked to fail\n")
os.Exit(1)
}

var result = map[string]string{
"result": "yes",
"query_value": query["value"],
"result": "yes",
}

if queryValue, ok := query["value"]; ok && queryValue != nil {
result["query_value"] = *queryValue
}

if len(os.Args) >= 2 {
result["argument"] = os.Args[1]
}

for queryKey, queryValue := range query {
result[queryKey] = queryValue
if queryValue != nil {
result[queryKey] = *queryValue
}
}

resultBytes, err := json.Marshal(result)
Expand Down

0 comments on commit 3892046

Please sign in to comment.