tedsmitt / ecsgo

Provides an interactive prompt to connect to ECS Containers using the ECS ExecuteCommand API.
Apache License 2.0
81 stars 13 forks source link

Panic in `internal.(*ExecCommand).Start` when `-c` flag is not specified #21

Closed j-boivie closed 1 year ago

j-boivie commented 1 year ago

Hi!

I've been testing this tool but ran into an issue. When I run the tool I can navigate to a task that has ECS exec enabled, but when I select the container to exec into the program panics like this:

? Select a task: panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1553488]

goroutine 6 [running]:
github.com/tedsmitt/ecsgo/internal.(*ExecCommand).executeInput(0xc00031caa0)
        github.com/tedsmitt/ecsgo/internal/exec.go:264 +0x88
github.com/tedsmitt/ecsgo/internal.(*ExecCommand).Start.func1()
        github.com/tedsmitt/ecsgo/internal/exec.go:72 +0x1e5
created by github.com/tedsmitt/ecsgo/internal.(*ExecCommand).Start
        github.com/tedsmitt/ecsgo/internal/exec.go:58 +0xee

I'm able to work around this by manually specifying the shell to run like ecsgo -c /bin/sh, which according to the README should be the default command.

tedsmitt commented 1 year ago

Hey @j-boivie thanks for raising this! I'm currently on vacation so will look at this when I get back next week 😄 In the meantime could you provide some details to help troubleshoot such as:

Thanks again!

j-boivie commented 1 year ago

Hi,

Sorry for the late response, from my testing I've noticed that this happens on every task I connect to, no matter names, number of containers in the task etc. I've tested this across two different AWS accounts as well.

The only thing I can think of is that all of the ECS resources typically have hyphens in the name, and that they all run some variant of debian:-slim or python:-slim as base image.

Another thing I noticed is that the tool doesn't return a list of all services on the cluster, although I can see all tasks when selecting *. I'm only ever getting a list of 10 services, so I'm guessing that the reason is that this call here doesn't handle a paginated response from the ECS api: https://github.com/tedsmitt/ecsgo/blob/95178ae50b5d102c5ffea4eb3f5723c6fad40353/internal/exec.go#L123-L126

But I can raise another issue for that if needed.

tedsmitt commented 1 year ago

@j-boivie No problem! Thanks for the additional info.

Based on the error message it looks like this is the offending line: https://github.com/tedsmitt/ecsgo/blob/95178ae50b5d102c5ffea4eb3f5723c6fad40353/internal/exec.go#L264

Would I be right in assuming these tasks are running on an EC2-backed ECS cluster? I suspect that the PlatformFamily value may only be available by default in Fargate tasks, and might have to manually be specified for tasks with EC2 launch type. I have just tested this and I can recreate the problem. I'm writing up a fix now.

RE: Pagination you're absolutely right, I've not implemented this yet embarrassingly, it has been on my todo for too long 😓 If you could raise a new issue for this that'd be great!

j-boivie commented 1 year ago

@j-boivie No problem! Thanks for the additional info.

Based on the error message it looks like this is the offending line:

https://github.com/tedsmitt/ecsgo/blob/95178ae50b5d102c5ffea4eb3f5723c6fad40353/internal/exec.go#L264

Would I be right in assuming these tasks are running on an EC2-backed ECS cluster? I suspect that the PlatformFamily value may only be available by default in Fargate tasks, and might have to manually be specified for tasks with EC2 launch type. I have just tested this and I can recreate the problem. I'm writing up a fix now.

You're absolutely right, somehow I had forgotten that Fargate was a thing... 😅, but that seems reasonable.

RE: Pagination you're absolutely right, I've not implemented this yet embarrassingly, it has been on my todo for too long 😓 If you could raise a new issue for this that'd be great!

Roger that, will create a new issue 👍

tedsmitt commented 1 year ago

@j-boivie I've just pushed the fix for this in release 0.4.4, can you let me know if all looks good on your side? 😁

j-boivie commented 1 year ago

@j-boivie I've just pushed the fix for this in release 0.4.4, can you let me know if all looks good on your side? 😁

All good here, thanks a lot for the fix! 🙇