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

feat: do not save imported state to statefile #589

Merged
merged 2 commits into from
Dec 9, 2024
Merged

Conversation

Apollorion
Copy link
Member

@Apollorion Apollorion commented Nov 25, 2024

We should not save peoples state to our state file. If an admin stack is managing many stacks with import_state it could cause peoples state to explode in size and it also causes memory to explode because of internal terraform comparisons. I have examples of this, if you'd like to see them.

Because of this attributes' DiffSuppressFunc, we will never see a change here so whats in state does not matter. I can also think of an easy way to allow diffing of this when using ForceNew as well, but I was unsure if this was purposefully added here since we're dealing with state so I left that alone. Happy to discuss that if warranted.

This change will:

  1. Ensure that any new stacks created with import_state will not save the imported state to this state file.
  2. Will migrate any existing states and remove the import_state keys value from the state file.

Migration Proof

Proof that state migration works, note the .resources[0].instances[0].schema_version changes from 0 to 1 and that .resources[0].instances[0].attributes.import_state changes from a whole state file to "".

Schema Version Change

$ cat terraform.tfstate.backup | jq '.resources[0].instances[0].schema_version'
0

$ cat terraform.tfstate | jq '.resources[0].instances[0].schema_version'
1

Attribute Change

$ cat terraform.tfstate.backup | jq '.resources[0].instances[0].attributes.import_state'
"{\"version\":4,\"terraform_version\":\"1.8.1\",\"serial\":1,\"lineage\":\"aaf9ffad-ce22-98be-5707-ec9ae75df7e2\",\"outputs\":{\"test_file\":{\"value\":\"this is the ultra test\",\"type\":\"string\"}},\"resources\":[{\"mode\":\"managed\",\"type\":\"null_resource\",\"name\":\"this\",\"provider\":\"provider[\\\"registry.opentofu.org/hashicorp/null\\\"]\",\"instances\":[{\"schema_version\":0,\"attributes\":{\"id\":\"5259082673849345590\",\"triggers\":{\"environment\":\"yes\"}},\"sensitive_attributes\":[]}]}],\"check_results\":null}\n"

cat terraform.tfstate | jq '.resources[0].instances[0].attributes.import_state'
""

Full Old State (v1.19.0 of the provider)

{"version":4,"terraform_version":"1.8.1","serial":1,"lineage":"4f4fe5e0-4806-5908-38c6-5883fcff123b","outputs":{},"resources":[{"mode":"managed","type":"spacelift_stack","name":"this","provider":"provider[\"spacelift.io/spacelift-io/spacelift\"]","instances":[{"schema_version":0,"attributes":{"additional_project_globs":null,"administrative":false,"after_apply":null,"after_destroy":null,"after_init":null,"after_perform":null,"after_plan":null,"after_run":null,"ansible":[],"autodeploy":false,"autoretry":false,"aws_assume_role_policy_statement":"{\"Action\":\"sts:AssumeRole\",\"Condition\":{\"StringEquals\":{\"sts:ExternalId\":\"Apollorion@state-import-check\"}},\"Effect\":\"Allow\",\"Principal\":{\"AWS\":\"324880187172\"}}","azure_devops":[],"before_apply":null,"before_destroy":null,"before_init":null,"before_perform":null,"before_plan":null,"bitbucket_cloud":[],"bitbucket_datacenter":[],"branch":"main","cloudformation":[],"description":"","enable_local_preview":false,"enable_well_known_secret_masking":false,"github_action_deploy":true,"github_enterprise":[],"gitlab":[],"id":"state-import-check","import_state":"{\"version\":4,\"terraform_version\":\"1.8.1\",\"serial\":1,\"lineage\":\"aaf9ffad-ce22-98be-5707-ec9ae75df7e2\",\"outputs\":{\"test_file\":{\"value\":\"this is the ultra test\",\"type\":\"string\"}},\"resources\":[{\"mode\":\"managed\",\"type\":\"null_resource\",\"name\":\"this\",\"provider\":\"provider[\\\"registry.opentofu.org/hashicorp/null\\\"]\",\"instances\":[{\"schema_version\":0,\"attributes\":{\"id\":\"5259082673849345590\",\"triggers\":{\"environment\":\"yes\"}},\"sensitive_attributes\":[]}]}],\"check_results\":null}\n","import_state_file":null,"kubernetes":[],"labels":null,"manage_state":true,"name":"state-import-check","project_root":"playground/archive_file","protect_from_deletion":false,"pulumi":[],"raw_git":[],"repository":"spacelift","runner_image":"","showcase":[],"slug":"state-import-check","space_id":"legacy","terraform_external_state_access":false,"terraform_smart_sanitization":false,"terraform_version":"","terraform_workflow_tool":"TERRAFORM_FOSS","terraform_workspace":"","terragrunt":[],"worker_pool_id":""},"sensitive_attributes":[[{"type":"get_attr","value":"import_state"}]],"private":"bnVsbA=="}]}],"check_results":null}

Full New State

{"version":4,"terraform_version":"1.8.1","serial":2,"lineage":"4f4fe5e0-4806-5908-38c6-5883fcff123b","outputs":{},"resources":[{"mode":"managed","type":"spacelift_stack","name":"this","provider":"provider[\"spacelift.io/spacelift-io/spacelift\"]","instances":[{"schema_version":1,"attributes":{"additional_project_globs":[],"administrative":false,"after_apply":[],"after_destroy":[],"after_init":[],"after_perform":[],"after_plan":[],"after_run":[],"ansible":[],"autodeploy":false,"autoretry":false,"aws_assume_role_policy_statement":"{\"Action\":\"sts:AssumeRole\",\"Condition\":{\"StringEquals\":{\"sts:ExternalId\":\"Apollorion@state-import-check\"}},\"Effect\":\"Allow\",\"Principal\":{\"AWS\":\"324880187172\"}}","azure_devops":[],"before_apply":[],"before_destroy":[],"before_init":[],"before_perform":[],"before_plan":[],"bitbucket_cloud":[],"bitbucket_datacenter":[],"branch":"main","cloudformation":[],"description":"","enable_local_preview":false,"enable_well_known_secret_masking":false,"github_action_deploy":true,"github_enterprise":[],"gitlab":[],"id":"state-import-check","import_state":"","import_state_file":null,"kubernetes":[],"labels":[],"manage_state":true,"name":"state-import-check","project_root":"playground/archive_file","protect_from_deletion":false,"pulumi":[],"raw_git":[],"repository":"spacelift","runner_image":"","showcase":[],"slug":"state-import-check","space_id":"legacy","terraform_external_state_access":false,"terraform_smart_sanitization":false,"terraform_version":"","terraform_workflow_tool":"TERRAFORM_FOSS","terraform_workspace":"","terragrunt":[],"worker_pool_id":""},"sensitive_attributes":[[{"type":"get_attr","value":"import_state"}]],"private":"bnVsbA=="}]}],"check_results":null}

Clean Tofu Apply with Dev Override

$ tofu apply
╷
│ Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│  - spacelift.io/spacelift-io/spacelift in /Users/apollorion/projects/src/github.com/spacelift-io/terraform-provider-spacelift/dist/terraform-provider-spacelift_darwin_arm64
│
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵
spacelift_stack.this: Refreshing state... [id=state-import-check]

No changes. Your infrastructure matches the configuration.

OpenTofu has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Proof State is still uploaded

Screenshot 2024-11-25 at 3 43 57 PM

@Apollorion Apollorion force-pushed the recreate-stack branch 3 times, most recently from 0b82070 to 9f9113d Compare November 25, 2024 20:11
@Apollorion Apollorion requested a review from a team November 25, 2024 20:28
Copy link
Contributor

@peterdeme peterdeme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one super small thing, otherwise lgtm

spacelift/resource_stack.go Show resolved Hide resolved
spacelift/resource_stack.go Show resolved Hide resolved
peterdeme
peterdeme previously approved these changes Nov 26, 2024
eliecharra
eliecharra previously approved these changes Nov 26, 2024
Copy link
Member

@eliecharra eliecharra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with @peterdeme that we should drop comments to explain the why. Otherwise, makes sense to me 👍🏻

@Apollorion Apollorion dismissed stale reviews from eliecharra and peterdeme via 25b9415 December 9, 2024 14:52
@Apollorion
Copy link
Member Author

Thanks @eliecharra and @peterdeme I've updated the PR with comments.

@peterdeme peterdeme enabled auto-merge (squash) December 9, 2024 14:54
@peterdeme peterdeme disabled auto-merge December 9, 2024 14:54
@peterdeme
Copy link
Contributor

@Apollorion approved, thanks!

@Apollorion Apollorion enabled auto-merge December 9, 2024 14:57
@Apollorion Apollorion merged commit ca71331 into main Dec 9, 2024
7 checks passed
@Apollorion Apollorion deleted the recreate-stack branch December 9, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants