Return errors on parse failures for config.Try*

CHANGELOG_PENDING.md

@@ -6,4 +6,8 @@
 ### Bug Fixes
 
 - [cli/plugin] - Dynamic provider binaries will now be found even if pulumi/bin is not on $PATH.
-  [#9396](https://github.com/pulumi/pulumi/pull/9396)
+  [#9396](https://github.com/pulumi/pulumi/pull/9396)
+
+- [sdk/go] - Fail appropriatly for `config.Try*` and `config.Require*` where the
+  key is present but of the wrong type.
+  [#9407](https://github.com/pulumi/pulumi/pull/9407)

pkg/codegen/nodejs/test.go

@@ -54,7 +54,7 @@ func Check(t *testing.T, path string, dependencies codegen.StringSet, linkLocal
 	err = os.WriteFile(filepath.Join(dir, "tsconfig.json"), tsConfigJSON, 0600)
 	require.NoError(t, err)
 
-	typeCheckGeneratedPackage(t, dir, true)
+	typeCheckGeneratedPackage(t, dir, linkLocal)
 }
 
 func typeCheckGeneratedPackage(t *testing.T, pwd string, linkLocal bool) {
@@ -63,7 +63,9 @@ func typeCheckGeneratedPackage(t *testing.T, pwd string, linkLocal bool) {
 	// other places at the moment, and yarn does not run into the
 	// ${VERSION} problem; use yarn for now.
 
-	test.RunCommand(t, "yarn_link", pwd, "yarn", "link", "@pulumi/pulumi")
+	if linkLocal {
+		test.RunCommand(t, "yarn_link", pwd, "yarn", "link", "@pulumi/pulumi")
+	}
 	test.RunCommand(t, "yarn_install", pwd, "yarn", "install")
 	tscOptions := &integration.ProgramTestOptions{
 		// Avoid Out of Memory error on CI:

pkg/codegen/pcl/rewrite_convert.go

@@ -1,7 +1,6 @@
 package pcl
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/hashicorp/hcl/v2"
@@ -288,10 +287,45 @@ func convertLiteralToString(from model.Expression) (string, bool) {
 	return "", false
 }
 
+// lowerConversion performs the main logic of LowerConversion. nil, false is
+// returned if there is no conversion (safe or unsafe) between `from` and `to`.
+// This can occur when a loosely typed program is converted, or if an other
+// rewrite violated the type system.
+func lowerConversion(from model.Expression, to model.Type) (model.Type, bool) {
+	switch to := to.(type) {
+	case *model.UnionType:
+		// Assignment: it just works
+		for _, to := range to.ElementTypes {
+			if to.AssignableFrom(from.Type()) {
+				return to, true
+			}
+		}
+		conversions := make([]model.ConversionKind, len(to.ElementTypes))
+		for i, to := range to.ElementTypes {
+			conversions[i] = to.ConversionFrom(from.Type())
+			if conversions[i] == model.SafeConversion {
+				// We found a safe conversion, and we will use it. We don't need
+				// to search for more conversions.
+				return to, true
+			}
+		}
+
+		// Unsafe conversions:
+		for i, to := range to.ElementTypes {
+			if conversions[i] == model.UnsafeConversion {
+				return to, true
+			}
+		}
+		return nil, false
+	default:
+		return to, true
+	}
+}
+
 // LowerConversion lowers a conversion for a specific value, such that
 // converting `from` to a value of the returned type will produce valid code.
-// The algorithm prioritizes safe conversions over unsafe conversions, and
-// panics if a conversion could not be found.
+// The algorithm prioritizes safe conversions over unsafe conversions. If no
+// conversion can be found, nil, false is returned.
 //
 // This is useful because it cuts out conversion steps which the caller doesn't
 // need to worry about. For example:
@@ -319,33 +353,8 @@ func convertLiteralToString(from model.Expression) (string, bool) {
 // since var(string) can be safely assigned to string, but unsafely assigned to
 // enum(string: "foo", "bar").
 func LowerConversion(from model.Expression, to model.Type) model.Type {
-	switch to := to.(type) {
-	case *model.UnionType:
-		// Assignment: it just works
-		for _, to := range to.ElementTypes {
-			if to.AssignableFrom(from.Type()) {
-				return to
-			}
-		}
-		conversions := make([]model.ConversionKind, len(to.ElementTypes))
-		for i, to := range to.ElementTypes {
-			conversions[i] = to.ConversionFrom(from.Type())
-			if conversions[i] == model.SafeConversion {
-				// We found a safe conversion, and we will use it. We don't need
-				// to search for more conversions.
-				return to
-			}
-		}
-
-		// Unsafe conversions:
-		for i, to := range to.ElementTypes {
-			if conversions[i] == model.UnsafeConversion {
-				return to
-			}
-		}
-		panic(fmt.Sprintf("Could not find a conversion from %s (%s) to type %s",
-			strings.TrimSpace(fmt.Sprintf("%s", from)), from.Type(), to))
-	default:
-		return to
+	if t, ok := lowerConversion(from, to); ok {
+		return t
 	}
+	return to
 }

sdk/go/pulumi/config/config_test.go

@@ -16,6 +16,7 @@ package config
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"reflect"
 	"testing"
@@ -39,6 +40,7 @@ func TestConfig(t *testing.T) {
 			"testpkg:sss":    "a string value",
 			"testpkg:bbb":    "true",
 			"testpkg:intint": "42",
+			"testpkg:badint": "4d2",
 			"testpkg:fpfpfp": "99.963",
 			"testpkg:obj": `
 				{
@@ -96,6 +98,10 @@ func TestConfig(t *testing.T) {
 	assert.Equal(t, "a string value", cfg.Require("sss"))
 	assert.Equal(t, true, cfg.RequireBool("bbb"))
 	assert.Equal(t, 42, cfg.RequireInt("intint"))
+	assert.PanicsWithError(t,
+		"unable to parse required configuration variable"+
+			" 'testpkg:badint'; unable to cast \"4d2\" of type string to int",
+		func() { cfg.RequireInt("badint") })
 	assert.Equal(t, 99.963, cfg.RequireFloat64("fpfpfp"))
 	cfg.RequireObject("obj", &testStruct)
 	assert.Equal(t, expectedTestStruct, testStruct)
@@ -122,7 +128,7 @@ func TestConfig(t *testing.T) {
 		_ = cfg.Require("missing")
 	}()
 
-	// Test Try, which returns an error for missing entries.
+	// Test Try, which returns an error for missing or invalid entries.
 	k1, err := cfg.Try("sss")
 	assert.Nil(t, err)
 	assert.Equal(t, "a string value", k1)
@@ -132,6 +138,9 @@ func TestConfig(t *testing.T) {
 	k3, err := cfg.TryInt("intint")
 	assert.Nil(t, err)
 	assert.Equal(t, 42, k3)
+	invalidInt, err := cfg.TryInt("badint")
+	assert.Error(t, err)
+	assert.Zero(t, invalidInt)
 	k4, err := cfg.TryFloat64("fpfpfp")
 	assert.Nil(t, err)
 	assert.Equal(t, 99.963, k4)
@@ -142,16 +151,21 @@ func TestConfig(t *testing.T) {
 	testStruct = TestStruct{}
 	// missing TryObject
 	err = cfg.TryObject("missing", &testStruct)
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 	assert.Equal(t, emptyTestStruct, testStruct)
+	assert.True(t, errors.Is(err, ErrMissingVar))
 	testStruct = TestStruct{}
 	// malformed TryObject
 	err = cfg.TryObject("malobj", &testStruct)
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 	assert.Equal(t, emptyTestStruct, testStruct)
+	assert.False(t, errors.Is(err, ErrMissingVar))
 	testStruct = TestStruct{}
 	_, err = cfg.Try("missing")
-	assert.NotNil(t, err)
+	assert.Error(t, err)
+	assert.Equal(t, err.Error(),
+		"missing required configuration variable 'testpkg:missing'; run `pulumi config` to set")
+	assert.True(t, errors.Is(err, ErrMissingVar))
 }
 
 func TestSecretConfig(t *testing.T) {