Skip to content

Commit

Permalink
fix(variable): change 'value' type to JSON
Browse files Browse the repository at this point in the history 
Changes the Variable 'value' type from string to JSON.

In PrefectHQ/prefect#13543 and associated
changes around May 2024, Variables were updated from simple strings to
JSON objects. The Terraform provider has still been treating them as
strings, so when folks tried to put JSON-compatible values in them,
Terraform would fail to work with them as found in
#254

Related to https://linear.app/prefect/issue/PLA-247/changing-variable-to-a-json-value-in-the-ui-makes-next-terraform-run

Related to #254
  • Loading branch information
@mitchnielsen
mitchnielsen committed Oct 4, 2024
1 parent 5933437 commit 07ce76d
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 29 deletions.
18 changes: 9 additions & 9 deletions internal/api/variables.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,23 @@ type VariablesClient interface {
// Variable is a representation of a variable.
type Variable struct {
BaseModel
Name string `json:"name"`
Value string `json:"value"`
Tags []string `json:"tags"`
Name string `json:"name"`
Value map[string]interface{} `json:"value"`
Tags []string `json:"tags"`
}

// VariableCreate is a subset of Variable used when creating variables.
type VariableCreate struct {
Name string `json:"name"`
Value string `json:"value"`
Tags []string `json:"tags"`
Name string `json:"name"`
Value map[string]interface{} `json:"value"`
Tags []string `json:"tags"`
}

// VariableUpdate is a subset of Variable used when updating variables.
type VariableUpdate struct {
Name string `json:"name"`
Value string `json:"value"`
Tags []string `json:"tags"`
Name string `json:"name"`
Value map[string]interface{} `json:"value"`
Tags []string `json:"tags"`
}

// VariableFilterSettings defines settings when searching for variables.
Expand Down
19 changes: 15 additions & 4 deletions internal/provider/datasources/variable.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package datasources

import (
"context"
"encoding/json"

"github.com/hashicorp/terraform-plugin-framework-jsontypes/jsontypes"
"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
"github.com/hashicorp/terraform-plugin-framework/types"
Expand All @@ -27,9 +29,9 @@ type VariableDataSourceModel struct {
AccountID customtypes.UUIDValue `tfsdk:"account_id"`
WorkspaceID customtypes.UUIDValue `tfsdk:"workspace_id"`

Name types.String `tfsdk:"name"`
Value types.String `tfsdk:"value"`
Tags types.List `tfsdk:"tags"`
Name types.String `tfsdk:"name"`
Value jsontypes.Normalized `tfsdk:"value"`
Tags types.List `tfsdk:"tags"`
}

// NewVariableDataSource returns a new VariableDataSource.
Expand Down Expand Up @@ -95,6 +97,7 @@ var variableAttributes = map[string]schema.Attribute{
"value": schema.StringAttribute{
Computed: true,
Description: "Value of the variable",
CustomType: jsontypes.NormalizedType{},
},
"tags": schema.ListAttribute{
Computed: true,
Expand Down Expand Up @@ -168,7 +171,15 @@ func (d *VariableDataSource) Read(ctx context.Context, req datasource.ReadReques
model.Updated = customtypes.NewTimestampPointerValue(variable.Updated)

model.Name = types.StringValue(variable.Name)
model.Value = types.StringValue(variable.Value)

byteSlice, err := json.Marshal(variable.Value)
if err != nil {
resp.Diagnostics.Append(helpers.SerializeDataErrorDiagnostic("data", "Variable Value", err))

return
}

model.Value = jsontypes.NewNormalizedValue(string(byteSlice))

list, diags := types.ListValueFrom(ctx, types.StringType, variable.Tags)
resp.Diagnostics.Append(diags...)
Expand Down
25 changes: 19 additions & 6 deletions internal/provider/resources/variable.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/avast/retry-go/v4"
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-framework-jsontypes/jsontypes"
"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
Expand Down Expand Up @@ -41,9 +42,9 @@ type VariableResourceModel struct {
AccountID customtypes.UUIDValue `tfsdk:"account_id"`
WorkspaceID customtypes.UUIDValue `tfsdk:"workspace_id"`

Name types.String `tfsdk:"name"`
Value types.String `tfsdk:"value"`
Tags types.List `tfsdk:"tags"`
Name types.String `tfsdk:"name"`
Value jsontypes.Normalized `tfsdk:"value"`
Tags types.List `tfsdk:"tags"`
}

// NewVariableResource returns a new VariableResource.
Expand Down Expand Up @@ -123,6 +124,7 @@ func (r *VariableResource) Schema(_ context.Context, _ resource.SchemaRequest, r
"value": schema.StringAttribute{
Description: "Value of the variable",
Required: true,
CustomType: jsontypes.NormalizedType{},
},
"tags": schema.ListAttribute{
Description: "Tags associated with the variable",
Expand All @@ -143,7 +145,6 @@ func copyVariableToModel(ctx context.Context, variable *api.Variable, tfModel *V
tfModel.Updated = customtypes.NewTimestampPointerValue(variable.Updated)

tfModel.Name = types.StringValue(variable.Name)
tfModel.Value = types.StringValue(variable.Value)

tags, diags := types.ListValueFrom(ctx, types.StringType, variable.Tags)
if diags.HasError() {
Expand All @@ -164,6 +165,12 @@ func (r *VariableResource) Create(ctx context.Context, req resource.CreateReques
return
}

var value map[string]interface{}
resp.Diagnostics.Append(plan.Value.Unmarshal(&value)...)
if resp.Diagnostics.HasError() {
return
}

var tags []string
resp.Diagnostics.Append(plan.Tags.ElementsAs(ctx, &tags, false)...)
if resp.Diagnostics.HasError() {
Expand All @@ -181,7 +188,7 @@ func (r *VariableResource) Create(ctx context.Context, req resource.CreateReques
func() (*api.Variable, error) {
return client.Create(ctx, api.VariableCreate{
Name: plan.Name.ValueString(),
Value: plan.Value.ValueString(),
Value: value,
Tags: tags,
})
},
Expand Down Expand Up @@ -281,6 +288,12 @@ func (r *VariableResource) Update(ctx context.Context, req resource.UpdateReques
return
}

var value map[string]interface{}
resp.Diagnostics.Append(plan.Value.Unmarshal(&value)...)
if resp.Diagnostics.HasError() {
return
}

var tags []string
resp.Diagnostics.Append(plan.Tags.ElementsAs(ctx, &tags, false)...)
if resp.Diagnostics.HasError() {
Expand All @@ -296,7 +309,7 @@ func (r *VariableResource) Update(ctx context.Context, req resource.UpdateReques

err = client.Update(ctx, variableID, api.VariableUpdate{
Name: plan.Name.ValueString(),
Value: plan.Value.ValueString(),
Value: value,
Tags: tags,
})
if err != nil {
Expand Down
22 changes: 12 additions & 10 deletions internal/provider/resources/variable_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package resources_test
import (
"context"
"fmt"
"reflect"
"testing"

"github.com/google/uuid"
Expand All @@ -17,7 +18,7 @@ func fixtureAccVariableResource(workspace, workspaceName, name, value string) st
%s
resource "prefect_variable" "%s" {
name = "%s"
value = "%s"
value = jsonencode({"foo" = "%s"})
workspace_id = prefect_workspace.%s.id
depends_on = [prefect_workspace.%s]
}
Expand All @@ -29,7 +30,7 @@ func fixtureAccVariableResourceWithTags(workspace, workspaceName, name, value st
%s
resource "prefect_variable" "%s" {
name = "%s"
value = "%s"
value = jsonencode({"foo" = "%s"})
tags = ["foo", "bar"]
workspace_id = prefect_workspace.%s.id
depends_on = [prefect_workspace.%s]
Expand All @@ -48,6 +49,9 @@ func TestAccResource_variable(t *testing.T) {
randomValue := testutils.NewRandomPrefixedString()
randomValue2 := testutils.NewRandomPrefixedString()

randomValueMap := map[string]interface{}{"foo": randomValue}
randomValue2Map := map[string]interface{}{"foo": randomValue2}

workspace, workspaceName := testutils.NewEphemeralWorkspace()
workspaceResourceName := "prefect_workspace." + workspaceName

Expand All @@ -64,29 +68,26 @@ func TestAccResource_variable(t *testing.T) {
Config: fixtureAccVariableResource(workspace, workspaceName, randomName, randomValue),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckVariableExists(resourceName, workspaceResourceName, &variable),
testAccCheckVariableValues(&variable, &api.Variable{Name: randomName, Value: randomValue}),
testAccCheckVariableValues(&variable, &api.Variable{Name: randomName, Value: randomValueMap}),
resource.TestCheckResourceAttr(resourceName, "name", randomName),
resource.TestCheckResourceAttr(resourceName, "value", randomValue),
),
},
{
// Check updating name + value of the variable resource
Config: fixtureAccVariableResource(workspace, workspaceName, randomName2, randomValue2),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckVariableExists(resourceName2, workspaceResourceName, &variable),
testAccCheckVariableValues(&variable, &api.Variable{Name: randomName2, Value: randomValue2}),
testAccCheckVariableValues(&variable, &api.Variable{Name: randomName2, Value: randomValue2Map}),
resource.TestCheckResourceAttr(resourceName2, "name", randomName2),
resource.TestCheckResourceAttr(resourceName2, "value", randomValue2),
),
},
{
// Check adding tags
Config: fixtureAccVariableResourceWithTags(workspace, workspaceName, randomName2, randomValue2),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckVariableExists(resourceName2, workspaceResourceName, &variable),
testAccCheckVariableValues(&variable, &api.Variable{Name: randomName2, Value: randomValue2}),
testAccCheckVariableValues(&variable, &api.Variable{Name: randomName2, Value: randomValue2Map}),
resource.TestCheckResourceAttr(resourceName2, "name", randomName2),
resource.TestCheckResourceAttr(resourceName2, "value", randomValue2),
resource.TestCheckResourceAttr(resourceName2, "tags.#", "2"),
resource.TestCheckResourceAttr(resourceName2, "tags.0", "foo"),
resource.TestCheckResourceAttr(resourceName2, "tags.1", "bar"),
Expand Down Expand Up @@ -134,8 +135,9 @@ func testAccCheckVariableValues(fetchedVariable *api.Variable, valuesToCheck *ap
if fetchedVariable.Name != valuesToCheck.Name {
return fmt.Errorf("Expected variable name to be %s, got %s", valuesToCheck.Name, fetchedVariable.Name)
}
if fetchedVariable.Value != valuesToCheck.Value {
return fmt.Errorf("Expected variable value to be %s, got %s", valuesToCheck.Name, fetchedVariable.Name)

if !reflect.DeepEqual(fetchedVariable.Value, valuesToCheck.Value) {
return fmt.Errorf("Expected variable value to be %s, got %s", valuesToCheck.Value, fetchedVariable.Value)
}

return nil
Expand Down

0 comments on commit 07ce76d

Please sign in to comment.