diff --git a/.changes/unreleased/ENHANCEMENTS-20230623-102412.yaml b/.changes/unreleased/ENHANCEMENTS-20230623-102412.yaml new file mode 100644 index 0000000000..1469e81572 --- /dev/null +++ b/.changes/unreleased/ENHANCEMENTS-20230623-102412.yaml @@ -0,0 +1,6 @@ +kind: ENHANCEMENTS +body: 'helper/schema: Added `Resource` type `SchemaFunc` field and `SchemaMap` method, + which can reduce resident memory usage with large schemas' +time: 2023-06-23T10:24:12.025356-04:00 +custom: + Issue: "1217" diff --git a/.changes/unreleased/NOTES-20230623-102528.yaml b/.changes/unreleased/NOTES-20230623-102528.yaml new file mode 100644 index 0000000000..5f791f4ffd --- /dev/null +++ b/.changes/unreleased/NOTES-20230623-102528.yaml @@ -0,0 +1,7 @@ +kind: NOTES +body: 'helper/schema: Consumers directly referencing the `Resource` type `Schema` + field should switch to the `SchemaMap` method to ensure new `SchemaFunc` field + data is properly retrieved' +time: 2023-06-23T10:25:28.864812-04:00 +custom: + Issue: "1217" diff --git a/helper/schema/core_schema.go b/helper/schema/core_schema.go index 3ded6fc612..736af218da 100644 --- a/helper/schema/core_schema.go +++ b/helper/schema/core_schema.go @@ -367,5 +367,5 @@ func (r *Resource) CoreConfigSchema() *configschema.Block { } func (r *Resource) coreConfigSchema() *configschema.Block { - return schemaMap(r.Schema).CoreConfigSchema() + return schemaMap(r.SchemaMap()).CoreConfigSchema() } diff --git a/helper/schema/field_reader.go b/helper/schema/field_reader.go index 02defaacf6..6aae74d95b 100644 --- a/helper/schema/field_reader.go +++ b/helper/schema/field_reader.go @@ -84,7 +84,7 @@ func addrToSchema(addr []string, schemaMap map[string]*Schema) []*Schema { case *Resource: current = &Schema{ Type: typeObject, - Elem: v.Schema, + Elem: v.SchemaMap(), } case *Schema: current = v diff --git a/helper/schema/field_reader_config.go b/helper/schema/field_reader_config.go index 2edd8e7f09..df317c20bb 100644 --- a/helper/schema/field_reader_config.go +++ b/helper/schema/field_reader_config.go @@ -303,7 +303,7 @@ func (r *ConfigFieldReader) hasComputedSubKeys(key string, schema *Schema) bool switch t := schema.Elem.(type) { case *Resource: - for k, schema := range t.Schema { + for k, schema := range t.SchemaMap() { if r.Config.IsComputed(prefix + k) { return true } diff --git a/helper/schema/grpc_provider.go b/helper/schema/grpc_provider.go index 6b632f8b40..831d78777a 100644 --- a/helper/schema/grpc_provider.go +++ b/helper/schema/grpc_provider.go @@ -1303,7 +1303,7 @@ func stripResourceModifiers(r *Resource) *Resource { newResource.CustomizeDiff = nil newResource.Schema = map[string]*Schema{} - for k, s := range r.Schema { + for k, s := range r.SchemaMap() { newResource.Schema[k] = stripSchema(s) } diff --git a/helper/schema/grpc_provider_test.go b/helper/schema/grpc_provider_test.go index 3aeb3377b7..82292689da 100644 --- a/helper/schema/grpc_provider_test.go +++ b/helper/schema/grpc_provider_test.go @@ -807,6 +807,23 @@ func TestApplyResourceChange(t *testing.T) { }, }, }, + { + Description: "CreateContext_SchemaFunc", + TestResource: &Resource{ + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "id": { + Type: TypeString, + Computed: true, + }, + } + }, + CreateContext: func(_ context.Context, rd *ResourceData, _ interface{}) diag.Diagnostics { + rd.SetId("bar") // expected in response + return nil + }, + }, + }, } for _, testCase := range testCases { @@ -1254,6 +1271,47 @@ func TestReadDataSource(t *testing.T) { }, }, }, + "SchemaFunc": { + server: NewGRPCProviderServer(&Provider{ + DataSourcesMap: map[string]*Resource{ + "test": { + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "id": { + Type: TypeString, + Computed: true, + }, + } + }, + ReadContext: func(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { + d.SetId("test-id") + return nil + }, + }, + }, + }), + req: &tfprotov5.ReadDataSourceRequest{ + TypeName: "test", + Config: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.EmptyObject, + cty.NullVal(cty.EmptyObject), + ), + }, + }, + expected: &tfprotov5.ReadDataSourceResponse{ + State: &tfprotov5.DynamicValue{ + MsgPack: mustMsgpackMarshal( + cty.Object(map[string]cty.Type{ + "id": cty.String, + }), + cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("test-id"), + }), + ), + }, + }, + }, "null-object": { server: NewGRPCProviderServer(&Provider{ DataSourcesMap: map[string]*Resource{ diff --git a/helper/schema/resource.go b/helper/schema/resource.go index 42030b7485..8a1472e45d 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -56,13 +56,25 @@ var ReservedResourceFields = []string{ // being implemented. type Resource struct { // Schema is the structure and type information for this component. This - // field is required for all Resource concepts. + // field, or SchemaFunc, is required for all Resource concepts. To prevent + // storing all schema information in memory for the lifecycle of a provider, + // use SchemaFunc instead. // // The keys of this map are the names used in a practitioner configuration, // such as the attribute or block name. The values describe the structure // and type information of that attribute or block. Schema map[string]*Schema + // SchemaFunc is the structure and type information for this component. This + // field, or Schema, is required for all Resource concepts. Use this field + // instead of Schema on top level Resource declarations to prevent storing + // all schema information in memory for the lifecycle of a provider. + // + // The keys of this map are the names used in a practitioner configuration, + // such as the attribute or block name. The values describe the structure + // and type information of that attribute or block. + SchemaFunc func() map[string]*Schema + // SchemaVersion is the version number for this resource's Schema // definition. This field is only valid when the Resource is a managed // resource. @@ -585,6 +597,17 @@ type Resource struct { UseJSONNumber bool } +// SchemaMap returns the schema information for this Resource whether it is +// defined via the SchemaFunc field or Schema field. The SchemaFunc field, if +// defined, takes precedence over the Schema field. +func (r *Resource) SchemaMap() map[string]*Schema { + if r.SchemaFunc != nil { + return r.SchemaFunc() + } + + return r.Schema +} + // ShimInstanceStateFromValue converts a cty.Value to a // terraform.InstanceState. func (r *Resource) ShimInstanceStateFromValue(state cty.Value) (*terraform.InstanceState, error) { @@ -594,7 +617,7 @@ func (r *Resource) ShimInstanceStateFromValue(state cty.Value) (*terraform.Insta // We now rebuild the state through the ResourceData, so that the set indexes // match what helper/schema expects. - data, err := schemaMap(r.Schema).Data(s, nil) + data, err := schemaMap(r.SchemaMap()).Data(s, nil) if err != nil { return nil, err } @@ -767,7 +790,8 @@ func (r *Resource) Apply( s *terraform.InstanceState, d *terraform.InstanceDiff, meta interface{}) (*terraform.InstanceState, diag.Diagnostics) { - data, err := schemaMap(r.Schema).Data(s, d) + schema := schemaMap(r.SchemaMap()) + data, err := schema.Data(s, d) if err != nil { return s, diag.FromErr(err) } @@ -824,7 +848,7 @@ func (r *Resource) Apply( } // Reset the data to be stateless since we just destroyed - data, err = schemaMap(r.Schema).Data(nil, d) + data, err = schema.Data(nil, d) if err != nil { return nil, append(diags, diag.FromErr(err)...) } @@ -868,7 +892,7 @@ func (r *Resource) Diff( return nil, fmt.Errorf("[ERR] Error decoding timeout: %s", err) } - instanceDiff, err := schemaMap(r.Schema).Diff(ctx, s, c, r.CustomizeDiff, meta, true) + instanceDiff, err := schemaMap(r.SchemaMap()).Diff(ctx, s, c, r.CustomizeDiff, meta, true) if err != nil { return instanceDiff, err } @@ -890,7 +914,7 @@ func (r *Resource) SimpleDiff( c *terraform.ResourceConfig, meta interface{}) (*terraform.InstanceDiff, error) { - instanceDiff, err := schemaMap(r.Schema).Diff(ctx, s, c, r.CustomizeDiff, meta, false) + instanceDiff, err := schemaMap(r.SchemaMap()).Diff(ctx, s, c, r.CustomizeDiff, meta, false) if err != nil { return instanceDiff, err } @@ -915,7 +939,7 @@ func (r *Resource) SimpleDiff( // Validate validates the resource configuration against the schema. func (r *Resource) Validate(c *terraform.ResourceConfig) diag.Diagnostics { - diags := schemaMap(r.Schema).Validate(c) + diags := schemaMap(r.SchemaMap()).Validate(c) if r.DeprecationMessage != "" { diags = append(diags, diag.Diagnostic{ @@ -937,7 +961,7 @@ func (r *Resource) ReadDataApply( ) (*terraform.InstanceState, diag.Diagnostics) { // Data sources are always built completely from scratch // on each read, so the source state is always nil. - data, err := schemaMap(r.Schema).Data(nil, d) + data, err := schemaMap(r.SchemaMap()).Data(nil, d) if err != nil { return nil, diag.FromErr(err) } @@ -978,10 +1002,12 @@ func (r *Resource) RefreshWithoutUpgrade( } } + schema := schemaMap(r.SchemaMap()) + if r.Exists != nil { // Make a copy of data so that if it is modified it doesn't // affect our Read later. - data, err := schemaMap(r.Schema).Data(s, nil) + data, err := schema.Data(s, nil) if err != nil { return s, diag.FromErr(err) } @@ -1004,7 +1030,7 @@ func (r *Resource) RefreshWithoutUpgrade( } } - data, err := schemaMap(r.Schema).Data(s, nil) + data, err := schema.Data(s, nil) if err != nil { return s, diag.FromErr(err) } @@ -1023,7 +1049,7 @@ func (r *Resource) RefreshWithoutUpgrade( state = nil } - schemaMap(r.Schema).handleDiffSuppressOnRefresh(ctx, s, state) + schema.handleDiffSuppressOnRefresh(ctx, s, state) return r.recordCurrentSchemaVersion(state), diags } @@ -1069,13 +1095,14 @@ func (r *Resource) InternalValidate(topSchemaMap schemaMap, writable bool) error } } + schema := schemaMap(r.SchemaMap()) tsm := topSchemaMap if r.isTopLevel() && writable { // All non-Computed attributes must be ForceNew if Update is not defined if !r.updateFuncSet() { nonForceNewAttrs := make([]string, 0) - for k, v := range r.Schema { + for k, v := range schema { if !v.ForceNew && !v.Computed { nonForceNewAttrs = append(nonForceNewAttrs, k) } @@ -1086,19 +1113,19 @@ func (r *Resource) InternalValidate(topSchemaMap schemaMap, writable bool) error } } else { nonUpdateableAttrs := make([]string, 0) - for k, v := range r.Schema { + for k, v := range schema { if v.ForceNew || v.Computed && !v.Optional { nonUpdateableAttrs = append(nonUpdateableAttrs, k) } } - updateableAttrs := len(r.Schema) - len(nonUpdateableAttrs) + updateableAttrs := len(schema) - len(nonUpdateableAttrs) if updateableAttrs == 0 { return fmt.Errorf( "All fields are ForceNew or Computed w/out Optional, Update is superfluous") } } - tsm = schemaMap(r.Schema) + tsm = schema // Destroy, and Read are required if !r.readFuncSet() { @@ -1157,7 +1184,7 @@ func (r *Resource) InternalValidate(topSchemaMap schemaMap, writable bool) error // Data source if r.isTopLevel() && !writable { - tsm = schemaMap(r.Schema) + tsm = schema for k := range tsm { if isReservedDataSourceFieldName(k) { return fmt.Errorf("%s is a reserved field name", k) @@ -1165,6 +1192,10 @@ func (r *Resource) InternalValidate(topSchemaMap schemaMap, writable bool) error } } + if r.SchemaFunc != nil && r.Schema != nil { + return fmt.Errorf("SchemaFunc and Schema should not both be set") + } + // check context funcs are not set alongside their nonctx counterparts if r.CreateContext != nil && r.Create != nil { return fmt.Errorf("CreateContext and Create should not both be set") @@ -1207,7 +1238,7 @@ func (r *Resource) InternalValidate(topSchemaMap schemaMap, writable bool) error return fmt.Errorf("Delete and DeleteWithoutTimeout should not both be set") } - return schemaMap(r.Schema).InternalValidate(tsm) + return schema.InternalValidate(tsm) } func isReservedDataSourceFieldName(name string) bool { @@ -1254,7 +1285,7 @@ func isReservedResourceFieldName(name string) bool { // // This function is useful for unit tests and ResourceImporter functions. func (r *Resource) Data(s *terraform.InstanceState) *ResourceData { - result, err := schemaMap(r.Schema).Data(s, nil) + result, err := schemaMap(r.SchemaMap()).Data(s, nil) if err != nil { // At the time of writing, this isn't possible (Data never returns // non-nil errors). We panic to find this in the future if we have to. @@ -1281,7 +1312,7 @@ func (r *Resource) Data(s *terraform.InstanceState) *ResourceData { // TODO: May be able to be removed with the above ResourceData function. func (r *Resource) TestResourceData() *ResourceData { return &ResourceData{ - schema: r.Schema, + schema: r.SchemaMap(), } } diff --git a/helper/schema/resource_test.go b/helper/schema/resource_test.go index debb55f08c..5c9fd629ba 100644 --- a/helper/schema/resource_test.go +++ b/helper/schema/resource_test.go @@ -1080,6 +1080,51 @@ func TestResourceInternalValidate(t *testing.T) { true, true, }, + 27: { // Non-Writable SchemaFunc and Schema should not both be set + In: &Resource{ + Schema: map[string]*Schema{ + "test": { + Type: TypeString, + Required: true, + }, + }, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "test": { + Type: TypeString, + Required: true, + }, + } + }, + Read: Noop, + }, + Writable: false, + Err: true, + }, + 28: { // Writable SchemaFunc and Schema should not both be set + In: &Resource{ + Schema: map[string]*Schema{ + "test": { + Type: TypeString, + Required: true, + }, + }, + SchemaFunc: func() map[string]*Schema { + return map[string]*Schema{ + "test": { + Type: TypeString, + Required: true, + }, + } + }, + Create: Noop, + Read: Noop, + Update: Noop, + Delete: Noop, + }, + Writable: true, + Err: true, + }, } for i, tc := range cases { diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 61739a1bb1..176288b0cd 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -939,7 +939,7 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro case *Resource: attrsOnly := attrsOnly || v.ConfigMode == SchemaConfigModeAttr - if err := schemaMap(t.Schema).internalValidate(topSchemaMap, attrsOnly); err != nil { + if err := schemaMap(t.SchemaMap()).internalValidate(topSchemaMap, attrsOnly); err != nil { return err } case *Schema: @@ -1069,7 +1069,7 @@ func checkKeysAgainstSchemaFlags(k string, keys []string, topSchemaMap schemaMap return fmt.Errorf("%s configuration block reference (%s) can only be used with TypeList and MaxItems: 1 configuration blocks", k, key) } - sm = schemaMap(subResource.Schema) + sm = subResource.SchemaMap() } if target == nil { @@ -1259,7 +1259,7 @@ func (m schemaMap) diffList( case *Resource: // This is a complex resource for i := 0; i < maxLen; i++ { - for k2, schema := range t.Schema { + for k2, schema := range t.SchemaMap() { subK := fmt.Sprintf("%s.%d.%s", k, i, k2) err := m.diff(ctx, subK, schema, diff, d, all) if err != nil { @@ -1506,7 +1506,7 @@ func (m schemaMap) diffSet( switch t := schema.Elem.(type) { case *Resource: // This is a complex resource - for k2, schema := range t.Schema { + for k2, schema := range t.SchemaMap() { subK := fmt.Sprintf("%s.%s.%s", k, code, k2) err := m.diff(ctx, subK, schema, diff, d, true) if err != nil { @@ -1973,7 +1973,7 @@ func (m schemaMap) validateList( switch t := schema.Elem.(type) { case *Resource: // This is a sub-resource - diags = append(diags, m.validateObject(key, t.Schema, c, p)...) + diags = append(diags, m.validateObject(key, t.SchemaMap(), c, p)...) case *Schema: diags = append(diags, m.validateType(key, raw, t, c, p)...) } diff --git a/helper/schema/serialize.go b/helper/schema/serialize.go index 459b61314e..d629240fd3 100644 --- a/helper/schema/serialize.go +++ b/helper/schema/serialize.go @@ -91,7 +91,7 @@ func SerializeResourceForHash(buf *bytes.Buffer, val interface{}, resource *Reso if val == nil { return } - sm := resource.Schema + sm := resource.SchemaMap() m := val.(map[string]interface{}) var keys []string allComputed := true diff --git a/helper/schema/shims.go b/helper/schema/shims.go index d6398d59cc..e8baebd70c 100644 --- a/helper/schema/shims.go +++ b/helper/schema/shims.go @@ -43,7 +43,7 @@ func diffFromValues(ctx context.Context, prior, planned, config cty.Value, res * removeConfigUnknowns(cfg.Config) removeConfigUnknowns(cfg.Raw) - diff, err := schemaMap(res.Schema).Diff(ctx, instanceState, cfg, cust, nil, false) + diff, err := schemaMap(res.SchemaMap()).Diff(ctx, instanceState, cfg, cust, nil, false) if err != nil { return nil, err }