From 44fd9b8d354c032a70325aa5ba257d2927c82543 Mon Sep 17 00:00:00 2001 From: Paddy Carver Date: Mon, 4 Jan 2021 07:46:25 -0800 Subject: [PATCH 1/2] Be more permissive when checking schemas are identical. Allow schemas to differ in the order of attributes if they are otherwise identical. Applies to the checks on provider schemas and provider_meta schemas to ensure the servers are in agreement. --- schema_server.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/schema_server.go b/schema_server.go index d7d3489..3915b14 100644 --- a/schema_server.go +++ b/schema_server.go @@ -3,6 +3,7 @@ package tfmux import ( "context" "fmt" + "sort" "strings" "github.com/google/go-cmp/cmp" @@ -11,6 +12,13 @@ import ( var _ tfprotov5.ProviderServer = SchemaServer{} +var sortCmpTransformer = cmp.Transformer("Sort", func(in []*tfprotov5.SchemaAttribute) []*tfprotov5.SchemaAttribute { + copied := make([]*tfprotov5.SchemaAttribute, len(in)) + copy(copied, in) + sort.Slice(copied, func(i, j int) bool { return copied[i].Name < copied[j].Name }) + return in +}) + // SchemaServerFactory is a generator for SchemaServers, which are Terraform // gRPC servers that route requests to different gRPC provider implementations // based on which gRPC provider implementation supports the resource the @@ -79,13 +87,13 @@ func NewSchemaServerFactory(ctx context.Context, servers ...func() tfprotov5.Pro } return factory, fmt.Errorf("error retrieving schema for %T:\n\n\tAttribute: %s\n\tSummary: %s\n\tDetail: %s", s, diag.Attribute, diag.Summary, diag.Detail) } - if resp.Provider != nil && factory.providerSchema != nil && !cmp.Equal(resp.Provider, factory.providerSchema) { + if resp.Provider != nil && factory.providerSchema != nil && !cmp.Equal(resp.Provider, factory.providerSchema, sortCmpTransformer) { return factory, fmt.Errorf("got a different provider schema from two servers (%T, %T). Provider schemas must be identical across providers.", factory.servers[factory.providerSchemaFrom](), s) } else if resp.Provider != nil { factory.providerSchemaFrom = pos factory.providerSchema = resp.Provider } - if resp.ProviderMeta != nil && factory.providerMetaSchema != nil && !cmp.Equal(resp.ProviderMeta, factory.providerMetaSchema) { + if resp.ProviderMeta != nil && factory.providerMetaSchema != nil && !cmp.Equal(resp.ProviderMeta, factory.providerMetaSchema, sortCmpTransformer) { return factory, fmt.Errorf("got a different provider_meta schema from two servers (%T, %T). Provider metadata schemas must be identical across providers.", factory.servers[factory.providerMetaSchemaFrom](), s) } else if resp.ProviderMeta != nil { factory.providerMetaSchemaFrom = pos From 6c12ef66da75f3094222e30cc341b30b1723a2bc Mon Sep 17 00:00:00 2001 From: Paddy Carver Date: Tue, 2 Feb 2021 04:33:21 -0800 Subject: [PATCH 2/2] Another pass at more lenient schema checks. Make sure that schemas are compared semantically, by not caring about the order of blocks or attributes in the results for Provider and ProviderMeta. Add tests to verify that this is the case. When Provider or ProviderMeta schemas don't match, include the diff in the error for easier debugging. --- schema_server.go | 24 ++-- schema_server_test.go | 320 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 333 insertions(+), 11 deletions(-) diff --git a/schema_server.go b/schema_server.go index 3915b14..2aba7a3 100644 --- a/schema_server.go +++ b/schema_server.go @@ -3,21 +3,23 @@ package tfmux import ( "context" "fmt" - "sort" "strings" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/terraform-plugin-go/tfprotov5" ) var _ tfprotov5.ProviderServer = SchemaServer{} -var sortCmpTransformer = cmp.Transformer("Sort", func(in []*tfprotov5.SchemaAttribute) []*tfprotov5.SchemaAttribute { - copied := make([]*tfprotov5.SchemaAttribute, len(in)) - copy(copied, in) - sort.Slice(copied, func(i, j int) bool { return copied[i].Name < copied[j].Name }) - return in -}) +var cmpOptions = []cmp.Option{ + cmpopts.SortSlices(func(i, j *tfprotov5.SchemaAttribute) bool { + return i.Name < j.Name + }), + cmpopts.SortSlices(func(i, j *tfprotov5.SchemaNestedBlock) bool { + return i.TypeName < j.TypeName + }), +} // SchemaServerFactory is a generator for SchemaServers, which are Terraform // gRPC servers that route requests to different gRPC provider implementations @@ -87,14 +89,14 @@ func NewSchemaServerFactory(ctx context.Context, servers ...func() tfprotov5.Pro } return factory, fmt.Errorf("error retrieving schema for %T:\n\n\tAttribute: %s\n\tSummary: %s\n\tDetail: %s", s, diag.Attribute, diag.Summary, diag.Detail) } - if resp.Provider != nil && factory.providerSchema != nil && !cmp.Equal(resp.Provider, factory.providerSchema, sortCmpTransformer) { - return factory, fmt.Errorf("got a different provider schema from two servers (%T, %T). Provider schemas must be identical across providers.", factory.servers[factory.providerSchemaFrom](), s) + if resp.Provider != nil && factory.providerSchema != nil && !cmp.Equal(resp.Provider, factory.providerSchema, cmpOptions...) { + return factory, fmt.Errorf("got a different provider schema from two servers (%T, %T). Provider schemas must be identical across providers. Diff: %s", factory.servers[factory.providerSchemaFrom](), s, cmp.Diff(resp.Provider, factory.providerSchema, cmpOptions...)) } else if resp.Provider != nil { factory.providerSchemaFrom = pos factory.providerSchema = resp.Provider } - if resp.ProviderMeta != nil && factory.providerMetaSchema != nil && !cmp.Equal(resp.ProviderMeta, factory.providerMetaSchema, sortCmpTransformer) { - return factory, fmt.Errorf("got a different provider_meta schema from two servers (%T, %T). Provider metadata schemas must be identical across providers.", factory.servers[factory.providerMetaSchemaFrom](), s) + if resp.ProviderMeta != nil && factory.providerMetaSchema != nil && !cmp.Equal(resp.ProviderMeta, factory.providerMetaSchema, cmpOptions...) { + return factory, fmt.Errorf("got a different provider_meta schema from two servers (%T, %T). Provider metadata schemas must be identical across providers. Diff: %s", factory.servers[factory.providerMetaSchemaFrom](), s, cmp.Diff(resp.ProviderMeta, factory.providerMetaSchema, cmpOptions...)) } else if resp.ProviderMeta != nil { factory.providerMetaSchemaFrom = pos factory.providerMetaSchema = resp.ProviderMeta diff --git a/schema_server_test.go b/schema_server_test.go index dc99814..dfb64b7 100644 --- a/schema_server_test.go +++ b/schema_server_test.go @@ -595,6 +595,326 @@ func TestSchemaServerGetProviderSchema_errorDuplicateDataSource(t *testing.T) { } } +func TestSchemaServerGetProviderSchema_providerOutOfOrder(t *testing.T) { + server1 := testFactory(&testServer{ + providerSchema: &tfprotov5.Schema{ + Version: 1, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "account_id", + Type: tftypes.String, + Required: true, + Description: "the account ID to make requests for", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "secret", + Type: tftypes.String, + Required: true, + Description: "the secret to authenticate with", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + BlockTypes: []*tfprotov5.SchemaNestedBlock{ + { + TypeName: "other_feature", + Nesting: tfprotov5.SchemaNestedBlockNestingModeList, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Description: "features to enable on the provider", + DescriptionKind: tfprotov5.StringKindPlain, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "enabled", + Type: tftypes.Bool, + Required: true, + Description: "whether the feature is enabled", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "feature_id", + Type: tftypes.Number, + Required: true, + Description: "The ID of the feature", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + }, + }, + { + TypeName: "feature", + Nesting: tfprotov5.SchemaNestedBlockNestingModeList, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Description: "features to enable on the provider", + DescriptionKind: tfprotov5.StringKindPlain, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "feature_id", + Type: tftypes.Number, + Required: true, + Description: "The ID of the feature", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "enabled", + Type: tftypes.Bool, + Required: true, + Description: "whether the feature is enabled", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + }, + }, + }, + }, + }, + }) + server2 := testFactory(&testServer{ + providerSchema: &tfprotov5.Schema{ + Version: 1, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "secret", + Type: tftypes.String, + Required: true, + Description: "the secret to authenticate with", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "account_id", + Type: tftypes.String, + Required: true, + Description: "the account ID to make requests for", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + BlockTypes: []*tfprotov5.SchemaNestedBlock{ + { + TypeName: "feature", + Nesting: tfprotov5.SchemaNestedBlockNestingModeList, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Description: "features to enable on the provider", + DescriptionKind: tfprotov5.StringKindPlain, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "enabled", + Type: tftypes.Bool, + Required: true, + Description: "whether the feature is enabled", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "feature_id", + Type: tftypes.Number, + Required: true, + Description: "The ID of the feature", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + }, + }, + { + TypeName: "other_feature", + Nesting: tfprotov5.SchemaNestedBlockNestingModeList, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Description: "features to enable on the provider", + DescriptionKind: tfprotov5.StringKindPlain, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "enabled", + Type: tftypes.Bool, + Required: true, + Description: "whether the feature is enabled", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "feature_id", + Type: tftypes.Number, + Required: true, + Description: "The ID of the feature", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + }, + }, + }, + }, + }, + }) + + _, err := NewSchemaServerFactory(context.Background(), server1, server2) + if err != nil { + t.Error(err) + } +} + +func TestSchemaServerGetProviderSchema_providerMetaOutOfOrder(t *testing.T) { + server1 := testFactory(&testServer{ + providerMetaSchema: &tfprotov5.Schema{ + Version: 1, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "account_id", + Type: tftypes.String, + Required: true, + Description: "the account ID to make requests for", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "secret", + Type: tftypes.String, + Required: true, + Description: "the secret to authenticate with", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + BlockTypes: []*tfprotov5.SchemaNestedBlock{ + { + TypeName: "feature", + Nesting: tfprotov5.SchemaNestedBlockNestingModeList, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Description: "features to enable on the provider", + DescriptionKind: tfprotov5.StringKindPlain, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "feature_id", + Type: tftypes.Number, + Required: true, + Description: "The ID of the feature", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "enabled", + Type: tftypes.Bool, + Required: true, + Description: "whether the feature is enabled", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + }, + }, + { + TypeName: "other_feature", + Nesting: tfprotov5.SchemaNestedBlockNestingModeList, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Description: "features to enable on the provider", + DescriptionKind: tfprotov5.StringKindPlain, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "enabled", + Type: tftypes.Bool, + Required: true, + Description: "whether the feature is enabled", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "feature_id", + Type: tftypes.Number, + Required: true, + Description: "The ID of the feature", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + }, + }, + }, + }, + }, + }) + server2 := testFactory(&testServer{ + providerMetaSchema: &tfprotov5.Schema{ + Version: 1, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "secret", + Type: tftypes.String, + Required: true, + Description: "the secret to authenticate with", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "account_id", + Type: tftypes.String, + Required: true, + Description: "the account ID to make requests for", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + BlockTypes: []*tfprotov5.SchemaNestedBlock{ + { + TypeName: "other_feature", + Nesting: tfprotov5.SchemaNestedBlockNestingModeList, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Description: "features to enable on the provider", + DescriptionKind: tfprotov5.StringKindPlain, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "feature_id", + Type: tftypes.Number, + Required: true, + Description: "The ID of the feature", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "enabled", + Type: tftypes.Bool, + Required: true, + Description: "whether the feature is enabled", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + }, + }, + { + TypeName: "feature", + Nesting: tfprotov5.SchemaNestedBlockNestingModeList, + Block: &tfprotov5.SchemaBlock{ + Version: 1, + Description: "features to enable on the provider", + DescriptionKind: tfprotov5.StringKindPlain, + Attributes: []*tfprotov5.SchemaAttribute{ + { + Name: "enabled", + Type: tftypes.Bool, + Required: true, + Description: "whether the feature is enabled", + DescriptionKind: tfprotov5.StringKindPlain, + }, + { + Name: "feature_id", + Type: tftypes.Number, + Required: true, + Description: "The ID of the feature", + DescriptionKind: tfprotov5.StringKindPlain, + }, + }, + }, + }, + }, + }, + }, + }) + + _, err := NewSchemaServerFactory(context.Background(), server1, server2) + if err != nil { + t.Error(err) + } +} + func TestSchemaServerGetProviderSchema_errorProviderMismatch(t *testing.T) { server1 := testFactory(&testServer{ providerSchema: &tfprotov5.Schema{