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

bug(targets): fixes bug that caused a panic in authorize-session #1496

Merged
merged 1 commit into from
Sep 1, 2021

Conversation

louisruch
Copy link
Collaborator

Fixes #1488

The panic is occurring here https://github.com/hashicorp/boundary/blob/main/internal/cmd/commands/targetscmd/funcs.go#L822 and the issue is at this point the target service has populated the CredentialLibrary struct and not the CredentialSource struct. We could also update the targetscmd to support both in the interim...

Panic for reference:

boundary targets authorize-session -id ttcp_5PS2dktESb
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x1f99a31]

goroutine 1 [running]:
github.com/hashicorp/boundary/internal/cmd/commands/targetscmd.printCustomActionOutputImpl(0xc00046c0a0)
	/Users/louisruch/boundary/internal/cmd/commands/targetscmd/funcs.go:822 +0x9d1
github.com/hashicorp/boundary/internal/cmd/commands/targetscmd.(*Command).Run(0xc00046c0a0, {0xc000134030, 0x2, 0x2})
	/Users/louisruch/boundary/internal/cmd/commands/targetscmd/targets.gen.go:324 +0x188b
github.com/mitchellh/cli.(*CLI).Run(0xc0001c6500)
	/Users/louisruch/go/pkg/mod/github.com/mitchellh/[email protected]/cli.go:262 +0x5f8
github.com/hashicorp/boundary/internal/cmd.RunCustom({0xc000134010, 0x60, 0x0}, 0x0)
	/Users/louisruch/boundary/internal/cmd/main.go:186 +0x9d6
github.com/hashicorp/boundary/internal/cmd.Run(...)
	/Users/louisruch/boundary/internal/cmd/main.go:92
main.main()
	/Users/louisruch/boundary/cmd/boundary/main.go:13 +0xc9

@mgaffney
Copy link
Member

mgaffney commented Sep 1, 2021

The build is failing and running the tests locally I get this error:

--- FAIL: TestAuthorizeSession (11.64s)
    target_service_test.go:2530: maxTTL: 16m48.200001664s, defaultTTL: 8m24.100000832s
    target_service_test.go:2626:
                Error Trace:    target_service_test.go:2626
                Error:          Should be empty, but was   (*targets.SessionAuthorization)(Inverse(protocmp.Transform, protocmp.Message{
                                        "@type": s"controller.api.resources.targets.v1.SessionAuthorization",
                                        "credentials": []protocmp.Message{
                                                {
                                                        "@type":              s"controller.api.resources.targets.v1.SessionCredential",
                                +                       "credential_library": s`{id:"clvlt_m8fjNwNfG9"  name:"Library Name"  description:"Library Description"  credential_store_id:"csvlt_DdlJtk72vq"  type:"vault"}`,
                                -                       "credential_source":  s`{id:"clvlt_m8fjNwNfG9"  name:"Library Name"  description:"Library Description"  credential_store_id:"csvlt_DdlJtk72vq"  type:"vault"}`,
                                                },
                                        },
                                        "endpoint": string("tcp://hcst_nddTezwcd1-0"),
                                        "host_id":  string("hst_z9zdFTaWbm"),
                                        ... // 5 identical entries
                                  }))
                Test:           TestAuthorizeSession

@louisruch
Copy link
Collaborator Author

The build is failing and running the tests locally I get this error:

--- FAIL: TestAuthorizeSession (11.64s)
    target_service_test.go:2530: maxTTL: 16m48.200001664s, defaultTTL: 8m24.100000832s
    target_service_test.go:2626:
                Error Trace:    target_service_test.go:2626
                Error:          Should be empty, but was   (*targets.SessionAuthorization)(Inverse(protocmp.Transform, protocmp.Message{
                                        "@type": s"controller.api.resources.targets.v1.SessionAuthorization",
                                        "credentials": []protocmp.Message{
                                                {
                                                        "@type":              s"controller.api.resources.targets.v1.SessionCredential",
                                +                       "credential_library": s`{id:"clvlt_m8fjNwNfG9"  name:"Library Name"  description:"Library Description"  credential_store_id:"csvlt_DdlJtk72vq"  type:"vault"}`,
                                -                       "credential_source":  s`{id:"clvlt_m8fjNwNfG9"  name:"Library Name"  description:"Library Description"  credential_store_id:"csvlt_DdlJtk72vq"  type:"vault"}`,
                                                },
                                        },
                                        "endpoint": string("tcp://hcst_nddTezwcd1-0"),
                                        "host_id":  string("hst_z9zdFTaWbm"),
                                        ... // 5 identical entries
                                  }))
                Test:           TestAuthorizeSession

ty

@louisruch
Copy link
Collaborator Author

Updated this PR to set both CredentialLibrary and CredentialSource since CredentialLibrary will only be deprecated in 0.6.0 we need to support both

jefferai
jefferai previously approved these changes Sep 1, 2021
When renaming credential libraries -> credential sources
the target service added the credential library information
to the SessionCredential response, while the target cmd was
parsing the credential source.

This prevents a panic like the following:

    boundary targets authorize-session -id ttcp_5PS2dktESb
    panic: runtime error: invalid memory address or nil pointer dereference
    [signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x1f99a31]

    goroutine 1 [running]:
    github.com/hashicorp/boundary/internal/cmd/commands/targetscmd.printCustomActionOutputImpl(0xc00046c0a0)
    	/Users/louisruch/boundary/internal/cmd/commands/targetscmd/funcs.go:822 +0x9d1
    github.com/hashicorp/boundary/internal/cmd/commands/targetscmd.(*Command).Run(0xc00046c0a0, {0xc000134030, 0x2, 0x2})
    	/Users/louisruch/boundary/internal/cmd/commands/targetscmd/targets.gen.go:324 +0x188b
    github.com/mitchellh/cli.(*CLI).Run(0xc0001c6500)
    	/Users/louisruch/go/pkg/mod/github.com/mitchellh/[email protected]/cli.go:262 +0x5f8
    github.com/hashicorp/boundary/internal/cmd.RunCustom({0xc000134010, 0x60, 0x0}, 0x0)
    	/Users/louisruch/boundary/internal/cmd/main.go:186 +0x9d6
    github.com/hashicorp/boundary/internal/cmd.Run(...)
    	/Users/louisruch/boundary/internal/cmd/main.go:92
    main.main()
    	/Users/louisruch/boundary/cmd/boundary/main.go:13 +0xc9

Fixes #1488
@louisruch louisruch merged commit 030c09c into main Sep 1, 2021
@louisruch louisruch deleted the louis-panic branch September 1, 2021 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

boundary targets authorize-session command failed with runtime error
3 participants