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

Fix waiting room rules resource ID #692

Merged
merged 1 commit into from
May 16, 2024

Conversation

piperswe
Copy link
Contributor

@piperswe piperswe commented Apr 2, 2024

The lack of ID was causing nil dereference errors further down.

panic: interface conversion: interface {} is nil, not string

goroutine 1 [running]:
github.com/cloudflare/cf-terraforming/internal/app/cf-terraforming/cmd.init.generateResources.func3(0x10591f540, {0x10529f7ab?, 0x4?, 0x10529f72b?})
        src/github.com/cloudflare/cf-terraforming/internal/app/cf-terraforming/cmd/generate.go:1276 +0x565c
github.com/spf13/cobra.(*Command).execute(0x10591f540, {0x14000159b00, 0x4, 0x4})
        pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:987 +0x828
github.com/spf13/cobra.(*Command).ExecuteC(0x10591f260)
        pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x344
github.com/spf13/cobra.(*Command).Execute(...)
        pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039
github.com/cloudflare/cf-terraforming/internal/app/cf-terraforming/cmd.Execute()
        src/github.com/cloudflare/cf-terraforming/internal/app/cf-terraforming/cmd/root.go:30 +0x24
main.main()
        src/github.com/cloudflare/cf-terraforming/cmd/cf-terraforming/main.go:8 +0x1c

This PR doesn't change the generated Terraform code, just the ID used for the Terraform state.

@jafowler
Copy link
Contributor

jafowler commented Apr 2, 2024

Can you add some description on the problem you're fixing? A before error and after would be ideal. Also some tests that show the problem before and after would be awesome.

@piperswe
Copy link
Contributor Author

piperswe commented Apr 3, 2024

Added description and error. The existing tests should have been failing, I don't know why they wouldn't be.

@jacobbednarz
Copy link
Member

the PR seems fine but i'm curious why this isn't failing in CI. what was the command invocation you were using here?

@jacobbednarz jacobbednarz merged commit 9dec653 into cloudflare:master May 16, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants