Add Plugin SDK to Plugin Framework migration tests for meta data sources
#27378
Comments
ewbankkit
changed the title
meta data sources
Community NoteVoting for Prioritization
Volunteering to Work on This Issue
|
ewbankkit
added
service/meta
tests
|
Some thoughts:
|
|
From internal Slack thread: Kit Ewbank Oct 24th at 2:17 PM Good afternoon TF DevEx 👋 if tags != nil {
if err := d.Set("tags", tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig).Map()); err != nil {
return fmt.Errorf("error setting tags: %w", err)
}
} else {
d.Set("tags", nil)
}to (v4.36.0) if tags != nil {
data.Tags = flex.FlattenFrameworkStringValueMap(ctx, tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig).Map())
} else {
data.Tags = types.Map{ElemType: types.StringType, Null: true}
}The issue was that Brian Flad 1 day ago
I will look into this right now; wouldn’t surprise me if the shims treated the null attribute the same as an empty map :eyeroll: func testCheckResourceAttr(is *terraform.InstanceState, name string, key string, value string) error {
v, ok := is.Attributes[key]
if !ok {
// Empty containers may be elided from the state.
// If the intent here is to check for an empty container, allow the key to
// also be non-existent.
if value == "0" && (strings.HasSuffix(key, ".#") || strings.HasSuffix(key, ".%")) {
return nil
}hashicorp/terraform-plugin-sdk@57c8f96 was the origin for that, which alludes to my suspicion there So, we essentially get to decide whether to frustrate the ecosystem by making a breaking change or make it opt-in and difficult to discover when it might not do what you’d expect. I’m not sure we want to get into the business of “well if the provider is installed via ProtoV5ProviderFactories/ProtoV6ProviderFactories (probably framework or go based, but not guaranteed) do a different behavior” but that’s another option if we squint really hard. :shifty_eyed_dog:
Relating to the second option there about a new TestCheckFunc, we could introduce something quite specific, e.g. I went through the existing sdk issues and didn’t see anything relating to this, so I guess a good first step is creating one if we want to do anything here. I could probably create and test that TestCheckFunc fairly quickly since I’m familiar with how the other ones are unit tested. I’m quite hesitant to suggest changing the existing TestCheckResourceAttr behavior because these code searches yield lots of potential issues Created hashicorp/terraform-plugin-sdk#1085
This is to be expected. Data sources don’t really have a plan per-se, only a refreshed state. Terraform used to (maybe it still does in some cases) compare a prior data source state versus current data source state, but data source schemas are not versioned so that comparison/plan is now removed in most cases to prevent errors that providers cannot avoid. func TestAccDefaultTagsDataSource_EmptyMapCheck(t *testing.T) {
t.Parallel()
var tags1, tags2 map[string]string
Test(t, TestCase{
Steps: []TestStep{
{
ExternalProviders: map[string]ExternalProvider{
"aws": {
Source: "registry.terraform.io/hashicorp/aws",
VersionConstraint: "4.35.0",
},
},
Config: `data "aws_default_tags" "test" {}`,
Check: ComposeAggregateTestCheckFunc(
testExtractResourceAttrMap("data.aws_default_tags.test", "tags", &tags1),
),
},
{
ExternalProviders: map[string]ExternalProvider{
"aws": {
Source: "registry.terraform.io/hashicorp/aws",
VersionConstraint: "4.36.0",
},
},
Config: `data "aws_default_tags" "test" {}`,
Check: ComposeAggregateTestCheckFunc(
testExtractResourceAttrMap("data.aws_default_tags.test", "tags", &tags2),
func(s *terraform.State) error {
if diff := cmp.Diff(tags1, tags2); diff != "" {
return fmt.Errorf("tags attribute value difference: %s", diff)
}
return nil
},
),
},
},
})
}
func testExtractResourceAttrMap(resourceName string, attributeName string, attributeValue *map[string]string) TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[resourceName]
if !ok {
return fmt.Errorf("resource name %s not found in state", resourceName)
}
if _, ok := rs.Primary.Attributes[attributeName]; ok {
return fmt.Errorf("attribute name %s found, but is not a map attribute in state", attributeName)
}
// Everything in a map will be in the flatmap with the attribute name,
// a period, then either the size sigil or a string key.
attributeValuePathPrefix := attributeName + "."
attributePathSize := attributeValuePathPrefix + "%"
sizeStr, ok := rs.Primary.Attributes[attributePathSize]
if !ok {
// attribute not found
*attributeValue = nil
return nil
}
size, err := strconv.Atoi(sizeStr)
if err != nil {
return fmt.Errorf("unable to convert map size %s to integer: %s", sizeStr, err)
}
m := make(map[string]string, size)
for attributePath, attributePathValue := range rs.Primary.Attributes {
// Ignore
if !strings.HasPrefix(attributePath, attributeValuePathPrefix) {
continue
}
if attributePath == attributePathSize {
continue
}
key := strings.TrimPrefix(attributePath, attributeValuePathPrefix)
m[key] = attributePathValue
}
*attributeValue = m
return nil
}
}sorry that’s written in from the sdk helper/resource package, but that’s the best thing I can figure there func TestAccDefaultTagsDataSource_EmptyMapCheck(t *testing.T) {
t.Parallel()
Test(t, TestCase{
Steps: []TestStep{
{
ExternalProviders: map[string]ExternalProvider{
"aws": {
Source: "registry.terraform.io/hashicorp/aws",
VersionConstraint: "4.35.0",
},
},
Config: `data "aws_default_tags" "test" {}`,
Check: ComposeAggregateTestCheckFunc(
TestCheckResourceAttrSet("data.aws_default_tags.test", "tags.%"),
),
},
{
ExternalProviders: map[string]ExternalProvider{
"aws": {
Source: "registry.terraform.io/hashicorp/aws",
VersionConstraint: "4.36.0",
},
},
Config: `data "aws_default_tags" "test" {}`,
Check: ComposeAggregateTestCheckFunc(
TestCheckResourceAttrSet("data.aws_default_tags.test", "tags.%"),
),
},
},
})
}😄 |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Add Plugin SDK to Plugin Framework migration acceptance tests for the
metadata sources.Relates
nullvs. empty map #27377In particular test for "zero" values of "complex" attributes (lists, sets, maps, nested blocks).
The text was updated successfully, but these errors were encountered: