Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement state rename command for renaming a resource #9098

Merged
merged 5 commits into from Mar 3, 2022

Conversation

Zaid-Ajaj
Copy link
Contributor

@Zaid-Ajaj Zaid-Ajaj commented Mar 3, 2022

Description

Implements state rename command. Fixes #2060

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • Yes, there are changes in this PR that warrants bumping the Pulumi Service API version

@Zaid-Ajaj Zaid-Ajaj requested a review from Frassle March 3, 2022 09:59
@Zaid-Ajaj Zaid-Ajaj added the area/cli UX of using the CLI (args, output, logs) label Mar 3, 2022
res := runStateEdit(stack, showPrompt, urn, func(snap *deploy.Snapshot, resource *resource.State) error {

// the resource is protected but the user didn't use --force
if !force && resource.Protect {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think "Protect" needs to be taken into consideration for renames. Protect is that you won't delete it, not that you won't change anything about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I will change it ✔️


// update the URN with only the name part changed
resource.URN = resource.URN.Rename(newResourceName)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to use runTotalStateEdit because we need to rename this resources URN but we also need to look at every other resource to see if it references this URN and update it's references (for things like Dependencies and PropertyDependencies)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this only a matter of changing runStateEdit to runTotalStateEdit and it will take care of the rest? I thought I actually needed to look for dependants rather than dependencies since only dependants would reference an existing resource

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes anything that is dependent on the resource being renamed will need a fix up. You don't actually need to build a dependency graph though, you can just do a straight forward iterate through everything and replace anywhere you see the old URN with the new URN I think.

urn := resource.URN(args[0])
newResourceName := args[1]
// Show the confirmation prompt if the user didn't pass the --yes parameter to skip it.
showPrompt := !yes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before running any stateEdit we should do a non-mutating pass of the current state to check the renamed URN isn't already present, and error if it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, I'll try to figure how to do that 👍

Copy link
Member

@Frassle Frassle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor changes to do but otherwise LGTM

@@ -0,0 +1,121 @@
// Copyright 2016-2018, Pulumi Corporation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Copyright 2016-2018, Pulumi Corporation.
// Copyright 2016-2022, Pulumi Corporation.

This command renames a resource from a stack's state. The resource is specified
by its Pulumi URN (use ` + "`pulumi stack --show-urns`" + ` to get it) and the new name of the resource.

Make sure that URNs are single-quoted to avoid having characters unexpectedly interpreted by the shell.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UX ❤️

Args: cmdutil.ExactArgs(2),
Run: cmdutil.RunResultFunc(func(cmd *cobra.Command, args []string) result.Result {
yes = yes || skipConfirmations()
urn := resource.URN(args[0])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably add a check here for urn.IsValid() and error out if false

if len(candidateResources) > 0 {
return errors.New("The chosen new name for the state corresponds to an already existing resource")
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally the checks above could come before the user prompt that we're going to modify state. I think this is fine for this PR but would be nice to have a think if there's a way to keep the code this simple but split the check/prompt/do work up.

@Zaid-Ajaj Zaid-Ajaj merged commit 39c44fb into master Mar 3, 2022
@pulumi-bot pulumi-bot deleted the implement-state-rename branch March 3, 2022 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli UX of using the CLI (args, output, logs)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add 'pulumi state' command for renaming resources
2 participants