terraform: remote state file exists check issue

[LanceNero]

In the official CLI implementation of Terraform, if the state file does not exist, a new one will be created, and there is no need to check that the state file already exists and with an error if file not exists.

# Test command
terraform apply -state test.tfstate. # if state file not exists ,terraform will create a new one
terraform destroy -state test1.tfstate  ### Terraform will not throw any error, the command will succeed execute, only report no resource has destroy

SUMMARY

Remove state file check condition and error block, because in the native implementation of terraform will not cause errors due to the non-existent file

Fixes community.general.terraform module paramter state_file throw error due to the non-existent file

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

Fixes community.general.terraform

ADDITIONAL INFORMATION

Terraform CLI result

# Test command
terraform apply -state test.tfstate. # if state file not exists ,terraform will create a new one
terraform destroy -state test1.tfstate  ### Terraform will not throw any error, the command will succeed execute, only report no resource has destroy

Ansible Module result

the following code will return an error for the file doesn't exists issue
site.yml

---
- name: "[ Part 1 ]Creating Infrastructure...."
  hosts: localhost
  connection : local
  gather_facts: no 
  vars:
    project_path: XXXXX
    custom_path: XXXXXXXXX

tasks:
    - name: "Creating VPC - Planing"
      terraform:
        project_path: "{{ project_path }}"
        variables: 
          aws_region: us-west-1
        state: present
        state_file: "{{ custom_path }}test.tfstate"
        force_init: true
        overwrite_init: false
        workspace: default
      register: present_result

[ansibullbot]

cc @m-yosefpor @rainerleber
click here for bot help

[ansibullbot]

The test licenses failed with 2 errors:

changelogs/fragments/6296-LanceNero-Terraform_statefile_check:0:0: found no copyright notice
changelogs/fragments/6296-LanceNero-Terraform_statefile_check:0:0: must have at least one license

The test ansible-test sanity --test changelog [explain] failed with 1 error:

changelogs/fragments/6296-LanceNero-Terraform_statefile_check:0:0: extension must be one of: .yml, .yaml

The test ansible-test sanity --test changelog [explain] failed with 1 error:

changelogs/fragments/6296-LanceNero-Terraform_statefile_check:0:0: extension must be one of: .yml, .yaml

The test ansible-test sanity --test changelog [explain] failed with 1 error:

changelogs/fragments/6296-LanceNero-Terraform_statefile_check:0:0: extension must be one of: .yml, .yaml

The test ansible-test sanity --test changelog [explain] failed with 1 error:

changelogs/fragments/6296-LanceNero-Terraform_statefile_check:0:0: extension must be one of: .yml, .yaml

The test ansible-test sanity --test changelog [explain] failed with 1 error:

changelogs/fragments/6296-LanceNero-Terraform_statefile_check:0:0: extension must be one of: .yml, .yaml

click here for bot help

[russoz]

hi @LanceNero thanks for your contribution!

Question: shouldn't the presence of the state file be required when state=absent, I mean, if we are decommissioning infrastructure, it should exist in a state file, no?

[LanceNero]

shouldn't the presence of the state file be required when state=absent, I mean, if we are decommissioning infrastructure, it should exist in a state file, no?

Hi Alexei

I think yes, the answer is NO. because even the state file not exists that will not cause any error from terraform cmd, and the user program does not have any wrong behavior, It just ran once and without any effect.

I think we don't need to add an error to block the user's program, but we cloud to provide a warning to let users know their program has not achieved their expected goals, then they will check the state file path.

but now, according to the current implementation, if I want to specify the state file output to another directory when the backend is local, this module will throw an error and block subsequent tasks.

• Scenario Example:
  -- Present State
  I want to create an infra by terraform and I wish to output the state file to a central place with a special name(not terraform.tfstate ) if the state file does not exist then create one

[russoz]

LGTM

[felixfontein]

Hmm, maybe I understand the code wrong, but aren't you also removing that condition when state is not absent? That looks very wrong to me.

(Also sanity tests are currently failing, the changelog fragment is missing the correct extension.)

[russoz]

Hmm, maybe I understand the code wrong, but aren't you also removing that condition when state is not absent? That looks very wrong to me.

Actually that was hist very first intention, and as far as I can tell, there is nothing wrong with that. For states planned and present, if a state file does not exist terraform will create it automatically.

My doubt was for state absent, but from @LanceNero (and also I checked in TF's docs) the important thing when running terraform destroy is to have the TF project files available. If a state file exists and contains the infra being destroyed, then it will be updated. If the state file does not exist the destroy operation still works.

(Also sanity tests are currently failing, the changelog fragment is missing the correct extension.)

Yeah, @LanceNero you need to fix that before we can merge this PR.

[felixfontein]

May be that is OK (I'm not familiar with terraform), but in my opinion the change for state=present would definitely not be a bugfix, but a feature.

[russoz]

Errr, I hadn't paid attention to that. Yes, definitely a feature not a bugfix.

And come to think about it, it would be nice to mention in the docs for the state_file parameter that the module will create the state file as needed, just like terraform itself would do. Just for the sake of being explicit and preventing future confusions for people not quite used to TF and/or the module.

[github-actions]

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

[LanceNero]

May be that is OK (I'm not familiar with terraform), but in my opinion the change for state=present would definitely not be a bugfix, but a feature.

Actually, previous implementations of this program did not perform the correct operation, when the state is set to "present"( planned stage will ignore this param) and we set the state_file param a value (even default name terraform.tfstate ) it wouldn't work and will throw an error, that means this param only work when the state is "absent". that's why I set the tag as bug_fix.

But If you think it should be classified as feature improvement, I'm ok with it, I've changed the fragment and PR comment to feature release.

[patchback]

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/bf780ea7387b4cbfb10dd4ef28c15e3fea76b94e/pr-6296

Backported as #6331

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[felixfontein]

@LanceNero thanks for your contribution!
@russoz thanks for reviewing!

[kemopq]

I checked the solution of this issue in a setup described in #6490 and it doesn't work.
TypeError: AnsibleModule.warn() got an unexpected keyword argument 'msg'