From 0ba649155011b1f9b9993315ecfe9770311681fd Mon Sep 17 00:00:00 2001 From: temp Date: Tue, 9 Aug 2022 11:10:34 -0400 Subject: [PATCH 1/2] no-op commit due to failed cherry-picking From 8f28b6923df603522bbeaa0789d6c5057afab3da Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 4 Aug 2022 16:50:09 -0400 Subject: [PATCH 2/2] validate deprecated attrs from static traversals We can't validate that data from deprecated nested attributes is used in the configuration, but we can at least catch the simple case where a deprecated attribute is referenced directly. --- .../configschema/validate_traversal.go | 30 ++++++-- .../configschema/validate_traversal_test.go | 68 +++++++++++++++++-- 2 files changed, 88 insertions(+), 10 deletions(-) diff --git a/internal/configs/configschema/validate_traversal.go b/internal/configs/configschema/validate_traversal.go index f8a9efe12bec..8320c9de3663 100644 --- a/internal/configs/configschema/validate_traversal.go +++ b/internal/configs/configschema/validate_traversal.go @@ -74,14 +74,34 @@ func (b *Block) StaticValidateTraversal(traversal hcl.Traversal) tfdiags.Diagnos } if attrS, exists := b.Attributes[name]; exists { + // Check for Deprecated status of this attribute. + // We currently can't provide the user with any useful guidance because + // the deprecation string is not part of the schema, but we can at + // least warn them. + // + // This purposely does not attempt to recurse into nested attribute + // types. Because nested attribute values are often not accessed via a + // direct traversal to the leaf attributes, we cannot reliably detect + // if a nested, deprecated attribute value is actually used from the + // traversal alone. More precise detection of deprecated attributes + // would require adding metadata like marks to the cty value itself, to + // be caught during evaluation. + if attrS.Deprecated { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: `Deprecated attribute`, + Detail: fmt.Sprintf(`The attribute %q is deprecated. Refer to the provider documentation for details.`, name), + Subject: next.SourceRange().Ptr(), + }) + } + // For attribute validation we will just apply the rest of the // traversal to an unknown value of the attribute type and pass - // through HCL's own errors, since we don't want to replicate all of - // HCL's type checking rules here. - val := cty.UnknownVal(attrS.Type) + // through HCL's own errors, since we don't want to replicate all + // of HCL's type checking rules here. + val := cty.UnknownVal(attrS.ImpliedType()) _, hclDiags := after.TraverseRel(val) - diags = diags.Append(hclDiags) - return diags + return diags.Append(hclDiags) } if blockS, exists := b.BlockTypes[name]; exists { diff --git a/internal/configs/configschema/validate_traversal_test.go b/internal/configs/configschema/validate_traversal_test.go index 2000d2e756e6..f39914881191 100644 --- a/internal/configs/configschema/validate_traversal_test.go +++ b/internal/configs/configschema/validate_traversal_test.go @@ -10,9 +10,46 @@ import ( func TestStaticValidateTraversal(t *testing.T) { attrs := map[string]*Attribute{ - "str": {Type: cty.String, Optional: true}, - "list": {Type: cty.List(cty.String), Optional: true}, - "dyn": {Type: cty.DynamicPseudoType, Optional: true}, + "str": {Type: cty.String, Optional: true}, + "list": {Type: cty.List(cty.String), Optional: true}, + "dyn": {Type: cty.DynamicPseudoType, Optional: true}, + "deprecated": {Type: cty.String, Computed: true, Deprecated: true}, + "nested_single": { + Optional: true, + NestedType: &Object{ + Nesting: NestingSingle, + Attributes: map[string]*Attribute{ + "optional": {Type: cty.String, Optional: true}, + }, + }, + }, + "nested_list": { + Optional: true, + NestedType: &Object{ + Nesting: NestingList, + Attributes: map[string]*Attribute{ + "optional": {Type: cty.String, Optional: true}, + }, + }, + }, + "nested_set": { + Optional: true, + NestedType: &Object{ + Nesting: NestingSet, + Attributes: map[string]*Attribute{ + "optional": {Type: cty.String, Optional: true}, + }, + }, + }, + "nested_map": { + Optional: true, + NestedType: &Object{ + Nesting: NestingMap, + Attributes: map[string]*Attribute{ + "optional": {Type: cty.String, Optional: true}, + }, + }, + }, } schema := &Block{ Attributes: attrs, @@ -168,6 +205,26 @@ func TestStaticValidateTraversal(t *testing.T) { `obj.map_block.anything.nonexist`, `Unsupported attribute: This object has no argument, nested block, or exported attribute named "nonexist".`, }, + { + `obj.nested_single.optional`, + ``, + }, + { + `obj.nested_list[0].optional`, + ``, + }, + { + `obj.nested_set[0].optional`, + `Invalid index: Elements of a set are identified only by their value and don't have any separate index or key to select with, so it's only possible to perform operations across all elements of the set.`, + }, + { + `obj.nested_map["key"].optional`, + ``, + }, + { + `obj.deprecated`, + `Deprecated attribute: The attribute "deprecated" is deprecated. Refer to the provider documentation for details.`, + }, } for _, test := range tests { @@ -187,8 +244,9 @@ func TestStaticValidateTraversal(t *testing.T) { t.Errorf("unexpected error: %s", diags.Err().Error()) } } else { - if diags.HasErrors() { - if got := diags.Err().Error(); got != test.WantError { + err := diags.ErrWithWarnings() + if err != nil { + if got := err.Error(); got != test.WantError { t.Errorf("wrong error\ngot: %s\nwant: %s", got, test.WantError) } } else {