Skip to content

Commit

Permalink
Introduce equality checking of PreparedConfig in PrepareProviderConfi…
Browse files Browse the repository at this point in the history
…g and ValidateProviderConfig (#54)

Reference: #51

Previously in real world usage:

```

    provider_test.go:11: Step 1/1 error: Error running pre-apply refresh: exit status 1

        Error: Invalid provider configuration

        Provider "registry.terraform.io/hashicorp/tf5muxprovider" requires explicit
        configuration. Add a provider block to the root module and configure the
        provider's required arguments as described in the provider documentation.

        Error: Plugin error

          with provider["registry.terraform.io/hashicorp/tf5muxprovider"],
          on <empty> line 0:
          (source code not available)

        The plugin returned an unexpected error from
        plugin.(*GRPCProvider).ValidateProviderConfig: rpc error: code = Unknown desc
        = got a PrepareProviderConfig PreparedConfig response from multiple servers,
        not sure which to use

    provider_test.go:11: Step 1/1 error: Error running pre-apply refresh: exit status 1

        Error: Plugin error

        The plugin returned an unexpected error from
        plugin6.(*GRPCProvider).ValidateProviderConfig: rpc error: code = Unknown
        desc = got a ValidateProviderConfig PreparedConfig response from multiple
        servers, not sure which to use

        Error: Invalid provider configuration

        Provider "registry.terraform.io/hashicorp/tf6muxprovider" requires explicit
        configuration. Add a provider block to the root module and configure the
        provider's required arguments as described in the provider documentation.
```

Updated unit testing before code changes:

```
--- FAIL: TestMuxServerPrepareProviderConfig (0.00s)
    --- FAIL: TestMuxServerPrepareProviderConfig/PreparedConfig-multiple-equal (0.00s)
        mux_server_PrepareProviderConfig_test.go:365: wanted no error, got error: got a PrepareProviderConfig PreparedConfig response from multiple servers, not sure which to use
FAIL
FAIL    github.com/hashicorp/terraform-plugin-mux/tf5muxserver  1.244s

--- FAIL: TestMuxServerValidateProviderConfig (0.00s)
    --- FAIL: TestMuxServerValidateProviderConfig/PreparedConfig-multiple-equal (0.00s)
        mux_server_ValidateProviderConfig_test.go:365: wanted no error, got error: got a ValidateProviderConfig PreparedConfig response from multiple servers, not sure which to use
FAIL
FAIL    github.com/hashicorp/terraform-plugin-mux/tf6muxserver  0.665s
```

Verified those integration tests are now passing by replacing the mux dependency with this branch. Once those tests are published, we can introduce the integration tests into this project's testing workflow.
  • Loading branch information
bflad authored Feb 17, 2022
1 parent bde9985 commit 82e4ba5
Show file tree
Hide file tree
Showing 13 changed files with 2,545 additions and 27 deletions.
7 changes: 7 additions & 0 deletions .changelog/54.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:bug
tf5muxserver: Prevent `PrepareProviderConfig` RPC error for multiple `PreparedConfig` responses when combining terraform-plugin-sdk/v2 providers
```

```release-note:bug
tf6muxserver: Prevent `ValidateProviderConfig` RPC error for multiple `PreparedConfig` responses when combining terraform-plugin-framework providers
```
38 changes: 38 additions & 0 deletions tf5muxserver/dynamic_value_equality.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package tf5muxserver

import (
"fmt"

"github.com/hashicorp/terraform-plugin-go/tfprotov5"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

// dynamicValueEquals performs equality checking of DynamicValue.
func dynamicValueEquals(schemaType tftypes.Type, i *tfprotov5.DynamicValue, j *tfprotov5.DynamicValue) (bool, error) {
if i == nil {
return j == nil, nil
}

if j == nil {
return false, nil
}

// Upstream will panic on DynamicValue.Unmarshal with nil Type
if schemaType == nil {
return false, fmt.Errorf("unable to unmarshal DynamicValue: missing Type")
}

iValue, err := i.Unmarshal(schemaType)

if err != nil {
return false, fmt.Errorf("unable to unmarshal DynamicValue: %w", err)
}

jValue, err := j.Unmarshal(schemaType)

if err != nil {
return false, fmt.Errorf("unable to unmarshal DynamicValue: %w", err)
}

return iValue.Equal(jValue), nil
}
283 changes: 283 additions & 0 deletions tf5muxserver/dynamic_value_equality_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,283 @@
package tf5muxserver

import (
"fmt"
"strings"
"testing"

"github.com/hashicorp/terraform-plugin-go/tfprotov5"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

func TestDynamicValueEquals(t *testing.T) {
t.Parallel()

testCases := map[string]struct {
schemaType tftypes.Type
dynamicValue1 func() (*tfprotov5.DynamicValue, error)
dynamicValue2 func() (*tfprotov5.DynamicValue, error)
expected bool
expectedError error
}{
"all-missing": {
schemaType: nil,
dynamicValue1: func() (*tfprotov5.DynamicValue, error) {
return nil, nil
},
dynamicValue2: func() (*tfprotov5.DynamicValue, error) {
return nil, nil
},
expected: true,
},
"first-missing": {
schemaType: nil,
dynamicValue1: func() (*tfprotov5.DynamicValue, error) {
return nil, nil
},
dynamicValue2: func() (*tfprotov5.DynamicValue, error) {
return &tfprotov5.DynamicValue{}, nil
},
expected: false,
},
"second-missing": {
schemaType: nil,
dynamicValue1: func() (*tfprotov5.DynamicValue, error) {
return &tfprotov5.DynamicValue{}, nil
},
dynamicValue2: func() (*tfprotov5.DynamicValue, error) {
return nil, nil
},
expected: false,
},
"missing-type": {
schemaType: nil,
dynamicValue1: func() (*tfprotov5.DynamicValue, error) {
dv, err := tfprotov5.NewDynamicValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
map[string]tftypes.Value{
"test_string_attribute": tftypes.NewValue(tftypes.String, "test-value"),
},
),
)
return &dv, err
},
dynamicValue2: func() (*tfprotov5.DynamicValue, error) {
dv, err := tfprotov5.NewDynamicValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
map[string]tftypes.Value{
"test_string_attribute": tftypes.NewValue(tftypes.String, "test-value"),
},
),
)
return &dv, err
},
expected: false,
expectedError: fmt.Errorf("unable to unmarshal DynamicValue: missing Type"),
},
"mismatched-type": {
schemaType: tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_bool_attribute": tftypes.Bool,
},
},
dynamicValue1: func() (*tfprotov5.DynamicValue, error) {
dv, err := tfprotov5.NewDynamicValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
map[string]tftypes.Value{
"test_string_attribute": tftypes.NewValue(tftypes.String, "test-value"),
},
),
)
return &dv, err
},
dynamicValue2: func() (*tfprotov5.DynamicValue, error) {
dv, err := tfprotov5.NewDynamicValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
map[string]tftypes.Value{
"test_string_attribute": tftypes.NewValue(tftypes.String, "test-value"),
},
),
)
return &dv, err
},
expected: false,
expectedError: fmt.Errorf("unable to unmarshal DynamicValue: unknown attribute \"test_string_attribute\""),
},
"String-different-value": {
schemaType: tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
dynamicValue1: func() (*tfprotov5.DynamicValue, error) {
dv, err := tfprotov5.NewDynamicValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
map[string]tftypes.Value{
"test_string_attribute": tftypes.NewValue(tftypes.String, "test-value-1"),
},
),
)
return &dv, err
},
dynamicValue2: func() (*tfprotov5.DynamicValue, error) {
dv, err := tfprotov5.NewDynamicValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
map[string]tftypes.Value{
"test_string_attribute": tftypes.NewValue(tftypes.String, "test-value-2"),
},
),
)
return &dv, err
},
expected: false,
},
"String-equal-value": {
schemaType: tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
dynamicValue1: func() (*tfprotov5.DynamicValue, error) {
dv, err := tfprotov5.NewDynamicValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
map[string]tftypes.Value{
"test_string_attribute": tftypes.NewValue(tftypes.String, "test-value"),
},
),
)
return &dv, err
},
dynamicValue2: func() (*tfprotov5.DynamicValue, error) {
dv, err := tfprotov5.NewDynamicValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
tftypes.NewValue(
tftypes.Object{
AttributeTypes: map[string]tftypes.Type{
"test_string_attribute": tftypes.String,
},
},
map[string]tftypes.Value{
"test_string_attribute": tftypes.NewValue(tftypes.String, "test-value"),
},
),
)
return &dv, err
},
expected: true,
},
}

for name, testCase := range testCases {
name, testCase := name, testCase

t.Run(name, func(t *testing.T) {
t.Parallel()

dynamicValue1, err := testCase.dynamicValue1()

if err != nil {
t.Fatalf("unable to create first DynamicValue: %s", err)
}

dynamicValue2, err := testCase.dynamicValue2()

if err != nil {
t.Fatalf("unable to create second DynamicValue: %s", err)
}

got, err := dynamicValueEquals(testCase.schemaType, dynamicValue1, dynamicValue2)

if err != nil {
if testCase.expectedError == nil {
t.Fatalf("wanted no error, got error: %s", err)
}

if !strings.Contains(err.Error(), testCase.expectedError.Error()) {
t.Fatalf("wanted error %q, got error: %s", testCase.expectedError.Error(), err.Error())
}
}

if err == nil && testCase.expectedError != nil {
t.Fatalf("got no error, wanted err: %s", testCase.expectedError)
}

if got != testCase.expected {
t.Errorf("expected %t, got %t", testCase.expected, got)
}
})
}
}
27 changes: 17 additions & 10 deletions tf5muxserver/mux_server_PrepareProviderConfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import (
)

// PrepareProviderConfig calls the PrepareProviderConfig method on each server
// in order, passing `req`. Only one may respond with a non-nil PreparedConfig
// or a non-empty Diagnostics.
// in order, passing `req`. Response diagnostics are appended from all servers.
// Response PreparedConfig must be equal across all servers with nil values
// skipped.
func (s muxServer) PrepareProviderConfig(ctx context.Context, req *tfprotov5.PrepareProviderConfigRequest) (*tfprotov5.PrepareProviderConfigResponse, error) {
rpc := "PrepareProviderConfig"
ctx = logging.InitContext(ctx)
Expand Down Expand Up @@ -42,16 +43,22 @@ func (s muxServer) PrepareProviderConfig(ctx context.Context, req *tfprotov5.Pre
resp.Diagnostics = append(resp.Diagnostics, res.Diagnostics...)
}

if res.PreparedConfig != nil {
// This could check equality to bypass the error, however
// DynamicValue does not implement Equals() and previous mux server
// implementations have not requested the enhancement.
if resp.PreparedConfig != nil {
return nil, fmt.Errorf("got a PrepareProviderConfig PreparedConfig response from multiple servers, not sure which to use")
}
// Do not check equality on missing PreparedConfig or unset PreparedConfig
if res.PreparedConfig == nil {
continue
}

resp.PreparedConfig = res.PreparedConfig
equal, err := dynamicValueEquals(schemaType(s.providerSchema), res.PreparedConfig, resp.PreparedConfig)

if err != nil {
return nil, fmt.Errorf("unable to compare PrepareProviderConfig PreparedConfig responses: %w", err)
}

if !equal {
return nil, fmt.Errorf("got different PrepareProviderConfig PreparedConfig response from multiple servers, not sure which to use")
}

resp.PreparedConfig = res.PreparedConfig
}

return resp, nil
Expand Down
Loading

0 comments on commit 82e4ba5

Please sign in to comment.