Merge 1c76854 into f629d53

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/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.PanicsWithValue(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) {

sdk/go/pulumi/templates/config-require.go.template

@@ -1,4 +1,4 @@
-// Copyright 2016-2018, Pulumi Corporation.
+// Copyright 2016-2022, Pulumi Corporation.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -18,17 +18,21 @@ package config
 
 import (
 	"encoding/json"
+    "fmt"
 
 	"github.com/spf13/cast"
 
-	"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
 	"github.com/pulumi/pulumi/sdk/v3/go/pulumi"
 )
 
+func failf(format string, a ...interface{}) {
+	panic(fmt.Errorf(format, a...))
+}
+
 func require(ctx *pulumi.Context, key, use, insteadOf string) string {
 	v, ok := get(ctx, key, use, insteadOf)
 	if !ok {
-		contract.Failf("missing required configuration variable '%s'; run `pulumi config` to set", key)
+		failf("missing required configuration variable '%s'; run `pulumi config` to set", key)
 	}
 	return v
 }
@@ -41,7 +45,7 @@ func Require(ctx *pulumi.Context, key string) string {
 func requireObject(ctx *pulumi.Context, key string, output interface{}, use, insteadOf string) {
 	v := require(ctx, key, use, insteadOf)
 	if err := json.Unmarshal([]byte(v), output); err != nil {
-		contract.Failf("unable to unmarshall required configuration variable '%s'; %s", key, err.Error())
+		failf("unable to unmarshall required configuration variable '%s'; %s", key, err.Error())
 	}
 }
 
@@ -55,7 +59,11 @@ func RequireObject(ctx *pulumi.Context, key string, output interface{}) {
 {{if .GenerateConfig}}
 func require{{.Name}}(ctx *pulumi.Context, key, use, insteadOf string) {{.Type}} {
 	v := require(ctx, key, use, insteadOf)
-	return cast.To{{.Name}}(v)
+    o, err := cast.To{{.Name}}E(v)
+    if err != nil {
+		failf("unable to parse required configuration variable '%s'; %s", key, err.Error())
+	}
+	return o
 }
 
 // Require{{.Name}} loads an optional configuration value by its key, as a {{.Type}}, or panics if it doesn't exist.