pulumi/pkg/engine/lifecycletest/provider_test.go

1724 lines
53 KiB
Go
Raw Permalink Normal View History

package lifecycletest
import (
"fmt"
2021-07-28 19:12:53 +00:00
"sync"
"testing"
"github.com/blang/semver"
2021-07-28 19:12:53 +00:00
"github.com/gofrs/uuid"
"github.com/stretchr/testify/assert"
2021-07-28 19:12:53 +00:00
"github.com/stretchr/testify/require"
"github.com/pulumi/pulumi/pkg/v3/display"
2023-11-21 15:16:13 +00:00
. "github.com/pulumi/pulumi/pkg/v3/engine" //nolint:revive
"github.com/pulumi/pulumi/pkg/v3/resource/deploy"
"github.com/pulumi/pulumi/pkg/v3/resource/deploy/deploytest"
"github.com/pulumi/pulumi/pkg/v3/resource/deploy/providers"
"github.com/pulumi/pulumi/sdk/v3/go/common/apitype"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource/config"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
"github.com/pulumi/pulumi/sdk/v3/go/common/tokens"
"github.com/pulumi/pulumi/sdk/v3/go/common/workspace"
)
func TestSingleResourceDefaultProviderLifecycle(t *testing.T) {
t.Parallel()
Reuse provider instances where possible (#14127) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13987. This reworks the registry to better track provider instances such that we can reuse unconfigured instances between Creates, Updates, and Sames. When we allocate a provider instance in the registry for a Check call we save it with the special id "unconfigured". This value should never make its way back to program SDKs, it's purely an internal value for the engine. When we do a Create, Update or Same we look to see if there's an unconfigured provider to use and if so configures that one, else it starts up a fresh one. (N.B. Update we can assume there will always be an unconfigured one from the Check call before). This has also fixed registry Create to use the ID `UnknownID` rather than `""`, have added some contract assertions to check that and fixed up some test fallout because of that (the tests had been getting away with leaving ID blank before). ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [ ] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-10-12 20:46:01 +00:00
startupCount := 0
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
Reuse provider instances where possible (#14127) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13987. This reworks the registry to better track provider instances such that we can reuse unconfigured instances between Creates, Updates, and Sames. When we allocate a provider instance in the registry for a Check call we save it with the special id "unconfigured". This value should never make its way back to program SDKs, it's purely an internal value for the engine. When we do a Create, Update or Same we look to see if there's an unconfigured provider to use and if so configures that one, else it starts up a fresh one. (N.B. Update we can assume there will always be an unconfigured one from the Check call before). This has also fixed registry Create to use the ID `UnknownID` rather than `""`, have added some contract assertions to check that and fixed up some test fallout because of that (the tests had been getting away with leaving ID blank before). ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [ ] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-10-12 20:46:01 +00:00
startupCount++
return &deploytest.Provider{}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: MakeBasicLifecycleSteps(t, 2),
}
p.Run(t, nil)
Reuse provider instances where possible (#14127) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13987. This reworks the registry to better track provider instances such that we can reuse unconfigured instances between Creates, Updates, and Sames. When we allocate a provider instance in the registry for a Check call we save it with the special id "unconfigured". This value should never make its way back to program SDKs, it's purely an internal value for the engine. When we do a Create, Update or Same we look to see if there's an unconfigured provider to use and if so configures that one, else it starts up a fresh one. (N.B. Update we can assume there will always be an unconfigured one from the Check call before). This has also fixed registry Create to use the ID `UnknownID` rather than `""`, have added some contract assertions to check that and fixed up some test fallout because of that (the tests had been getting away with leaving ID blank before). ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [ ] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-10-12 20:46:01 +00:00
// We should have started the provider 10 times, twice for each of the steps in the basic lifecycle (one preview,
// one up), but zero for the last refresh step where the provider is not needed.
assert.Equal(t, 10, startupCount)
}
func TestSingleResourceExplicitProviderLifecycle(t *testing.T) {
t.Parallel()
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(providers.MakeProviderType("pkgA"), "provA", true)
assert.NoError(t, err)
provID := resp.ID
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
Provider: provRef.String(),
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: MakeBasicLifecycleSteps(t, 2),
}
p.Run(t, nil)
}
func TestSingleResourceDefaultProviderUpgrade(t *testing.T) {
t.Parallel()
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
}
provURN := p.NewProviderURN("pkgA", "default", "")
resURN := p.NewURN("pkgA:m:typA", "resA", "")
// Create an old snapshot with an existing copy of the single resource and no providers.
old := &deploy.Snapshot{
Resources: []*resource.State{{
Type: resURN.Type(),
URN: resURN,
Custom: true,
ID: "0",
Inputs: resource.PropertyMap{},
Outputs: resource.PropertyMap{},
}},
}
isRefresh := false
validate := func(project workspace.Project, target deploy.Target, entries JournalEntries,
_ []Event, err error,
) error {
require.NoError(t, err)
// Should see only sames: the default provider should be injected into the old state before the update
// runs.
for _, entry := range entries {
switch urn := entry.Step.URN(); urn {
case provURN, resURN:
expect := deploy.OpSame
if isRefresh {
expect = deploy.OpRefresh
}
assert.Equal(t, expect, entry.Step.Op())
default:
t.Fatalf("unexpected resource %v", urn)
}
}
[engine] Only record a resource's chosen alias. (#9288) As we discovered when removing aliases from the state entirely, the snapshotter needs to be alias-aware so that it can fix up references to resources that were aliased. After a resource operation finishes, the snapshotter needs to write out a new copy of the snapshot. However, at the time we write the snapshot, there may be resources that have not yet been registered that refer to the just-registered resources by a different URN due to aliasing. Those references need to be fixed up prior to writing the snapshot in order to preserve the snapshot's integrity (in particular, the property that all URNs refer to resources that exist in the snapshot). For example, consider the following simple dependency graph: A <-- B. When that graph is serialized, B will contain a reference to A in its dependency list. Let the next run of the program produces the graph A' <-- B where A' is aliased to A. After A' is registered, the snapshotter needs to write a snapshot that contains its state, but B must also be updated so it references A' instead of A, which will no longer be in the snapshot. These changes take advantage of the fact that although a resource can provide multiple aliases, it can only ever resolve those aliases to a single resource in the existing state. Therefore, at the time the statefile is fixed up, each resource in the statefile could only have been aliased to a single old resource, and it is sufficient to store only the URN of the chosen resource rather than all possible aliases. In addition to preserving the ability to fix up references to aliased resources, retaining the chosen alias allows the history of a logical resource to be followed across aliases.
2022-03-28 15:36:08 +00:00
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, 2)
return err
}
// Run a single update step using the base snapshot.
p.Steps = []TestStep{{Op: Update, Validate: validate}}
p.Run(t, old)
// Run a single refresh step using the base snapshot.
isRefresh = true
p.Steps = []TestStep{{Op: Refresh, Validate: validate}}
p.Run(t, old)
// Run a single destroy step using the base snapshot.
isRefresh = false
p.Steps = []TestStep{{
Op: Destroy,
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
_ []Event, err error,
) error {
require.NoError(t, err)
// Should see two deletes: the default provider should be injected into the old state before the update
// runs.
deleted := make(map[resource.URN]bool)
for _, entry := range entries {
switch urn := entry.Step.URN(); urn {
case provURN, resURN:
deleted[urn] = true
assert.Equal(t, deploy.OpDelete, entry.Step.Op())
default:
t.Fatalf("unexpected resource %v", urn)
}
}
assert.Len(t, deleted, 2)
[engine] Only record a resource's chosen alias. (#9288) As we discovered when removing aliases from the state entirely, the snapshotter needs to be alias-aware so that it can fix up references to resources that were aliased. After a resource operation finishes, the snapshotter needs to write out a new copy of the snapshot. However, at the time we write the snapshot, there may be resources that have not yet been registered that refer to the just-registered resources by a different URN due to aliasing. Those references need to be fixed up prior to writing the snapshot in order to preserve the snapshot's integrity (in particular, the property that all URNs refer to resources that exist in the snapshot). For example, consider the following simple dependency graph: A <-- B. When that graph is serialized, B will contain a reference to A in its dependency list. Let the next run of the program produces the graph A' <-- B where A' is aliased to A. After A' is registered, the snapshotter needs to write a snapshot that contains its state, but B must also be updated so it references A' instead of A, which will no longer be in the snapshot. These changes take advantage of the fact that although a resource can provide multiple aliases, it can only ever resolve those aliases to a single resource in the existing state. Therefore, at the time the statefile is fixed up, each resource in the statefile could only have been aliased to a single old resource, and it is sufficient to store only the URN of the chosen resource rather than all possible aliases. In addition to preserving the ability to fix up references to aliased resources, retaining the chosen alias allows the history of a logical resource to be followed across aliases.
2022-03-28 15:36:08 +00:00
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, 0)
return err
},
}}
p.Run(t, old)
// Run a partial lifecycle using the base snapshot, skipping the initial update step.
p.Steps = MakeBasicLifecycleSteps(t, 2)[1:]
p.Run(t, old)
}
func TestSingleResourceDefaultProviderReplace(t *testing.T) {
t.Parallel()
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{
DiffConfigF: func(urn resource.URN, oldInputs, oldOutputs, newInputs resource.PropertyMap,
all: Reformat with gofumpt Per team discussion, switching to gofumpt. [gofumpt][1] is an alternative, stricter alternative to gofmt. It addresses other stylistic concerns that gofmt doesn't yet cover. [1]: https://github.com/mvdan/gofumpt See the full list of [Added rules][2], but it includes: - Dropping empty lines around function bodies - Dropping unnecessary variable grouping when there's only one variable - Ensuring an empty line between multi-line functions - simplification (`-s` in gofmt) is always enabled - Ensuring multi-line function signatures end with `) {` on a separate line. [2]: https://github.com/mvdan/gofumpt#Added-rules gofumpt is stricter, but there's no lock-in. All gofumpt output is valid gofmt output, so if we decide we don't like it, it's easy to switch back without any code changes. gofumpt support is built into the tooling we use for development so this won't change development workflows. - golangci-lint includes a gofumpt check (enabled in this PR) - gopls, the LSP for Go, includes a gofumpt option (see [installation instrutions][3]) [3]: https://github.com/mvdan/gofumpt#installation This change was generated by running: ```bash gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error) ``` The following files were manually tweaked afterwards: - pkg/cmd/pulumi/stack_change_secrets_provider.go: one of the lines overflowed and had comments in an inconvenient place - pkg/cmd/pulumi/destroy.go: `var x T = y` where `T` wasn't necessary - pkg/cmd/pulumi/policy_new.go: long line because of error message - pkg/backend/snapshot_test.go: long line trying to assign three variables in the same assignment I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 16:36:39 +00:00
ignoreChanges []string,
) (plugin.DiffResult, error) {
// Always require replacement.
keys := []resource.PropertyKey{}
for k := range newInputs {
keys = append(keys, k)
}
return plugin.DiffResult{ReplaceKeys: keys}, nil
},
}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Config: config.Map{
config.MustMakeKey("pkgA", "foo"): config.NewValue("bar"),
},
}
// Build a basic lifecycle.
steps := MakeBasicLifecycleSteps(t, 2)
// Run the lifecycle through its no-op update+refresh.
p.Steps = steps[:4]
snap := p.Run(t, nil)
// Change the config and run an update. We expect everything to require replacement.
p.Config[config.MustMakeKey("pkgA", "foo")] = config.NewValue("baz")
p.Steps = []TestStep{{
Op: Update,
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
_ []Event, err error,
) error {
provURN := p.NewProviderURN("pkgA", "default", "")
resURN := p.NewURN("pkgA:m:typA", "resA", "")
// Look for replace steps on the provider and the resource.
replacedProvider, replacedResource := false, false
for _, entry := range entries {
if entry.Kind != JournalEntrySuccess || entry.Step.Op() != deploy.OpDeleteReplaced {
continue
}
switch urn := entry.Step.URN(); urn {
case provURN:
replacedProvider = true
case resURN:
replacedResource = true
default:
t.Fatalf("unexpected resource %v", urn)
}
}
assert.True(t, replacedProvider)
assert.True(t, replacedResource)
return err
},
}}
snap = p.Run(t, snap)
// Resume the lifecycle with another no-op update.
p.Steps = steps[2:]
p.Run(t, snap)
}
func TestSingleResourceExplicitProviderReplace(t *testing.T) {
t.Parallel()
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{
DiffConfigF: func(urn resource.URN, oldInputs, oldOutputs, newInputs resource.PropertyMap,
all: Reformat with gofumpt Per team discussion, switching to gofumpt. [gofumpt][1] is an alternative, stricter alternative to gofmt. It addresses other stylistic concerns that gofmt doesn't yet cover. [1]: https://github.com/mvdan/gofumpt See the full list of [Added rules][2], but it includes: - Dropping empty lines around function bodies - Dropping unnecessary variable grouping when there's only one variable - Ensuring an empty line between multi-line functions - simplification (`-s` in gofmt) is always enabled - Ensuring multi-line function signatures end with `) {` on a separate line. [2]: https://github.com/mvdan/gofumpt#Added-rules gofumpt is stricter, but there's no lock-in. All gofumpt output is valid gofmt output, so if we decide we don't like it, it's easy to switch back without any code changes. gofumpt support is built into the tooling we use for development so this won't change development workflows. - golangci-lint includes a gofumpt check (enabled in this PR) - gopls, the LSP for Go, includes a gofumpt option (see [installation instrutions][3]) [3]: https://github.com/mvdan/gofumpt#installation This change was generated by running: ```bash gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error) ``` The following files were manually tweaked afterwards: - pkg/cmd/pulumi/stack_change_secrets_provider.go: one of the lines overflowed and had comments in an inconvenient place - pkg/cmd/pulumi/destroy.go: `var x T = y` where `T` wasn't necessary - pkg/cmd/pulumi/policy_new.go: long line because of error message - pkg/backend/snapshot_test.go: long line trying to assign three variables in the same assignment I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 16:36:39 +00:00
ignoreChanges []string,
) (plugin.DiffResult, error) {
// Always require replacement.
keys := []resource.PropertyKey{}
for k := range newInputs {
keys = append(keys, k)
}
return plugin.DiffResult{ReplaceKeys: keys}, nil
},
}, nil
}),
}
providerInputs := resource.PropertyMap{
resource.PropertyKey("foo"): resource.NewStringProperty("bar"),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(providers.MakeProviderType("pkgA"), "provA", true,
deploytest.ResourceOptions{Inputs: providerInputs})
assert.NoError(t, err)
provID := resp.ID
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
Provider: provRef.String(),
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
}
// Build a basic lifecycle.
steps := MakeBasicLifecycleSteps(t, 2)
// Run the lifecycle through its no-op update+refresh.
p.Steps = steps[:4]
snap := p.Run(t, nil)
// Change the config and run an update. We expect everything to require replacement.
providerInputs[resource.PropertyKey("foo")] = resource.NewStringProperty("baz")
p.Steps = []TestStep{{
Op: Update,
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
_ []Event, err error,
) error {
provURN := p.NewProviderURN("pkgA", "provA", "")
resURN := p.NewURN("pkgA:m:typA", "resA", "")
// Look for replace steps on the provider and the resource.
replacedProvider, replacedResource := false, false
for _, entry := range entries {
if entry.Kind != JournalEntrySuccess || entry.Step.Op() != deploy.OpDeleteReplaced {
continue
}
switch urn := entry.Step.URN(); urn {
case provURN:
replacedProvider = true
case resURN:
replacedResource = true
default:
t.Fatalf("unexpected resource %v", urn)
}
}
assert.True(t, replacedProvider)
assert.True(t, replacedResource)
return err
},
}}
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
snap = p.RunWithName(t, snap, "0")
// Resume the lifecycle with another no-op update.
p.Steps = steps[2:]
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
p.RunWithName(t, snap, "1")
}
2021-07-28 19:12:53 +00:00
type configurableProvider struct {
id string
replace bool
creates *sync.Map
deletes *sync.Map
}
func (p *configurableProvider) configure(news resource.PropertyMap) error {
p.id = news["id"].StringValue()
return nil
}
func (p *configurableProvider) create(urn resource.URN, inputs resource.PropertyMap, timeout float64,
all: Reformat with gofumpt Per team discussion, switching to gofumpt. [gofumpt][1] is an alternative, stricter alternative to gofmt. It addresses other stylistic concerns that gofmt doesn't yet cover. [1]: https://github.com/mvdan/gofumpt See the full list of [Added rules][2], but it includes: - Dropping empty lines around function bodies - Dropping unnecessary variable grouping when there's only one variable - Ensuring an empty line between multi-line functions - simplification (`-s` in gofmt) is always enabled - Ensuring multi-line function signatures end with `) {` on a separate line. [2]: https://github.com/mvdan/gofumpt#Added-rules gofumpt is stricter, but there's no lock-in. All gofumpt output is valid gofmt output, so if we decide we don't like it, it's easy to switch back without any code changes. gofumpt support is built into the tooling we use for development so this won't change development workflows. - golangci-lint includes a gofumpt check (enabled in this PR) - gopls, the LSP for Go, includes a gofumpt option (see [installation instrutions][3]) [3]: https://github.com/mvdan/gofumpt#installation This change was generated by running: ```bash gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error) ``` The following files were manually tweaked afterwards: - pkg/cmd/pulumi/stack_change_secrets_provider.go: one of the lines overflowed and had comments in an inconvenient place - pkg/cmd/pulumi/destroy.go: `var x T = y` where `T` wasn't necessary - pkg/cmd/pulumi/policy_new.go: long line because of error message - pkg/backend/snapshot_test.go: long line trying to assign three variables in the same assignment I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 16:36:39 +00:00
preview bool,
) (resource.ID, resource.PropertyMap, resource.Status, error) {
2021-07-28 19:12:53 +00:00
uid, err := uuid.NewV4()
if err != nil {
return "", nil, resource.StatusUnknown, err
}
id := resource.ID(uid.String())
p.creates.Store(id, p.id)
return id, inputs, resource.StatusOK, nil
}
Send old inputs to Delete (#14051) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/14115. This was missed as part of https://github.com/pulumi/pulumi/pull/13139. Adds a new configure flag (sends_old_inputs_to_delete) which the engine will now always set to true. If that's set providers can rely on the old inputs being sent to delete, otherwise they'll get nil. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [ ] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-10-13 14:12:26 +00:00
func (p *configurableProvider) delete(urn resource.URN, id resource.ID, oldInputs, oldOutputs resource.PropertyMap,
all: Reformat with gofumpt Per team discussion, switching to gofumpt. [gofumpt][1] is an alternative, stricter alternative to gofmt. It addresses other stylistic concerns that gofmt doesn't yet cover. [1]: https://github.com/mvdan/gofumpt See the full list of [Added rules][2], but it includes: - Dropping empty lines around function bodies - Dropping unnecessary variable grouping when there's only one variable - Ensuring an empty line between multi-line functions - simplification (`-s` in gofmt) is always enabled - Ensuring multi-line function signatures end with `) {` on a separate line. [2]: https://github.com/mvdan/gofumpt#Added-rules gofumpt is stricter, but there's no lock-in. All gofumpt output is valid gofmt output, so if we decide we don't like it, it's easy to switch back without any code changes. gofumpt support is built into the tooling we use for development so this won't change development workflows. - golangci-lint includes a gofumpt check (enabled in this PR) - gopls, the LSP for Go, includes a gofumpt option (see [installation instrutions][3]) [3]: https://github.com/mvdan/gofumpt#installation This change was generated by running: ```bash gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error) ``` The following files were manually tweaked afterwards: - pkg/cmd/pulumi/stack_change_secrets_provider.go: one of the lines overflowed and had comments in an inconvenient place - pkg/cmd/pulumi/destroy.go: `var x T = y` where `T` wasn't necessary - pkg/cmd/pulumi/policy_new.go: long line because of error message - pkg/backend/snapshot_test.go: long line trying to assign three variables in the same assignment I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 16:36:39 +00:00
timeout float64,
) (resource.Status, error) {
2021-07-28 19:12:53 +00:00
p.deletes.Store(id, p.id)
return resource.StatusOK, nil
}
// TestSingleResourceExplicitProviderAliasUpdateDelete verifies that providers respect aliases during updates, and
// that the correct instance of an explicit provider is used to delete a removed resource.
func TestSingleResourceExplicitProviderAliasUpdateDelete(t *testing.T) {
t.Parallel()
2021-07-28 19:12:53 +00:00
var creates, deletes sync.Map
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
configurable := &configurableProvider{
creates: &creates,
deletes: &deletes,
}
return &deploytest.Provider{
DiffConfigF: func(urn resource.URN, oldInputs, oldOutputs, newInputs resource.PropertyMap,
all: Reformat with gofumpt Per team discussion, switching to gofumpt. [gofumpt][1] is an alternative, stricter alternative to gofmt. It addresses other stylistic concerns that gofmt doesn't yet cover. [1]: https://github.com/mvdan/gofumpt See the full list of [Added rules][2], but it includes: - Dropping empty lines around function bodies - Dropping unnecessary variable grouping when there's only one variable - Ensuring an empty line between multi-line functions - simplification (`-s` in gofmt) is always enabled - Ensuring multi-line function signatures end with `) {` on a separate line. [2]: https://github.com/mvdan/gofumpt#Added-rules gofumpt is stricter, but there's no lock-in. All gofumpt output is valid gofmt output, so if we decide we don't like it, it's easy to switch back without any code changes. gofumpt support is built into the tooling we use for development so this won't change development workflows. - golangci-lint includes a gofumpt check (enabled in this PR) - gopls, the LSP for Go, includes a gofumpt option (see [installation instrutions][3]) [3]: https://github.com/mvdan/gofumpt#installation This change was generated by running: ```bash gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error) ``` The following files were manually tweaked afterwards: - pkg/cmd/pulumi/stack_change_secrets_provider.go: one of the lines overflowed and had comments in an inconvenient place - pkg/cmd/pulumi/destroy.go: `var x T = y` where `T` wasn't necessary - pkg/cmd/pulumi/policy_new.go: long line because of error message - pkg/backend/snapshot_test.go: long line trying to assign three variables in the same assignment I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 16:36:39 +00:00
ignoreChanges []string,
) (plugin.DiffResult, error) {
2021-07-28 19:12:53 +00:00
return plugin.DiffResult{}, nil
},
ConfigureF: configurable.configure,
CreateF: configurable.create,
DeleteF: configurable.delete,
}, nil
}),
}
providerInputs := resource.PropertyMap{
resource.PropertyKey("id"): resource.NewStringProperty("first"),
}
providerName := "provA"
aliases := []resource.URN{}
registerResource := true
var resourceID resource.ID
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(providers.MakeProviderType("pkgA"), providerName, true,
2021-07-28 19:12:53 +00:00
deploytest.ResourceOptions{
Inputs: providerInputs,
AliasURNs: aliases,
2021-07-28 19:12:53 +00:00
})
assert.NoError(t, err)
provID := resp.ID
2021-07-28 19:12:53 +00:00
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
2021-07-28 19:12:53 +00:00
assert.NoError(t, err)
if registerResource {
resp, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
2021-07-28 19:12:53 +00:00
Provider: provRef.String(),
})
assert.NoError(t, err)
resourceID = resp.ID
2021-07-28 19:12:53 +00:00
}
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
2021-07-28 19:12:53 +00:00
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
2021-07-28 19:12:53 +00:00
}
// Build a basic lifecycle.
steps := MakeBasicLifecycleSteps(t, 2)
// Run the lifecycle through its initial update+refresh.
p.Steps = steps[:4]
snap := p.Run(t, nil)
// Add a provider alias to the original URN.
aliases = []resource.URN{
p.NewProviderURN("pkgA", "provA", ""),
}
// Change the provider name and configuration and remove the resource. This will cause an Update for the provider
// and a Delete for the resource. The updated provider instance should be used to perform the delete.
providerName = "provB"
providerInputs[resource.PropertyKey("id")] = resource.NewStringProperty("second")
registerResource = false
p.Steps = []TestStep{{Op: Update}}
_ = p.Run(t, snap)
// Check the identity of the provider that performed the delete.
deleterID, ok := deletes.Load(resourceID)
require.True(t, ok)
assert.Equal(t, "second", deleterID)
}
// TestSingleResourceExplicitProviderAliasReplace verifies that providers respect aliases,
// and propagate replaces as a result of an aliased provider diff.
func TestSingleResourceExplicitProviderAliasReplace(t *testing.T) {
t.Parallel()
2021-07-28 19:12:53 +00:00
var creates, deletes sync.Map
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
configurable := &configurableProvider{
replace: true,
creates: &creates,
deletes: &deletes,
}
return &deploytest.Provider{
DiffConfigF: func(urn resource.URN, oldInputs, oldOutputs, newInputs resource.PropertyMap,
all: Reformat with gofumpt Per team discussion, switching to gofumpt. [gofumpt][1] is an alternative, stricter alternative to gofmt. It addresses other stylistic concerns that gofmt doesn't yet cover. [1]: https://github.com/mvdan/gofumpt See the full list of [Added rules][2], but it includes: - Dropping empty lines around function bodies - Dropping unnecessary variable grouping when there's only one variable - Ensuring an empty line between multi-line functions - simplification (`-s` in gofmt) is always enabled - Ensuring multi-line function signatures end with `) {` on a separate line. [2]: https://github.com/mvdan/gofumpt#Added-rules gofumpt is stricter, but there's no lock-in. All gofumpt output is valid gofmt output, so if we decide we don't like it, it's easy to switch back without any code changes. gofumpt support is built into the tooling we use for development so this won't change development workflows. - golangci-lint includes a gofumpt check (enabled in this PR) - gopls, the LSP for Go, includes a gofumpt option (see [installation instrutions][3]) [3]: https://github.com/mvdan/gofumpt#installation This change was generated by running: ```bash gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error) ``` The following files were manually tweaked afterwards: - pkg/cmd/pulumi/stack_change_secrets_provider.go: one of the lines overflowed and had comments in an inconvenient place - pkg/cmd/pulumi/destroy.go: `var x T = y` where `T` wasn't necessary - pkg/cmd/pulumi/policy_new.go: long line because of error message - pkg/backend/snapshot_test.go: long line trying to assign three variables in the same assignment I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 16:36:39 +00:00
ignoreChanges []string,
) (plugin.DiffResult, error) {
2021-07-28 19:12:53 +00:00
keys := []resource.PropertyKey{}
for k := range newInputs {
2021-07-28 19:12:53 +00:00
keys = append(keys, k)
}
return plugin.DiffResult{ReplaceKeys: keys}, nil
},
ConfigureF: configurable.configure,
CreateF: configurable.create,
DeleteF: configurable.delete,
}, nil
}),
}
providerInputs := resource.PropertyMap{
resource.PropertyKey("id"): resource.NewStringProperty("first"),
}
providerName := "provA"
aliases := []resource.URN{}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(providers.MakeProviderType("pkgA"), providerName, true,
2021-07-28 19:12:53 +00:00
deploytest.ResourceOptions{
Inputs: providerInputs,
AliasURNs: aliases,
2021-07-28 19:12:53 +00:00
})
assert.NoError(t, err)
provID := resp.ID
2021-07-28 19:12:53 +00:00
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
2021-07-28 19:12:53 +00:00
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
2021-07-28 19:12:53 +00:00
Provider: provRef.String(),
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
2021-07-28 19:12:53 +00:00
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
2021-07-28 19:12:53 +00:00
}
// Build a basic lifecycle.
steps := MakeBasicLifecycleSteps(t, 2)
// Run the lifecycle through its no-op update+refresh.
p.Steps = steps[:4]
snap := p.Run(t, nil)
// add a provider alias to the original URN
aliases = []resource.URN{
p.NewProviderURN("pkgA", "provA", ""),
}
// change the provider name
providerName = "provB"
// run an update expecting no-op respecting the aliases.
p.Steps = []TestStep{{
Op: Update,
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
_ []Event, err error,
) error {
2021-07-28 19:12:53 +00:00
for _, entry := range entries {
if entry.Step.Op() != deploy.OpSame {
t.Fatalf("update should contain no changes: %v", entry.Step.URN())
}
}
return err
2021-07-28 19:12:53 +00:00
},
}}
snap = p.Run(t, snap)
// Change the config and run an update maintaining the alias. We expect everything to require replacement.
providerInputs[resource.PropertyKey("id")] = resource.NewStringProperty("second")
p.Steps = []TestStep{{
Op: Update,
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
_ []Event, err error,
) error {
2021-07-28 19:12:53 +00:00
provURN := p.NewProviderURN("pkgA", providerName, "")
resURN := p.NewURN("pkgA:m:typA", "resA", "")
// Find the delete and create IDs for the resource.
var createdID, deletedID resource.ID
// Look for replace steps on the provider and the resource.
replacedProvider, replacedResource := false, false
for _, entry := range entries {
op := entry.Step.Op()
if entry.Step.URN() == resURN {
switch op {
case deploy.OpCreateReplacement:
createdID = entry.Step.New().ID
case deploy.OpDeleteReplaced:
deletedID = entry.Step.Old().ID
}
}
if entry.Kind != JournalEntrySuccess || op != deploy.OpDeleteReplaced {
continue
}
switch urn := entry.Step.URN(); urn {
case provURN:
replacedProvider = true
case resURN:
replacedResource = true
default:
t.Fatalf("unexpected resource %v", urn)
}
}
assert.True(t, replacedProvider)
assert.True(t, replacedResource)
// Check the identities of the providers that performed the create and delete.
//
// For a replacement, the newly-created provider should be used to create the new resource, and the original
// provider should be used to delete the old resource.
creatorID, ok := creates.Load(createdID)
require.True(t, ok)
assert.Equal(t, "second", creatorID)
deleterID, ok := deletes.Load(deletedID)
require.True(t, ok)
assert.Equal(t, "first", deleterID)
return err
2021-07-28 19:12:53 +00:00
},
}}
snap = p.Run(t, snap)
// Resume the lifecycle with another no-op update.
p.Steps = steps[2:]
p.Run(t, snap)
}
func TestSingleResourceExplicitProviderDeleteBeforeReplace(t *testing.T) {
t.Parallel()
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{
DiffConfigF: func(urn resource.URN, oldInputs, oldOutputs, newInputs resource.PropertyMap,
all: Reformat with gofumpt Per team discussion, switching to gofumpt. [gofumpt][1] is an alternative, stricter alternative to gofmt. It addresses other stylistic concerns that gofmt doesn't yet cover. [1]: https://github.com/mvdan/gofumpt See the full list of [Added rules][2], but it includes: - Dropping empty lines around function bodies - Dropping unnecessary variable grouping when there's only one variable - Ensuring an empty line between multi-line functions - simplification (`-s` in gofmt) is always enabled - Ensuring multi-line function signatures end with `) {` on a separate line. [2]: https://github.com/mvdan/gofumpt#Added-rules gofumpt is stricter, but there's no lock-in. All gofumpt output is valid gofmt output, so if we decide we don't like it, it's easy to switch back without any code changes. gofumpt support is built into the tooling we use for development so this won't change development workflows. - golangci-lint includes a gofumpt check (enabled in this PR) - gopls, the LSP for Go, includes a gofumpt option (see [installation instrutions][3]) [3]: https://github.com/mvdan/gofumpt#installation This change was generated by running: ```bash gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error) ``` The following files were manually tweaked afterwards: - pkg/cmd/pulumi/stack_change_secrets_provider.go: one of the lines overflowed and had comments in an inconvenient place - pkg/cmd/pulumi/destroy.go: `var x T = y` where `T` wasn't necessary - pkg/cmd/pulumi/policy_new.go: long line because of error message - pkg/backend/snapshot_test.go: long line trying to assign three variables in the same assignment I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 16:36:39 +00:00
ignoreChanges []string,
) (plugin.DiffResult, error) {
// Always require replacement.
keys := []resource.PropertyKey{}
for k := range newInputs {
keys = append(keys, k)
}
return plugin.DiffResult{ReplaceKeys: keys, DeleteBeforeReplace: true}, nil
},
}, nil
}),
}
providerInputs := resource.PropertyMap{
resource.PropertyKey("foo"): resource.NewStringProperty("bar"),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(providers.MakeProviderType("pkgA"), "provA", true,
deploytest.ResourceOptions{Inputs: providerInputs})
assert.NoError(t, err)
provID := resp.ID
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
Provider: provRef.String(),
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
}
// Build a basic lifecycle.
steps := MakeBasicLifecycleSteps(t, 2)
// Run the lifecycle through its no-op update+refresh.
p.Steps = steps[:4]
snap := p.Run(t, nil)
// Change the config and run an update. We expect everything to require replacement.
providerInputs[resource.PropertyKey("foo")] = resource.NewStringProperty("baz")
p.Steps = []TestStep{{
Op: Update,
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
_ []Event, err error,
) error {
provURN := p.NewProviderURN("pkgA", "provA", "")
resURN := p.NewURN("pkgA:m:typA", "resA", "")
// Look for replace steps on the provider and the resource.
createdProvider, createdResource := false, false
deletedProvider, deletedResource := false, false
for _, entry := range entries {
if entry.Kind != JournalEntrySuccess {
continue
}
switch urn := entry.Step.URN(); urn {
case provURN:
if entry.Step.Op() == deploy.OpDeleteReplaced {
assert.False(t, createdProvider)
assert.False(t, createdResource)
assert.True(t, deletedResource)
deletedProvider = true
} else if entry.Step.Op() == deploy.OpCreateReplacement {
assert.True(t, deletedProvider)
assert.True(t, deletedResource)
assert.False(t, createdResource)
createdProvider = true
}
case resURN:
if entry.Step.Op() == deploy.OpDeleteReplaced {
assert.False(t, deletedProvider)
assert.False(t, deletedResource)
deletedResource = true
} else if entry.Step.Op() == deploy.OpCreateReplacement {
assert.True(t, deletedProvider)
assert.True(t, deletedResource)
assert.True(t, createdProvider)
createdResource = true
}
default:
t.Fatalf("unexpected resource %v", urn)
}
}
assert.True(t, deletedProvider)
assert.True(t, deletedResource)
return err
},
}}
snap = p.Run(t, snap)
// Resume the lifecycle with another no-op update.
p.Steps = steps[2:]
p.Run(t, snap)
}
// TestDefaultProviderDiff tests that the engine can gracefully recover whenever a resource's default provider changes
// and there is no diff in the provider's inputs.
func TestDefaultProviderDiff(t *testing.T) {
t.Parallel()
const resName, resBName = "resA", "resB"
Don't load providers at startup This changes the provider registry to no longer load all the providers from the old state on startup (in `NewRegistry`) instead the load logic has been moved to the `Same` method. The step_executor and step_generator have been fixed up to ensure that for cases where a resource might not have had it's provider created yet (i.e. for DBR'ing the old version of a resource, for refreshes or deletes) they ask the `Deployment` to look up the provider in the old state and `Same` it in the registry. All of the above means we only load providers we're going to use (even taking --targets into account). One fix mot done in this change is to auto-update providers for deletes. That is given a program state with two resources both using V1 of a provider, if you run the program to update one of those resource to use V2 of the provider but to delete the other resource currently we'll still load V1 to do that delete. It _might_ be possible (although this is definitly questionable) to see that another resource changed it's provider from V1 to V2 and to just assume the same change should have happened to the deleted resource. This could be helpful for not loading old provider versions at all, but can be done in two passes now pretty easily. Just run `up` without any program changes except for the SDK version bump to update all the provider references to V2 of the provider, then do another `up` that deletes the second resource. Fixes https://github.com/pulumi/pulumi/issues/12177.
2023-04-12 09:35:20 +00:00
expect1710 := true
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("0.17.10"), func() (plugin.Provider, error) {
Don't load providers at startup This changes the provider registry to no longer load all the providers from the old state on startup (in `NewRegistry`) instead the load logic has been moved to the `Same` method. The step_executor and step_generator have been fixed up to ensure that for cases where a resource might not have had it's provider created yet (i.e. for DBR'ing the old version of a resource, for refreshes or deletes) they ask the `Deployment` to look up the provider in the old state and `Same` it in the registry. All of the above means we only load providers we're going to use (even taking --targets into account). One fix mot done in this change is to auto-update providers for deletes. That is given a program state with two resources both using V1 of a provider, if you run the program to update one of those resource to use V2 of the provider but to delete the other resource currently we'll still load V1 to do that delete. It _might_ be possible (although this is definitly questionable) to see that another resource changed it's provider from V1 to V2 and to just assume the same change should have happened to the deleted resource. This could be helpful for not loading old provider versions at all, but can be done in two passes now pretty easily. Just run `up` without any program changes except for the SDK version bump to update all the provider references to V2 of the provider, then do another `up` that deletes the second resource. Fixes https://github.com/pulumi/pulumi/issues/12177.
2023-04-12 09:35:20 +00:00
// If we don't expect to load this assert if called
if !expect1710 {
assert.Fail(t, "unexpected call to 0.17.10 provider")
}
return &deploytest.Provider{}, nil
}),
deploytest.NewProviderLoader("pkgA", semver.MustParse("0.17.11"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
deploytest.NewProviderLoader("pkgA", semver.MustParse("0.17.12"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
}
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
runProgram := func(
base *deploy.Snapshot, versionA, versionB string, expectedStep display.StepOp, name string,
) *deploy.Snapshot {
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, err := monitor.RegisterResource("pkgA:m:typA", resName, true, deploytest.ResourceOptions{
Version: versionA,
})
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", resBName, true, deploytest.ResourceOptions{
Version: versionB,
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: []TestStep{
{
Op: Update,
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
events []Event, err error,
) error {
for _, entry := range entries {
if entry.Kind != JournalEntrySuccess {
continue
}
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
switch entry.Step.URN().Name() {
case resName, resBName:
assert.Equal(t, expectedStep, entry.Step.Op())
}
}
return err
},
},
},
}
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
return p.RunWithName(t, base, name)
}
// This test simulates the upgrade scenario of old-style default providers to new-style versioned default providers.
//
// The first update creates a stack using a language host that does not report a version to the engine. As a result,
// the engine makes up a default provider for "pkgA" and calls it "default". It then creates the two resources that
// we are creating and associates them with the default provider.
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
snap := runProgram(nil, "", "", deploy.OpCreate, "0")
for _, res := range snap.Resources {
switch {
case providers.IsDefaultProvider(res.URN):
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.Equal(t, "default", res.URN.Name())
case res.URN.Name() == resName || res.URN.Name() == resBName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.Equal(t, "default", provRef.URN().Name())
}
}
// The second update switches to a language host that does report a version to the engine. As a result, the engine
// uses this version to make a new provider, with a different URN, and uses that provider to operate on resA and
// resB.
//
// Despite switching out the provider, the engine should still generate a Same step for resA. It is vital that the
// engine gracefully react to changes in the default provider in this manner. See pulumi/pulumi#2753 for what
// happens when it doesn't.
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
snap = runProgram(snap, "0.17.10", "0.17.10", deploy.OpSame, "1")
for _, res := range snap.Resources {
switch {
case providers.IsDefaultProvider(res.URN):
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.Equal(t, "default_0_17_10", res.URN.Name())
case res.URN.Name() == resName || res.URN.Name() == resBName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.Equal(t, "default_0_17_10", provRef.URN().Name())
}
}
// The third update changes the version that the language host reports to the engine. This simulates a scenario in
// which a user updates their SDK to a new version of a provider package. In order to simulate side-by-side
// packages with different versions, this update requests distinct package versions for resA and resB.
Don't load providers at startup This changes the provider registry to no longer load all the providers from the old state on startup (in `NewRegistry`) instead the load logic has been moved to the `Same` method. The step_executor and step_generator have been fixed up to ensure that for cases where a resource might not have had it's provider created yet (i.e. for DBR'ing the old version of a resource, for refreshes or deletes) they ask the `Deployment` to look up the provider in the old state and `Same` it in the registry. All of the above means we only load providers we're going to use (even taking --targets into account). One fix mot done in this change is to auto-update providers for deletes. That is given a program state with two resources both using V1 of a provider, if you run the program to update one of those resource to use V2 of the provider but to delete the other resource currently we'll still load V1 to do that delete. It _might_ be possible (although this is definitly questionable) to see that another resource changed it's provider from V1 to V2 and to just assume the same change should have happened to the deleted resource. This could be helpful for not loading old provider versions at all, but can be done in two passes now pretty easily. Just run `up` without any program changes except for the SDK version bump to update all the provider references to V2 of the provider, then do another `up` that deletes the second resource. Fixes https://github.com/pulumi/pulumi/issues/12177.
2023-04-12 09:35:20 +00:00
expect1710 = false
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
snap = runProgram(snap, "0.17.11", "0.17.12", deploy.OpSame, "2")
for _, res := range snap.Resources {
switch {
case providers.IsDefaultProvider(res.URN):
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.True(t, res.URN.Name() == "default_0_17_11" || res.URN.Name() == "default_0_17_12")
case res.URN.Name() == resName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.Equal(t, "default_0_17_11", provRef.URN().Name())
case res.URN.Name() == resBName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.Equal(t, "default_0_17_12", provRef.URN().Name())
}
}
}
// TestDefaultProviderDiffReplacement tests that, when replacing a default provider for a resource, the engine will
// replace the resource if DiffConfig on the new provider returns a diff for the provider's new state.
func TestDefaultProviderDiffReplacement(t *testing.T) {
t.Parallel()
const resName, resBName = "resA", "resB"
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("0.17.10"), func() (plugin.Provider, error) {
return &deploytest.Provider{
// This implementation of DiffConfig always requests replacement.
DiffConfigF: func(_ resource.URN, oldInputs, oldOutputs, newInputs resource.PropertyMap,
all: Reformat with gofumpt Per team discussion, switching to gofumpt. [gofumpt][1] is an alternative, stricter alternative to gofmt. It addresses other stylistic concerns that gofmt doesn't yet cover. [1]: https://github.com/mvdan/gofumpt See the full list of [Added rules][2], but it includes: - Dropping empty lines around function bodies - Dropping unnecessary variable grouping when there's only one variable - Ensuring an empty line between multi-line functions - simplification (`-s` in gofmt) is always enabled - Ensuring multi-line function signatures end with `) {` on a separate line. [2]: https://github.com/mvdan/gofumpt#Added-rules gofumpt is stricter, but there's no lock-in. All gofumpt output is valid gofmt output, so if we decide we don't like it, it's easy to switch back without any code changes. gofumpt support is built into the tooling we use for development so this won't change development workflows. - golangci-lint includes a gofumpt check (enabled in this PR) - gopls, the LSP for Go, includes a gofumpt option (see [installation instrutions][3]) [3]: https://github.com/mvdan/gofumpt#installation This change was generated by running: ```bash gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error) ``` The following files were manually tweaked afterwards: - pkg/cmd/pulumi/stack_change_secrets_provider.go: one of the lines overflowed and had comments in an inconvenient place - pkg/cmd/pulumi/destroy.go: `var x T = y` where `T` wasn't necessary - pkg/cmd/pulumi/policy_new.go: long line because of error message - pkg/backend/snapshot_test.go: long line trying to assign three variables in the same assignment I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 16:36:39 +00:00
ignoreChanges []string,
) (plugin.DiffResult, error) {
keys := []resource.PropertyKey{}
for k := range newInputs {
keys = append(keys, k)
}
return plugin.DiffResult{
Changes: plugin.DiffSome,
ReplaceKeys: keys,
}, nil
},
}, nil
}),
deploytest.NewProviderLoader("pkgA", semver.MustParse("0.17.11"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
}
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
runProgram := func(base *deploy.Snapshot, name, versionA, versionB string,
all: Reformat with gofumpt Per team discussion, switching to gofumpt. [gofumpt][1] is an alternative, stricter alternative to gofmt. It addresses other stylistic concerns that gofmt doesn't yet cover. [1]: https://github.com/mvdan/gofumpt See the full list of [Added rules][2], but it includes: - Dropping empty lines around function bodies - Dropping unnecessary variable grouping when there's only one variable - Ensuring an empty line between multi-line functions - simplification (`-s` in gofmt) is always enabled - Ensuring multi-line function signatures end with `) {` on a separate line. [2]: https://github.com/mvdan/gofumpt#Added-rules gofumpt is stricter, but there's no lock-in. All gofumpt output is valid gofmt output, so if we decide we don't like it, it's easy to switch back without any code changes. gofumpt support is built into the tooling we use for development so this won't change development workflows. - golangci-lint includes a gofumpt check (enabled in this PR) - gopls, the LSP for Go, includes a gofumpt option (see [installation instrutions][3]) [3]: https://github.com/mvdan/gofumpt#installation This change was generated by running: ```bash gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error) ``` The following files were manually tweaked afterwards: - pkg/cmd/pulumi/stack_change_secrets_provider.go: one of the lines overflowed and had comments in an inconvenient place - pkg/cmd/pulumi/destroy.go: `var x T = y` where `T` wasn't necessary - pkg/cmd/pulumi/policy_new.go: long line because of error message - pkg/backend/snapshot_test.go: long line trying to assign three variables in the same assignment I have included mention of gofumpt in the CONTRIBUTING.md.
2023-03-03 16:36:39 +00:00
expectedSteps ...display.StepOp,
) *deploy.Snapshot {
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, err := monitor.RegisterResource("pkgA:m:typA", resName, true, deploytest.ResourceOptions{
Version: versionA,
})
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", resBName, true, deploytest.ResourceOptions{
Version: versionB,
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: []TestStep{
{
Op: Update,
Validate: func(project workspace.Project, target deploy.Target, entries JournalEntries,
events []Event, err error,
) error {
for _, entry := range entries {
if entry.Kind != JournalEntrySuccess {
continue
}
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
switch entry.Step.URN().Name() {
case resName:
assert.Subset(t, expectedSteps, []display.StepOp{entry.Step.Op()})
case resBName:
assert.Subset(t,
[]display.StepOp{deploy.OpCreate, deploy.OpSame}, []display.StepOp{entry.Step.Op()})
}
}
return err
},
},
},
}
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
return p.RunWithName(t, base, name)
}
// This test simulates the upgrade scenario of default providers, except that the requested upgrade results in the
// provider getting replaced. Because of this, the engine should decide to replace resA. It should not decide to
// replace resB, as its change does not require replacement.
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
snap := runProgram(nil, "0", "", "", deploy.OpCreate)
for _, res := range snap.Resources {
switch {
case providers.IsDefaultProvider(res.URN):
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.Equal(t, "default", res.URN.Name())
case res.URN.Name() == resName || res.URN.Name() == resBName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.Equal(t, "default", provRef.URN().Name())
}
}
// Upon update, now that the language host is sending a version, DiffConfig reports that there's a diff between the
// old and new provider and so we must replace resA.
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
snap = runProgram(snap, "1", "0.17.10", "0.17.11",
deploy.OpCreateReplacement, deploy.OpReplace, deploy.OpDeleteReplaced)
for _, res := range snap.Resources {
switch {
case providers.IsDefaultProvider(res.URN):
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.True(t, res.URN.Name() == "default_0_17_10" || res.URN.Name() == "default_0_17_11")
case res.URN.Name() == resName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.Equal(t, "default_0_17_10", provRef.URN().Name())
case res.URN.Name() == resBName:
provRef, err := providers.ParseReference(res.Provider)
assert.NoError(t, err)
Allow anything in resource names (#14107) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> Fixes https://github.com/pulumi/pulumi/issues/13968. Fixes https://github.com/pulumi/pulumi/issues/8949. This requires changing the parsing of URN's slightly, it is _very_ likely that providers will need to update to handle URNs like this correctly. This changes resource names to be `string` not `QName`. We never validated this before and it turns out that users have put all manner of text for resource names so we just updating the system to correctly reflect that. ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-11-20 08:59:00 +00:00
assert.Equal(t, "default_0_17_11", provRef.URN().Name())
}
}
}
func TestProviderVersionDefault(t *testing.T) {
t.Parallel()
version := ""
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
version = "1.0.0"
return &deploytest.Provider{}, nil
}),
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.5.0"), func() (plugin.Provider, error) {
version = "1.5.0"
return &deploytest.Provider{}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(providers.MakeProviderType("pkgA"), "provA", true)
assert.NoError(t, err)
provID := resp.ID
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
Provider: provRef.String(),
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: MakeBasicLifecycleSteps(t, 2),
}
p.Run(t, nil)
assert.Equal(t, "1.5.0", version)
}
func TestProviderVersionOption(t *testing.T) {
t.Parallel()
version := ""
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
version = "1.0.0"
return &deploytest.Provider{}, nil
}),
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.5.0"), func() (plugin.Provider, error) {
version = "1.5.0"
return &deploytest.Provider{}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(providers.MakeProviderType("pkgA"), "provA", true,
deploytest.ResourceOptions{
Version: "1.0.0",
})
assert.NoError(t, err)
provID := resp.ID
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
Provider: provRef.String(),
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: MakeBasicLifecycleSteps(t, 2),
}
p.Run(t, nil)
assert.Equal(t, "1.0.0", version)
}
func TestProviderVersionInput(t *testing.T) {
t.Parallel()
version := ""
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
version = "1.0.0"
return &deploytest.Provider{}, nil
}),
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.5.0"), func() (plugin.Provider, error) {
version = "1.5.0"
return &deploytest.Provider{}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(providers.MakeProviderType("pkgA"), "provA", true,
deploytest.ResourceOptions{
Inputs: resource.PropertyMap{
"version": resource.NewStringProperty("1.0.0"),
},
})
assert.NoError(t, err)
provID := resp.ID
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
Provider: provRef.String(),
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: MakeBasicLifecycleSteps(t, 2),
}
p.Run(t, nil)
assert.Equal(t, "1.0.0", version)
}
func TestProviderVersionInputAndOption(t *testing.T) {
t.Parallel()
version := ""
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
version = "1.0.0"
return &deploytest.Provider{}, nil
}),
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.5.0"), func() (plugin.Provider, error) {
version = "1.5.0"
return &deploytest.Provider{}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(providers.MakeProviderType("pkgA"), "provA", true,
deploytest.ResourceOptions{
Inputs: resource.PropertyMap{
"version": resource.NewStringProperty("1.5.0"),
},
Version: "1.0.0",
})
assert.NoError(t, err)
provID := resp.ID
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
Provider: provRef.String(),
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: MakeBasicLifecycleSteps(t, 2),
}
p.Run(t, nil)
assert.Equal(t, "1.0.0", version)
}
func TestPluginDownloadURLPassthrough(t *testing.T) {
t.Parallel()
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
}
pkgAPluginDownloadURL := "get.pulumi.com/${VERSION}"
pkgAType := providers.MakeProviderType("pkgA")
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(pkgAType, "provA", true, deploytest.ResourceOptions{
PluginDownloadURL: pkgAPluginDownloadURL,
})
assert.NoError(t, err)
provID := resp.ID
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
Provider: provRef.String(),
})
assert.NoError(t, err)
return nil
})
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
steps := MakeBasicLifecycleSteps(t, 2)
steps[0].ValidateAnd(func(project workspace.Project, target deploy.Target, entries JournalEntries,
_ []Event, err error,
) error {
for _, e := range entries {
r := e.Step.New()
if r.Type == pkgAType && r.Inputs["pluginDownloadURL"].StringValue() != pkgAPluginDownloadURL {
return fmt.Errorf("Found unexpected value %v", r.Inputs["pluginDownloadURL"])
}
}
return nil
})
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: steps,
}
p.Run(t, nil)
}
// Check that creating a resource with pluginDownloadURL set will instantiate a default provider with
// pluginDownloadURL set.
func TestPluginDownloadURLDefaultProvider(t *testing.T) {
t.Parallel()
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
}
url := "get.pulumi.com"
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, err := monitor.RegisterResource("pkgA::Foo", "foo", true, deploytest.ResourceOptions{
PluginDownloadURL: url,
})
return err
})
snapshot := (&TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: deploytest.NewPluginHostF(nil, nil, programF, loaders...)},
// The first step is the update. We don't want the full lifecycle because we want to see the
// created resources.
Steps: MakeBasicLifecycleSteps(t, 2)[:1],
}).Run(t, nil)
foundDefaultProvider := false
for _, r := range snapshot.Resources {
if providers.IsDefaultProvider(r.URN) {
actualURL, err := providers.GetProviderDownloadURL(r.Inputs)
assert.NoError(t, err)
assert.Equal(t, url, actualURL)
foundDefaultProvider = true
}
}
assert.Truef(t, foundDefaultProvider, "Found resources: %#v", snapshot.Resources)
}
func TestMultipleResourceDenyDefaultProviderLifecycle(t *testing.T) {
t.Parallel()
cases := []struct {
name string
f deploytest.ProgramFunc
disabled string
expectFail bool
}{
{
name: "default-blocked",
f: func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgB:m:typB", "resB", true)
assert.NoError(t, err)
return nil
},
disabled: `["pkgA"]`,
expectFail: true,
},
{
name: "explicit-not-blocked",
f: func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource(providers.MakeProviderType("pkgA"), "provA", true)
assert.NoError(t, err)
provRef, err := providers.NewReference(resp.URN, resp.ID)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
Provider: provRef.String(),
})
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgB:m:typB", "resB", true)
assert.NoError(t, err)
return nil
},
disabled: `["pkgA"]`,
expectFail: false,
},
{
name: "wildcard",
f: func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, err := monitor.RegisterResource("pkgA:m:typA", "resA", true)
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgB:m:typB", "resB", true)
assert.NoError(t, err)
return nil
},
disabled: `["*"]`,
expectFail: true,
},
}
for _, tt := range cases {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
deploytest.NewProviderLoader("pkgB", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(tt.f)
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
c := config.Map{}
k := config.MustMakeKey("pulumi", "disable-default-providers")
c[k] = config.NewValue(tt.disabled)
expectedCreated := 4
if tt.expectFail {
expectedCreated = 0
}
update := MakeBasicLifecycleSteps(t, expectedCreated)[:1]
update[0].ExpectFailure = tt.expectFail
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: update,
Config: c,
}
p.Run(t, nil)
})
}
}
func TestProviderVersionAssignment(t *testing.T) {
t.Parallel()
prog := func(opts ...deploytest.ResourceOptions) deploytest.ProgramFunc {
return func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, err := monitor.RegisterResource("pkgA:r:typA", "resA", true, opts...)
if err != nil {
return err
}
_, err = monitor.RegisterResource("pulumi:providers:pkgA", "provA", true, opts...)
if err != nil {
return err
}
return nil
}
}
cases := []struct {
name string
plugins []workspace.PluginSpec
snapshot *deploy.Snapshot
validate func(t *testing.T, r *resource.State)
versions []string
prog deploytest.ProgramFunc
}{
{
name: "empty",
versions: []string{"1.0.0"},
validate: func(*testing.T, *resource.State) {},
prog: prog(),
},
{
name: "default-version",
versions: []string{"1.0.0", "1.1.0"},
plugins: []workspace.PluginSpec{{
Name: "pkgA",
Version: &semver.Version{Major: 1, Minor: 1},
PluginDownloadURL: "example.com/default",
Kind: apitype.ResourcePlugin,
}},
validate: func(t *testing.T, r *resource.State) {
if providers.IsProviderType(r.Type) && !providers.IsDefaultProvider(r.URN) {
assert.Equal(t, r.Inputs["version"].StringValue(), "1.1.0")
assert.Equal(t, r.Inputs["pluginDownloadURL"].StringValue(), "example.com/default")
}
},
prog: prog(),
},
{
name: "specified-provider",
versions: []string{"1.0.0", "1.1.0"},
plugins: []workspace.PluginSpec{{
Name: "pkgA",
Version: &semver.Version{Major: 1, Minor: 1},
Kind: apitype.ResourcePlugin,
}},
validate: func(t *testing.T, r *resource.State) {
if providers.IsProviderType(r.Type) && !providers.IsDefaultProvider(r.URN) {
_, hasVersion := r.Inputs["version"]
assert.False(t, hasVersion)
assert.Equal(t, r.Inputs["pluginDownloadURL"].StringValue(), "example.com/download")
}
},
prog: prog(deploytest.ResourceOptions{PluginDownloadURL: "example.com/download"}),
},
{
name: "higher-in-snapshot",
versions: []string{"1.3.0", "1.1.0"},
prog: prog(),
plugins: []workspace.PluginSpec{{
Name: "pkgA",
Version: &semver.Version{Major: 1, Minor: 1},
Kind: apitype.ResourcePlugin,
}},
snapshot: &deploy.Snapshot{
Resources: []*resource.State{
{
Type: "providers:pulumi:pkgA",
URN: "this:is:a:urn::ofaei",
Inputs: map[resource.PropertyKey]resource.PropertyValue{
"version": resource.NewPropertyValue("1.3.0"),
},
},
},
},
validate: func(t *testing.T, r *resource.State) {
if providers.IsProviderType(r.Type) && !providers.IsDefaultProvider(r.URN) {
assert.Equal(t, r.Inputs["version"].StringValue(), "1.1.0")
}
},
},
}
for _, c := range cases {
c := c
t.Run(c.name, func(t *testing.T) {
t.Parallel()
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(c.prog, c.plugins...)
loaders := []*deploytest.ProviderLoader{}
for _, v := range c.versions {
loaders = append(loaders,
deploytest.NewProviderLoader("pkgA", semver.MustParse(v), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}))
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
update := []TestStep{{Op: Update, Validate: func(
project workspace.Project, target deploy.Target, entries JournalEntries,
events []Event, err error,
) error {
require.NoError(t, err)
snap, err := entries.Snap(target.Snapshot)
require.NoError(t, err)
assert.Len(t, snap.Resources, 3)
for _, r := range snap.Resources {
c.validate(t, r)
}
return nil
}}}
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
Steps: update,
}
p.Run(t, &deploy.Snapshot{})
})
}
}
// TestDeletedWithOptionInheritance checks that a resource that sets its parent to another resource inherits
// that resource's DeletedWith option.
func TestDeletedWithOptionInheritance(t *testing.T) {
t.Parallel()
Better handle property dependencies and `deletedWith` (#16088) A resource `A` depends on a resource `B` if: 1. `B` is `A`'s `Provider`. 2. `B` is `A`'s `Parent`. 3. `B` appears in `A`'s `Dependencies`. 4. `B` appears in one or more of `A`'s `PropertyDependencies`. 5. `B` is referenced by `A`'s `DeletedWith` field. While cases 1, 2, and 3 (providers, parents, and dependencies) are handled fairly consistently, there have been a number of cases where the newer features of `PropertyDependencies` (case 4) and `DeletedWith` (case 5) have been neglected. This commit addresses some of these omissions. Specifically: * When refreshing state, it's important that we remove URNs that point to resources that we've identified as deleted. Presently we check pointers to parents and dependencies, but not property dependencies or `deletedWith`. This commit fixes these gaps where dangling URNs could lurk. * Linked to the above, this commit extends snapshot integrity checks to include property dependencies and `deletedWith`. Some tests that previously used now invalid states have to be repaired or removed as a result of this. * Fixing snapshot integrity checking reveals that dependency graph checks also fail to consider property dependencies and `deletedWith`. This probably hasn't bitten us since property dependencies are currently rolled up into dependencies (for temporary backwards compatibility) and `deletedWith` is typically an optimisation (moreover, one that only matters during deletes), so operations being parallelised due a perceived lack of dependency still succeed. However, tests that previously passed now fail as we can spot these races with our better integrity checks. This commit thus fixes up dependency graph construction and bulks out its test suite to cover the new cases. These bugs were discovered as part of the investigation into #16052, though they may not be directly responsible for it (though the issues with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
var deletionDepURN resource.URN
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
Better handle property dependencies and `deletedWith` (#16088) A resource `A` depends on a resource `B` if: 1. `B` is `A`'s `Provider`. 2. `B` is `A`'s `Parent`. 3. `B` appears in `A`'s `Dependencies`. 4. `B` appears in one or more of `A`'s `PropertyDependencies`. 5. `B` is referenced by `A`'s `DeletedWith` field. While cases 1, 2, and 3 (providers, parents, and dependencies) are handled fairly consistently, there have been a number of cases where the newer features of `PropertyDependencies` (case 4) and `DeletedWith` (case 5) have been neglected. This commit addresses some of these omissions. Specifically: * When refreshing state, it's important that we remove URNs that point to resources that we've identified as deleted. Presently we check pointers to parents and dependencies, but not property dependencies or `deletedWith`. This commit fixes these gaps where dangling URNs could lurk. * Linked to the above, this commit extends snapshot integrity checks to include property dependencies and `deletedWith`. Some tests that previously used now invalid states have to be repaired or removed as a result of this. * Fixing snapshot integrity checking reveals that dependency graph checks also fail to consider property dependencies and `deletedWith`. This probably hasn't bitten us since property dependencies are currently rolled up into dependencies (for temporary backwards compatibility) and `deletedWith` is typically an optimisation (moreover, one that only matters during deletes), so operations being parallelised due a perceived lack of dependency still succeed. However, tests that previously passed now fail as we can spot these races with our better integrity checks. This commit thus fixes up dependency graph construction and bulks out its test suite to cover the new cases. These bugs were discovered as part of the investigation into #16052, though they may not be directly responsible for it (though the issues with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
deletionDep, err := monitor.RegisterResource("pkgA:m:typA", "deletable", false)
assert.NoError(t, err)
parentResp, err := monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
Better handle property dependencies and `deletedWith` (#16088) A resource `A` depends on a resource `B` if: 1. `B` is `A`'s `Provider`. 2. `B` is `A`'s `Parent`. 3. `B` appears in `A`'s `Dependencies`. 4. `B` appears in one or more of `A`'s `PropertyDependencies`. 5. `B` is referenced by `A`'s `DeletedWith` field. While cases 1, 2, and 3 (providers, parents, and dependencies) are handled fairly consistently, there have been a number of cases where the newer features of `PropertyDependencies` (case 4) and `DeletedWith` (case 5) have been neglected. This commit addresses some of these omissions. Specifically: * When refreshing state, it's important that we remove URNs that point to resources that we've identified as deleted. Presently we check pointers to parents and dependencies, but not property dependencies or `deletedWith`. This commit fixes these gaps where dangling URNs could lurk. * Linked to the above, this commit extends snapshot integrity checks to include property dependencies and `deletedWith`. Some tests that previously used now invalid states have to be repaired or removed as a result of this. * Fixing snapshot integrity checking reveals that dependency graph checks also fail to consider property dependencies and `deletedWith`. This probably hasn't bitten us since property dependencies are currently rolled up into dependencies (for temporary backwards compatibility) and `deletedWith` is typically an optimisation (moreover, one that only matters during deletes), so operations being parallelised due a perceived lack of dependency still succeed. However, tests that previously passed now fail as we can spot these races with our better integrity checks. This commit thus fixes up dependency graph construction and bulks out its test suite to cover the new cases. These bugs were discovered as part of the investigation into #16052, though they may not be directly responsible for it (though the issues with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
DeletedWith: deletionDep.URN,
})
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resB", true, deploytest.ResourceOptions{
Parent: parentResp.URN,
})
assert.NoError(t, err)
Better handle property dependencies and `deletedWith` (#16088) A resource `A` depends on a resource `B` if: 1. `B` is `A`'s `Provider`. 2. `B` is `A`'s `Parent`. 3. `B` appears in `A`'s `Dependencies`. 4. `B` appears in one or more of `A`'s `PropertyDependencies`. 5. `B` is referenced by `A`'s `DeletedWith` field. While cases 1, 2, and 3 (providers, parents, and dependencies) are handled fairly consistently, there have been a number of cases where the newer features of `PropertyDependencies` (case 4) and `DeletedWith` (case 5) have been neglected. This commit addresses some of these omissions. Specifically: * When refreshing state, it's important that we remove URNs that point to resources that we've identified as deleted. Presently we check pointers to parents and dependencies, but not property dependencies or `deletedWith`. This commit fixes these gaps where dangling URNs could lurk. * Linked to the above, this commit extends snapshot integrity checks to include property dependencies and `deletedWith`. Some tests that previously used now invalid states have to be repaired or removed as a result of this. * Fixing snapshot integrity checking reveals that dependency graph checks also fail to consider property dependencies and `deletedWith`. This probably hasn't bitten us since property dependencies are currently rolled up into dependencies (for temporary backwards compatibility) and `deletedWith` is typically an optimisation (moreover, one that only matters during deletes), so operations being parallelised due a perceived lack of dependency still succeed. However, tests that previously passed now fail as we can spot these races with our better integrity checks. This commit thus fixes up dependency graph construction and bulks out its test suite to cover the new cases. These bugs were discovered as part of the investigation into #16052, though they may not be directly responsible for it (though the issues with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
deletionDepURN = deletionDep.URN
return nil
})
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{
DiffF: func(
urn resource.URN, id resource.ID, oldInputs, oldOutputs, newInputs resource.PropertyMap, ignoreChanges []string,
) (plugin.DiffResult, error) {
return plugin.DiffResult{}, nil
},
}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
}
project := p.GetProject()
snap, err := TestOp(Update).Run(project, p.GetTarget(t, nil), p.Options, false, p.BackendClient, nil)
Better handle property dependencies and `deletedWith` (#16088) A resource `A` depends on a resource `B` if: 1. `B` is `A`'s `Provider`. 2. `B` is `A`'s `Parent`. 3. `B` appears in `A`'s `Dependencies`. 4. `B` appears in one or more of `A`'s `PropertyDependencies`. 5. `B` is referenced by `A`'s `DeletedWith` field. While cases 1, 2, and 3 (providers, parents, and dependencies) are handled fairly consistently, there have been a number of cases where the newer features of `PropertyDependencies` (case 4) and `DeletedWith` (case 5) have been neglected. This commit addresses some of these omissions. Specifically: * When refreshing state, it's important that we remove URNs that point to resources that we've identified as deleted. Presently we check pointers to parents and dependencies, but not property dependencies or `deletedWith`. This commit fixes these gaps where dangling URNs could lurk. * Linked to the above, this commit extends snapshot integrity checks to include property dependencies and `deletedWith`. Some tests that previously used now invalid states have to be repaired or removed as a result of this. * Fixing snapshot integrity checking reveals that dependency graph checks also fail to consider property dependencies and `deletedWith`. This probably hasn't bitten us since property dependencies are currently rolled up into dependencies (for temporary backwards compatibility) and `deletedWith` is typically an optimisation (moreover, one that only matters during deletes), so operations being parallelised due a perceived lack of dependency still succeed. However, tests that previously passed now fail as we can spot these races with our better integrity checks. This commit thus fixes up dependency graph construction and bulks out its test suite to cover the new cases. These bugs were discovered as part of the investigation into #16052, though they may not be directly responsible for it (though the issues with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
for _, res := range snap.Resources[2:] {
assert.Equal(t, deletionDepURN, res.DeletedWith)
}
assert.NoError(t, err)
}
// TestDeletedWithOptionInheritanceMLC checks that an MLC's DeletedWith option is propagated to resources that
// set an MLC as its parent. MLC's are remote and at the time of writing their RegisterResource call asks the
// resource monitor to ask the constructor to call the necessary RegisterResource calls on the program's behalf.
func TestDeletedWithOptionInheritanceMLC(t *testing.T) {
t.Parallel()
Better handle property dependencies and `deletedWith` (#16088) A resource `A` depends on a resource `B` if: 1. `B` is `A`'s `Provider`. 2. `B` is `A`'s `Parent`. 3. `B` appears in `A`'s `Dependencies`. 4. `B` appears in one or more of `A`'s `PropertyDependencies`. 5. `B` is referenced by `A`'s `DeletedWith` field. While cases 1, 2, and 3 (providers, parents, and dependencies) are handled fairly consistently, there have been a number of cases where the newer features of `PropertyDependencies` (case 4) and `DeletedWith` (case 5) have been neglected. This commit addresses some of these omissions. Specifically: * When refreshing state, it's important that we remove URNs that point to resources that we've identified as deleted. Presently we check pointers to parents and dependencies, but not property dependencies or `deletedWith`. This commit fixes these gaps where dangling URNs could lurk. * Linked to the above, this commit extends snapshot integrity checks to include property dependencies and `deletedWith`. Some tests that previously used now invalid states have to be repaired or removed as a result of this. * Fixing snapshot integrity checking reveals that dependency graph checks also fail to consider property dependencies and `deletedWith`. This probably hasn't bitten us since property dependencies are currently rolled up into dependencies (for temporary backwards compatibility) and `deletedWith` is typically an optimisation (moreover, one that only matters during deletes), so operations being parallelised due a perceived lack of dependency still succeed. However, tests that previously passed now fail as we can spot these races with our better integrity checks. This commit thus fixes up dependency graph construction and bulks out its test suite to cover the new cases. These bugs were discovered as part of the investigation into #16052, though they may not be directly responsible for it (though the issues with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
var deletionDepURN resource.URN
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
Better handle property dependencies and `deletedWith` (#16088) A resource `A` depends on a resource `B` if: 1. `B` is `A`'s `Provider`. 2. `B` is `A`'s `Parent`. 3. `B` appears in `A`'s `Dependencies`. 4. `B` appears in one or more of `A`'s `PropertyDependencies`. 5. `B` is referenced by `A`'s `DeletedWith` field. While cases 1, 2, and 3 (providers, parents, and dependencies) are handled fairly consistently, there have been a number of cases where the newer features of `PropertyDependencies` (case 4) and `DeletedWith` (case 5) have been neglected. This commit addresses some of these omissions. Specifically: * When refreshing state, it's important that we remove URNs that point to resources that we've identified as deleted. Presently we check pointers to parents and dependencies, but not property dependencies or `deletedWith`. This commit fixes these gaps where dangling URNs could lurk. * Linked to the above, this commit extends snapshot integrity checks to include property dependencies and `deletedWith`. Some tests that previously used now invalid states have to be repaired or removed as a result of this. * Fixing snapshot integrity checking reveals that dependency graph checks also fail to consider property dependencies and `deletedWith`. This probably hasn't bitten us since property dependencies are currently rolled up into dependencies (for temporary backwards compatibility) and `deletedWith` is typically an optimisation (moreover, one that only matters during deletes), so operations being parallelised due a perceived lack of dependency still succeed. However, tests that previously passed now fail as we can spot these races with our better integrity checks. This commit thus fixes up dependency graph construction and bulks out its test suite to cover the new cases. These bugs were discovered as part of the investigation into #16052, though they may not be directly responsible for it (though the issues with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
deletionDep, err := monitor.RegisterResource("pkgA:m:typComponent", "deletable", false)
assert.NoError(t, err)
parentResp, err := monitor.RegisterResource("pkgA:m:typComponent", "resA", false, deploytest.ResourceOptions{
Remote: true,
Better handle property dependencies and `deletedWith` (#16088) A resource `A` depends on a resource `B` if: 1. `B` is `A`'s `Provider`. 2. `B` is `A`'s `Parent`. 3. `B` appears in `A`'s `Dependencies`. 4. `B` appears in one or more of `A`'s `PropertyDependencies`. 5. `B` is referenced by `A`'s `DeletedWith` field. While cases 1, 2, and 3 (providers, parents, and dependencies) are handled fairly consistently, there have been a number of cases where the newer features of `PropertyDependencies` (case 4) and `DeletedWith` (case 5) have been neglected. This commit addresses some of these omissions. Specifically: * When refreshing state, it's important that we remove URNs that point to resources that we've identified as deleted. Presently we check pointers to parents and dependencies, but not property dependencies or `deletedWith`. This commit fixes these gaps where dangling URNs could lurk. * Linked to the above, this commit extends snapshot integrity checks to include property dependencies and `deletedWith`. Some tests that previously used now invalid states have to be repaired or removed as a result of this. * Fixing snapshot integrity checking reveals that dependency graph checks also fail to consider property dependencies and `deletedWith`. This probably hasn't bitten us since property dependencies are currently rolled up into dependencies (for temporary backwards compatibility) and `deletedWith` is typically an optimisation (moreover, one that only matters during deletes), so operations being parallelised due a perceived lack of dependency still succeed. However, tests that previously passed now fail as we can spot these races with our better integrity checks. This commit thus fixes up dependency graph construction and bulks out its test suite to cover the new cases. These bugs were discovered as part of the investigation into #16052, though they may not be directly responsible for it (though the issues with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
DeletedWith: deletionDep.URN,
})
assert.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resB", true, deploytest.ResourceOptions{
Parent: parentResp.URN,
})
assert.NoError(t, err)
Better handle property dependencies and `deletedWith` (#16088) A resource `A` depends on a resource `B` if: 1. `B` is `A`'s `Provider`. 2. `B` is `A`'s `Parent`. 3. `B` appears in `A`'s `Dependencies`. 4. `B` appears in one or more of `A`'s `PropertyDependencies`. 5. `B` is referenced by `A`'s `DeletedWith` field. While cases 1, 2, and 3 (providers, parents, and dependencies) are handled fairly consistently, there have been a number of cases where the newer features of `PropertyDependencies` (case 4) and `DeletedWith` (case 5) have been neglected. This commit addresses some of these omissions. Specifically: * When refreshing state, it's important that we remove URNs that point to resources that we've identified as deleted. Presently we check pointers to parents and dependencies, but not property dependencies or `deletedWith`. This commit fixes these gaps where dangling URNs could lurk. * Linked to the above, this commit extends snapshot integrity checks to include property dependencies and `deletedWith`. Some tests that previously used now invalid states have to be repaired or removed as a result of this. * Fixing snapshot integrity checking reveals that dependency graph checks also fail to consider property dependencies and `deletedWith`. This probably hasn't bitten us since property dependencies are currently rolled up into dependencies (for temporary backwards compatibility) and `deletedWith` is typically an optimisation (moreover, one that only matters during deletes), so operations being parallelised due a perceived lack of dependency still succeed. However, tests that previously passed now fail as we can spot these races with our better integrity checks. This commit thus fixes up dependency graph construction and bulks out its test suite to cover the new cases. These bugs were discovered as part of the investigation into #16052, though they may not be directly responsible for it (though the issues with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
deletionDepURN = deletionDep.URN
return nil
})
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{
DiffF: func(
urn resource.URN, id resource.ID, oldInputs, oldOutputs, newInputs resource.PropertyMap, ignoreChanges []string,
) (plugin.DiffResult, error) {
return plugin.DiffResult{}, nil
},
ConstructF: func(monitor *deploytest.ResourceMonitor, typ, name string,
parent resource.URN, inputs resource.PropertyMap,
info plugin.ConstructInfo, options plugin.ConstructOptions,
) (plugin.ConstructResult, error) {
require.Equal(t, "resA", name)
require.Equal(t, "pkgA:m:typComponent", typ)
resp, err := monitor.RegisterResource(tokens.Type(typ), name, false, deploytest.ResourceOptions{
DeletedWith: options.DeletedWith,
})
require.NoError(t, err)
_, err = monitor.RegisterResource("pkgA:m:typA", "resC", true, deploytest.ResourceOptions{
Parent: resp.URN,
})
require.NoError(t, err)
return plugin.ConstructResult{
URN: resp.URN,
}, nil
},
}, nil
}),
}
Lifecycle tests shouldn't use a closed host (#14063) <!--- Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation. --> # Description <!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. --> This PR fixes the inadvertent use of a closed plugin host in the lifecycle tests. The tests override the host that is provided to the engine, for good reasons, but that same host is re-used across multiple engine operations. Since the engine closes the supplied host at the end of each operation, subsequent operations are handed a closed host. In order to detect engine bugs related to the use of a closed host (see https://github.com/pulumi/pulumi/pull/14057), the fake host should return an error if it is used after being closed (as does the real host). This PR addresses this. The detailed change is to shift to using a host factory that produces a host in `TestOp.Run`. The `TestPlan` now takes a `TestUpdateOptions` with `HostF` and an embedded `UpdateOptions`. Note that two tests fail due to https://github.com/pulumi/pulumi/pull/14057 which was being masked by the problem that is fixed here. This PR disables those tests and the other PR will re-enable them. - `TestCanceledRefresh` - `TestProviderCancellation` ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt` <!--- Please provide details if the checkbox below is to be left unchecked. --> - [x] I have added tests that prove my fix is effective or that my feature works <!--- User-facing changes require a CHANGELOG entry. --> - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change <!-- If the change(s) in this PR is a modification of an existing call to the Pulumi Cloud, then the service should honor older versions of the CLI where this change would not exist. You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add it to the service. --> - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version <!-- @Pulumi employees: If yes, you must submit corresponding changes in the service repo. -->
2023-09-28 21:50:18 +00:00
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
}
project := p.GetProject()
snap, err := TestOp(Update).Run(project, p.GetTarget(t, nil), p.Options, false, p.BackendClient, nil)
Better handle property dependencies and `deletedWith` (#16088) A resource `A` depends on a resource `B` if: 1. `B` is `A`'s `Provider`. 2. `B` is `A`'s `Parent`. 3. `B` appears in `A`'s `Dependencies`. 4. `B` appears in one or more of `A`'s `PropertyDependencies`. 5. `B` is referenced by `A`'s `DeletedWith` field. While cases 1, 2, and 3 (providers, parents, and dependencies) are handled fairly consistently, there have been a number of cases where the newer features of `PropertyDependencies` (case 4) and `DeletedWith` (case 5) have been neglected. This commit addresses some of these omissions. Specifically: * When refreshing state, it's important that we remove URNs that point to resources that we've identified as deleted. Presently we check pointers to parents and dependencies, but not property dependencies or `deletedWith`. This commit fixes these gaps where dangling URNs could lurk. * Linked to the above, this commit extends snapshot integrity checks to include property dependencies and `deletedWith`. Some tests that previously used now invalid states have to be repaired or removed as a result of this. * Fixing snapshot integrity checking reveals that dependency graph checks also fail to consider property dependencies and `deletedWith`. This probably hasn't bitten us since property dependencies are currently rolled up into dependencies (for temporary backwards compatibility) and `deletedWith` is typically an optimisation (moreover, one that only matters during deletes), so operations being parallelised due a perceived lack of dependency still succeed. However, tests that previously passed now fail as we can spot these races with our better integrity checks. This commit thus fixes up dependency graph construction and bulks out its test suite to cover the new cases. These bugs were discovered as part of the investigation into #16052, though they may not be directly responsible for it (though the issues with dependency graphs are certainly a candidate).
2024-05-03 17:08:06 +00:00
for _, res := range snap.Resources[2:] {
assert.Equal(t, deletionDepURN, res.DeletedWith)
}
assert.NoError(t, err)
}
// TestComponentProvidersInheritance is to test that the `providers` map is propagated to child resources. The rules
// around providers inheritances are _weird_. They are only used for remote construct calls, but they propagate through
// any "component parent", not custom resource parents. This is probably just badly spec'd behavior from the first
// release that we're now stuck with.
func TestComponentProvidersInheritance(t *testing.T) {
t.Parallel()
programF := deploytest.NewLanguageRuntimeF(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
resp, err := monitor.RegisterResource("pulumi:providers:pkg", "provA", true)
assert.NoError(t, err)
provID := resp.ID
if provID == "" {
provID = providers.UnknownID
}
provRef, err := providers.NewReference(resp.URN, provID)
assert.NoError(t, err)
respA, err := monitor.RegisterResource("my_component", "resA", false, deploytest.ResourceOptions{
Providers: map[string]string{"pkgA": provRef.String()},
})
assert.NoError(t, err)
// resB _should_ see the explicit provider in it's construct options because it's parent is a component with
// providers set.
_, err = monitor.RegisterResource("pkg:index:component", "resB", false, deploytest.ResourceOptions{
Remote: true,
Parent: respA.URN,
})
assert.NoError(t, err)
respC, err := monitor.RegisterResource("pkg:index:type", "resC", true, deploytest.ResourceOptions{
Providers: map[string]string{"pkgA": provRef.String()},
})
assert.NoError(t, err)
// resD _should NOT_ see the explicit provider in it's construct options because it's parent is a custom.
_, err = monitor.RegisterResource("pkg:index:component", "resD", false, deploytest.ResourceOptions{
Remote: true,
Parent: respC.URN,
})
assert.NoError(t, err)
return nil
})
loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkg", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{
DiffF: func(
urn resource.URN, id resource.ID, oldInputs, oldOutputs, newInputs resource.PropertyMap, ignoreChanges []string,
) (plugin.DiffResult, error) {
return plugin.DiffResult{}, nil
},
ConstructF: func(monitor *deploytest.ResourceMonitor, typ, name string,
parent resource.URN, inputs resource.PropertyMap,
info plugin.ConstructInfo, options plugin.ConstructOptions,
) (plugin.ConstructResult, error) {
assert.Equal(t, "pkg:index:component", typ)
if name == "resB" {
assert.Contains(t, options.Providers["pkgA"], "urn:pulumi:test::test::pulumi:providers:pkg::provA::")
} else {
assert.Equal(t, "resD", name)
assert.NotContains(t, options.Providers, "pkgA")
}
resp, err := monitor.RegisterResource(tokens.Type(typ), name, false, deploytest.ResourceOptions{})
assert.NoError(t, err)
return plugin.ConstructResult{
URN: resp.URN,
}, nil
},
}, nil
}),
}
hostF := deploytest.NewPluginHostF(nil, nil, programF, loaders...)
p := &TestPlan{
Add display to the engine tests (#16050) We want to add more test coverage to the display code. The best way to do that is to add it to the engine tests, that already cover most of the pulumi functionality. It's probably not really possible to review all of the output, but at least it gives us a baseline, which we can work with. There's a couple of tests that are flaky for reasons I don't quite understand yet. I marked them as to skip and we can look at them later. I'd rather get in the baseline tests sooner, rather than spending a bunch of time looking at that. The output differences also seem very minor, so not super concerning. The biggest remaining issue is that this doesn't interact well with the Chdir we're doing in the engine. We could either pass the CWD through, or just try to get rid of that Chdir. So this should only be merged after https://github.com/pulumi/pulumi/pull/15607. I've tried to split this into a few commits, separating out adding the testdata, so it's hopefully a little easier to review, even though the PR is still quite large. One other thing to note is that we're comparing that the output has all the same lines, and not that it is exactly the same. Because of how the engine is implemented, there's a bunch of race conditions otherwise, that would make us have to skip a bunch of tests, just because e.g. resource A is sometimes deleted before resource B and sometimes it's the other way around. The biggest downside of that is that running with `PULUMI_ACCEPT` will produce a diff even when there are no changes. Hopefully we won't have to run that way too often though, so it might not be a huge issue? --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
2024-05-13 07:18:25 +00:00
Options: TestUpdateOptions{T: t, HostF: hostF},
}
project := p.GetProject()
_, err := TestOp(Update).Run(project, p.GetTarget(t, nil), p.Options, false, p.BackendClient, nil)
assert.NoError(t, err)
}