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

Panic on random_string when all properties set to false #549

Closed
1 task done
guineveresaenger opened this issue Apr 9, 2024 · 3 comments · Fixed by #551
Closed
1 task done

Panic on random_string when all properties set to false #549

guineveresaenger opened this issue Apr 9, 2024 · 3 comments · Fixed by #551

Comments

@guineveresaenger
Copy link

Terraform CLI and Provider Versions

Terraform v1.5.7
on darwin_arm64

  • provider registry.terraform.io/hashicorp/random v3.6.0

Terraform Configuration

terraform {
  required_providers {
    random = {
      version = "3.6.0"
    }
  }
}

provider "random" {

}

# Create a random string resource
resource "random_string" "my_string" {
  length  = 16
  special = false
  upper   = false
  number  = false
  lower   = false
}

# Output the generated random string
output "random_string_result" {
  value = random_string.my_string.result
}

Expected Behavior

I should see an error telling me my configuration is nonsensical.

Actual Behavior

I got a panic.

Steps to Reproduce

  1. terraform apply
  2. See panic stack trace:
Stack trace from the terraform-provider-random_v3.6.0_x5 plugin:

panic: crypto/rand: argument to Int is <= 0

goroutine 26 [running]:
crypto/rand.Int({0x102d2e600?, 0x14000142380?}, 0xffffffffffffffff?)
	crypto/rand/util.go:64 +0x29c
github.com/terraform-providers/terraform-provider-random/internal/random.generateRandomBytes(0x14000182c08, 0x10)
	github.com/terraform-providers/terraform-provider-random/internal/random/string.go:90 +0xc8
github.com/terraform-providers/terraform-provider-random/internal/random.CreateString({0x10, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, {0x0, ...}})
	github.com/terraform-providers/terraform-provider-random/internal/random/string.go:67 +0x35c
github.com/terraform-providers/terraform-provider-random/internal/provider.(*stringResource).Create(0xd?, {0x102d34738, 0x14000354cf0}, {{{{0x102d38960, 0x14000355ce0}, {0x102c585e0, 0x14000355b60}}, {0x102d39b18, 0x14000136d20}}, {{{0x102d38960, ...}, ...}, ...}, ...}, ...)
	github.com/terraform-providers/terraform-provider-random/internal/provider/resource_string.go:71 +0x2c8
github.com/hashicorp/terraform-plugin-framework/internal/fwserver.(*Server).CreateResource(0x1400015d600, {0x102d34738, 0x14000354cf0}, 0x14000183320, 0x140001832c0)
	github.com/hashicorp/terraform-plugin-framework@v1.4.2/internal/fwserver/server_createresource.go:101 +0x430
github.com/hashicorp/terraform-plugin-framework/internal/fwserver.(*Server).ApplyResourceChange(0x14000183480?, {0x102d34738, 0x14000354cf0}, 0x14000356410, 0x14000183480)
	github.com/hashicorp/terraform-plugin-framework@v1.4.2/internal/fwserver/server_applyresourcechange.go:57 +0x370
github.com/hashicorp/terraform-plugin-framework/internal/proto5server.(*Server).ApplyResourceChange(0x1400015d600, {0x102d34738?, 0x14000354ba0?}, 0x14000356370)
	github.com/hashicorp/terraform-plugin-framework@v1.4.2/internal/proto5server/server_applyresourcechange.go:55 +0x314
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ApplyResourceChange(0x140001e0640, {0x102d34738?, 0x140003541b0?}, 0x14000540070)
	github.com/hashicorp/terraform-plugin-go@v0.19.1/tfprotov5/tf5server/server.go:859 +0x3bc
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ApplyResourceChange_Handler({0x102d080a0?, 0x140001e0640}, {0x102d34738, 0x140003541b0}, 0x14000412080, 0x0)
	github.com/hashicorp/terraform-plugin-go@v0.19.1/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:467 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0x140001de1e0, {0x102d34738, 0x140003540f0}, {0x102d38c98, 0x1400048a1a0}, 0x14000358000, 0x1400033e210, 0x10317a538, 0x0)
	google.golang.org/grpc@v1.59.0/server.go:1343 +0xc6c
google.golang.org/grpc.(*Server).handleStream(0x140001de1e0, {0x102d38c98, 0x1400048a1a0}, 0x14000358000)
	google.golang.org/grpc@v1.59.0/server.go:1737 +0x9f0
