GCloud backend

[pbzdyl]

Hello,

I have implemented a new backend: GCloud. It uses Google Storage for persisting state files and for distributed locks. It supports multiple workspaces.

Thanks to the authors of remote-state/consul and remote-state/s3 as I used their implementation as a basis for my code. Disclaimer: I am new to Go and Terraform, so please forgive me any dumbness in the code!

Features:

• remote locking
• multiple workspaces
• uses account key JSON file (credentials option, either file path to the JSON file or directly JSON contents)
• if credentials is not specified, uses Google Application Default Credentials

Example configuration:

terraform {
  backend "gcloud" {
    bucket = "my-bucket"
    state_dir = "terraform-state"
    credentials = "path/to/account-key.json"
  }
}

Actual state file URL will be: gs://${bucket}/${state_dir}/${workspace_name}.tfstate
and a lock file will be created at: gc://${bucket}/${state_dir}/$workspace_name}.tflock

State locks use Google Cloud Storage Concurrency Control mechanism.

I am aware there is already the gcs backend. However, my understanding is that it is a legacy remote client implementation which doesn't support multiple workspaces. There would also be a risk of regressions potentially impacting deployments that use it.

This is of course still work in progress as I am not sure if this PR would be accepted or not. If there is a chance it could be merged, I would complete the PR with tests, documentation updates and anything suggested by the reviewers.

Best regards,
Piotr

[sl1pm4t]

Hi @pbzdyl
I'm glad my colleague pointed this PR out to me, because I was about to start work on implementing a GCS backend.

I contributed the legacy gcs remote last year, and you're correct that it doesn't support workspaces or locking etc - simply because those concepts didn't exist in Terraform back then.

If this backend is accepted, I don't see any need to keep the existing gcs remote. Just like the legacy s3 remote was removed when the s3 backend was implemented.
EDIT: For this reason, I think this backend should also be renamed to gcs.

I'll give this backend a try - thanks!

[nodesocket]

👍 for this replacing the existing gcs integration if everything checks out.

[pbzdyl]

Hello @sl1pm4t ,

Thanks! Please, do share your feedback from testing this PR. If you have any suggestions for the PR's code, I would be happy to hear too as I am new to terraform and Go so I am sure there is something I could improve.

I am not sure removing the current gcs implementation and replacing it with this one using the same name would be welcomed by users of the current gcs as it would be backward incompatible. By using a different name, users could migrate from one backend to another by changing the configuration and moving their state.

[octo]

Hi @pbzdyl, thank you very much for your work!

I was about to start working on implementing locking for the GCS backend when I stumbled into this PR. I'm quite interested in moving this along; are you still interested in working on this? I'd be happy to collaborate on this; I could e.g. send PRs against your feature branch if that's okay with you …?

Best regards,
—octo

[pbzdyl]

Hello @octo,

Yes - definitely. However, I am not sure if this PR will be accepted as there has been no feedback from Terraform team.

What kind of changes would you like to add to this PR?

Regards,
Piotr

[octo]

@pbzdyl Great! Mostly coding style changes for now, but there are two noteworthy changes:

1. Return the GCS object's generation as the lock ID. This allows Unlock() to call "delete if generation matches" immediately, removing one round-trip.
2. Hold on to the context passed to the configure() callback to avoid calling context.Background().

The changes are in the pr/15592 branch. I'll open a PR against your repo in a minute.

Thanks and best regards,
—octo

[jbardin]

Hi,

Sorry about the delayed response here, and thanks for the work you put into this!
I hope to catch up with a review shortly, but I have a preliminary question; Are there some incompatibilities that are preventing updating the existing "gcs" remote state to a backend? We generally would prefer a transparent upgrade path for existing users, rather than creating an entirely new backend.

[pbzdyl]

Hello @jbardin,

Do you mean removing current gcs implementation and renaming the new backend to gcs? I am not sure I have enough knowledge about the internals but I will try to implement it this way and test it locally.

[octo]

Hi @jbardin,

This PR adds support for named states, which are stored under a configurable directory. Previously, the gcs backend was using a state file.

I think it should be possible to add backwards compatibility and rename this package to "gcs" so it can serve as a drop-in replacement. Do you know if another backend has implemented something similar so we can go for consistent behavior?

Best regards,
—octo

[jbardin]

Hi @octo,

I think the existing "remote-state" backends were all migrated from legacy remote state implementations, with azure being the most recent. S3 might be a good learning example, as it probably has the most "hacks" for legacy behavior, and is one of the more complex requiring S3 and DynamoDB in concert.

[octo]

Great, thanks @jbardin!

[octo]

Hi everybody!

pbzdyl#2 has a lot of updates to this PR and should appear here once merged by @pbzdyl.

How do you want to deal with the merge conflicts in vendor/? Should I rebase onto or merge the master branch? I can also remove the commit that changes vendor/ and we can do this as the last commit of the PR.

Best regards,
—octo

[octo]

Thank you @pbzdyl!

@jbardin I've done all the changes I was intending to make; from my POV, this is good to go. If anything's missing, please let me know and I'll make sure to add / change that.

Best regards,
—octo

[jbardin]

Hi @octo,

Thanks for the update, I'll take a look ASAP.
As for the conflicts, we usually prefer to rebase on master then force push the new commits into the PR. The vendor files however are a little different, since you can't "resolve" conflicts in there. I would drop the first commit, rebase on master, then use govendor to update the missing dependencies.

Don't worry if you can't get the vendor files to work out. If it turns into too much of a mess, I can transfer this whole PR into a branch here and fix them at the end.

[pbzdyl]

I have removed the commit updating vendor files and rebased on the latest master branch to resolve merge conflicts. I will try to update vendor files tomorrow.

[pbzdyl]

@jbardin, I was able to update vendor files.

[pbzdyl]

I was testing the new backend today and I noticed that we probably should add (ignored) project configuration property to make the new backend fully compatible with the old one so that users with existing gcs configuration can start using the new one without any configuration changes.

@jbardin, @octo What would be the preferred way of handling it? Shall it be marked with Deprecated or Removed?

Similarly, should path be flagged with Deprecated?

[octo]

I just tried the new code for the first time, and there are some small issues that need fixing:

• The old code had a project option which the new code is missing. I'll have to find out what it was used for and do something sensible.
• The credentials option is evaluated relative to the process's working directory, not the terraform working directory which is unexpected. I.e. if example.tf references creds.json, this should be interpreted as relative to example.tf, not the (meaningless) directory from which terraform init /path/to/terraform/wd was executed.
• We should make an effort to create the bucket for the user if it doesn't exist. This is a UX issue that only pops up when initializing, but I think is worth fixing in code to give new users an easier time getting started.

I'll send patches for those issues soon. If you know a way to determine the terraform working directory (without having access to command.Meta.DataDir(), I assume) I'd love to hear it :)

Best regards,
—octo

[jbardin]

@octo: Everything looks good to me, thanks for hanging in there!

I'm going to hold off merging briefly, just until I can get some hands on testing with existing gcs remote states to try and catch any regressions. I do want to get that done ASAP though, so we don't start to lag behind and run into more merge conflicts, and can get this in before we start the process for the next major release.

[octo]

@jbardin Awesome, thank you! :)

[octo]

Hi @jbardin, is there any update? Is there anything I can help with to move this forward?

Best regards,
—octo

[jbardin]

@octo: Thanks, just trying to schedule time to QA a build with this. It needs to be in before the beta process for 0.11 ;)

[jbardin]

moved this over to a branch where I could resolve the merge conflict asap.

[octo]

🎉 Thanks @pbzdyl and @jbardin!

[pbzdyl]

Thank you @octo - all the hard work was done by you!

[sl1pm4t]

Thanks @pbzdyl + @octo + @jbardin and all who helped with this one!

[ghost]

All,

Can someone check the following bug that seems to be related to be this PR?

#16741

[ghost]

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.