Skip to content
This repository has been archived by the owner on Feb 16, 2023. It is now read-only.

Integrated Terraform State Backend #285

Draft
wants to merge 11 commits into
base: develop
Choose a base branch
from
Draft

Conversation

jpcoenen
Copy link
Member

@jpcoenen jpcoenen commented May 7, 2020

This is an MVP of intgerating a tfstate backend into the secrethub run command.

It can be used by configuring the following backend in in terraform:

terraform {
  backend "http" {
    address        = "http://localhost:8118"
    unlock_address = "http://localhost:8118"
    lock_address   = "http://localhost:8118"
    username       = "path/to/state/directory"
  }
}

The state is served by wrapping Terraform commands in secrethub run --tfstate. For example:

secrethub run --tfstate -- terraform init -reconfigure
secrethub run --tfstate -- terraform apply

This opens up an HTTP-endpoint that Terraform can connect to. This HTTP-endpoint can only be connected to from processes that are a child of secrethub run. In other words: it should not be possible for another process than Terraform to connect to this endpoint and read/write the state.

Though not strictly necessary, it is also possible to set a password on the listener. If the secret path/to/state/directory/password exists, the value of it must be set in the password field of the Terraform backend. If this secret does not exist, the password field can be omitted.

jpcoenen added 9 commits May 7, 2020 11:20
Previous version had compatibility issues with the latest version of lsof on MacOS.
This should be handled in secrethub-go instead.
Backend now returns an error when invoked on Windows 386. Support can be added later.
@SimonBarendse
Copy link
Member

We'd love to get your feedback on this!

If you don't want to build the binary yourself, here they are for your convenience:
secrethub-linux-amd64.tar.gz
secrethub-darwin-amd64.tar.gz
secrethub-windows-amd64.zip

return b.respondError(http.StatusUnauthorized, "password stored at %s should be set as auth password", passwordPath), nil
}
if password != secret {
return b.respondError(http.StatusForbidden, "provided password does not password stored at %s", passwordPath), nil

Choose a reason for hiding this comment

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

provided password does not password stored at %s => provided password does not match the password stored at %s

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch 👌

@antonbabenko
Copy link

Here is my feedback when I try to use this backend:

  1. Setting username to valid username makes sense to me (username = "antonbabenko")
[SecretHub]: set user to a valid repository or directory, got: antonbabenko

The error message is confusing.

  1. I set username to a correct value (<namespace>/<repo>) and it works. I see that we can't rename username into path, then there should be clear instructions on what is expected as a value.

  2. When running secrethub run --tfstate -- terraform apply with missconfigured username (eg, username = "antonbabenko/demo/terraform.tfstate" and this key already contains previous tfstate. Terraform apply fails like this:

[SecretHub]: Directory not found (server.dir_not_found)
Failed to save state: HTTP error: 404

Error: Failed to persist state to backend.

The error shown above has prevented Terraform from writing the updated state
to the configured backend. To allow for recovery, the state has been written
to the file "errored.tfstate" in the current working directory.

Running "terraform apply" again at this point will create a forked state,
making it harder to recover.

To retry writing this state, use the following command:
    terraform state push errored.tfstate

When trying to overwrite existing key with the one which was produced and stored locally (errored.tfstate), I run into another issue:

$ secrethub run --tfstate -- terraform state push errored.tfstate
Acquiring state lock. This may take a few moments...
[SecretHub]: Directory not found (server.dir_not_found)
Error locking state: Error acquiring the state lock: Unexpected HTTP response code 404

Terraform acquires a state lock to protect the state from being written
by multiple users at the same time. Please resolve the issue above and try
again. For most commands, you can disable locking with the "-lock=false"
flag, but this is not recommended.

As a solution for this, maybe secrethub CLI should do basic verification that key is readable and/or correct?

  1. Currently, each state should be placed in separate repo or directory to prevent accidental overrides, which is very easy to forget. I don't see a way to customize the name in HTTP backend. Do you have any ideas on how this can be improved for the users?

  2. When trying to access the running process from separate execution the error message ([SecretHub]: can only be reached from a process spawned with secrethub run) is being printed into the main window where prompt for the confirmation is being expected. It would be better to raise the same error in the window where this belongs to (after Error: Error loading state: HTTP remote state endpoint invalid auth).

That's it for now :) Let me know when an update is coming.

@jpcoenen
Copy link
Member Author

jpcoenen commented Jun 12, 2020

Thanks a lot for the feedback @antonbabenko! Seems like you did some really thorough testing 😀

Setting username to valid username makes sense to me (username = "antonbabenko")
[SecretHub]: set user to a valid repository or directory, got: antonbabenko
The error message is confusing.

I think you already figured out that we're a little boun to what we can do with the HTTP State Backend; that's why the username field is used. But I do definitely agree that this error message could be way more helpful. I'll work on that.

When running secrethub run --tfstate -- terraform apply with missconfigured username (eg, username = "antonbabenko/demo/terraform.tfstate" and this key already contains previous tfstate. Terraform apply fails like this:

An explicit check for the existence of the directory will probably make this a bit more user-friendly.

Currently, each state should be placed in separate repo or directory to prevent accidental overrides, which is very easy to forget. I don't see a way to customize the name in HTTP backend. Do you have any ideas on how this can be improved for the users?

Hmm. That's an interesting question. The reason for using a directory as input instead of the path to the secret where the state is stored, is that this directory is both used to store the state and the lock. I do not directly see a way around that.

What would happen if you used the same S3 path for 2 different Terraform projects? Is that different from how this works? Or is it mainly that there is some confusion from the username being used to specify the path where the state is stored?

When trying to access the running process from separate execution the error message ([SecretHub]: can only be reached from a process spawned with secrethub run) is being printed into the main window where prompt for the confirmation is being expected. It would be better to raise the same error in the window where this belongs to (after Error: Error loading state: HTTP remote state endpoint invalid auth).

You mean that it would be better to also print can only be reached from a process spawned with secrethub run in the terminal where you execute the failing terraform command? Because I agree that would be helpful, but it's not possible, unfortunately. Terraform does not allow any customization of the errors it displays. The messages prepended with [SecretHub are output by the SecretHub CLI, not by Terraform.

Or.. are you running secrethub run -- terraform apply in two terminals simultaneously in this case? Because in that case, it would be possible to make some improvements.


To conclude: really great feedback! I'll get to work on the above points in the near future. Though I cannot precisely say when, I will let you know once a new iteration is done.

One important question that's still on my mind: if we do some iterations (starting with the points you mentioned), how usable would you find it?

@jpcoenen
Copy link
Member Author

@antonbabenko I've just updated the error message you describe in point 1 and have introduced an explicit check for the existence of the given directory, which should help with point 3.

Here is the new build:
secrethub-linux-amd64.tar.gz
secrethub-darwin-amd64.tar.gz
secrethub-windows-amd64.zip

@antonbabenko
Copy link

Nice! I will try to take a look at this release and comment during next week.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants