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

Maps containing keys with spaces are not properly validated when those keys are used as resource names #25116

Closed
ssttgg opened this issue Jun 3, 2020 · 5 comments
Labels
bug powershell Issues that seem to be specific to launching Terraform from PowerShell UX User experience enhancements

Comments

@ssttgg
Copy link

ssttgg commented Jun 3, 2020

I can't seem run terraform import using a resource address string that has a space in it, it seems to trip up the cli argument parsing quite early on.

I've created lots of resources keyed like this using for_each and up until this point, I've encountered no other issues with spaces and just assumed they were fully supported.

Tested with Powershell on Windows 10.

Terraform Version

Terraform v0.12.26
+ provider.null v2.1.2

Terraform Configuration Files

locals {
  things = {
    "thing one" : 1
    "thing two" : 2
    "thing_three" : 3
  }
}

resource "null_resource" "things" {
  for_each = local.things
}

Steps to Reproduce

terraform init
terraform apply
terraform import 'null_resource.things[\"thing one\"]' 800778739314064726

Debug Output

Expected Behavior

It should accept the resource address string as a single valid address and attempt to execute the import.

Actual Behavior

A wall of red text, the import usage output, is shown.

@pkolyvas
Copy link
Contributor

pkolyvas commented Jun 3, 2020

Hi Steve!

Resource names don't accept spaces, so I feel that the issue here is really one of poor UX rather than a bug or enhancement request.

From our documentation:
Note: Resource names must start with a letter or underscore, and may contain only letters, digits, underscores, and dashes.

@ssttgg
Copy link
Author

ssttgg commented Jun 3, 2020

Hi Petros, thanks very much for the response.

I'm just a casual user of terraform, but it seems to me to be a bit more than a simple UX issue with the import command?

It looks like terraform is allowing me to for_each a resource with keys/resource indexes that include spaces but these addresses (despite displaying with spaces in the plan/apply output) are somehow not considered valid resource-addresses when passed to import (I note that the docs state these must be alphanumeric)?

Also, if it's just a UX issue, what is the workaround to allow me to import these resources? The docs for import clearly imply I should be able to make this work with indexed resources, but this isn't true. This is not hypothetical for me, I literally have production resources with spaces used in the key that I need to import - so any guidance would be much appreciated!

Cheers, Steve.

@pkolyvas
Copy link
Contributor

pkolyvas commented Jun 3, 2020

Sorry I wasn't more clear about my UX point.

Referring to terraform's documentation around identifiers, resource names can't have spaces.

The disconnect is between map keys being able to have spaces where resource names cannot. It seems the issue is that we're not properly validating that for you. This sits in that grey area where our UX isn't helping that be understood and a bug where we've also allowed you to get partway there.

In this specific case it might be helpful to know that the name used to identify resources in Terraform, ex null_resource.some_thing doesn't have to correspond to the human-readable name of the resource used by a given cloud provider or service.

The resource name in Terraform is a way to identify it within terraform's configuration and state, not specifically to represent the resource's proper name, which is often an attribute of the resource in Terraform.

ex:.

resource "aws_instance" "example_instance" {
  ami                  = aws_ami.example.id
  instance_type = "t3.nano"
  subnet_id        = aws_subnet.example.id

  tags = {
    Name = "My Name Example"
  }
}```

@pkolyvas pkolyvas changed the title import does not support spaces in resource addresses Maps containing keys with spaces are not properly validated when those keys are used as resource names Jun 3, 2020
@pkolyvas pkolyvas added bug UX User experience enhancements labels Jun 3, 2020
@apparentlymart
Copy link
Member

Hi @ssttgg. Thanks for reporting this!

From reviewing your logs, I can see that you are running Terraform on Windows. The quoting syntax you showed is not supported under the Windows command line syntax: quotes like ' are a syntax from Unix shells, like on Linux or Mac OS X.

The correct way to include spaces in an argument on Windows is to place the relevant portion in " quotes. Because the argument itself already includes literal quotes we must still escape those with backslashes, which leads to the following for your example:

terraform import "null_resource.things[\"thing one\"]" 800778739314064726

The situation is more complicated still when you're using PowerShell, because you must both use suitable quoting/escaping for PowerShell's benefit and suitable quoting/escaping for the underlying Windows command line processing conventions. I would recommend not running command line utilities like Terraform under PowerShell for that reason -- PowerShell's syntax is more tailored for running its own cmdlets, not normal Windows command line programs -- but if you need to run Terraform under PowerShell then you will need to study its escaping rules to formulate a string that both PowerShell and the native Windows command line conventions can understand.

Unfortunately I'm not familiar enough with PowerShell to be able to craft a convenient rule for it. The only thing I can offer is a very extreme version using the Start-Process cmdlet directly, which makes it very explicit how PowerShell should process the arguments:

& Start-Process -FilePath "terraform" -ArgumentList "import `"\`"null_resource.things[\\\`"thing one\\\`"]\`"`" 800778739314064726"

I'm sure there is a shorter way to write this, but I don't know the answer myself. If at all possible I'd recommend using the normal Windows Command Prompt, because that command interpreter natively understands the way Windows command line programs interpret arguments and so doesn't require any unusual additional escaping.

To help with understanding how a command line is being processed, Terraform outputs an early log line showing its understanding of the boundaries between the arguments, which we can see in the log you shared:

2020/06/03 12:03:31 [INFO] CLI command args: []string{"import", "null_resource.things[\"thing", "one\"]", "6926265835913175150"}

These are the raw arguments being received by Terraform after the command line was parsed using the Windows command line conventions. Before Terraform code began running, the incorrect parsing was already done, giving Terraform the following four arguments:

  • import
  • "null_resource.things["thing
  • one"]
  • 6926265835913175150

Unfortunately these are OS-level differences that Terraform cannot abstract away because the processing happens before any Terraform code is running. Windows has a different history than Unix (it's derived from MS-DOS) and so it has some different conventions based on that heritage. PowerShell in turn tries to blend Unix-style shell syntax with some syntax of its own, which makes things difficult in more complicated situations like this when lots of escaping is needed.

We've had some similar issues in the past, including #11112 and #17032. The second one has some more detailed discussion about research I did last time we were looking in this area, in case you're curious. Since we already have that one still open, I'm going to close this to consolidate all of the PowerShell-parsing-related concerns into one place. Thanks again for reporting!

@apparentlymart apparentlymart added the powershell Issues that seem to be specific to launching Terraform from PowerShell label Jun 24, 2020
@ghost
Copy link

ghost commented Jul 4, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Jul 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug powershell Issues that seem to be specific to launching Terraform from PowerShell UX User experience enhancements
Projects
None yet
Development

No branches or pull requests

3 participants