google.golang.org/grpc.(*Server).serveStreams.func1.1()
	google.golang.org/grpc@v1.59.0/server.go:986 +0x88
created by google.golang.org/grpc.(*Server).serveStreams.func1
	google.golang.org/grpc@v1.59.0/server.go:997 +0x174

Error: The terraform-provider-random_v3.6.0_x5 plugin crashed!

This is always indicative of a bug within the plugin. It would be immensely
helpful if you could report the crash with the plugin's maintainers so that it
can be fixed. The output above should help diagnose the issue.

How much impact is this issue causing?

Low

Logs

https://gist.github.com/guineveresaenger/79f6457a8ad86c182c9e46c589edb8bd

Additional Information

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@github-actions github-actions bot added the crash label Apr 9, 2024
bendbennett added a commit that referenced this issue Apr 10, 2024
…pper, lower, numeric/number are set to false (#549)

Previously, a panic would be generated if the following configuration was used:

```
resource "random_string" "random" {
  length  = 16
  special = false
  upper   = false
  lower   = false
  numeric = false
}
```

```
╷
│ Error: Plugin did not respond
│
│   with random_string.random,
│   on resource.tf line 1, in resource "random_string" "random":
│    1: resource "random_string" "random" {
│
│ The plugin encountered an error, and failed to respond to the plugin.(*GRPCProvider).ApplyResourceChange call. The plugin logs may contain more details.
╵

Stack trace from the terraform-provider-random plugin:

panic: crypto/rand: argument to Int is <= 0
```

This change will cause validation to fail if special, upper, lower, and numeric/number are all set to false

Output from acceptance testing:

```console
TF_ACC=1 go test -count=1 -run='TestAccResourceString_NumericFalse' -timeout=10m -v ./internal/provider
=== RUN   TestAccResourceString_NumericFalse
=== PAUSE TestAccResourceString_NumericFalse
=== CONT  TestAccResourceString_NumericFalse
--- PASS: TestAccResourceString_NumericFalse (0.15s)
```
@bendbennett
Copy link
Contributor

Hi @guineveresaenger 👋

Thank you for reporting this bug.
It looks like we were both working on a fix at the same time!

I'm interested to hear the more about the use-case for assigning special, upper, lower and numeric/number all to false at the same time. Can you provide some further context on when you are doing this and why?

@guineveresaenger
Copy link
Author

hi @bendbennett! thank you for the quick fix, I'm going to close mine in favor of yours.

I'm interested to hear the more about the use-case for assigning special, upper, lower and numeric/number all to false at the same time. Can you provide some further context on when you are doing this and why?

Honestly, we were running edge case tests and this came up - this did not reflect a real-world error, which, while not impossible, would be unusual to have happen, since defaults are set to true. Anyway, an error message is a much nicer experience than a flat out panic. Thanks again!

bendbennett added a commit that referenced this issue Apr 15, 2024
… set is empty (#551)

* resource/random_password+random_string: Prevent panic when special, upper, lower, numeric/number are set to false (#549)

Previously, a panic would be generated if the following configuration was used:

```
resource "random_string" "random" {
  length  = 16
  special = false
  upper   = false
  lower   = false
  numeric = false
}
```

```
╷
│ Error: Plugin did not respond
│
│   with random_string.random,
│   on resource.tf line 1, in resource "random_string" "random":
│    1: resource "random_string" "random" {
│
│ The plugin encountered an error, and failed to respond to the plugin.(*GRPCProvider).ApplyResourceChange call. The plugin logs may contain more details.
╵

Stack trace from the terraform-provider-random plugin:

panic: crypto/rand: argument to Int is <= 0
```

This change will cause validation to fail if special, upper, lower, and numeric/number are all set to false

Output from acceptance testing:

```console
TF_ACC=1 go test -count=1 -run='TestAccResourceString_NumericFalse' -timeout=10m -v ./internal/provider
=== RUN   TestAccResourceString_NumericFalse
=== PAUSE TestAccResourceString_NumericFalse
=== CONT  TestAccResourceString_NumericFalse
--- PASS: TestAccResourceString_NumericFalse (0.15s)
```

* Adding changelog entries and copyright header (#549)

* Linting

* Updating description for number and numeric attributes on resource_password and resource_string
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants