Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImportResourceState Errantly Includes timeouts in State #1145

Closed
bflad opened this issue Feb 13, 2023 · 1 comment · Fixed by #1146
Closed

ImportResourceState Errantly Includes timeouts in State #1145

bflad opened this issue Feb 13, 2023 · 1 comment · Fixed by #1146
Assignees
Labels
bug Something isn't working
Milestone

Comments

@bflad
Copy link
Contributor

bflad commented Feb 13, 2023

SDK version

2.24.0 / Terraform 1.3.8 specifically

Relevant provider source code

&schema.Resource{
	// ...
	Importer: &schema.ResourceImporter{
		StateContext: schema.ImportStatePassthroughContext,
	},
}

Or potentially any resource with:

  • A non-refreshable attribute (e.g. "virtual" or "Terraform only" attribute)
  • Import support
  • No special import handling for that attribute

Terraform Configuration Files

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "4.54.0"
    }
  }
  required_version = "1.3.8"
}

provider "aws" {
  region = "us-west-2"
}

resource "aws_security_group" "test" {
  name = "tf-acc-test-6316391654197628380-0"

  tags = {
    Name = "tf-acc-test-6316391654197628380"
  }
}

Expected Behavior

After importing a resource, Terraform plans should never contain anything relating to the SDK-defined timeouts block.

Actual Behavior

$ terraform plan -refresh=false                                                                  

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # aws_security_group.test will be updated in-place
  ~ resource "aws_security_group" "test" {
        id                     = "sg-0329f3c7cf561ad15"
        name                   = "tf-acc-test-6316391654197628380-0"
      + revoke_rules_on_delete = false
        tags                   = {
            "Name" = "tf-acc-test-6316391654197628380"
        }
        # (7 unchanged attributes hidden)

      - timeouts {}
    }

Plan: 0 to add, 1 to change, 0 to destroy.

The imported state contains a timeouts single nested block with create and delete set to null.

$ ❯ terraform show -json | jq '.values.root_module.resources[] | select(.address == "aws_security_group.test") | .values'
{
  "arn": "arn:aws:ec2:us-west-2:--OMITTED--:security-group/sg-0329f3c7cf561ad15",
  "description": "Managed by Terraform",
  "egress": [],
  "id": "sg-0329f3c7cf561ad15",
  "ingress": [],
  "name": "tf-acc-test-6316391654197628380-0",
  "name_prefix": "",
  "owner_id": "--OMITTED--",
  "revoke_rules_on_delete": null,
  "tags": {
    "Name": "tf-acc-test-6316391654197628380"
  },
  "tags_all": {
    "Name": "tf-acc-test-6316391654197628380"
  },
  "timeouts": {
    "create": null,
    "delete": null
  },
  "vpc_id": "vpc-0c4883cac90b9bab6"
}

Steps to Reproduce

For aws_security_group:

  • An AWS environment with an EC2 Security Group accessible for import
  • terraform import aws_security_group.test sg-0329f3c7cf561ad15
  • terraform plan or terraform plan -refresh=false

References

@bflad bflad added bug Something isn't working tf-devex-triage labels Feb 13, 2023
bflad added a commit that referenced this issue Feb 13, 2023
Reference: #1145
Reference: hashicorp/terraform#32463

Terraform 1.3.8 and later now correctly handles null values for single nested blocks. This means Terraform will now report differences between a null block and known block with null values. This SDK only supported single nested blocks via its timeouts functionality.

This change is a very targeted removal of any potential `timeouts` block values in a resource state from the `ImportResourceState` RPC. Since configuration is not available during that RPC, it is never valid to return any data beyond null for that block. This will prevent unexpected differences on the first plan after import, where Terraform will report the block removal for configurations which do not contain the block.

New unit test failure prior to code updates:

```
--- FAIL: TestImportResourceState_Timeouts_Removed (0.00s)
    /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider_test.go:1159: unexpected difference:   cty.Value(
        - 	{
        - 		ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{...}}},
        - 		v:  map[string]any{"id": string("test"), "string_attribute": nil, "timeouts": nil},
        - 	},
        + 	{
        + 		ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{...}}},
        + 		v: map[string]any{
        + 			"id":               string("test"),
        + 			"string_attribute": nil,
        + 			"timeouts":         map[string]any{"create": nil, "read": nil},
        + 		},
        + 	},
          )
```
@bflad bflad added this to the v2.25.0 milestone Feb 13, 2023
@bflad bflad self-assigned this Feb 13, 2023
bflad added a commit that referenced this issue Feb 14, 2023
Reference: #1145
Reference: hashicorp/terraform#32463

Terraform 1.3.8 and later now correctly handles null values for single nested blocks. This means Terraform will now report differences between a null block and known block with null values. This SDK only supported single nested blocks via its timeouts functionality.

This change is a very targeted removal of any potential `timeouts` block values in a resource state from the `ImportResourceState` RPC. Since configuration is not available during that RPC, it is never valid to return any data beyond null for that block. This will prevent unexpected differences on the first plan after import, where Terraform will report the block removal for configurations which do not contain the block.

New unit test failure prior to code updates:

```
--- FAIL: TestImportResourceState_Timeouts_Removed (0.00s)
    /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider_test.go:1159: unexpected difference:   cty.Value(
        - 	{
        - 		ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{...}}},
        - 		v:  map[string]any{"id": string("test"), "string_attribute": nil, "timeouts": nil},
        - 	},
        + 	{
        + 		ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{...}}},
        + 		v: map[string]any{
        + 			"id":               string("test"),
        + 			"string_attribute": nil,
        + 			"timeouts":         map[string]any{"create": nil, "read": nil},
        + 		},
        + 	},
          )
```
@github-actions
Copy link

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.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant