search

hashicorp / terraform-ls

Terraform Language Server
Mozilla Public License 2.0
992 stars 131 forks source link

Investigate optimisation opportunities for workspaces w/ symlinks #1398

Open radeksimko opened 1 year ago

radeksimko commented 1 year ago

Originally posted by @briceburg in https://github.com/hashicorp/terraform-ls/issues/1375#issuecomment-1714496313

Ever since switching to a monolithic structure (where all terraform configurations under a single repo) the vs code extension has become unresponsive/unusable for me. even when using v0.31.5 of the language server.

Repo structure looks like;

.
├── CODEOWNERS
├── README.md
├── acme
│   ├── bar
│   │   ├── README.md
│   │   ├── __global.tf
│   │   ├── docker-compose.yml
│   │   ├── environments
│   │   │   ├── _common.tf
│   │   │   ├── _variables-legacy.tf
│   │   │   ├── dev
│   │   │   │   ├── __global.tf -> ../../__global.tf
│   │   │   │   ├── _common.tf -> ../_common.tf
│   │   │   │   ├── _variables-legacy.tf -> ../_variables-legacy.tf
│   │   │   │   ├── main.tf
│   │   │   │   ├── service_account.yaml -> ../../service_account.yaml
│   │   │   │   ├── vgs.auto.tfvars -> ../../secrets/acme-secrets/vgs.auto.tfvars
│   │   │   │   └── vgs.dev.sh -> ../../secrets/acme-secrets/vgs.dev.sh
│   │   │   ├── prod
│   │   │   │   ├── __global.tf -> ../../__global.tf
│   │   │   │   ├── _common.tf -> ../_common.tf
│   │   │   │   ├── _variables-legacy.tf -> ../_variables-legacy.tf
│   │   │   │   ├── main.tf
│   │   │   │   ├── migration-20230731.tf
│   │   │   │   ├── service_account.yaml -> ../../service_account.yaml
│   │   │   │   ├── vgs.auto.tfvars -> ../../secrets/acme-secrets/vgs.auto.tfvars
│   │   │   │   └── vgs.prod.sh -> ../../secrets/acme-secrets/vgs.prod.sh
│   │   │   ├── sandbox
│   │   │   │   ├── __global.tf -> ../../__global.tf
│   │   │   │   ├── _common.tf -> ../_common.tf
│   │   │   │   ├── _variables-legacy.tf -> ../_variables-legacy.tf
│   │   │   │   ├── main.tf
│   │   │   │   ├── migration-20230730.tf
│   │   │   │   ├── service_account.yaml -> ../../service_account.yaml
│   │   │   │   ├── vgs.auto.tfvars -> ../../secrets/acme-secrets/vgs.auto.tfvars
│   │   │   │   └── vgs.sandbox.sh -> ../../secrets/acme-secrets/vgs.sandbox.sh
│   │   │   └── stg
│   │   │       ├── __global.tf -> ../../__global.tf
│   │   │       ├── _common.tf -> ../_common.tf
│   │   │       ├── _variables-legacy.tf -> ../_variables-legacy.tf
│   │   │       ├── main.tf
│   │   │       ├── service_account.yaml -> ../../service_account.yaml
│   │   │       ├── vgs.auto.tfvars -> ../../secrets/acme-secrets/vgs.auto.tfvars
│   │   │       └── vgs.stg.sh -> ../../secrets/acme-secrets/vgs.stg.sh
│   │   ├── modules
│   │   │   └── main
│   │   │       ├── locals.tf
│   │   │       ├── main.tf
│   │   │       ├── outputs.tf
│   │   │       └── variables.tf
│   │   ├── secrets
│   │   └── service_account.yaml
│   └── foo
│       ├── README.md
│       ├── __global.tf
│       ├── environments
│       │   ├── _common.tf
│       │   ├── _variables-legacy.tf
│       │   ├── review
│       │   │   ├── __global.tf -> ../../__global.tf
│       │   │   ├── _common.tf -> ../_common.tf
│       │   │   ├── _variables-legacy.tf -> ../_variables-legacy.tf
│       │   │   ├── cloudflare.auto.tfvars -> ../../secrets/acme-secrets/cloudflare.auto.tfvars
│       │   │   ├── datadog_secrets.auto.tfvars -> ../../secrets/acme-secrets/datadog_secrets.auto.tfvars
│       │   │   ├── main.tf
│       │   │   └── service_secrets.auto.tfvars -> ../../secrets/acme-secrets/service_secrets.auto.tfvars
│       │   └── sandbox
│       │       ├── __global.tf -> ../../__global.tf
│       │       ├── _common.tf -> ../_common.tf
│       │       ├── _variables-legacy.tf -> ../_variables-legacy.tf
│       │       ├── cloudflare.auto.tfvars -> ../../secrets/acme-secrets/cloudflare.auto.tfvars
│       │       ├── datadog_secrets.auto.tfvars -> ../../secrets/acme-secrets/datadog_secrets.auto.tfvars
│       │       ├── main.tf
│       │       ├── migration-20230721.tf
│       │       └── service_secrets.auto.tfvars -> ../../secrets/acme-secrets/service_secrets.auto.tfvars
│       ├── modules
│       │   └── main
│       │       ├── data.tf
│       │       ├── locals.tf
│       │       ├── main.tf
│       │       ├── outputs.tf
│       │       ├── services-mailcatch.tf
│       │       ├── services.tf
│       │       └── variables.tf
│       ├── namespaces
│       │   ├── _common.tf
│       │   ├── review
│       │   │   ├── __global.tf -> ../../__global.tf
│       │   │   ├── _common.tf -> ../_common.tf
│       │   │   ├── main.tf
│       │   │   └── security.tf
│       │   └── sandbox
│       │       ├── __global.tf -> ../../__global.tf
│       │       ├── _common.tf -> ../_common.tf
│       │       ├── main.tf
│       │       └── security.tf
│       └── secrets
├── platform
│   ├── backstage
│   │   └── TBD
│   └── sample-app
│       ├── README.md
│       ├── __global.tf
│       ├── environments
│       │   ├── _common.tf
│       │   ├── dev
│       │   │   ├── __global.tf -> ../../__global.tf
│       │   │   ├── _common.tf -> ../_common.tf
│       │   │   └── main.tf
│       │   └── review
│       │       ├── __global.tf -> ../../__global.tf
│       │       ├── _common.tf -> ../_common.tf
│       │       └── main.tf
│       ├── modules
│       │   └── main
│       │       ├── data.tf
│       │       ├── locals.tf
│       │       ├── main.tf
│       │       ├── outputs.tf
│       │       ├── providers.tf
│       │       └── variables.tf
│       └── namespaces
│           ├── _common.tf
│           ├── dev
│           │   ├── __global.tf -> ../../__global.tf
│           │   ├── _common.tf -> ../_common.tf
│           │   └── main.tf
│           └── review
│               ├── __global.tf -> ../../__global.tf
│               ├── _common.tf -> ../_common.tf
│               └── main.tf
└── sso
    ├── README.md
    ├── namespaces
    │   ├── _common.tf
    │   ├── dev
    │   │   ├── _common.tf -> ../_common.tf
    │   │   ├── main.tf
    │   │   └── security.tf
    │   ├── prod
    │   │   ├── _common.tf -> ../_common.tf
    │   │   ├── main.tf
    │   │   └── security.tf
    │   ├── sandbox
    │   │   ├── _common.tf -> ../_common.tf
    │   │   ├── main.tf
    │   │   └── security.tf
    │   └── stg
    │       ├── _common.tf -> ../_common.tf
    │       ├── main.tf
    │       └── security.tf
    └── token-handler
        ├── README.md
        ├── __global.tf
        ├── environments
        │   ├── _common.tf
        │   ├── _variables-legacy.tf
        │   ├── dev
        │   │   ├── __global.tf -> ../../__global.tf
        │   │   ├── _common.tf -> ../_common.tf
        │   │   ├── _variables-legacy.tf -> ../_variables-legacy.tf
        │   │   ├── cloudflare.auto.tfvars -> ../../secrets/acme-secrets/cloudflare.auto.tfvars
        │   │   ├── datadog.auto.tfvars -> ../../secrets/acme-secrets/datadog.auto.tfvars
        │   │   └── main.tf
        │   ├── prod
        │   │   ├── __global.tf -> ../../__global.tf
        │   │   ├── _common.tf -> ../_common.tf
        │   │   ├── _variables-legacy.tf -> ../_variables-legacy.tf
        │   │   ├── cloudflare.auto.tfvars -> ../../secrets/acme-secrets/cloudflare.auto.tfvars
        │   │   ├── datadog.auto.tfvars -> ../../secrets/acme-secrets/datadog.auto.tfvars
        │   │   └── main.tf
        │   ├── sandbox
        │   │   ├── __global.tf -> ../../__global.tf
        │   │   ├── _common.tf -> ../_common.tf
        │   │   ├── _variables-legacy.tf -> ../_variables-legacy.tf
        │   │   ├── cloudflare.auto.tfvars -> ../../secrets/acme-secrets/cloudflare.auto.tfvars
        │   │   ├── datadog.auto.tfvars -> ../../secrets/acme-secrets/datadog.auto.tfvars
        │   │   └── main.tf
        │   └── stg
        │       ├── __global.tf -> ../../__global.tf
        │       ├── _common.tf -> ../_common.tf
        │       ├── _variables-legacy.tf -> ../_variables-legacy.tf
        │       ├── cloudflare.auto.tfvars -> ../../secrets/acme-secrets/cloudflare.auto.tfvars
        │       ├── datadog.auto.tfvars -> ../../secrets/acme-secrets/datadog.auto.tfvars
        │       └── main.tf
        ├── modules
        │   ├── api-gateway
        │   │   ├── data.tf
        │   │   ├── main.tf
        │   │   ├── outputs.tf
        │   │   └── variables.tf
        │   └── main
        │       ├── data.tf
        │       ├── discovery.tf
        │       ├── locals.tf
        │       ├── main.tf
        │       ├── outputs.tf
        │       └── variables.tf
        └── secrets
radeksimko commented 1 year ago

@briceburg Thank you for describing the tree with symlinks. We can try to reproduce this but based on some other issues I have filed before I can imagine symlinks generally causing issues, performance-related or not.

I have transferred your comment here as the issue you commented on primarily focuses on different problem, which is processing that happens after file is opened.

The optimisation there could help in your case but it doesn't target symlinks in any way, so it would be incidental if you see improvements, rather than intentional.

Related:

radeksimko commented 1 year ago

We have done some investigating in https://github.com/hashicorp/terraform-ls/issues/1409 and have some ideas on how we can improve the performance when links are involved. It is however non-trivial, so while we still want to address this problem, we will have to weigh the complexity/effort against other work.

For these reasons (to aid prioritisation): @briceburg would you mind describing your motivations in more detail? As far as I understand, you're dealing with repetition between different customers, apps and different environments. What I'm specifically curious about is what problems are symlinks solving for you, which modules cannot solve, or solve in a suboptimal way?

On a separate note, I'm vaguely aware that symlinks are generally Unix concept and they may not translate well to Windows. How do you deal with this problem? Do you just assume/ensure that all users, CI systems etc. always run on Unix?

github-actions[bot] commented 1 year ago

Marking this issue as stale due to inactivity over the last 30 days. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

Thank you for understanding.

briceburg commented 11 months ago

@radeksimko I am terribly sorry for my delayed response.

would you mind describing your motivations in more detail?

primarily our reliance on symlinks is to maintain DRYness when defining providers, common variables/locals, and other core bits of the root configuration. while we previously used workspaces to handle this, in practice it became cumbersome and less flexible than having separate root configurations per environment per google's advice. we do use modules in this root configuration as would be expected, e.g.;

# foo-project/environments/production/main.tf

module "main" {
  source = "../../modules/main"
  ...
}

... but chose to use symlinks as a happy medium to stay DRY when switching from workspaces to using environment directories.

I'm vaguely aware that symlinks are generally Unix concept and they may not translate well to Windows. How do you deal with this problem?

thankfully git properly handles symlinks and I believe Windows 10+ support is better (os x is fine) -- although you are correct in that we ensure our CI systems are containerized and or under a linux runner.

notorand-it commented 9 months ago

Maybe I am wrong, but why not just let the language server open and parse symlinks when they match the *\.tf regex? It looks like the program is intentionally checking whether an inode object IS NOT a symlink before opening it. Terraform itself works well with symlynks, which are a useful way to centralize stuff without copying it over and over.

DownRangeDevOps commented 9 months ago

Workspaces are an obvious footgun. Symlinks are a more robust solution. It would be appreciated if terraform-ls parsed them correctly to eliminate false-positive not defined errors.

notorand-it commented 8 months ago

Indeed. Also because terraform-ls should conform to terraform itself. This deviating behavior by the language server is simply breaking the rules set by the same developer/company.

theherk commented 5 months ago

I'm in a similar situation, where I use a directory per state, but this causes some issues with the LSP.

In env/dev-eu-north-1, for example:

_main.tf -> ../../../shared/bases/main.tf
_providers.tf -> ../../../shared/bases/providers.tf
_variables.tf -> ../../../shared/bases/variables.tf
_versions.tf -> ../../../shared/bases/versions.tf
main.tf -> ../main.tf
outputs.tf -> ../outputs.tf
terraform.tfvars
variables.tf -> ../variables.tf
versions.tf

We get several shared base configurations that are linked into many states. Then, for a given domain's environment, we link to the directory just above, so that all states have the base configurations and the domain configurations. In that way, the only actual non-link files in the environment directory are the backend configuration in versions.tf and the variables in terraform.tfvars.

This works extremely well, except it make the LSP complain a bit and jumping to definitions doesn't work.

I'm not sure what the actual solution could be though, because in the files actual location, the source paths are just not correct, and I don't see a way for the LSP to intuit where it should be looking. It doesn't know where the file could be potentially linked.

However, it would be done with a comment directive like:

module "core" {
  # terraform-ls: source = "./module/domain-core"
  source = "../modules/domain-core"

  # ...
}