hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.31k stars 9.49k forks source link

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

Closed EugenKon closed 1 week ago

EugenKon commented 1 month ago

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

jbardin commented 1 month ago

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!

EugenKon commented 1 month ago

@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

jbardin commented 1 month ago

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 commented 1 month ago

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"
  }
}
EugenKon commented 1 month ago

@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.

jbardin commented 1 month ago

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.

EugenKon commented 1 month ago

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

No deprecation warning for -state at the moment.

jbardin commented 1 month ago

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.

melsonic commented 3 weeks ago

Hi @jbardin, can I work on this one?

crw commented 3 weeks ago

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

melsonic commented 3 weeks ago

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

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

crw commented 3 weeks ago

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

melsonic commented 3 weeks ago

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

sure, thanks @crw

liamcervante commented 2 weeks ago

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.

melsonic commented 2 weeks ago

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