diff --git a/CHANGELOG.md b/CHANGELOG.md index 53754346a..f5efb2718 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ CHANGELOG ## HEAD (Unreleased) +- Gracefully handle errors when resources use duplicate aliases. - Use the update token for renew_lease calls and update the API version to 5. [#3348](https://github.com/pulumi/pulumi/pull/3348) diff --git a/pkg/backend/snapshot.go b/pkg/backend/snapshot.go index 8ce72d8e9..7ff5be852 100644 --- a/pkg/backend/snapshot.go +++ b/pkg/backend/snapshot.go @@ -562,7 +562,9 @@ func (sm *SnapshotManager) snap() *deploy.Snapshot { // saveSnapshot persists the current snapshot and optionally verifies it afterwards. func (sm *SnapshotManager) saveSnapshot() error { snap := sm.snap() - snap.NormalizeURNReferences() + if err := snap.NormalizeURNReferences(); err != nil { + return errors.Wrap(err, "failed to normalize URN references") + } if err := sm.persister.Save(snap); err != nil { return errors.Wrap(err, "failed to save snapshot") } diff --git a/pkg/engine/lifecycle_test.go b/pkg/engine/lifecycle_test.go index 717119c95..12e0591db 100644 --- a/pkg/engine/lifecycle_test.go +++ b/pkg/engine/lifecycle_test.go @@ -3720,6 +3720,44 @@ func TestAliases(t *testing.T) { "urn:pulumi:test::test::pkgA:index:t1$pkgA:index:t2::n2", }, }}, []deploy.StepOp{deploy.OpSame, deploy.OpReplace, deploy.OpCreateReplacement, deploy.OpDeleteReplaced}) + + // ensure failure when different resources use duplicate aliases + snap = updateProgramWithResource(nil, []Resource{{ + t: "pkgA:index:t1", + name: "n1", + aliases: []resource.URN{ + "urn:pulumi:test::test::pkgA:index:t1::n1", + }, + }, { + t: "pkgA:index:t2", + name: "n2", + aliases: []resource.URN{ + "urn:pulumi:test::test::pkgA:index:t1::n1", + }, + }}, []deploy.StepOp{deploy.OpCreate}) + + err := snap.NormalizeURNReferences() + assert.Equal(t, err.Error(), + "Two resources ('urn:pulumi:test::test::pkgA:index:t1::n1'"+ + " and 'urn:pulumi:test::test::pkgA:index:t2::n2') aliased to the same: 'urn:pulumi:test::test::pkgA:index:t1::n1'") + + // ensure different resources can use different aliases + snap = updateProgramWithResource(nil, []Resource{{ + t: "pkgA:index:t1", + name: "n1", + aliases: []resource.URN{ + "urn:pulumi:test::test::pkgA:index:t1::n1", + }, + }, { + t: "pkgA:index:t2", + name: "n2", + aliases: []resource.URN{ + "urn:pulumi:test::test::pkgA:index:t1::n2", + }, + }}, []deploy.StepOp{deploy.OpCreate}) + + err = snap.NormalizeURNReferences() + assert.Nil(t, err) } func TestPersistentDiff(t *testing.T) { diff --git a/pkg/resource/deploy/snapshot.go b/pkg/resource/deploy/snapshot.go index f1a57c490..d556c685f 100644 --- a/pkg/resource/deploy/snapshot.go +++ b/pkg/resource/deploy/snapshot.go @@ -75,7 +75,7 @@ func NewSnapshot(manifest Manifest, secretsManager secrets.Manager, // of a resource in the resources map. // // Note: This method modifies the snapshot (and resource.States in the snapshot) in-place. -func (snap *Snapshot) NormalizeURNReferences() { +func (snap *Snapshot) NormalizeURNReferences() error { if snap != nil { aliased := make(map[resource.URN]resource.URN) fixUrn := func(urn resource.URN) resource.URN { @@ -109,12 +109,14 @@ func (snap *Snapshot) NormalizeURNReferences() { // same resource multiple times. That's fine, only error if we see the same alias, // but it maps to *different* resources. if otherUrn, has := aliased[alias]; has && otherUrn != state.URN { - contract.Assertf(!has, "Two resources ('%s' and '%s') aliased to the same: '%s'", otherUrn, state.URN, alias) + return errors.Errorf("Two resources ('%s' and '%s') aliased to the same: '%s'", otherUrn, state.URN, alias) } aliased[alias] = state.URN } } } + + return nil } // VerifyIntegrity checks a snapshot to ensure it is well-formed. Because of the cost of this operation,