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

🐛 Unmanaged Terraform Variables are being unset #221

Open
KamalAman opened this issue Jul 18, 2023 · 5 comments
Open

🐛 Unmanaged Terraform Variables are being unset #221

KamalAman opened this issue Jul 18, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@KamalAman
Copy link

KamalAman commented Jul 18, 2023

Operator Version, Kind and Kubernetes Version

  • Operator version: beta7
  • Kind: Workspace
  • Kubernetes version: 1.25.5

YAML Manifest File

# Copy-paste your YAML manifest here

Output Log

2023-07-18T11:55:07Z	INFO	Reconcile Variables	{"workspace": {"name":"WORKSPACE","namespace":"tf-workspaces"}, "msg": "deleting 1 terraform variables from the workspace ID ws-iANiBEofJqkNYnEg"}

Output of relevant kubectl commands

Steps To Reproduce

  1. Create a workspace
  2. Go to terraform cloud and manually add a new variable to the workspace. Ensure that the variables name is one that shouldn't be managed by the Workspace definition
  3. Wait and see the variable become deleted

Expected Behavior

What should have happened?
Variables that the Workspaces does not manage should not be modified / removed.

Actual Behavior

What actually happened?
The additional variable is deleted

Additional Context

This messes up the ability to have advanced configuration outside of the scope of the operator. Hopefully this issue gets resolved ASAP as I think it is a very important workflow to support.

References

Community Note

  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.
@KamalAman KamalAman added the bug Something isn't working label Jul 18, 2023
@KamalAman
Copy link
Author

KamalAman commented Jul 18, 2023

I know that the request might be a little bit difficult due to the following code only having access to its current state and the remote state:

image

Ideally and CRD modification you would calculate a diff to know which variables were managed to clean up, or you might have some sort of internal storage mechanism to get the true diff between the previous state and the current state.

Other simpler solutions that are less ideal that would solve my use case is a variable whitelist/blacklist to tell the operator what to manage, or even if it is just the ability to disable variable deletion all together.

@marceloboeira
Copy link

marceloboeira commented Nov 14, 2023

I think that's a feature not a bug, e.g.: if someone goes and messes up the variables you probably want the source of truth to be the CRD and not what someone might've changed in the UI by mistake.

It's a common thing for the IaC world, is there a scenario for this being the case?

There could be a parameter to define the strategy when drift is detected:

onDriftStragegy: reconcile (default/todays behaviour, delete everything out of CRD definition)
onDriftStrategy: tolerate (keep externally managed variables but overrides same variables with CRD definition)
onDriftStrategy: ignore (keep externally managed variables and ignore value changes in the UI) -> probably useless

It should be straightforward to implement the solution, but understanding the requirements and expected/common patterns might be more complex. Maybe variable sets are a better option for such scenarios with different groups of variables.

@srlynch1
Copy link

Generally I would say this is expected behavior.

If a consumer is defining variables in the CRD this is the source of truth.

If there is a need for variables to be enforced or have precedence this is something that was recently added to project based variable sets. This allows overrides to be applied by an administrator. Varsets are also independent of the workspace.

@certara-mchamberland
Copy link

certara-mchamberland commented Dec 8, 2023

I also have a use case here: I'm a Terragrunt user, and my current terragrunt setup provides a number of "standard" variables that people writing modules in my org can use for standardization. I have Terragrunt provision the workspaces then write a .auto.tfvars for the module hosted in that workspace, but it would be a lot more convenient (and transparent) to have it write directly in the workspace module, except the workspace module would presumably have a perpetual diff from TFC deleting the extra vars.

In general, I think this issue should be seen as a feature request for the TFE provider: We can't use the TFE provider to manage a workspace if TFC will go mess with it behind the scene and create drift. Providing a simple switch at the project and workspace level to override this behaviour would help.

@KamalAman
Copy link
Author

I do agree, if the Workspace CRD is managing a variable and someone manually changes it in TFC, it should get reverted. You should not be able to mess with a managed variable.

My main idea here is: If the operator in managing a variable, manage it. If it is not managing it, leave it alone.

If a variable is being managed, and then it's then removed from the Workspace CRD, it should be removed from TFC.
But after that, that variable so no longer being managed.

This should be opt in functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants