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

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

Closed
EugenKon opened this issue Aug 14, 2024 · 16 comments · Fixed by #35660
Closed

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

EugenKon opened this issue Aug 14, 2024 · 16 comments · Fixed by #35660

Comments

@EugenKon
Copy link

EugenKon commented Aug 14, 2024

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 EugenKon added bug new new issue not yet triaged labels Aug 14, 2024
@jbardin
Copy link
Member

jbardin commented Aug 14, 2024

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 jbardin added the waiting for reproduction unable to reproduce issue without further information label Aug 14, 2024
@EugenKon
Copy link
Author

EugenKon commented Aug 14, 2024

@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
Copy link
Member

jbardin commented Aug 14, 2024

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
Copy link
Member

jbardin commented Aug 14, 2024

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 jbardin added waiting-response An issue/pull request is waiting for a response from the community and removed waiting for reproduction unable to reproduce issue without further information labels Aug 14, 2024
@EugenKon
Copy link
Author

EugenKon commented Aug 14, 2024

@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
Copy link
Member

jbardin commented Aug 14, 2024

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 jbardin closed this as completed Aug 14, 2024
@EugenKon
Copy link
Author

EugenKon commented Aug 14, 2024

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

No deprecation warning for -state at the moment.

@jbardin
Copy link
Member

jbardin commented Aug 14, 2024

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 jbardin reopened this Aug 14, 2024
@jbardin jbardin added enhancement cli and removed bug waiting-response An issue/pull request is waiting for a response from the community new new issue not yet triaged labels Aug 14, 2024
@jbardin jbardin changed the title Terraform can not destroy resource if provider was not connected show deprecation warning if -state is used in conjunction with plan or apply Aug 14, 2024
@melsonic
Copy link
Contributor

Hi @jbardin, can I work on this one?

@crw
Copy link
Contributor

crw commented Aug 20, 2024

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

@melsonic
Copy link
Contributor

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.

@crw
Copy link
Contributor

crw commented Aug 22, 2024

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

@melsonic
Copy link
Contributor

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

sure, thanks @crw

@liamcervante
Copy link
Member

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
Copy link
Contributor

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

Copy link
Contributor

github-actions bot commented Oct 6, 2024

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 Oct 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants