Merge #11533 #11537

11533: Allow opting out of PULUMI_OPTIMIZED_CHECKPOINT_PATCH r=t0yv0 a=t0yv0  # Description Allow users to opt out of deltaCheckpointUpdates even if the backend indicates it should be used. This remains necessary while PULUMI_OPTIMIZED_CHECKPOINT_PATCH has higher memory requirements on the client and may cause out-of-memory issues in constrained environments.  Part of https://github.com/pulumi/home/issues/2426 ## Checklist  - [ ] I have added tests that prove my fix is effective or that my feature works  - [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change  - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version  11537: Fix panic in mapper support for map[string]*string r=t0yv0 a=t0yv0  # Description `map[string]*string` does not turn around through mapper, causing a panic. ```go go test -run TestReproduceMapStringPointerTurnaroundIssue ~/code/pulumi/sdk/go/common/util/mapper --- FAIL: TestReproduceMapStringPointerTurnaroundIssue (0.00s) mapper_test.go:608: encodedMap: map[args:map[key:value]] panic: reflect: Elem of invalid type string [recovered] panic: reflect: Elem of invalid type string goroutine 20 [running]: testing.tRunner.func1.2({0x11cb480, 0xc0000992b0}) /nix/store/0c30lcag5r6ahw3qj0x7lkshpry4yqwl-go-1.19/share/go/src/testing/testing.go:1396 +0x24e testing.tRunner.func1() /nix/store/0c30lcag5r6ahw3qj0x7lkshpry4yqwl-go-1.19/share/go/src/testing/testing.go:1399 +0x39f panic({0x11cb480, 0xc0000992b0}) /nix/store/0c30lcag5r6ahw3qj0x7lkshpry4yqwl-go-1.19/share/go/src/runtime/panic.go:884 +0x212 reflect.(*rtype).Elem(0x11d1500?) /nix/store/0c30lcag5r6ahw3qj0x7lkshpry4yqwl-go-1.19/share/go/src/reflect/type.go:972 +0x134 github.com/pulumi/pulumi/sdk/v3/go/common/util/mapper.(*mapper).adjustValueForAssignment(0xc0000a2240, {0x11d1500?, 0xc000099290?, 0x98?}, {0x126b940?, 0x11c6160}, {0x126b940?, 0x11dd060}, {0xc0000a4a80, 0xf}) /Users/t0yv0/code/pulumi/sdk/go/common/util/mapper/mapper_decode.go:158 +0x2ea github.com/pulumi/pulumi/sdk/v3/go/common/util/mapper.(*mapper).adjustValueForAssignment(0xc0000a2240, {0x11d5c20?, 0xc00009d470?, 0x11de1e0?}, {0x126b940?, 0x11d5680}, {0x126b940?, 0x11dd060}, {0x11c0b24, 0x4}) /Users/t0yv0/code/pulumi/sdk/go/common/util/mapper/mapper_decode.go:204 +0x1a15 github.com/pulumi/pulumi/sdk/v3/go/common/util/mapper.(*mapper).DecodeValue(0xc0000a2240, 0xc0000a0060?, {0x126b940?, 0x11dd060}, {0x11c0b24, 0x4}, {0xc0000aac80?, 0xc0000a0060?}, 0x1) /Users/t0yv0/code/pulumi/sdk/go/common/util/mapper/mapper_decode.go:117 +0x8ee github.com/pulumi/pulumi/sdk/v3/go/common/util/mapper.(*mapper).Decode(0xc0000a2240, 0xc0000a8380?, {0x11c36a0?, 0xc0000a0060?}) /Users/t0yv0/code/pulumi/sdk/go/common/util/mapper/mapper_decode.go:55 +0x8db github.com/pulumi/pulumi/sdk/v3/go/common/util/mapper.TestReproduceMapStringPointerTurnaroundIssue.func2(0xc000083520?) /Users/t0yv0/code/pulumi/sdk/go/common/util/mapper/mapper_test.go:592 +0xe9 github.com/pulumi/pulumi/sdk/v3/go/common/util/mapper.TestReproduceMapStringPointerTurnaroundIssue(0xc000083520) /Users/t0yv0/code/pulumi/sdk/go/common/util/mapper/mapper_test.go:610 +0x11f testing.tRunner(0xc000083520, 0x1231f88) /nix/store/0c30lcag5r6ahw3qj0x7lkshpry4yqwl-go-1.19/share/go/src/testing/testing.go:1446 +0x10b created by testing.(*T).Run /nix/store/0c30lcag5r6ahw3qj0x7lkshpry4yqwl-go-1.19/share/go/src/testing/testing.go:1493 +0x35f exit status 2 FAIL github.com/pulumi/pulumi/sdk/v3/go/common/util/mapper 0.674s ```  Fixes # (issue) ## Checklist  - [ ] I have added tests that prove my fix is effective or that my feature works  - [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change  - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version  Co-authored-by: Anton Tayanovskyy <anton@pulumi.com>
pulumi · Dec 6, 2022 · c463045 · c463045
3 parents bb1ef96 + 590003f + 751d795
commit c463045
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 8 deletions.
diff --git a/pkg/backend/httpstate/backend.go b/pkg/backend/httpstate/backend.go
@@ -1798,6 +1798,15 @@ func doDetectCapabilities(ctx context.Context, d diag.Sink, client *client.Clien
 		d.Warningf(diag.Message("" /*urn*/, "failed to decode capabilities: %v"), err)
 		return capabilities{}
 	}
+
+	// Allow users to opt out of deltaCheckpointUpdates even if the backend indicates it should be used. This
+	// remains necessary while PULUMI_OPTIMIZED_CHECKPOINT_PATCH has higher memory requirements on the client and
+	// may cause out-of-memory issues in constrained environments.
+	switch strings.ToLower(os.Getenv("PULUMI_OPTIMIZED_CHECKPOINT_PATCH")) {
+	case "0", "false":
+		caps.deltaCheckpointUpdates = nil
+	}
+
 	return caps
 }
 

diff --git a/sdk/go/common/util/mapper/mapper_decode.go b/sdk/go/common/util/mapper/mapper_decode.go
@@ -151,16 +151,19 @@ func (md *mapper) adjustValueForAssignment(val reflect.Value,
 			// A simple conversion exists to make this right.
 			val = val.Convert(to)
 		} else if to.Kind() == reflect.Ptr && val.Type().AssignableTo(to.Elem()) {
-			// If the target is a pointer, turn the target into a pointer.  If it's not addressable, make a copy.
-			if val.CanAddr() {
-				val = val.Addr()
+			// Here the destination type (to) is a pointer to a type that accepts val.
+			var adjusted reflect.Value // var adjusted *toElem
+			if val.CanAddr() && val.Addr().Type().AssignableTo(to) {
+				// If taking the address of val makes this right, do it.
+				adjusted = val.Addr() // adjusted = &val
 			} else {
-				slot := reflect.New(val.Type().Elem())
-				copy := reflect.ValueOf(val.Interface())
-				contract.Assert(copy.CanAddr())
-				slot.Set(copy)
-				val = slot
+				// Otherwise create a fresh pointer of the desired type and point it to val.
+				adjusted = reflect.New(to.Elem()) // adjusted = new(toElem)
+				adjusted.Elem().Set(val)          // *adjusted = val
 			}
+			// In either case, the loop condition should be sastisfied at this point.
+			contract.Assert(adjusted.Type().AssignableTo(to))
+			return adjusted, nil
 		} else if val.Kind() == reflect.Interface {
 			// It could be that the source is an interface{} with the right element type (or the right element type
 			// through a series of successive conversions); go ahead and give it a try.

diff --git a/sdk/go/common/util/mapper/mapper_test.go b/sdk/go/common/util/mapper/mapper_test.go
@@ -573,3 +573,44 @@ func TestBasicUnmap(t *testing.T) {
 		assert.False(t, hash1h)
 	}
 }
+
+func TestReproduceMapStringPointerTurnaroundIssue(t *testing.T) {
+	t.Parallel()
+
+	type X struct {
+		Args map[string]*string `pulumi:"args,optional"`
+	}
+
+	xToMap := func(build X) (map[string]interface{}, error) {
+		m, err := New(nil).Encode(build)
+		if err != nil {
+			return nil, err
+		}
+		return m, nil
+	}
+
+	xFromMap := func(pm map[string]interface{}) (X, error) {
+		var build X
+		err := New(nil).Decode(pm, &build)
+		if err != nil {
+			return X{}, err
+		}
+		return build, nil
+	}
+
+	value := "value"
+	expected := X{
+		Args: map[string]*string{
+			"key": &value,
+		},
+	}
+
+	encodedMap, err := xToMap(expected)
+	require.NoError(t, err)
+	t.Logf("encodedMap: %v", encodedMap)
+
+	back, err2 := xFromMap(encodedMap)
+	require.NoError(t, err2)
+
+	require.Equal(t, expected, back)
+}