Return errors on parse failures for config.Try* (#9407)

* Return a concrete type when missing a key

* Add tests + fix implementation

* Return an error for when parsing fails

* Implement the same fix for errors

* Update changelog

* Update CHANGELOG_PENDING.md

* Remove contract.Failf from require

* Fix nit + lower conversion panic

* Fix test

* Link correctly

* Fix indentation + revert lowerConversion change

CHANGELOG_PENDING.md

@@ -9,4 +9,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:

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

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.

sdk/go/pulumi/templates/config-try.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.
@@ -25,11 +25,28 @@ import (
 	"github.com/pulumi/pulumi/sdk/v3/go/pulumi"
 )
 
+var ErrMissingVar = missingVariable{}
+
+type missingVariable struct {
+	key string
+}
+
+func (m missingVariable) Error() string {
+	if m.key == "" {
+		return "missing required configuration variable"
+	}
+	return fmt.Sprintf("missing required configuration variable '%s'; run `pulumi config` to set", m.key)
+}
+
+func (m missingVariable) Is(target error) bool {
+	_, ok := target.(missingVariable)
+	return ok
+}
+
 func try(ctx *pulumi.Context, key, use, insteadOf string) (string, error) {
 	v, ok := get(ctx, key, use, insteadOf)
 	if !ok {
-		return "",
-			fmt.Errorf("missing required configuration variable '%s'; run `pulumi config` to set", key)
+		return "", missingVariable{key}
 	}
 	return v, nil
 }
@@ -60,10 +77,11 @@ func try{{.Name}}(ctx *pulumi.Context, key, use, insteadOf string) ({{.Type}}, e
 	if err != nil {
 		return {{.DefaultConfig}}, err
 	}
-	return cast.To{{.Name}}(v), nil
+	return cast.To{{.Name}}E(v)
 }
 
-// Try{{.Name}} loads an optional configuration value by its key, as a {{.Type}}, or returns an error if it doesn't exist.
+// Try{{.Name}} loads an optional configuration value by its key, as a {{.Type}},
+// or returns an error if it doesn't exist or can't be parsed.
 func Try{{.Name}}(ctx *pulumi.Context, key string) ({{.Type}}, error) {
 	return try{{.Name}}(ctx, key, "TrySecret{{.Name}}", "Try{{.Name}}")
 }