fix: upgrade docker/docker to moby - #348
Conversation
This fixes b/524928356
There was a problem hiding this comment.
Code Review
This pull request migrates the Docker and Docker Swarm service discovery implementations from the deprecated github.com/docker/docker client to the github.com/moby/moby client. However, several critical compilation errors were identified where string fields (such as IP addresses and CIDR blocks) are treated as netip.Addr or have .Addr() called directly on them. The feedback suggests refactoring ipString to parse string addresses, using netip.ParsePrefix to handle CIDR strings, and adding the missing "net/netip" imports.
| if err != nil { | ||
| return nil, fmt.Errorf("error while parsing address %s: %w", p.Addr, err) | ||
| } | ||
| ip := p.Addr.Addr().String() |
There was a problem hiding this comment.
Calling .String() directly on an unassigned or zero-value netip.Addr evaluates to the literal string "invalid IP", which could corrupt target addresses. Since you added ipString() in docker.go, we should use it here as well to safely return an empty string.
| ip := p.Addr.Addr().String() | |
| ip := ipString(p.Addr.Addr()) |
| if err != nil { | ||
| return nil, fmt.Errorf("error while parsing address %s: %w", address, err) | ||
| } | ||
| ip := address.Addr().String() |
There was a problem hiding this comment.
Similar to services.go, calling .String() directly on a zero-value netip.Addr returns "invalid IP". We should wrap this in ipString() to keep address formatting safe.
| ip := address.Addr().String() | |
| ip := ipString(address.Addr()) |
This fixes b/524928356