fix `pulumi new` creates new project when stack reference project name and project name don't match #13250

dixler · 2023-06-22T18:19:57Z

Description

pulumi new --stack and --name check that the project name matches before creating a new project.

Fixes #13248

Checklist

I have run make tidy to update any new dependencies
I have run make lint to verify my code passes the lint check
- I have formatted my code using gofumpt

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 Cloud API version

pulumi-bot · 2023-06-22T18:20:44Z

Changelog

[uncommitted] (2023-06-29)

Bug Fixes

[cli/new] When providing a --stack and --name to pulumi new, the project names must match before creating Pulumi.yaml.
#13250

Frassle · 2023-06-23T07:56:40Z

pkg/cmd/pulumi/new.go

@@ -82,6 +82,11 @@ func runNew(ctx context.Context, args newArgs) error {
 		IsInteractive: args.interactive,
 	}

+	// Check project name and stack reference project name are the same.


I wonder if we should just make --stack have to be a plain name. I.e. validate it's a tokens.Name not a QName. I realize the scuppers the idea of using a fully qualified ref here to set the org, but I think I'm coming around to preferring your idea of adding an "--org" flag to override that. Hell maybe we could even add a prompt as part of the new workflow to give users the choice for org if they haven't set that flag (The backend can suggest which orgs the user has access to, and if they only have one we can just default select it and not prompt so most users wouldn't see any difference here).

This might also interact with your other PR (#13234) so if we like this approach I'd put that PR on a short hold to make this change first.

I think this is sensible.

This generally sounds reasonable to me, but we should discuss a concrete proposal in ideation.

Though, I don't think we can get away with forcing --stack to be only a plain name, because it can be a fully qualified name already today. As such, this change to do this validation looks reasonable to me.

Though, I don't think we can get away with forcing --stack to be only a plain name, because it can be a fully qualified name already today.

I think we've only run into problems trying to pass a ref here internally, I haven't seen any users doing this? I suspect it would be OK to do this.

As such, this change to do this validation looks reasonable to me.

If we are going to allow a ref here then yes we should validate the project matches but we should do that by calling the backends ParseStackReference method and checking the refs Project() property. Not by doing the string split currently proposed in this change.
That Parse will also cover a lot of other validation of the ref like checking its actually a valid name.

abhinav

Seems like a reasonable change.
Minus the minor comments, would appreciate comments on some of the key behavioral bits.
And we should maybe tweak the error message a bit, WDYT?

pkg/backend/mock.go

pkg/cmd/pulumi/new.go

abhinav · 2023-06-26T22:27:57Z

pkg/cmd/pulumi/new_test.go

+			valid:       false,
+		},
+		{
+			projectName: "",
+			stackRef:    "org/foo/dev",
+			valid:       true,
+		},
+		{
+			projectName: "foo",
+			stackRef:    "",
+			valid:       true,
+		},


optional: can omit zero-value fields.

Suggested change

valid: false,

},

{

projectName: "",

stackRef: "org/foo/dev",

valid: true,

},

{

projectName: "foo",

stackRef: "",

valid: true,

},

},

{

stackRef: "org/foo/dev",

valid: true,

},

{

projectName: "foo",

valid: true,

},

We could, but I feel that it would be nicer to show this implicit detail since it's a list and I imagine it's easier to chunk if they all have the same structure.

pkg/cmd/pulumi/new_test.go

Frassle · 2023-06-29T18:44:31Z

pkg/cmd/pulumi/new.go

+	}
+
+	// Catch the case where the user has specified a fully qualified stack reference.
+	if strings.Count(stackName, "/") == 2 {


No need to check for if there's two "/" here, just let the backend do the parse.

Frassle · 2023-06-29T18:45:34Z

pkg/cmd/pulumi/new_test.go

+			valid:       true,
+		},
+	}
+	b, err := currentBackend(context.Background(), nil, display.Options{})


This should not be testing against the real backend.

…oject.

Addressed PR review comments.

Assuming Justin's checked my comments and I'm off today

dixler · 2023-06-30T15:21:35Z

bors merge

13250: fix `pulumi new` creates new project when stack reference project name and project name don't match r=dixler a=dixler  # Description `pulumi new` `--stack` and `--name` check that the project name matches before creating a new project. Fixes #13248 ## Checklist - [x] I have run `make tidy` to update any new dependencies - [x] I have run `make lint` to verify my code passes the lint check - [x] I have formatted my code using `gofumpt`  - [x] I have added tests that prove my fix is effective or that my feature works  - [x] 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 Cloud API version  Co-authored-by: Kyle Dixler <kyle@pulumi.com>

dixler · 2023-06-30T16:39:41Z

bors merge

bors · 2023-06-30T16:39:43Z

Already running a review

bors · 2023-06-30T16:43:57Z

Build failed:

bors-ok

dixler · 2023-06-30T18:16:57Z

bors merge

bors · 2023-06-30T19:11:18Z

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

bors-ok

dixler changed the title ~~Dixler/13248/pulumi new project contradiction~~ fix pulumi new creates new project when stack reference project name and project name don't match Jun 22, 2023

dixler marked this pull request as ready for review June 22, 2023 18:22

dixler requested a review from a team June 22, 2023 18:23

Frassle reviewed Jun 23, 2023

View reviewed changes

dixler force-pushed the dixler/13248/pulumi-new-project-contradiction branch from e1fa860 to 8135649 Compare June 26, 2023 17:45

dixler requested a review from Frassle June 26, 2023 17:46

abhinav reviewed Jun 26, 2023

View reviewed changes

dixler force-pushed the dixler/13248/pulumi-new-project-contradiction branch 2 times, most recently from d96d39d to d03d607 Compare June 27, 2023 18:55

dixler requested a review from justinvp June 28, 2023 16:52

justinvp approved these changes Jun 29, 2023

View reviewed changes

Frassle previously requested changes Jun 29, 2023

View reviewed changes

dixler requested a review from Frassle June 29, 2023 19:33

dixler force-pushed the dixler/13248/pulumi-new-project-contradiction branch 2 times, most recently from c69227f to d6651d6 Compare June 29, 2023 22:34

Kyle Dixler added 3 commits June 29, 2023 15:34

pulumi new --stack and --name now must match before creating pr…

dd648f6

…oject.

updated MockBackend to have mock implementation of ParseStackReference

c0012be

Addressed PR review comments.

address PR comments

d6651d6

justinvp approved these changes Jun 29, 2023

View reviewed changes

bors bot merged commit 97bc12a into master Jun 30, 2023
48 checks passed

bors bot deleted the dixler/13248/pulumi-new-project-contradiction branch June 30, 2023 19:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix `pulumi new` creates new project when stack reference project name and project name don't match #13250

fix `pulumi new` creates new project when stack reference project name and project name don't match #13250

dixler commented Jun 22, 2023 •

edited

Loading

pulumi-bot commented Jun 22, 2023 •

edited

Loading

Frassle Jun 23, 2023

dixler Jun 23, 2023

justinvp Jun 23, 2023

Frassle Jun 24, 2023

abhinav left a comment

abhinav Jun 26, 2023

dixler Jun 27, 2023 •

edited

Loading

Frassle Jun 29, 2023

Frassle Jun 29, 2023

dixler commented Jun 30, 2023

dixler commented Jun 30, 2023

bors bot commented Jun 30, 2023

bors bot commented Jun 30, 2023

dixler commented Jun 30, 2023

bors bot commented Jun 30, 2023

fix pulumi new creates new project when stack reference project name and project name don't match #13250

fix pulumi new creates new project when stack reference project name and project name don't match #13250

Conversation

dixler commented Jun 22, 2023 • edited Loading

Description

Checklist

pulumi-bot commented Jun 22, 2023 • edited Loading

Changelog

[uncommitted] (2023-06-29)

Bug Fixes

Frassle Jun 23, 2023

Choose a reason for hiding this comment

dixler Jun 23, 2023

Choose a reason for hiding this comment

justinvp Jun 23, 2023

Choose a reason for hiding this comment

Frassle Jun 24, 2023

Choose a reason for hiding this comment

abhinav left a comment

Choose a reason for hiding this comment

abhinav Jun 26, 2023

Choose a reason for hiding this comment

dixler Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

Frassle Jun 29, 2023

Choose a reason for hiding this comment

Frassle Jun 29, 2023

Choose a reason for hiding this comment

dixler commented Jun 30, 2023

dixler commented Jun 30, 2023

bors bot commented Jun 30, 2023

bors bot commented Jun 30, 2023

dixler commented Jun 30, 2023

bors bot commented Jun 30, 2023

fix `pulumi new` creates new project when stack reference project name and project name don't match #13250

fix `pulumi new` creates new project when stack reference project name and project name don't match #13250

dixler commented Jun 22, 2023 •

edited

Loading

pulumi-bot commented Jun 22, 2023 •

edited

Loading

dixler Jun 27, 2023 •

edited

Loading