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

Treat schema availability as not essential #83

Closed
radeksimko opened this issue May 6, 2020 · 19 comments · Fixed by #171
Closed

Treat schema availability as not essential #83

radeksimko opened this issue May 6, 2020 · 19 comments · Fixed by #171
Labels
enhancement New feature or request
Milestone

Comments

@radeksimko
Copy link
Member

radeksimko commented May 6, 2020

Current Version

0.1.0

Use-cases

Sometimes the user may open a directory which has not been terraform initd - which is then surfaced as initialize error and the server doesn't start at all as unavailability of schema is treated as essential for its functioning.

We could still provide limited functionality that doesn't require schema - e.g. formatting and make schema watching/obtaining non-essential.

Proposal

Provide limited functionality if schema isn't available and inform the user of the limited functionality via diagnostics.

We should also figure out how/whether we can detect when user runs terraform init for the first time, so they don't have to reopen the whole folder/IDE after doing so. This practically depends on whether we can watch file/dir creations in a given path.

References

#84

@paultyng
Copy link
Contributor

paultyng commented May 6, 2020

Should we just init with -backend=false for them?

@radeksimko
Copy link
Member Author

radeksimko commented May 6, 2020

As per our offline discussion - I believe that:

  1. if any side executes init it should be the client, through workspace/executeCommand, not the server directly
  2. a good client should not be downloading hundreds of MB on the background without the user knowing
    • I'd be ok with a client config option which turns auto-init on and off, but that should certainly ship as off by default

@radeksimko
Copy link
Member Author

I just noticed that this is also important for when the client for any reason triggers initialize for a directory that isn't terraform at all - apparently happens with our private build of the VSCode extension currently.

It will just initialize whatever project is currently active in VSCode, which it probably shouldn't, but the server also shouldn't just error out in such case.

@paultyng
Copy link
Contributor

Not sure, but this is possibly a duplicate of or at least related to #106

@devTechi
Copy link

devTechi commented Jun 16, 2020

Hi,
for me this extensions stopped somehow working due to this issue (and others).

We aren't using terraform locally at all. So no possibility to do a local terraform init. And terraform init -backend=false gave me errors since we are still using version 0.11.* in some repositories.

And the settings terraform.indexing also stopped working. I guess because of the new language server, right?

@radeksimko
Copy link
Member Author

Hi @devTechi
I understand it can be hard to understand what functionality is/was provided by the client (e.g. VSCode extension) or the server, but when it comes to VSCode settings, that's most likely client side.

Before asking question or filing an issue on the client side though, please note that the VSCode v2 extension was slimmed down in functionality with the aim of reimplementing it in the language server, so it's reusable across other IDEs too. This means that some of the old v1 settings may not be applicable/relevant, because the client doesn't contain the same functionality.

If there's a particular functionality you miss from v1 I would encourage you to file an issue here. We can figure out what needs to be done on the server or client side.

We aren't using terraform locally at all. So no possibility to do a local terraform init.

The language server does and will depend on terraform for the foreseeable future as the CLI represents a stable consumable API from Terraform's perspective (unlike importing Go packages from the project, which is explicitly not supported).

We plan to explore some ways of gathering data directly from the plugin/provider binaries in the future, so that might reduce the reliance on Terraform in some scenarios, but it's likely that you'd still need to get the provider binaries somehow to the local workstation and terraform init would likely remain the main way to do that.

Pulling schema data from providers directly is essential for the language server to scale and work with any version of any provider and it also means we don't have to bundle schemas of 100+ popular providers with the language server in any way.

With all that said I understand that initializing the backend(s) is an excessive requirement and it's something I'd like to address somehow. The main reason it's a requirement from Terraform's perspective is because it needs to read not only the config, but also the state, to understand which providers to initialize. In order to access the state, it needs to initialize the backend.

In many/most language server scenarios though the user doesn't really care about the state (assuming it won't plan or apply) and so there should be a way of obtaining schema data purely based on configuration. I captured this under hashicorp/terraform#24261

I do plan to close this issue when language server doesn't error out on missing schema, but we can create a separate one to track the backend initialization requirement?

@FernandoMiguel
Copy link

@devTechi is your issue with the repetitive download of providers?
you can use a common folder to store all those and avoid the downloading

@devTechi
Copy link

devTechi commented Jun 17, 2020

@radeksimko
I do understand the shift from client to server. Makes sense. Some explanation of using and relying on the state backend from my perspective as a user not that much (nevertheless I understand the reason). What I liked was the linking of resources in my project, so that I could navigate through resources within the same module easily. Now I can't anymore.

Downloading the providers is not an issue and could be manageable via settings (like a providers list or something). I am using AWS, archive, template and null in most cases. But AWS is the main provider I need. But I don't have access to the S3 statebucket at all. Just via pipelines. This is a customers requirement. This won't be changed.

@FernandoMiguel
The issue is not regarding repetitive downloading. That was no problem. But getting the state locally is not feasible.

That being said. For me this extension stopped working (besides the syntax highlighting).

And thanks for the quick answer :)

@radeksimko
Copy link
Member Author

Some explanation of using and relying on the state backend from my perspective as a user not that much

That is fair. To be clear while the Terraform CLI was designed to be stable as an API, it predates any language server, so it will likely need some tweaking along the way and we will work with the relevant team to tweak it accordingly.

Downloading the providers is not an issue and could be manageable via settings (like a providers list or something). I am using AWS, archive, template and null in most cases. But AWS is the main provider I need.

Selecting, downloading and most importantly updating providers is actually quite a complex problem when done right (with verification, semver resolution etc.). More importantly that problem is already solved in the existing configuration and terraform init, so trying to reinvent it just to instrument the language server could be suboptimal from UX and effort perspective. i.e. I'm not sure you (or any other user) would be too excited about having a separate workflow/config from terraform init, just so you can edit these files in your editor.

But I don't have access to the S3 statebucket at all. Just via pipelines. This is a customers requirement. This won't be changed.

That is a reasonable requirement and I wouldn't expect you (or your customer) to change that practice. I just think that the language server needs to have a way of accessing the schema without having to access the state. I'd like for that solution to live in Terraform itself (e.g. as a flag of terraform providers schema and/or terraform init) in the long run, but we may need to find a short-term solution too for users which would use terraform older than the version where the patch with flags would land.

I'm sorry it's not ideal experience right now, but I hope the explanation makes sense.

@FernandoMiguel
Copy link

but wouldnt an init without accessing the real state, and just download and init the providers locally achieve all that?

@devTechi
Copy link

@radeksimko
Well, if I could run terraform init with a mocked state backend or through whatever flag it would also be a really nice thing. Of course I have Terraform installed locally and could do whatever I need to.

Regarding terraform init -backend=false: This doesn't work for me right now as we are on some repositories still on version 0.11.*
Upgrading would of course be possible, but it is much code and it is timeconsuming. That is why we cannot do it for every repository.
Locally I habe the newest Terraform version. Remotely we are using the tf-docker-version we need for that project.

@FernandoMiguel
Copy link

FernandoMiguel commented Jun 17, 2020

Regarding terraform init -backend=false: This doesn't work for me right now as we are on some repositories still on version 0.11.*
Locally I habe the newest Terraform version. Remotely we are using the tf-docker-version we need for that project.

your local terraform should be the same version as the pinned for your codebase/remotestate.
having tf 0.12 or 0.13 if your CI is 0.11 will cause issues in coding and validation

do invest a week in upgrading to 0.12, before 0.11 is EOL.
you will gain huge benefits on a much better HCLv2 and be ready for 0.13

@devTechi
Copy link

Why should it be the same? It wasn't the same before and it was no problem. Of course I had some issues with the changed coding styles and validation. But I knew that. And I have other projects where I have the latest Terraform version. So this is not the solution.
And upgrading oh many, many, many projects is far more than a week. The benefit after that is more or less nothing, because many of those projects doesn't change that much.

@FernandoMiguel
Copy link

Why should it be the same? It wasn't the same before and it was no problem. Of course I had some issues with the changed coding styles and validation. But I knew that. And I have other projects where I have the latest Terraform version. So this is not the solution.
And upgrading oh many, many, many projects is far more than a week. The benefit after that is more or less nothing, because many of those projects doesn't change that much.

i've migrated two orgs with very large code bases, and it took me less than a week on both.
for the most part 012upgrade does all the work.
then you apply and you are done.

the benefits is that now you have a new TF with lots of bugs fixed, and allows you to use new features on your code like for_each

@devTechi
Copy link

Thanks. I know TF 0.12 and know why I would upgrade

@danieladams456
Copy link

I'm not sure if this is the correct place to comment or not. It has to do with no .tf files or initialized .terraform directory at the root level, but more specifically how other folders are arranged in the workspace. Alexander seems to have a similar setup.

Readme.md
-subproject-one
  --.terraform.d
  --main.tf
  --other.tf
-subproject-two
  --.terraform.d
  --main.tf
  --other.tf

In this case subproject one and two might have different providers. It would be nice for the language server to work up the folder tree for the currently editing file and find where its parent project providers are initialized for suggesting the appropriate autocompletes, etc.

@radeksimko I have not read enough of the code and issues to know if this is not already the design goal, just wanted to throw out my use case. If it would be better as another issue, just let me know and I can create one.
Thanks!

@radeksimko
Copy link
Member Author

@danieladams456 This feedback is very useful - thank you! It would be even more useful under #32 (we are collating all the different hierarchies there)

This issue will get closed probably much sooner, but only because "unsupported" hierarchies don't error anymore. #32 is about supporting more hierarchies.

@danieladams456
Copy link

@radeksimko Perfect thanks! I'll copy/paste over there.

@ghost
Copy link

ghost commented Jul 19, 2020

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 context necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Jul 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants