command/meta: Fix PSS migration during provider upgrade#38244
command/meta: Fix PSS migration during provider upgrade#38244radeksimko wants to merge 2 commits intomainfrom
Conversation
dcecb82 to
447450d
Compare
SarahFrench
left a comment
There was a problem hiding this comment.
I've taken a look and left some comments. I'm going to ask internally for advice about how to handle the provider cache issue.
Also, while debugging I found some fixes needed for diagnostics: #38275 . It could be worth cherry-picking the change into your branch or rebasing once it's merged.
| factories, err := m.ProviderFactories() | ||
| factories, err := m.providerFactoriesFromLocks(locks) |
There was a problem hiding this comment.
Should this be using ProviderFactoriesFromLocks instead?
| // running provider instance inside the returned backend.Backend instance. | ||
| // Stopping the provider process is the responsibility of the calling code. | ||
|
|
||
| resp := provider.GetProviderSchema() |
There was a problem hiding this comment.
Through some debugging I've found that we're falling foul of the caching Core performs when accessing a provider's schemas. The cache's keys are based on provider Addr and that'll match between both versions of the same provider. We always use the newer version of the provider first, so that's in the cache when savedStateStore is invoked. This'll cause errors when processing config later in this method if there's a schema change (in provider or store) between the new and old provider versions.
I guess our options are to either update the cache to allow caching a schema per version:
type schemaCache struct {
mu sync.Mutex
- m map[addrs.Provider]ProviderSchema
+ m map[addrs.Provider]map[versions.Version]ProviderSchema
}Or we allow calling code to skip the cache. This would require updating the providers.Interface interface to pass in an argument that indicates we want to skip the cache:
terraform/internal/providers/provider.go
Lines 17 to 19 in 447450d
- GetProviderSchema() GetProviderSchemaResponse
+ GetProviderSchema(GetProviderSchemaRequest) GetProviderSchemaResponse&
type GetProviderSchemaRequest struct {
SkipCache bool
}I don't think this needs to touch the protocol itself through, as the cache is specific to Core (versus this protocol for rpcapi where the cache is external?).
However, I think we aim to make the providers.Interface interface match the plugin protocol, so updating the interface feels like a misstep (or at least something to discuss within the team).
Fixes #
Target Release
1.15.x
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry