Work towards removing the packages under "internal/legacy" #34772
Conversation
apparentlymart
force-pushed
the
f-no-more-helperschema
branch
3 times, most recently
from
714e60b
to
e0dec7f
Compare
apparentlymart
force-pushed
the
f-no-more-helperschema
branch
from
658f73c
to
e42f008
Compare
|
The current state of this PR: All of the remote state backends are updated to use The more troublesome part of this is that the Terraform Core team isn't equipped to test most of these backends, since the accounts used for testing belong to the provider teams that maintain them. Therefore while I've made sure everything still compiles and run the tiny subset of tests that don't require real API access, I can't actually be sure that this hasn't broken parts of the backend functionality. We'll need to find a way to coordinate with the provider teams that maintain these backends to run the acceptance tests and do any other additional testing any teams feel is required based on their knowledge of gaps in the acceptance test coverage. |
apparentlymart
force-pushed
the
f-no-more-helperschema
branch
from
2537402
to
1ff650b
Compare
This now uses the backendbase package's "SDK-like" helpers instead, which provide a much smaller subset of the former legacy SDK functionality but enough for what this backend needs.
As part of our efforts to remove the large snapshot of the legacy SDK that we've been using, this replaces the uses of that package with our new lightweight "backendbase" package, which is a collection of helpers that can substitute for most of the parts of the legacy SDK that backends tend to use. For this backend for now we'll use the "SDK like" helpers which aim to retain some of the legacy-SDK-specific assumptions that aren't technically true anymore, such as the idea that null and empty string are equivalent. Hopefully one day we'll be able to wean this backend off of that more completely, but this is a pragmatic way to get away from the legacy SDK without a large rewrite. This also drew attention to the fact that this backend was previously relying on the weird context.Context value that the SDK was passing in to the configure method, even though that context isn't connected up to anything particularly useful. In the long run we should change the backend APIs to pass context.Context to each method that might make network requests, but for now we're using context.TODO so that we can more easily find these cases and update them later once there's a real context to use.
As with most of our remote state backends, this one was depending on just a tiny slice of the (enormous and now-poorly-understood) legacy SDK. In an effort to eliminate the legacy SDK snapshot from this codebase, this replaces it with functionality from our new "backendbase" package, which aims to provide just a narrow set of utilities to minimize the churn caused by removing the legacy SDK and thus reduce the risk of this change. This is currently using the "SDK-like" utilities, which emulate some of the questionable-but-convenient assumptions the legacy SDK makes, such as the idea that empty string and null are equivalent. Hopefully in future we can wean this backend even further off of these older assumptions, but the priority for now is to eliminate the legacy SDK without significantly disturbing the shape of the existing working code.
The legacy SDK is a very heavy dependency that this backend was only using tiny parts of. We'll now instead use our new "backendbase" package, which aims to provide a smaller set of helper functions that cover the main use-cases that the existing backends were relying on the SDK to meet, but with considerably less code and fewer layers of abstraction.
This package no longer has any callers. This also demotes the dependency on github.com/mitchellh/mapstructure to now only be indirect, so this slightly slims down Terraform's direct dependencies but we'll need to remove some more of the legacy packages to see significant benefits in that department.
This package is no longer called from anywhere in this codebase.
apparentlymart
force-pushed
the
f-no-more-helperschema
branch
from
1ff650b
to
2ee74e7
Compare
|
I merged a safer subset of this as #34806 so that we could at least make some progress. I don't expect this PR to actually move forward as-is, so I'm going to close it. Now that the |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Way back when we were working on Terraform v0.12 we retained a snapshot of the entire legacy Terraform SDK in this codebase purely to defer reworking the remote state backends to no longer depend on it.
Not only is the legacy SDK a huge amount of code that nobody at HashiCorp really understands anymore, it also has several unmaintained dependencies that we don't need anywhere else in Terraform that we'd rather no longer depend on.
The goal with this PR, then, is to make some further progress towards removing our use of this huge heap of unmaintained legacy code. This PR is currently a draft because it's unclear whether it'll be feasible to eradicate all of this in one go or if some of the backends are too complicated to migrate in a timely fashion.