diff --git a/.changelog/325.txt b/.changelog/325.txt new file mode 100644 index 00000000..024fa8ed --- /dev/null +++ b/.changelog/325.txt @@ -0,0 +1,11 @@ +```release-note:bug +`resource/davinci_flow`: Fix issue whereby flow variables cannot be updated, leading to error. +``` + +```release-note:bug +`resource/davinci_flow`: Fixed `flow_variables.type` so that it refers to the data type of the variable (as is the original intention), rather than the type of the variable object. +``` + +```release-note:enhancement +`resource/davinci_flow`: Added `flow_variables.value` to allow the variable's default value to be updated. +``` diff --git a/docs/resources/flow.md b/docs/resources/flow.md index f447e3ad..dcd2044e 100644 --- a/docs/resources/flow.md +++ b/docs/resources/flow.md @@ -72,7 +72,7 @@ resource "davinci_flow" "my_awesome_main_flow" { - `flow_configuration_json` (String, Sensitive) The parsed configuration of the DaVinci Flow import JSON. Drift is calculated based on this attribute. - `flow_export_json` (String, Sensitive) The DaVinci Flow export in raw JSON format following successful import, including target environment metadata. -- `flow_variables` (Attributes Set) Returned list of Flow Context variables. These are variable resources that are created and managed by the Flow resource via `flow_json`. (see [below for nested schema](#nestedatt--flow_variables)) +- `flow_variables` (Attributes Set) List of Flow variables that will be updated in the DaVinci instance. These are variable resources that are created and managed by the Flow resource, where variables are embedded in the `flow_json` DaVinci export file. (see [below for nested schema](#nestedatt--flow_variables)) - `id` (String) The ID of this resource. @@ -115,6 +115,7 @@ Read-Only: - `mutable` (Boolean) A boolean that specifies whether the variable is mutable. If `true`, the variable can be modified by the flow. If `false`, the variable is read-only and cannot be modified by the flow. - `name` (String) The user friendly name of the variable in the UI. - `type` (String) The variable's data type. Expected to be one of `string`, `number`, `boolean`, `object`. +- `value` (String) A string that represents the variable's default value. ## Import diff --git a/go.mod b/go.mod index 60e5e7fe..efba7089 100644 --- a/go.mod +++ b/go.mod @@ -5,14 +5,18 @@ go 1.21.1 require ( github.com/bflad/tfproviderlint v0.29.0 github.com/golangci/golangci-lint v1.59.0 + github.com/google/go-cmp v0.6.0 github.com/hashicorp/go-changelog v0.0.0-20221013053416-ba40b3a8c7ff + github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/terraform-plugin-docs v0.19.2 github.com/hashicorp/terraform-plugin-framework v1.8.0 github.com/hashicorp/terraform-plugin-framework-validators v0.12.0 + github.com/hashicorp/terraform-plugin-go v0.23.0 github.com/hashicorp/terraform-plugin-log v0.9.0 github.com/hashicorp/terraform-plugin-mux v0.16.0 github.com/hashicorp/terraform-plugin-sdk/v2 v2.33.0 github.com/katbyte/terrafmt v0.5.3 + github.com/patrickcping/pingone-go-sdk-v2 v0.11.8 github.com/patrickcping/pingone-go-sdk-v2/management v0.39.0 github.com/pavius/impi v0.0.3 github.com/samir-gandhi/davinci-client-go v0.3.0 @@ -284,14 +288,12 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/fatih/color v1.17.0 // indirect github.com/golang/protobuf v1.5.4 // indirect - github.com/google/go-cmp v0.6.0 // indirect github.com/google/uuid v1.6.0 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-checkpoint v0.5.0 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 // indirect github.com/hashicorp/go-hclog v1.5.0 // indirect - github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-plugin v1.6.0 // indirect github.com/hashicorp/go-uuid v1.0.3 // indirect github.com/hashicorp/go-version v1.7.0 // indirect @@ -300,7 +302,6 @@ require ( github.com/hashicorp/logutils v1.0.0 // indirect github.com/hashicorp/terraform-exec v0.20.0 // indirect github.com/hashicorp/terraform-json v0.21.0 // indirect - github.com/hashicorp/terraform-plugin-go v0.23.0 github.com/hashicorp/terraform-registry-address v0.2.3 // indirect github.com/hashicorp/terraform-svchost v0.1.1 // indirect github.com/hashicorp/yamux v0.1.1 // indirect @@ -314,7 +315,6 @@ require ( github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect github.com/oklog/run v1.1.0 // indirect - github.com/patrickcping/pingone-go-sdk-v2 v0.11.8 github.com/posener/complete v1.2.3 // indirect github.com/shopspring/decimal v1.3.1 // indirect github.com/spf13/cast v1.5.0 // indirect diff --git a/internal/service/davinci/resource_flow.go b/internal/service/davinci/resource_flow.go index 03220e68..39ecfed5 100644 --- a/internal/service/davinci/resource_flow.go +++ b/internal/service/davinci/resource_flow.go @@ -6,6 +6,7 @@ import ( "net/http" "strings" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework-validators/setvalidator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/attr" @@ -53,18 +54,6 @@ type FlowSubflowLinkResourceModel struct { Name types.String `tfsdk:"name"` } -type FlowVariablesResourceModel struct { - Id types.String `tfsdk:"id"` - Name types.String `tfsdk:"name"` - Description types.String `tfsdk:"description"` - FlowId types.String `tfsdk:"flow_id"` - Context types.String `tfsdk:"context"` - Type types.String `tfsdk:"type"` - Mutable types.Bool `tfsdk:"mutable"` - Min types.Int64 `tfsdk:"min"` - Max types.Int64 `tfsdk:"max"` -} - var ( flowVariablesTFObjectTypes = map[string]attr.Type{ "id": types.StringType, @@ -73,6 +62,7 @@ var ( "flow_id": types.StringType, "context": types.StringType, "type": types.StringType, + "value": types.StringType, "mutable": types.BoolType, "min": types.Int64Type, "max": types.Int64Type, @@ -170,7 +160,7 @@ func (r *FlowResource) Schema(ctx context.Context, req resource.SchemaRequest, r ) flowVariablesDescription := framework.SchemaAttributeDescriptionFromMarkdown( - "Returned list of Flow Context variables. These are variable resources that are created and managed by the Flow resource via `flow_json`.", + "List of Flow variables that will be updated in the DaVinci instance. These are variable resources that are created and managed by the Flow resource, where variables are embedded in the `flow_json` DaVinci export file.", ) flowVariablesContextDescription := framework.SchemaAttributeDescriptionFromMarkdown( @@ -181,6 +171,10 @@ func (r *FlowResource) Schema(ctx context.Context, req resource.SchemaRequest, r "The variable's data type. Expected to be one of `string`, `number`, `boolean`, `object`.", ) + flowVariablesValueDescription := framework.SchemaAttributeDescriptionFromMarkdown( + "A string that represents the variable's default value.", + ) + flowVariablesMutableDescription := framework.SchemaAttributeDescriptionFromMarkdown( "A boolean that specifies whether the variable is mutable. If `true`, the variable can be modified by the flow. If `false`, the variable is read-only and cannot be modified by the flow.", ) @@ -306,6 +300,12 @@ func (r *FlowResource) Schema(ctx context.Context, req resource.SchemaRequest, r Computed: true, }, + "value": schema.StringAttribute{ + Description: flowVariablesValueDescription.Description, + MarkdownDescription: flowVariablesValueDescription.MarkdownDescription, + Computed: true, + }, + "mutable": schema.BoolAttribute{ Description: flowVariablesMutableDescription.Description, MarkdownDescription: flowVariablesMutableDescription.MarkdownDescription, @@ -797,6 +797,93 @@ func (r *FlowResource) Update(ctx context.Context, req resource.UpdateRequest, r } } + // Variables + if !plan.FlowVariables.Equal(state.FlowVariables) { + var flowVarPlan, flowVarState []VariableResourceModel + resp.Diagnostics.Append(plan.FlowVariables.ElementsAs(ctx, &flowVarPlan, false)...) + resp.Diagnostics.Append(state.FlowVariables.ElementsAs(ctx, &flowVarState, false)...) + // If there are errors, keep going as it's a read to state left + + if !resp.Diagnostics.HasError() { + for _, flowVar := range flowVarPlan { + var flowVarStateFound bool + for _, flowVarState := range flowVarState { + if flowVar.Id.Equal(flowVarState.Id) { + flowVarStateFound = true + + // test for modifications + if !cmp.Equal(flowVar, flowVarState) { + flowVariable := flowVar.expand() + + _, err := sdk.DoRetryable( + ctx, + r.Client, + environmentID, + func() (interface{}, *http.Response, error) { + return r.Client.UpdateVariableWithResponse(environmentID, flowVariable) + }, + ) + if err != nil { + resp.Diagnostics.AddError( + "Error adding flow variable", + fmt.Sprintf("Error adding flow variable as part of flow update: %s", err), + ) + } + } + } + } + + if !flowVarStateFound { + // add the new variable + flowVariable := flowVar.expand() + + _, err := sdk.DoRetryable( + ctx, + r.Client, + environmentID, + func() (interface{}, *http.Response, error) { + return r.Client.CreateVariableWithResponse(environmentID, flowVariable) + }, + ) + if err != nil { + resp.Diagnostics.AddError( + "Error adding flow variable", + fmt.Sprintf("Error adding flow variable as part of flow update: %s", err), + ) + } + } + } + + for _, flowVar := range flowVarState { + var flowVarPlanFound bool + for _, flowVarPlan := range flowVarPlan { + if flowVar.Id.Equal(flowVarPlan.Id) { + flowVarPlanFound = true + break + } + } + + if !flowVarPlanFound { + // remove the variable + _, err := sdk.DoRetryable( + ctx, + r.Client, + environmentID, + func() (interface{}, *http.Response, error) { + return r.Client.DeleteVariableWithResponse(environmentID, flowVar.Id.ValueString()) + }, + ) + if err != nil { + resp.Diagnostics.AddError( + "Error removing flow variable", + fmt.Sprintf("Error removing flow variable as part of flow update: %s", err), + ) + } + } + } + } + } + // Do an export for state // Run the API call sdkRes, err := sdk.DoRetryable( @@ -1183,7 +1270,8 @@ func flowVariablesToTF(apiObject []davinci.FlowVariable) (types.Set, diag.Diagno "description": framework.StringOkToTF(variable.Label, true), "flow_id": framework.StringOkToTF(variable.FlowID, true), "context": framework.StringOkToTF(variable.Context, true), - "type": framework.StringToTF(variable.Type), + "type": framework.StringOkToTF(variable.Fields.Type, true), + "value": framework.StringOkToTF(variable.Fields.Value, true), "mutable": framework.BoolOkToTF(variable.Fields.Mutable, true), "min": framework.Int32OkToTF(variable.Fields.Min, true), "max": framework.Int32OkToTF(variable.Fields.Max, true), diff --git a/internal/service/davinci/resource_flow_test.go b/internal/service/davinci/resource_flow_test.go index e8e3d989..a4ea6ac5 100644 --- a/internal/service/davinci/resource_flow_test.go +++ b/internal/service/davinci/resource_flow_test.go @@ -120,8 +120,7 @@ func testAccResourceFlow_Basic(t *testing.T, withBootstrapConfig bool) { "min": regexp.MustCompile(`^0$`), "mutable": regexp.MustCompile(`^true$`), "name": regexp.MustCompile(`^fdgdfgfdg$`), - "type": regexp.MustCompile(`^property$`), - //"type": regexp.MustCompile(`^string$`), + "type": regexp.MustCompile(`^string$`), }), resource.TestMatchTypeSetElemNestedAttrs(resourceFullName, "flow_variables.*", map[string]*regexp.Regexp{ "context": regexp.MustCompile(`^flow$`), @@ -131,9 +130,8 @@ func testAccResourceFlow_Basic(t *testing.T, withBootstrapConfig bool) { "min": regexp.MustCompile(`^4$`), "mutable": regexp.MustCompile(`^true$`), "name": regexp.MustCompile(`^test123$`), - "type": regexp.MustCompile(`^property$`), - //"type": regexp.MustCompile(`^number$`), - //"value": regexp.MustCompile(`^10$`), + "type": regexp.MustCompile(`^number$`), + "value": regexp.MustCompile(`^10$`), }), ), } @@ -194,21 +192,21 @@ func testAccResourceFlow_Basic(t *testing.T, withBootstrapConfig bool) { Steps: []resource.TestStep{ // Create full from scratch fullStep, + minimalStep, { - Config: fullStepHcl, + Config: minimalStepHcl, Destroy: true, }, // Create minimal from scratch minimalStep, + fullStep, { - Config: minimalStepHcl, + Config: fullStepHcl, Destroy: true, }, // Test updates - fullStep, minimalStep, updateStep, - fullStep, // Test importing the resource { ResourceName: resourceFullName, @@ -372,6 +370,7 @@ func testAccResourceFlow_ConnectionSubflowLinks_WithMappingIDs(t *testing.T, wit "connection_link", "subflow_link", "flow_json", + "flow_export_json", }, }, }, @@ -503,6 +502,7 @@ func testAccResourceFlow_ConnectionSubflowLinks_WithoutMappingIDs(t *testing.T, "connection_link", "subflow_link", "flow_json", + "flow_export_json", }, }, }, diff --git a/internal/service/davinci/resource_variable.go b/internal/service/davinci/resource_variable.go index 4439ea63..b63b7b2a 100644 --- a/internal/service/davinci/resource_variable.go +++ b/internal/service/davinci/resource_variable.go @@ -7,6 +7,7 @@ import ( "regexp" "strings" + "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -361,3 +362,57 @@ func getVariableAttributes(d *schema.ResourceData) dv.VariablePayload { } return variablePayload } + +// Framework +type VariableResourceModel struct { + Id types.String `tfsdk:"id"` + Name types.String `tfsdk:"name"` + Description types.String `tfsdk:"description"` + FlowId types.String `tfsdk:"flow_id"` + Context types.String `tfsdk:"context"` + Type types.String `tfsdk:"type"` + Value types.String `tfsdk:"value"` + Mutable types.Bool `tfsdk:"mutable"` + Min types.Int64 `tfsdk:"min"` + Max types.Int64 `tfsdk:"max"` +} + +func (p *VariableResourceModel) expand() *dv.VariablePayload { + + data := dv.VariablePayload{ + Context: p.Context.ValueString(), + Type: p.Type.ValueString(), + } + + if !p.Name.IsNull() { + data.Name = p.Name.ValueStringPointer() + } + + if !p.Description.IsNull() { + data.Description = p.Description.ValueStringPointer() + } + + if !p.FlowId.IsNull() { + data.FlowId = p.FlowId.ValueStringPointer() + } + + if !p.Value.IsNull() { + data.Value = p.Value.ValueStringPointer() + } + + if !p.Mutable.IsNull() { + data.Mutable = p.Mutable.ValueBoolPointer() + } + + if !p.Min.IsNull() { + min := int(p.Min.ValueInt64()) + data.Min = &min + } + + if !p.Max.IsNull() { + max := int(p.Max.ValueInt64()) + data.Max = &max + } + + return &data +}