From 3c5e0f29903c87475076c96e5ee37272eea08b15 Mon Sep 17 00:00:00 2001 From: Erhan Cagirici Date: Sun, 14 Jan 2024 01:07:20 +0300 Subject: [PATCH] diff detection with plan and state comparison & refactor Signed-off-by: Erhan Cagirici --- .../external_terraform_plugin_framework.go | 240 ++++++++++-------- 1 file changed, 130 insertions(+), 110 deletions(-) diff --git a/pkg/controller/external_terraform_plugin_framework.go b/pkg/controller/external_terraform_plugin_framework.go index da69adb2..48d09934 100644 --- a/pkg/controller/external_terraform_plugin_framework.go +++ b/pkg/controller/external_terraform_plugin_framework.go @@ -7,6 +7,8 @@ package controller import ( "context" "encoding/json" + "fmt" + "math" "math/big" "strings" "time" @@ -143,15 +145,11 @@ func (c *TerraformPluginFrameworkConnector) Connect(ctx context.Context, mg xpre tfState = copyParameters(tfState, params) } - tfStateJsonBytes, err := json.Marshal(tfState) + tfStateDynamicValue, err := protov5DynamicValueFromMap(tfState, resourceSchema.Type().TerraformType(ctx)) if err != nil { - return nil, errors.Wrap(err, "could not marshal TF state map") + return nil, errors.Wrap(err, "cannot construct dynamic value for TF state") } - - opTracker.SetFrameworkTFState(&tfprotov5.DynamicValue{ - JSON: tfStateJsonBytes, - }) - + opTracker.SetFrameworkTFState(tfStateDynamicValue) } configuredProviderServer, err := c.configureProvider(ctx, ts) @@ -184,24 +182,32 @@ func (c *TerraformPluginFrameworkConnector) getResourceSchema(ctx context.Contex } func (c *TerraformPluginFrameworkConnector) configureProvider(ctx context.Context, ts terraform.Setup) (tfprotov5.ProviderServer, error) { + var schemaResp fwprovider.SchemaResponse + ts.FrameworkProvider.Schema(ctx, fwprovider.SchemaRequest{}, &schemaResp) + if schemaResp.Diagnostics.HasError() { + var diagErrors []string + for _, tfdiag := range schemaResp.Diagnostics.Errors() { + diagErrors = append(diagErrors, fmt.Sprintf("%s: %s", tfdiag.Summary(), tfdiag.Detail())) + } + return nil, fmt.Errorf("cannot retrieve provider schema") + } providerServer := providerserver.NewProtocol5(ts.FrameworkProvider)() - tsBytes, err := json.Marshal(ts.Configuration) + + providerConfigDynamicVal, err := protov5DynamicValueFromMap(ts.Configuration, schemaResp.Schema.Type().TerraformType(ctx)) if err != nil { - return nil, errors.Wrap(err, "cannot marshal ts config") + return nil, errors.Wrap(err, "cannot construct dynamic value for TF provider config") } + configureProviderReq := &tfprotov5.ConfigureProviderRequest{ TerraformVersion: "crossTF000", - Config: &tfprotov5.DynamicValue{ - JSON: tsBytes, - }, + Config: providerConfigDynamicVal, } providerResp, err := providerServer.ConfigureProvider(ctx, configureProviderReq) if err != nil { return nil, err } - // TODO(erhan): improve diag reporting - if hasFatalDiag := hasFatalDiagnostics(providerResp.Diagnostics); hasFatalDiag { - return nil, errors.Errorf("provider configure request returned fatal diagnostics") + if fatalDiags := getFatalDiagnostics(providerResp.Diagnostics); fatalDiags != nil { + return nil, errors.Wrap(fatalDiags, "provider configure request failed") } return providerServer, nil } @@ -210,56 +216,30 @@ func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context tfStateValue tftypes.Value) (*tfprotov5.DynamicValue, bool, error) { valueTerraformType := n.resourceSchema.Type().TerraformType(ctx) - paramBytes, err := json.Marshal(n.params) - if err != nil { - return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot convert params to json bytes") - } - - tfConfigValue, err := tftypes.ValueFromJSONWithOpts(paramBytes, valueTerraformType, tftypes.ValueFromJSONOpts{IgnoreUndefinedAttributes: true}) - if err != nil { - return &tfprotov5.DynamicValue{}, false, err - } - - tfConfig, err := tfprotov5.NewDynamicValue(valueTerraformType, tfConfigValue) - if err != nil { - return &tfprotov5.DynamicValue{}, false, err - } - tfPlannedState, err := tfprotov5.NewDynamicValue(valueTerraformType, tfConfigValue.Copy()) + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, valueTerraformType) if err != nil { - return &tfprotov5.DynamicValue{}, false, err + return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot construct dynamic value for TF Config") } - diffs, err := tfStateValue.Diff(tfConfigValue) + // + tfPlannedStateDynamicVal, err := protov5DynamicValueFromMap(n.params, valueTerraformType) if err != nil { - return &tfprotov5.DynamicValue{}, false, err - } - - // process diffs - processedDiffs := diffs[:0] - for _, diff := range diffs { - if !diff.Value2.IsNull() { - processedDiffs = append(processedDiffs, diff) - } - } - - if len(processedDiffs) == 0 { - return nil, false, nil + return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot construct dynamic value for TF Planned State") } prcReq := &tfprotov5.PlanResourceChangeRequest{ TypeName: n.config.Name, PriorState: n.opTracker.GetFrameworkTFState(), - Config: &tfConfig, - ProposedNewState: &tfPlannedState, + Config: tfConfigDynamicVal, + ProposedNewState: tfPlannedStateDynamicVal, } planResponse, err := n.server.PlanResourceChange(ctx, prcReq) if err != nil { return &tfprotov5.DynamicValue{}, false, errors.Wrap(err, "cannot plan change") } - // TODO: improve diag reporting - if isFatal := hasFatalDiagnostics(planResponse.Diagnostics); isFatal { - return &tfprotov5.DynamicValue{}, false, errors.New("plan resource change has fatal diags") + if fatalDiags := getFatalDiagnostics(planResponse.Diagnostics); fatalDiags != nil { + return &tfprotov5.DynamicValue{}, false, errors.Wrap(fatalDiags, "plan resource change request failed") } if len(planResponse.RequiresReplace) > 0 { @@ -272,7 +252,17 @@ func (n *terraformPluginFrameworkExternalClient) getDiffPlan(ctx context.Context return nil, false, errors.New(sb.String()) } - return planResponse.PlannedState, len(processedDiffs) > 0, nil + plannedStateValue, err := planResponse.PlannedState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) + if err != nil { + return nil, false, errors.Wrap(err, "cannot unmarshal planned state") + } + + diffso, err := plannedStateValue.Diff(tfStateValue) + if err != nil { + return nil, false, errors.Wrap(err, "cannot compare prior state and plan") + } + + return planResponse.PlannedState, len(diffso) > 0, nil } @@ -295,9 +285,8 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg return managed.ExternalObservation{}, errors.Wrap(err, "cannot read resource") } - // TODO(erhan): improve diag reporting - if shouldError := hasFatalDiagnostics(readResponse.Diagnostics); shouldError { - return managed.ExternalObservation{}, errors.New("read returned diags") + if fatalDiags := getFatalDiagnostics(readResponse.Diagnostics); fatalDiags != nil { + return managed.ExternalObservation{}, errors.Wrap(fatalDiags, "read resource request failed") } tfStateValue, err := readResponse.NewState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) @@ -310,7 +299,7 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg var stateValueMap map[string]any if resourceExists { - if conv, err := valueToGo(tfStateValue); err != nil { + if conv, err := tfValueToMap(tfStateValue); err != nil { return managed.ExternalObservation{}, errors.Wrap(err, "cannot convert instance state to JSON map") } else { stateValueMap = conv.(map[string]any) @@ -377,18 +366,16 @@ func (n *terraformPluginFrameworkExternalClient) Observe(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg xpresource.Managed) (managed.ExternalCreation, error) { n.logger.Debug("Creating the external resource") - configJsonBytes, err := json.Marshal(n.params) + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceSchema.Type().TerraformType(ctx)) if err != nil { - return managed.ExternalCreation{}, errors.Wrap(err, "cannot convert params to json bytes") + return managed.ExternalCreation{}, errors.Wrap(err, "cannot construct dynamic value for TF Config") } applyRequest := &tfprotov5.ApplyResourceChangeRequest{ TypeName: n.config.Name, PriorState: n.opTracker.GetFrameworkTFState(), PlannedState: n.plannedState, - Config: &tfprotov5.DynamicValue{ - JSON: configJsonBytes, - }, + Config: tfConfigDynamicVal, } start := time.Now() applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) @@ -396,29 +383,21 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, "cannot create resource") } - - // TODO(erhan): check diags reporting - if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { - return managed.ExternalCreation{}, errors.Errorf("failed to create the resource:") + if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil { + return managed.ExternalCreation{}, errors.Wrap(fatalDiags, "resource creation failed with diags") } - // TODO(erhan): refactor schema - res := *n.resource - schemaResp := &fwresource.SchemaResponse{} - res.Schema(ctx, fwresource.SchemaRequest{}, schemaResp) - - newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(schemaResp.Schema.Type().TerraformType(ctx)) + newStateAfterApplyVal, err := applyResponse.NewState.Unmarshal(n.resourceSchema.Type().TerraformType(ctx)) if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, "cannot unmarshal planned state") } - // TODO(erhan): check if new state ID is available if newStateAfterApplyVal.IsNull() { return managed.ExternalCreation{}, errors.New("new state is empty after creation") } var stateValueMap map[string]any - if goval, err := valueToGo(newStateAfterApplyVal); err != nil { + if goval, err := tfValueToMap(newStateAfterApplyVal); err != nil { return managed.ExternalCreation{}, errors.New("cannot convert native state to go map") } else { stateValueMap = goval.(map[string]any) @@ -435,7 +414,6 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg return managed.ExternalCreation{}, errors.Errorf("could not set observation: %v", err) } - // TODO(erhan): check config.Sensitive conn, err := resource.GetConnectionDetails(stateValueMap, mg.(resource.Terraformed), n.config) if err != nil { return managed.ExternalCreation{}, errors.Wrap(err, "cannot get connection details") @@ -447,18 +425,16 @@ func (n *terraformPluginFrameworkExternalClient) Create(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg xpresource.Managed) (managed.ExternalUpdate, error) { n.logger.Debug("Updating the external resource") - configJsonBytes, err := json.Marshal(n.params) + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceSchema.Type().TerraformType(ctx)) if err != nil { - return managed.ExternalUpdate{}, errors.Wrap(err, "cannot convert params to json bytes") + return managed.ExternalUpdate{}, errors.Wrap(err, "cannot construct dynamic value for TF Config") } applyRequest := &tfprotov5.ApplyResourceChangeRequest{ TypeName: n.config.Name, PriorState: n.opTracker.GetFrameworkTFState(), PlannedState: n.plannedState, - Config: &tfprotov5.DynamicValue{ - JSON: configJsonBytes, - }, + Config: tfConfigDynamicVal, } start := time.Now() applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) @@ -466,8 +442,8 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg if err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, "cannot update resource") } - if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { - return managed.ExternalUpdate{}, errors.Errorf("failed to create the resource:") + if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil { + return managed.ExternalUpdate{}, errors.Errorf("resource update failed") } n.opTracker.SetFrameworkTFState(applyResponse.NewState) @@ -481,7 +457,7 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg } var stateValueMap map[string]any - if goval, err := valueToGo(newStateAfterApplyVal); err != nil { + if goval, err := tfValueToMap(newStateAfterApplyVal); err != nil { return managed.ExternalUpdate{}, errors.New("cannot convert native state to go map") } else { stateValueMap = goval.(map[string]any) @@ -497,9 +473,10 @@ func (n *terraformPluginFrameworkExternalClient) Update(ctx context.Context, mg func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ xpresource.Managed) error { n.logger.Debug("Deleting the external resource") - configJsonBytes, err := json.Marshal(n.params) + + tfConfigDynamicVal, err := protov5DynamicValueFromMap(n.params, n.resourceSchema.Type().TerraformType(ctx)) if err != nil { - return errors.Wrap(err, "cannot convert params to json bytes") + return errors.Wrap(err, "cannot construct dynamic value for TF Config") } schemaType := n.resourceSchema.Type().TerraformType(ctx) @@ -513,9 +490,7 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x TypeName: n.config.Name, PriorState: n.opTracker.GetFrameworkTFState(), PlannedState: &plannedState, - Config: &tfprotov5.DynamicValue{ - JSON: configJsonBytes, - }, + Config: tfConfigDynamicVal, } start := time.Now() applyResponse, err := n.server.ApplyResourceChange(ctx, applyRequest) @@ -523,9 +498,8 @@ func (n *terraformPluginFrameworkExternalClient) Delete(ctx context.Context, _ x if err != nil { return errors.Wrap(err, "cannot delete resource") } - // TODO(erhan): improve diagnostics reporting - if fatal := hasFatalDiagnostics(applyResponse.Diagnostics); fatal { - return errors.Errorf("failed to delete the resource with diags") + if fatalDiags := getFatalDiagnostics(applyResponse.Diagnostics); fatalDiags != nil { + return errors.Wrap(fatalDiags, "resource deletion failed with diags") } n.opTracker.SetFrameworkTFState(applyResponse.NewState) @@ -554,72 +528,118 @@ func (n *terraformPluginFrameworkExternalClient) setExternalName(mg xpresource.M return oldName != newName, nil } -func valueToGo(input tftypes.Value) (any, error) { +func tfValueToMap(input tftypes.Value) (any, error) { + if !input.IsKnown() { + return nil, fmt.Errorf("cannot convert unknown value") + } if input.IsNull() { return nil, nil } valType := input.Type() - if valType.Is(tftypes.Object{}) || valType.Is(tftypes.Map{}) { + switch { + case valType.Is(tftypes.Object{}), valType.Is(tftypes.Map{}): destInterim := make(map[string]tftypes.Value) dest := make(map[string]any) if err := input.As(&destInterim); err != nil { return nil, err } for k, v := range destInterim { - if res, err := valueToGo(v); err != nil { + if res, err := tfValueToMap(v); err != nil { return nil, err } else { dest[k] = res } } return dest, nil - } else if valType.Is(tftypes.Set{}) || valType.Is(tftypes.List{}) || valType.Is(tftypes.Tuple{}) { + case valType.Is(tftypes.Set{}), valType.Is(tftypes.List{}), valType.Is(tftypes.Tuple{}): destInterim := make([]tftypes.Value, 0) dest := make([]any, 0) if err := input.As(&destInterim); err != nil { return nil, err } for i, v := range destInterim { - if res, err := valueToGo(v); err != nil { + if res, err := tfValueToMap(v); err != nil { return nil, err } else { dest[i] = res } } return dest, nil - } else if valType.Is(tftypes.Bool) { + case valType.Is(tftypes.Bool): var x bool if err := input.As(&x); err != nil { return nil, err } return x, nil - } else if valType.Is(tftypes.Number) { - var x big.Float - if err := input.As(&x); err != nil { + case valType.Is(tftypes.Number): + var valBigF big.Float + if err := input.As(&valBigF); err != nil { return nil, err } - xf, _ := x.Float64() - return xf, nil - } else if valType.Is(tftypes.String) { + // try to parse as integer + if valBigF.IsInt() { + intVal, accuracy := valBigF.Int64() + if accuracy != 0 { + return nil, fmt.Errorf("value %s cannot be represented as a 64-bit integer", valBigF) + } + return intVal, nil + } else { + xf, accuracy := valBigF.Float64() + // Underflow + // Reference: https://pkg.go.dev/math/big#Float.Float64 + if xf == 0 && accuracy != big.Exact { + return nil, fmt.Errorf("value %s cannot be represented as a 64-bit floating point", valBigF) + } + + // Overflow + // Reference: https://pkg.go.dev/math/big#Float.Float64 + if math.IsInf(xf, 0) { + return nil, fmt.Errorf("value %s cannot be represented as a 64-bit floating point", valBigF) + } + return xf, nil + } + case valType.Is(tftypes.String): var x string if err := input.As(&x); err != nil { return nil, err } return x, nil - } else if valType.Is(tftypes.DynamicPseudoType) { - // TODO: check if we need to handle DynamicPseudoType - return nil, nil - } else { - return nil, nil + case valType.Is(tftypes.DynamicPseudoType): + return nil, errors.New("DynamicPseudoType conversion is not supported") + default: + return nil, fmt.Errorf("input value has unknown type: %s", valType.String()) } } -func hasFatalDiagnostics(diags []*tfprotov5.Diagnostic) bool { - shouldError := false +func getFatalDiagnostics(diags []*tfprotov5.Diagnostic) error { + var errs error + var diagErrors []string for _, tfdiag := range diags { if tfdiag.Severity == tfprotov5.DiagnosticSeverityInvalid || tfdiag.Severity == tfprotov5.DiagnosticSeverityError { - shouldError = true + diagErrors = append(diagErrors, fmt.Sprintf("%s: %s", tfdiag.Summary, tfdiag.Detail)) } } - return shouldError + if len(diagErrors) > 0 { + errs = errors.New(strings.Join(diagErrors, "\n")) + } + return errs +} + +func protov5DynamicValueFromMap(data map[string]any, terraformType tftypes.Type) (*tfprotov5.DynamicValue, error) { + jsonBytes, err := json.Marshal(data) + if err != nil { + return nil, errors.Wrap(err, "cannot marshal json") + } + + tfValue, err := tftypes.ValueFromJSONWithOpts(jsonBytes, terraformType, tftypes.ValueFromJSONOpts{IgnoreUndefinedAttributes: true}) + if err != nil { + return nil, errors.Wrap(err, "cannot construct tf value from json") + } + + dynamicValue, err := tfprotov5.NewDynamicValue(terraformType, tfValue) + if err != nil { + return nil, errors.Wrap(err, "cannot construct dynamic value from tf value") + } + + return &dynamicValue, nil }