terraform icon indicating copy to clipboard operation
terraform copied to clipboard

show deprecation warning if `-state` is used in conjunction with `plan` or `apply`

Open EugenKon opened this issue 1 year ago • 15 comments

Terraform Version

$ terraform --version
Terraform v1.8.5
on darwin_amd64
+ provider registry.terraform.io/hashicorp/aws v5.62.0
+ provider registry.terraform.io/hashicorp/external v2.3.3
+ provider registry.terraform.io/hashicorp/local v2.5.1
+ provider registry.terraform.io/hashicorp/null v3.2.2
+ provider registry.terraform.io/hashicorp/random v3.6.2
+ provider registry.terraform.io/hashicorp/tls v4.0.5


### Terraform Configuration Files

```terraform
terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "~> 5.0"
    }

    local = {
      source  = "hashicorp/local"
      version = "~> 2.5"
    }

    # Needed to upgrade old Portal projects
    null = {
      source  = "hashicorp/null"
      version = "~> 3.2"
    }
  }
}

provider "aws" {
  region = local.aws_region
  default_tags {
    tags = {
      Project = local.project_name
    }
  }
}
# Somewhere in other parts of a cluster configuration
resource "random_uuid" "consul_token" {
}
resource "tls_private_key" "pk" {
  algorithm = "ED25519"
}

Debug Output

│ Error: failed to read schema for module.private-cloud.random_uuid.consul_token in registry.terraform.io/hashicorp/random: failed to instantiate provider "registry.terraform.io/hashicorp/random" to obtain schema: unavailable provider "registry.terraform.io/hashicorp/random"
│ Error: failed to read provider configuration schema for registry.terraform.io/hashicorp/tls: failed to instantiate provider "registry.terraform.io/hashicorp/tls" to obtain schema: unavailable provider "registry.terraform.io/hashicorp/tls"

Expected Behavior

Terraform should load providers automatically for resources to delete them as it done for the same resources to create them.

Actual Behavior

I need to configure providers explicitly to delete resources not in configuration but in a state.

Steps to Reproduce

  1. We had a configuration without 'tls_private_key' and 'random_uuid' resources and without 'tls' and 'random' providers.
  2. We added into a configuration 'tls_private_key' and 'random_uuid' resources. Still a configuration does not have 'tls' and 'random' providers.
  3. terraform plan/apply works fine
  4. Then we decided to rollback cluster to the old configuration without 'tls_private_key' and 'random_uuid' resources and still without 'tls' and 'random' providers.
  5. terraform plan/apply failed with the errors above

I assume, that terraform should connect tls and random providers automatically for resource delation as it was done to create these resources. For the first case, when resources are created, this is done from the information in a configuration. For the second case, when resources should be deleted, this could be done from the information in a terrafomr.tfstate file.

Additional Context

Because for the first case terraform core loaded tls and random providers automatically, I decided that the same should be done by terraform core for the second case. Thus this does not belongs to 'terraform-provider-tls' and/or 'terraform-provider-random'.

References

No response

EugenKon avatar Aug 14 '24 02:08 EugenKon

Hi @EugenKon,

That is definitely not the typical behavior here, and I'm not sure how you've managed to get resources which are not causing Terraform to initialize a provider. Even if you had configured providers within the module then removed them, which would cause an error because the provider config is not persisted, it would be a different error from this.

Can you show the output of terraform providers? That may give us some more information about the providers required by the configuration and state.

Thanks!

jbardin avatar Aug 14 '24 14:08 jbardin

@jbardin Yes, sure.

This is how it looks on upgraded cluster:

$ terraform -chdir=derived-src/aws/ providers

Providers required by configuration:
.
├── provider[registry.terraform.io/hashicorp/external]
├── provider[registry.terraform.io/hashicorp/aws] ~> 5.0
├── provider[registry.terraform.io/hashicorp/local] ~> 2.5
├── provider[registry.terraform.io/hashicorp/null] ~> 3.2
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   ├── provider[registry.terraform.io/hashicorp/local]
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   ├── provider[registry.terraform.io/hashicorp/aws]
│   ├── provider[registry.terraform.io/hashicorp/random]
│   ├── provider[registry.terraform.io/hashicorp/tls]
│   └── provider[registry.terraform.io/hashicorp/local]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
└── module.XXX
    └── provider[registry.terraform.io/hashicorp/aws]

This is before downgrade (a cluster with configuration without tls_private_key and random_uuid resources:

terraform -chdir=derived-src/aws/ providers

Providers required by configuration:
.
├── provider[registry.terraform.io/hashicorp/aws] ~> 5.0
├── provider[registry.terraform.io/hashicorp/local] ~> 2.5
├── provider[registry.terraform.io/hashicorp/null] ~> 3.2
├── provider[registry.terraform.io/hashicorp/external]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   ├── provider[registry.terraform.io/hashicorp/aws]
│   └── provider[registry.terraform.io/hashicorp/local]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
└── module.XXX
    └── provider[registry.terraform.io/hashicorp/aws]

Plan

terraform -chdir=derived-src/aws/  plan -state ../../state.d/terraform.tfstate -out ../../state.d/terraform.plan

╷
│ Error: failed to read schema for module.XXX.tls_private_key.pk in registry.terraform.io/hashicorp/tls: failed to instantiate provider "registry.terraform.io/hashicorp/tls" to obtain schema: unavailable provider "registry.terraform.io/hashicorp/tls"
│
│

After adding required providers into providers.tf

    tls = {
      source  = "hashicorp/tls"
      version = "~> 4.0"
    }

    random = {
      source  = "hashicorp/random"
      version = "~> 3.0"  
    }
$ terraform -chdir=derived-src/aws/ init
$ terraform -chdir=derived-src/aws/ providers

Providers required by configuration:
.
├── provider[registry.terraform.io/hashicorp/aws] ~> 5.0
├── provider[registry.terraform.io/hashicorp/local] ~> 2.5
├── provider[registry.terraform.io/hashicorp/null] ~> 3.2
├── provider[registry.terraform.io/hashicorp/external]
├── provider[registry.terraform.io/hashicorp/tls] ~> 4.0
├── provider[registry.terraform.io/hashicorp/random] ~> 3.0
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   ├── provider[registry.terraform.io/hashicorp/aws]
│   └── provider[registry.terraform.io/hashicorp/local]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
└── module.XXX
    └── provider[registry.terraform.io/hashicorp/aws]

I ran plan again:

  # module.XXX.random_uuid.consul_token will be destroyed
  # (because random_uuid.consul_token is not in configuration)
  - resource "random_uuid" "consul_token" {
      - id     = "YYY-7b19-ce4f-XXX" -> null
      - result = "YYY-7b19-ce4f-XXX" -> null
    }
...
# module.XXX.tls_private_key.pk will be destroyed
# (because tls_private_key.pk is not in configuration)
- resource "tls_private_key" "pk" {
...
Plan: 1 to add, 5 to change, 12 to destroy.

After plan was applied here is output for already the downgraded cluster (I mean the cluster without tls and random resources at the configuration):

$ terraform -chdir=derived-src/aws/ providers

Providers required by configuration:
.
├── provider[registry.terraform.io/hashicorp/aws] ~> 5.0
├── provider[registry.terraform.io/hashicorp/local] ~> 2.5
├── provider[registry.terraform.io/hashicorp/null] ~> 3.2
├── provider[registry.terraform.io/hashicorp/tls] ~> 4.0
├── provider[registry.terraform.io/hashicorp/external]
├── provider[registry.terraform.io/hashicorp/random] ~> 3.0
├── module.XXX
│   ├── provider[registry.terraform.io/hashicorp/aws]
│   └── provider[registry.terraform.io/hashicorp/local]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
├── module.XXX
│   └── provider[registry.terraform.io/hashicorp/aws]
└── module.XXX
    └── provider[registry.terraform.io/hashicorp/aws]

*PS. By the way, could it be possible to hide generated secret for "random_uuid" "consul_token" from the output?

  - resource "random_uuid" "consul_token" {
      - id     = "YYY-7b19-ce4f-XXX" -> null
      - result = "YYY-7b19-ce4f-XXX" -> null
    }

I can not find anything related to this here: https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/uuid

EugenKon avatar Aug 14 '24 15:08 EugenKon

Thanks for all the extra info @EugenKon! I'm guessing that it has something to do with the -state flag, since that is pretty rarely used. I'll try and see if I can come up with a reproduction around that feature.

(Also, there's unfortunately not any way to hide normal resource plan data from the output right now, unless that provider declares the attributes as sensitive in the resource schema)

jbardin avatar Aug 14 '24 16:08 jbardin

So I've managed to get a slightly different error, but I still think this is caused by the legacy -state flag. If you are trying to run init again to fetch the providers for the root module, the init command doesn't know about the -state flag and can't tell what providers are only required by the state. This also applies to the providers command, which isn't going to see any existing state.

Does terraform work correctly if you configure the local backend to point to the state file? I think it would be something like:

terraform {
  backend "local" {
    path = "../../../../state.d/terraform.tfstate"
  }
}

jbardin avatar Aug 14 '24 17:08 jbardin

@jbardin The give configuration resolves the problem also. It works as expected: no error messages about 'tls' and 'random' providers. I hope my feedback helped you.

Thanks.

EugenKon avatar Aug 14 '24 20:08 EugenKon

Thanks @EugenKon. Since the -state flag was deprecated years ago, and will cause Terraform to initialize incorrectly in cases like you have (which is why it's deprecated), I'm going to close this as terraform is working correctly given the situation.

jbardin avatar Aug 14 '24 20:08 jbardin

@jbardin It would be nice to warn about -state deprecation like this: image

No deprecation warning for -state at the moment.

EugenKon avatar Aug 14 '24 20:08 EugenKon

That's a good point, it is deprecated in the documentation, but perhaps we could add a message if it's used in conjunction with plan or apply.

jbardin avatar Aug 14 '24 20:08 jbardin

Hi @jbardin, can I work on this one?

melsonic avatar Aug 19 '24 20:08 melsonic

Hi @melsonic, please read https://github.com/hashicorp/terraform/blob/main/.github/CONTRIBUTING.md. Particularly the "Proposing a Change" section. Thanks!

crw avatar Aug 20 '24 20:08 crw

Hi @melsonic, please read https://github.com/hashicorp/terraform/blob/main/.github/CONTRIBUTING.md. Particularly the "Proposing a Change" section. Thanks!

Hi @crw, I have been through the codebase and found the following information

  • It is using Cobra for CLI interactions.
  • In the plan command, for example, a State field is inside plan. We can check if the -state is used in conjunction with plan by comparing the value that the State pointer points to (after parsing the args) with a zero value composite literal of type State.

Let me know if I missed some point, or I can proceed with the implementation.

melsonic avatar Aug 22 '24 17:08 melsonic

Thanks for the proposal, I will raise it in triage!

crw avatar Aug 22 '24 18:08 crw

Thanks for the proposal, I will raise it in triage!

sure, thanks @crw

melsonic avatar Aug 22 '24 18:08 melsonic

Hi @melsonic, I'm happy to review a PR for this if you raise one 👍

The -state flag is enabled through the extendedFlagSet function: https://github.com/hashicorp/terraform/blob/main/internal/command/arguments/extended.go.

It is actually only included in this way through plan.go, apply.go, and refresh.go which luckily are all the commands we want to add the warning for.

My approach would be to update the extendedFlagSet function so that it returns diagnostics alongside the initialised flags, and these diagnostics could include a warning about the state flag being deprecated.

Alternatively, we could avoid editing the shared function, and update the call sites so they check if the State field is not empty and add a warning in this case. These functions are all already configured to return diagnostics so there'd be no change externally if we went with this approach, but we'd be adding the warning in three places instead of a single one.

Hopefully that makes sense, and I'll leave it up to you if you want to take either of those approaches.

liamcervante avatar Aug 27 '24 16:08 liamcervante

Hey @liamcervante , thanks for your input. I will start working on this and submit a PR.

melsonic avatar Aug 28 '24 11:08 melsonic

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[bot] avatar Oct 06 '24 02:10 github-actions[bot]