[Bug]: HealthStrategy tries to dereference a null Health pointer #801

massenz · 2023-02-05T00:33:29Z

Testcontainers version

0.17.0

Using the latest Testcontainers version?

Yes

Host OS

Ubuntu 22.04

Host arch

amd64

Go version

1.19

Docker version

Client: Docker Engine - Community
 Version:           23.0.0
 API version:       1.42
 Go version:        go1.19.5
 Git commit:        e92dd87
 Built:             Wed Feb  1 17:47:51 2023
 OS/Arch:           linux/amd64
 Context:           default

Server: Docker Engine - Community
 Engine:
  Version:          23.0.0
  API version:      1.42 (minimum version 1.12)
  Go version:       go1.19.5
  Git commit:       d7573ab
  Built:            Wed Feb  1 17:47:51 2023
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.16
  GitCommit:        31aa4358a36870b21a992d3ad2bef29e1d693bec
 runc:
  Version:          1.1.4
  GitCommit:        v1.1.4-0-g5fd4c4d
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Docker info

Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.10.2
    Path:     /usr/libexec/docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.15.1
    Path:     /usr/libexec/docker/cli-plugins/docker-compose
  scan: Docker Scan (Docker Inc.)
    Version:  v0.23.0
    Path:     /usr/libexec/docker/cli-plugins/docker-scan

Server:
 Containers: 5
  Running: 2
  Paused: 0
  Stopped: 3
 Images: 578
 Server Version: 23.0.0
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: systemd
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 31aa4358a36870b21a992d3ad2bef29e1d693bec
 runc version: v1.1.4-0-g5fd4c4d
 init version: de40ad0
 Security Options:
  apparmor
  seccomp
   Profile: builtin
  cgroupns
 Kernel Version: 5.15.0-58-generic
 Operating System: Ubuntu 22.04.1 LTS
 OSType: linux
 Architecture: x86_64
 CPUs: 12
 Total Memory: 31.29GiB
 Name: gondor
 ID: DVVB:WRMF:MXNZ:2HZ3:JYZC:BQSD:6E7X:RY7E:IHRI:SV3A:ERUG:KTPF
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Username: massenz
 Registry: https://index.docker.io/v1/
 Experimental: false
 Insecure Registries:
  192.168.99.101:30500
  127.0.0.0/8
 Live Restore Enabled: false

What happened?

I used the code exactly as in the docs (and other variations too) and the tests failed with a panic (see log):

	req := testcontainers.ContainerRequest{
		Image:        OpaImage,
		ExposedPorts: []string{OpaPort},
		WaitingFor: wait.ForHealthCheck().
			WithPollInterval(1 * time.Second),
	}

Relevant log output

└─( ko ./gentests/internals

Running Suite: Internals Suite
==============================
Random Seed: 1675555046
Will run 1 of 1 specs

2023/02/04 15:57:27 github.com/testcontainers/testcontainers-go - Connected to docker: 
  Server Version: 23.0.0
  API Version: 1.42
  Operating System: Ubuntu 22.04.1 LTS
  Total Memory: 32045 MB
2023/02/04 15:57:27 Starting container id: 286d87d82a44 image: docker.io/testcontainers/ryuk:0.3.4
2023/02/04 15:57:27 Waiting for container id 286d87d82a44 image: docker.io/testcontainers/ryuk:0.3.4
2023/02/04 15:57:28 Container is ready id: 286d87d82a44 image: docker.io/testcontainers/ryuk:0.3.4
2023/02/04 15:57:28 Starting container id: f6de79a5282d image: openpolicyagent/opa:0.48.0
2023/02/04 15:57:28 Waiting for container id f6de79a5282d image: openpolicyagent/opa:0.48.0
•! Panic [1.011 seconds]
Gentests/Internals/Server
/home/marco/Development/CopilotIQ/opa-tests/gentests/internals/server_test.go:10
  when the tests start
  /home/marco/Development/CopilotIQ/opa-tests/gentests/internals/server_test.go:12
    should have a running OPA server [It]
    /home/marco/Development/CopilotIQ/opa-tests/gentests/internals/server_test.go:13

    Test Panicked
    runtime error: invalid memory address or nil pointer dereference
    /opt/go/src/runtime/panic.go:260

    Full Stack Trace
    github.com/testcontainers/testcontainers-go/wait.(*HealthStrategy).WaitUntilReady(0xc000120000, {0xb72898?, 0xc00002e110?}, {0xb73d60, 0xc000000320})
    	/home/marco/.local/go/pkg/mod/github.com/testcontainers/testcontainers-go@v0.17.0/wait/health.go:79 +0xd9
    github.com/testcontainers/testcontainers-go.(*DockerContainer).Start(0xc000000320, {0xb72898, 0xc00002e110})
    	/home/marco/.local/go/pkg/mod/github.com/testcontainers/testcontainers-go@v0.17.0/docker.go:198 +0x24f
    github.com/testcontainers/testcontainers-go.GenericContainer({_, _}, {{{{0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}, 0x0, 0x0, ...}, ...}, ...})
    	/home/marco/.local/go/pkg/mod/github.com/testcontainers/testcontainers-go@v0.17.0/generic.go:75 +0x3d4
    github.com/CopilotIQ/opa-tests/gentests/internals.NewOpaContainer({0xb72898, 0xc00002e110}, {0xa87143, 0xc})
    	/home/marco/Development/CopilotIQ/opa-tests/gentests/internals/server.go:36 +0x1ae
    github.com/CopilotIQ/opa-tests/gentests/internals_test.glob..func1.1.1()
    	/home/marco/Development/CopilotIQ/opa-tests/gentests/internals/server_test.go:14 +0x45
    github.com/onsi/ginkgo/internal/leafnodes.(*runner).runSync(0xc00003c4e0?)
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/internal/leafnodes/runner.go:113 +0xb1
    github.com/onsi/ginkgo/internal/leafnodes.(*runner).run(0x417610?)
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/internal/leafnodes/runner.go:64 +0x125
    github.com/onsi/ginkgo/internal/leafnodes.(*ItNode).Run(0x400?)
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/internal/leafnodes/it_node.go:26 +0x7b
    github.com/onsi/ginkgo/internal/spec.(*Spec).runSample(0xc0003ac000, 0xc000071af0?, {0xb6d600, 0xc000300ec0})
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/internal/spec/spec.go:215 +0x2a9
    github.com/onsi/ginkgo/internal/spec.(*Spec).Run(0xc0003ac000, {0xb6d600, 0xc000300ec0})
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/internal/spec/spec.go:138 +0xf2
    github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).runSpec(0xc000360780, 0xc0003ac000)
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/internal/specrunner/spec_runner.go:200 +0xf1
    github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).runSpecs(0xc000360780)
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/internal/specrunner/spec_runner.go:170 +0x1b6
    github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).Run(0xc000360780)
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/internal/specrunner/spec_runner.go:66 +0xc5
    github.com/onsi/ginkgo/internal/suite.(*Suite).Run(0xc0002f6690, {0x7f263aa33ca0, 0xc0000d6d00}, {0xa896b1, 0xf}, {0xc00035e8f0, 0x1, 0x1}, {0xb73360, 0xc000300ec0}, ...)
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/internal/suite/suite.go:62 +0x4c5
    github.com/onsi/ginkgo.RunSpecsWithCustomReporters({0xb6e2a0?, 0xc0000d6d00}, {0xa896b1, 0xf}, {0xc000060718, 0x1, 0xc000060728?})
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/ginkgo_dsl.go:226 +0x189
    github.com/onsi/ginkgo.RunSpecs({0xb6e2a0, 0xc0000d6d00}, {0xa896b1, 0xf})
    	/home/marco/.local/go/pkg/mod/github.com/onsi/ginkgo@v1.12.1/ginkgo_dsl.go:207 +0x10b
    github.com/CopilotIQ/opa-tests/gentests/internals_test.TestInternals(0x0?)
    	/home/marco/Development/CopilotIQ/opa-tests/gentests/internals/internals_suite_test.go:12 +0x85
    testing.tRunner(0xc0000d6d00, 0xac79e0)
    	/opt/go/src/testing/testing.go:1446 +0x10b
    created by testing.(*T).Run
    	/opt/go/src/testing/testing.go:1493 +0x35f
------------------------------


Summarizing 1 Failure:

[Panic!] Gentests/Internals/Server when the tests start [It] should have a running OPA server 
/opt/go/src/runtime/panic.go:260

Ran 1 of 1 Specs in 1.011 seconds
FAIL! -- 0 Passed | 1 Failed | 0 Pending | 0 Skipped
--- FAIL: TestInternals (1.01s)
FAIL

Ginkgo ran 1 suite in 2.027766693s
Test Suite Failed

Additional information

This is caused by this line:

if state.Health.Status != "healthy" {

due to Health being nil (at least, initially) - however, the following should work (I've rewritten the strategy and tested to work, but it's been a very limited testing on just that one container):

if state.Health == nil || state.Health.Status != "healthy" {

Whether this is "intended" I am not sure (but it achieves what I want, to wait for the container to be ready).

In any event, as a matter of best practice, before dereferencing "two levels deep" it would be better to defend against nil values.

The text was updated successfully, but these errors were encountered:

mdelapenya · 2023-02-05T07:46:48Z

Hi @massenz thanks for opening this issue. I think you are right, that double dereference should be checked.

Would you mind sending a patch for it, hopefully adding a test for a nil health check? I can do it my self, but later this week 🙏

Thanks!

massenz · 2023-02-05T19:27:48Z

Of course, I'd love to!
❤️ TestContainers, would be really nice to be a (tiny, tiny) contributor 😄

Probably later in the day, or tomorrow at the latest.

* [#801] Fix nil pointer dereference in HealthStrategy * Added another test to prove HealthStrategy fails for nil Health * Refactored error checking * Update wait/health_test.go Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com> * Update wait/health_test.go Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com> * Update wait/health_test.go Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com> --------- Co-authored-by: Manuel de la Peña <social.mdelapenya@gmail.com>

massenz · 2023-02-09T08:44:14Z

Thank you so much, @mdelapenya for jumping so quickly on this one and helping out.
As I mentioned, I ❤️ Testcontainers, and it makes me so happy to have made a tiny, tiny contribution! 🎉

If there is any issue that you think you'd like me take a look at, I'd be happy to continue contributing
🙏

mdelapenya · 2023-02-09T09:10:19Z

My pleasure @massenz!

Feel free to visit the issues, but I'm specially proud of the new example modules system. Would you like to contribute an example module for the technology you use the most? You can find docs here: https://golang.testcontainers.org/examples/ and a list of target modules here: #636

In any other case, contributions are always welcomed, be it in code, in docs, in tests... 🙏

mdelapenya · 2023-02-09T09:10:48Z

Ah, last one tiny thing: it's Testcontainers, no camel-case 🤫

massenz added the bug An issue with the library label Feb 5, 2023

mdelapenya added the low hanging fruit Issues that are good for newcomers. label Feb 5, 2023

massenz added a commit to massenz/testcontainers-go that referenced this issue Feb 6, 2023

[testcontainers#801] Fix nil pointer dereference in HealthStrategy

6eef6ce

massenz mentioned this issue Feb 6, 2023

fix: nil pointer dereference in HealthStrategy #802

Merged

massenz added a commit to massenz/testcontainers-go that referenced this issue Feb 7, 2023

[testcontainers#801] Fix nil pointer dereference in HealthStrategy

edb570b

massenz added a commit to massenz/testcontainers-go that referenced this issue Feb 8, 2023

[testcontainers#801] Fix nil pointer dereference in HealthStrategy

33a27ca

mdelapenya closed this as completed in #802 Feb 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug]: HealthStrategy tries to dereference a null Health pointer #801

[Bug]: HealthStrategy tries to dereference a null Health pointer #801

massenz commented Feb 5, 2023 •

edited

Loading

mdelapenya commented Feb 5, 2023

massenz commented Feb 5, 2023

massenz commented Feb 9, 2023 •

edited

Loading

mdelapenya commented Feb 9, 2023

mdelapenya commented Feb 9, 2023

[Bug]: HealthStrategy tries to dereference a null Health pointer #801

[Bug]: HealthStrategy tries to dereference a null Health pointer #801

Comments

massenz commented Feb 5, 2023 • edited Loading

Testcontainers version

Using the latest Testcontainers version?

Host OS

Host arch

Go version

Docker version

Docker info

What happened?

Relevant log output

Additional information

mdelapenya commented Feb 5, 2023

massenz commented Feb 5, 2023

massenz commented Feb 9, 2023 • edited Loading

mdelapenya commented Feb 9, 2023

mdelapenya commented Feb 9, 2023

massenz commented Feb 5, 2023 •

edited

Loading

massenz commented Feb 9, 2023 •

edited

Loading