Handle blank project names in 'new' (#5504)

The logic for validating prompted values in 'new' wasn't quite right,
leading to the possibility of creating Pulumi.yaml files with blank
project names.

This manifests in various ways and I've hit it a number of times
over the past few months because of the way we handle project/stack
name conflicts in 'new' -- which itself is a bit annoying too:

https://github.com/pulumi/pulumi/blob/master/pkg/cmd/pulumi/new.go#L206-L207

Because we substitue a default value of "", and because the prompting
logic assumed default values are always valid, we would skip validation
and therefore accept a blank Pulumi.yaml file.

This generates an invalid project which causes errors elsewhere, such as

    error: failed to load Pulumi project located at ".../Pulumi.yaml":
        project is missing a 'name' attribute

I hit this all the time with our getting started guide because I've
gone through it so many times and have leftover stacks from prior
run-throughs. I wouldn't be surprised if a lot of people hit this.

The solution here validates all values, including the default.

Note also that we failed to validate the value used by 'new --yes'
which meant you could bypass all validation by passing --yes, leading
to similar outcomes.

I've added a couple new tests for these cases. There is a risk we
depend on illegal default values somewhere which will now be rejected,
but that would seem strange, and assuming the tests pass, I would
assume that's not true. Let me know if that's wrong.

Fixes pulumi/pulumi#3255.

CHANGELOG.md

@@ -6,6 +6,9 @@ CHANGELOG
 - [cli] Remove eternal loop if a configured passphrase is invalid.
   [#5507](https://github.com/pulumi/pulumi/pull/5507)
 
+- Correctly validate project names during 'pulumi new'
+  [#5504](https://github.com/pulumi/pulumi/pull/5504)
+
 ## 2.11.2 (2020-10-01)
 
 - feat(autoapi): expose EnvVars LocalWorkspaceOption to set in ctor

pkg/cmd/pulumi/new.go

@@ -961,59 +961,68 @@ func promptForValue(
 	yes bool, valueType string, defaultValue string, secret bool,
 	isValidFn func(value string) error, opts display.Options) (string, error) {
 
-	if yes {
-		return defaultValue, nil
-	}
-
+	var value string
 	for {
-		var prompt string
-
-		if defaultValue == "" {
-			prompt = opts.Color.Colorize(
-				fmt.Sprintf("%s%s:%s ", colors.SpecPrompt, valueType, colors.Reset))
+		// If we are auto-accepting the default (--yes), just set it and move on to validating.
+		// Otherwise, prompt the user interactively for a value.
+		if yes {
+			value = defaultValue
 		} else {
-			defaultValuePrompt := defaultValue
+			var prompt string
+			if defaultValue == "" {
+				prompt = opts.Color.Colorize(
+					fmt.Sprintf("%s%s:%s ", colors.SpecPrompt, valueType, colors.Reset))
+			} else {
+				defaultValuePrompt := defaultValue
+				if secret {
+					defaultValuePrompt = "[secret]"
+				}
+
+				prompt = opts.Color.Colorize(
+					fmt.Sprintf("%s%s:%s (%s) ", colors.SpecPrompt, valueType, colors.Reset, defaultValuePrompt))
+			}
+			fmt.Print(prompt)
+
+			// Read the value.
+			var err error
 			if secret {
-				defaultValuePrompt = "[secret]"
+				value, err = cmdutil.ReadConsoleNoEcho("")
+				if err != nil {
+					return "", err
+				}
+			} else {
+				value, err = cmdutil.ReadConsole("")
+				if err != nil {
+					return "", err
+				}
 			}
+			value = strings.TrimSpace(value)
 
-			prompt = opts.Color.Colorize(
-				fmt.Sprintf("%s%s:%s (%s) ", colors.SpecPrompt, valueType, colors.Reset, defaultValuePrompt))
-		}
-		fmt.Print(prompt)
-
-		// Read the value.
-		var err error
-		var value string
-		if secret {
-			value, err = cmdutil.ReadConsoleNoEcho("")
-			if err != nil {
-				return "", err
-			}
-		} else {
-			value, err = cmdutil.ReadConsole("")
-			if err != nil {
-				return "", err
+			// If the user simply hit ENTER, choose the default value.
+			if value == "" {
+				value = defaultValue
 			}
 		}
-		value = strings.TrimSpace(value)
 
-		if value != "" {
-			var validationError error
-			if isValidFn != nil {
-				validationError = isValidFn(value)
+		// Ensure the resulting value is valid; note that we even validate the default, since sometimes
+		// we will have invalid default values, like "" for the project name.
+		if isValidFn != nil {
+			if validationError := isValidFn(value); validationError != nil {
+				// If validation failed, let the user know. If interactive, we will print the error and
+				// prompt the user again; otherwise, in the case of --yes, we fail and report an error.
+				msg := fmt.Sprintf("Sorry, '%s' is not a valid %s. %s.", value, valueType, validationError)
+				if yes {
+					return "", errors.New(msg)
+				}
+				fmt.Printf("%s\n", msg)
+				continue
 			}
-
-			if validationError == nil {
-				return value, nil
-			}
-
-			// The value is invalid, let the user know and try again
-			fmt.Printf("Sorry, '%s' is not a valid %s. %s.\n", value, valueType, validationError)
-			continue
 		}
-		return defaultValue, nil
+
+		break
 	}
+
+	return value, nil
 }
 
 // templatesToOptionArrayAndMap returns an array of option strings and a map of option strings to templates.

pkg/cmd/pulumi/new_test.go

@@ -401,17 +401,25 @@ func TestGeneratingProjectWithInvalidPromptedNameFails(t *testing.T) {
 	}
 
 	// Generate-only command is not creating any stacks, so don't bother with with the name uniqueness check.
-	var args = newArgs{
+	err := runNew(newArgs{
 		generateOnly:      true,
 		interactive:       true,
 		prompt:            promptMock("not#valid", ""),
 		secretsProvider:   "default",
 		templateNameOrURL: "typescript",
-	}
-
-	err := runNew(args)
+	})
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "project name may only contain")
+
+	err = runNew(newArgs{
+		generateOnly:      true,
+		interactive:       true,
+		prompt:            promptMock("", ""),
+		secretsProvider:   "default",
+		templateNameOrURL: "typescript",
+	})
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "project name may not be empty")
 }
 
 func TestInvalidTemplateName(t *testing.T) {

sdk/go/common/workspace/templates.go

@@ -514,6 +514,10 @@ var (
 // ValidateProjectName ensures a project name is valid, if it is not it returns an error with a message suitable
 // for display to an end user.
 func ValidateProjectName(s string) error {
+	if s == "" {
+		return errors.New("A project name may not be empty")
+	}
+
 	if len(s) > 100 {
 		return errors.New("A project name must be 100 characters or less")
 	}

sdk/go/common/workspace/templates_test.go

@@ -283,6 +283,11 @@ func TestProjectNames(t *testing.T) {
 			projectName: "Pulumi.Test",
 			expectError: true,
 		},
+		{
+			testName:    "Empty Project Name",
+			projectName: "",
+			expectError: true,
+		},
 	}
 
 	for _, tt := range tests {