Skip to content

Fix waiting room rules resource ID #692

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 16, 2024
Merged

Fix waiting room rules resource ID #692

merged 1 commit into from
May 16, 2024

Conversation

piperswe
Copy link
Contributor

@piperswe piperswe commented Apr 2, 2024 •
edited
Loading

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
Contributor

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@jacobbednarz jacobbednarz jacobbednarz approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@piperswe @jafowler @jacobbednarz