error instead of panic when different resources use the same alias (#3457)

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)
 

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")
 	}

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) {

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,