Skip to content

Commit

Permalink
Remove Removed and prune related code
Browse files Browse the repository at this point in the history
  • Loading branch information
paultyng committed Apr 28, 2020
1 parent ac1c092 commit f19078c
Show file tree
Hide file tree
Showing 15 changed files with 3 additions and 682 deletions.
40 changes: 0 additions & 40 deletions helper/resource/testing_new_import_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import (

"github.com/hashicorp/terraform-plugin-sdk/v2/acctest"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource/testing"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/internal/addrs"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
)

Expand Down Expand Up @@ -94,23 +92,6 @@ func testStepNewImportState(t testing.T, c TestCase, wd *tftest.WorkingDir, step
r.Primary.ID)
}

// We'll try our best to find the schema for this resource type
// so we can ignore Removed fields during validation. If we fail
// to find the schema then we won't ignore them and so the test
// will need to rely on explicit ImportStateVerifyIgnore, though
// this shouldn't happen in any reasonable case.
// KEM CHANGE FROM OLD FRAMEWORK: Fail test if this happens.
var rsrcSchema *schema.Resource
providerAddr, diags := addrs.ParseAbsProviderConfigStr("provider." + r.Provider + "." + r.Type)
if diags.HasErrors() {
t.Fatalf("Failed to find schema for resource with ID %s", r.Primary)
}

providerType := providerAddr.ProviderConfig.Type
if provider, ok := step.providers[providerType]; ok {
rsrcSchema = provider.ResourcesMap[r.Type]
}

// don't add empty flatmapped containers, so we can more easily
// compare the attributes
skipEmpty := func(k, v string) bool {
Expand Down Expand Up @@ -153,27 +134,6 @@ func testStepNewImportState(t testing.T, c TestCase, wd *tftest.WorkingDir, step
}
}

// Also remove any attributes that are marked as "Removed" in the
// schema, if we have a schema to check that against.
if rsrcSchema != nil {
for k := range actual {
for _, schema := range rsrcSchema.SchemasForFlatmapPath(k) {
if schema.Removed != "" {
delete(actual, k)
break
}
}
}
for k := range expected {
for _, schema := range rsrcSchema.SchemasForFlatmapPath(k) {
if schema.Removed != "" {
delete(expected, k)
break
}
}
}
}

if !reflect.DeepEqual(actual, expected) {
// Determine only the different attributes
for k, v := range expected {
Expand Down
8 changes: 0 additions & 8 deletions helper/schema/field_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package schema
import (
"fmt"
"strconv"
"strings"
)

// FieldReaders are responsible for decoding fields out of data into
Expand Down Expand Up @@ -42,13 +41,6 @@ func (r *FieldReadResult) ValueOrZero(s *Schema) interface{} {
return s.ZeroValue()
}

// SchemasForFlatmapPath tries its best to find a sequence of schemas that
// the given dot-delimited attribute path traverses through.
func SchemasForFlatmapPath(path string, schemaMap map[string]*Schema) []*Schema {
parts := strings.Split(path, ".")
return addrToSchema(parts, schemaMap)
}

// addrToSchema finds the final element schema for the given address
// and the given schema. It returns all the schemas that led to the final
// schema. These are in order of the address (out to in).
Expand Down
9 changes: 1 addition & 8 deletions helper/schema/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ func isReservedDataSourceFieldName(name string) bool {
func isReservedResourceFieldName(name string, s *Schema) bool {
// Allow phasing out "id"
// See https://github.com/terraform-providers/terraform-provider-aws/pull/1626#issuecomment-328881415
if name == "id" && (s.Deprecated != "" || s.Removed != "") {
if name == "id" && s.Deprecated != "" {
return false
}

Expand Down Expand Up @@ -793,13 +793,6 @@ func (r *Resource) TestResourceData() *ResourceData {
}
}

// SchemasForFlatmapPath tries its best to find a sequence of schemas that
// the given dot-delimited attribute path traverses through in the schema
// of the receiving Resource.
func (r *Resource) SchemasForFlatmapPath(path string) []*Schema {
return SchemasForFlatmapPath(path, r.Schema)
}

// Returns true if the resource is "top level" i.e. not a sub-resource.
func (r *Resource) isTopLevel() bool {
// TODO: This is a heuristic; replace with a definitive attribute?
Expand Down
19 changes: 1 addition & 18 deletions helper/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,14 +208,6 @@ type Schema struct {
// how to address the deprecation.
Deprecated string

// When Removed is set, this attribute has been removed from the schema
//
// Removed attributes can be left in the Schema to generate informative error
// messages for the user when they show up in resource configurations.
// This string is the message shown to the user with instructions on
// what do to about the removed attribute.
Removed string

// ValidateFunc allows individual fields to define arbitrary validation
// logic. It is yielded the provided config value as an interface{} that is
// guaranteed to be of the proper Schema type, and it can yield warnings or
Expand Down Expand Up @@ -881,7 +873,7 @@ func (m schemaMap) internalValidate(topSchemaMap schemaMap, attrsOnly bool) erro
return fmt.Errorf("%s: ValidateFunc and ValidateDiagFunc cannot both be set", k)
}

if v.Deprecated == "" && v.Removed == "" {
if v.Deprecated == "" {
if !isValidFieldName(k) {
return fmt.Errorf("%s: Field name may only contain lowercase alphanumeric characters & underscores.", k)
}
Expand Down Expand Up @@ -2097,15 +2089,6 @@ func (m schemaMap) validateType(
})
}

if schema.Removed != "" {
diags = append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "Removed Attribute",
Detail: schema.Removed,
AttributePath: path,
})
}

return diags
}

Expand Down
42 changes: 0 additions & 42 deletions helper/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3541,17 +3541,6 @@ func TestSchemaMap_InternalValidate(t *testing.T) {
false,
},

"invalid field name format of a Removed field": {
map[string]*Schema{
"WithCapitals": {
Type: TypeString,
Optional: true,
Removed: "Use with_underscores instead",
},
},
false,
},

"ConfigModeBlock with Elem *Resource": {
map[string]*Schema{
"block": {
Expand Down Expand Up @@ -4619,37 +4608,6 @@ func TestSchemaMap_Validate(t *testing.T) {
Warnings: nil,
},

"Removed attribute usage generates error": {
Schema: map[string]*Schema{
"long_gone": {
Type: TypeString,
Optional: true,
Removed: "no longer supported by Cloud API",
},
},

Config: map[string]interface{}{
"long_gone": "still here!",
},

Err: true,
Errors: []error{
fmt.Errorf("Error: Removed Attribute: no longer supported by Cloud API"),
},
},

"Removed generates no errors if attr not used": {
Schema: map[string]*Schema{
"long_gone": {
Type: TypeString,
Optional: true,
Removed: "no longer supported by Cloud API",
},
},

Err: false,
},

"Conflicting attributes generate error": {
Schema: map[string]*Schema{
"whitelist": {
Expand Down
41 changes: 0 additions & 41 deletions internal/addrs/module_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ import (
// creation.
type ModuleInstance []ModuleInstanceStep

var (
_ Targetable = ModuleInstance(nil)
)

func parseModuleInstance(traversal hcl.Traversal) (ModuleInstance, tfdiags.Diagnostics) {
mi, remain, diags := parseModuleInstancePrefix(traversal)
if len(remain) != 0 {
Expand Down Expand Up @@ -243,40 +239,3 @@ func (m ModuleInstance) String() string {
}
return buf.String()
}

// TargetContains implements Targetable by returning true if the given other
// address either matches the receiver, is a sub-module-instance of the
// receiver, or is a targetable absolute address within a module that
// is contained within the reciever.
func (m ModuleInstance) TargetContains(other Targetable) bool {
switch to := other.(type) {

case ModuleInstance:
if len(to) < len(m) {
// Can't be contained if the path is shorter
return false
}
// Other is contained if its steps match for the length of our own path.
for i, ourStep := range m {
otherStep := to[i]
if ourStep != otherStep {
return false
}
}
// If we fall out here then the prefixed matched, so it's contained.
return true

case absResource:
return m.TargetContains(to.Module)

case absResourceInstance:
return m.TargetContains(to.Module)

default:
return false
}
}

func (m ModuleInstance) targetableSigil() {
// ModuleInstance is targetable
}
141 changes: 0 additions & 141 deletions internal/addrs/provider_config.go

This file was deleted.

Loading

0 comments on commit f19078c

Please sign in to comment.