diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ce6cac..ba3f55c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,36 @@ env: BASE_IMAGE_NAME: ${{ github.repository }} jobs: + go-checks: + if: github.ref != 'refs/heads/main' + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: true + + - name: Check formatting + run: | + unformatted=$(gofmt -l ./cmd ./pkg) + if [ -n "$unformatted" ]; then + echo "The following files are not gofmt-formatted:" + echo "$unformatted" + exit 1 + fi + + - name: Vet + run: go vet ./... + + - name: Run unit tests + run: go test -race ./... + test: if: github.ref != 'refs/heads/main' runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 0460596..6bc86d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added + +- Unit tests for the pure parsing/config helpers in `dinghy-layer`, `dns-server`, `config`, and `utils` ([#101](https://github.com/sparkfabrik/http-proxy/issues/101)) +- CI `go-checks` job running `gofmt`, `go vet`, and `go test -race` on every non-`main` branch ([#101](https://github.com/sparkfabrik/http-proxy/issues/101)) - Expose DNS server TCP port 19322 alongside UDP port for Lima virtualization compatibility ([#56](https://github.com/sparkfabrik/http-proxy/issues/56)) - Add `upgrade` command to pull latest Docker images and recreate only changed containers, preserving volumes (grafana/prometheus data) ([#96](https://github.com/sparkfabrik/http-proxy/pull/96)) - Add `self-update` command to update the script and compose files from the git repository, with guards against non-git installs and dirty working trees ([#96](https://github.com/sparkfabrik/http-proxy/pull/96)) ### Fixed + +- Make backend IP and port selection deterministic for `VIRTUAL_HOST` containers attached to multiple networks or exposing multiple ports; previously Go map iteration could route to a different network IP or port across restarts ([#101](https://github.com/sparkfabrik/http-proxy/issues/101)) +- Lower generated DNS A-record TTL from 3600s to 60s so a changed `HTTP_PROXY_DNS_TARGET_IP` propagates quickly instead of being cached by the OS stub resolver ([#101](https://github.com/sparkfabrik/http-proxy/issues/101)) +- Guard against a nil-pointer panic in `join-networks` when a container reports no network settings ([#101](https://github.com/sparkfabrik/http-proxy/issues/101)) +- Make signal-driven shutdown deterministic in the event-driven services by giving a single owner control of signal handling, and abort the event-stream reconnect backoff promptly on shutdown ([#101](https://github.com/sparkfabrik/http-proxy/issues/101)) +- Reconnect to the Docker event stream when the daemon closes it (for example on daemon restart) instead of busy-looping on the closed channel ([#101](https://github.com/sparkfabrik/http-proxy/issues/101)) +- Reject a non-IPv4 `HTTP_PROXY_DNS_TARGET_IP` at startup; the DNS server answers only A records, so an IPv6 target would otherwise be silently truncated ([#101](https://github.com/sparkfabrik/http-proxy/issues/101)) - Fixed restart command to automatically start containers when not running instead of failing ([#40](https://github.com/sparkfabrik/http-proxy/issues/40)) - The `spark-http-proxy restart` command now intelligently detects container state - When containers are not running: automatically starts them using existing recreate logic @@ -23,4 +33,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Apply `disable-hsts` middleware at the HTTPS entrypoint level to ensure ALL HTTPS traffic (both dinghy-layer and native Traefik routes) benefits from this development-friendly configuration ### Added + - CHANGELOG.md file to track project changes diff --git a/cmd/dinghy-layer/main.go b/cmd/dinghy-layer/main.go index db87070..16e8118 100644 --- a/cmd/dinghy-layer/main.go +++ b/cmd/dinghy-layer/main.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "regexp" + "sort" "strconv" "strings" @@ -82,17 +83,6 @@ func (cl *CompatibilityLayer) SetDependencies(dockerClient *client.Client, logge cl.logger = logger } -// TraefikLabels represents the Traefik labels that would be applied to containers -// for manual configuration. This struct is used for reference but the actual -// implementation generates dynamic configuration files instead of container labels. -type TraefikLabels struct { - Enable string - Rule string - Port string - RouterName string - ServiceName string -} - // ContainerInfo holds essential container information extracted from Docker // container inspection. This struct contains the minimal set of data needed // to generate Traefik configuration from nginx-proxy environment variables. @@ -207,17 +197,13 @@ func (cl *CompatibilityLayer) processContainer(ctx context.Context, containerID return nil } - // Skip if traefik labels are already set. - // Check for traefik labels (any label starting with "traefik.") - labels := inspect.Config.Labels - for label := range labels { - if strings.HasPrefix(label, "traefik.") { - cl.logger.Debug("Skipping container with existing Traefik label", - "container_id", utils.FormatDockerID(containerID), - "container_name", containerInfo.Name, - "label", label) - return nil - } + // Skip if traefik labels are already set; native labels take precedence and + // Traefik's Docker provider handles those containers directly. + if utils.HasTraefikLabel(inspect.Config.Labels) { + cl.logger.Debug("Skipping container with existing Traefik label", + "container_id", utils.FormatDockerID(containerID), + "container_name", containerInfo.Name) + return nil } cl.logger.Info("Found container with VIRTUAL_HOST", @@ -311,12 +297,22 @@ func (cl *CompatibilityLayer) generateTraefikConfig(inspect types.ContainerJSON, } func getContainerIP(inspect types.ContainerJSON) string { - // Try to get IP from the first network - if inspect.NetworkSettings != nil && inspect.NetworkSettings.Networks != nil { - for _, network := range inspect.NetworkSettings.Networks { - if network.IPAddress != "" { - return network.IPAddress - } + if inspect.NetworkSettings == nil || inspect.NetworkSettings.Networks == nil { + return "" + } + + // Sort network names so the chosen IP is deterministic across restarts and + // events. Go map iteration order is randomized, which would otherwise make + // the routed backend IP vary for containers attached to multiple networks. + names := make([]string, 0, len(inspect.NetworkSettings.Networks)) + for name := range inspect.NetworkSettings.Networks { + names = append(names, name) + } + sort.Strings(names) + + for _, name := range names { + if ip := inspect.NetworkSettings.Networks[name].IPAddress; ip != "" { + return ip } } return "" @@ -496,27 +492,59 @@ func generateServiceName(containerName string) string { } func getDefaultPort(inspect types.ContainerJSON) string { - // Get the first exposed port or return "80" as default + // Prefer the lowest exposed TCP port, then fall back to the lowest bound TCP + // port. Sorting makes the selection deterministic; Go map iteration order is + // randomized, which would otherwise pick a different port across restarts for + // containers that expose more than one port. + var exposed []int if inspect.Config.ExposedPorts != nil { for port := range inspect.Config.ExposedPorts { - if strings.HasSuffix(string(port), "/tcp") { - return strings.TrimSuffix(string(port), "/tcp") + if n := tcpPortNumber(string(port)); n > 0 { + exposed = append(exposed, n) } } } + if port := lowestTCPPort(exposed); port != "" { + return port + } - // Check port bindings + var bound []int if inspect.NetworkSettings != nil && inspect.NetworkSettings.Ports != nil { for port := range inspect.NetworkSettings.Ports { - if strings.HasSuffix(string(port), "/tcp") { - return strings.TrimSuffix(string(port), "/tcp") + if n := tcpPortNumber(string(port)); n > 0 { + bound = append(bound, n) } } } + if port := lowestTCPPort(bound); port != "" { + return port + } return "80" } +// lowestTCPPort returns the smallest port in the slice as a string, or "" if empty. +func lowestTCPPort(ports []int) string { + if len(ports) == 0 { + return "" + } + sort.Ints(ports) + return strconv.Itoa(ports[0]) +} + +// tcpPortNumber returns the numeric port for a Docker "/tcp" port string, +// or 0 if the string is not a TCP port or cannot be parsed. +func tcpPortNumber(port string) int { + if !strings.HasSuffix(port, "/tcp") { + return 0 + } + n, err := strconv.Atoi(strings.TrimSuffix(port, "/tcp")) + if err != nil { + return 0 + } + return n +} + // configFileName returns the config file name for a container func (cl *CompatibilityLayer) configFileName(containerID string) string { return fmt.Sprintf("%s.yaml", utils.FormatDockerID(containerID)) diff --git a/cmd/dinghy-layer/main_test.go b/cmd/dinghy-layer/main_test.go new file mode 100644 index 0000000..8a98cf6 --- /dev/null +++ b/cmd/dinghy-layer/main_test.go @@ -0,0 +1,325 @@ +package main + +import ( + "testing" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/network" + "github.com/docker/go-connections/nat" + "github.com/sparkfabrik/http-proxy/pkg/logger" +) + +func testLayer() *CompatibilityLayer { + return &CompatibilityLayer{ + logger: logger.New("test"), + config: &CompatibilityConfig{TraefikDynamicDir: "/tmp"}, + } +} + +func inspectWithIP(name, ip string) types.ContainerJSON { + return types.ContainerJSON{ + ContainerJSONBase: &types.ContainerJSONBase{Name: name}, + Config: &container.Config{}, + NetworkSettings: &types.NetworkSettings{ + Networks: map[string]*network.EndpointSettings{ + "default": {IPAddress: ip}, + }, + }, + } +} + +func TestParseVirtualHosts(t *testing.T) { + tests := []struct { + name string + in string + want []virtualHost + }{ + {"empty", "", nil}, + {"single", "app.loc", []virtualHost{{hostname: "app.loc"}}}, + {"single with port", "app.loc:8080", []virtualHost{{hostname: "app.loc", port: "8080"}}}, + {"multiple", "app.loc,api.loc", []virtualHost{{hostname: "app.loc"}, {hostname: "api.loc"}}}, + {"whitespace trimmed", " app.loc , api.loc ", []virtualHost{{hostname: "app.loc"}, {hostname: "api.loc"}}}, + {"empty entries skipped", "app.loc,,api.loc,", []virtualHost{{hostname: "app.loc"}, {hostname: "api.loc"}}}, + {"non-numeric colon not a port", "app.loc:abc", []virtualHost{{hostname: "app.loc:abc"}}}, + {"out-of-range port not a port", "app.loc:70000", []virtualHost{{hostname: "app.loc:70000"}}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseVirtualHosts(tt.in) + if len(got) != len(tt.want) { + t.Fatalf("parseVirtualHosts(%q) = %+v, want %+v", tt.in, got, tt.want) + } + for i := range got { + if got[i] != tt.want[i] { + t.Errorf("entry %d = %+v, want %+v", i, got[i], tt.want[i]) + } + } + }) + } +} + +func TestIsPort(t *testing.T) { + tests := []struct { + in string + want bool + }{ + {"80", true}, + {"65535", true}, + {"1", true}, + {"0", false}, + {"65536", false}, + {"-1", false}, + {"abc", false}, + {"", false}, + } + for _, tt := range tests { + if got := isPort(tt.in); got != tt.want { + t.Errorf("isPort(%q) = %v, want %v", tt.in, got, tt.want) + } + } +} + +func TestIsWildcardHost(t *testing.T) { + tests := []struct { + in string + want bool + }{ + {"app.loc", false}, + {"*.app.loc", true}, + {"~^api\\..*\\.loc$", true}, + } + for _, tt := range tests { + if got := isWildcardHost(tt.in); got != tt.want { + t.Errorf("isWildcardHost(%q) = %v, want %v", tt.in, got, tt.want) + } + } +} + +func TestConvertWildcardToRegex(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {"wildcard", "*.app.loc", `^.*\.app\.loc$`}, + {"regex passthrough", "~^api\\.loc$", `^api\.loc$`}, + {"plain host escaped", "app.loc", `^app\.loc$`}, + {"too long rejected", string(make([]byte, 254)), ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := convertWildcardToRegex(tt.in); got != tt.want { + t.Errorf("convertWildcardToRegex(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +func TestConvertWildcardToRegexRejectsTooManyWildcards(t *testing.T) { + if got := convertWildcardToRegex("*.*.*.*.*.*.loc"); got != "" { + t.Errorf("expected empty result for excessive wildcards, got %q", got) + } +} + +func TestGenerateServiceName(t *testing.T) { + tests := []struct { + in string + want string + }{ + {"/my-app", "my-app"}, + {"/my_app", "my-app"}, + {"/My.App", "My-App"}, + {"/a--b", "a-b"}, + {"/-app-", "app"}, + {"/", "service"}, + {"/!@#", "service"}, + } + for _, tt := range tests { + if got := generateServiceName(tt.in); got != tt.want { + t.Errorf("generateServiceName(%q) = %q, want %q", tt.in, got, tt.want) + } + } +} + +func TestTCPPortNumber(t *testing.T) { + tests := []struct { + in string + want int + }{ + {"80/tcp", 80}, + {"8080/tcp", 8080}, + {"53/udp", 0}, + {"abc/tcp", 0}, + {"80", 0}, + } + for _, tt := range tests { + if got := tcpPortNumber(tt.in); got != tt.want { + t.Errorf("tcpPortNumber(%q) = %d, want %d", tt.in, got, tt.want) + } + } +} + +func TestGetEffectivePort(t *testing.T) { + empty := types.ContainerJSON{Config: &container.Config{}} + + // Host-level port wins over VIRTUAL_PORT. + if got := getEffectivePort([]virtualHost{{hostname: "a", port: "9000"}}, "8080", empty); got != "9000" { + t.Errorf("host port should win, got %q", got) + } + // VIRTUAL_PORT used when no host port. + if got := getEffectivePort([]virtualHost{{hostname: "a"}}, "8080", empty); got != "8080" { + t.Errorf("VIRTUAL_PORT should be used, got %q", got) + } + // Falls back to 80 when nothing specified. + if got := getEffectivePort([]virtualHost{{hostname: "a"}}, "", empty); got != "80" { + t.Errorf("default should be 80, got %q", got) + } +} + +func TestGetContainerIPDeterministic(t *testing.T) { + inspect := types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + Networks: map[string]*network.EndpointSettings{ + "zeta": {IPAddress: "172.0.0.9"}, + "alpha": {IPAddress: "172.0.0.1"}, + "beta": {IPAddress: "172.0.0.2"}, + }, + }, + } + // Lowest network name ("alpha") must always win, regardless of map order. + for i := 0; i < 20; i++ { + if got := getContainerIP(inspect); got != "172.0.0.1" { + t.Fatalf("getContainerIP = %q, want 172.0.0.1 (deterministic)", got) + } + } +} + +func TestGetContainerIPSkipsEmpty(t *testing.T) { + inspect := types.ContainerJSON{ + NetworkSettings: &types.NetworkSettings{ + Networks: map[string]*network.EndpointSettings{ + "alpha": {IPAddress: ""}, + "beta": {IPAddress: "172.0.0.2"}, + }, + }, + } + if got := getContainerIP(inspect); got != "172.0.0.2" { + t.Errorf("getContainerIP = %q, want 172.0.0.2", got) + } +} + +func TestGetContainerIPNilSettings(t *testing.T) { + if got := getContainerIP(types.ContainerJSON{}); got != "" { + t.Errorf("getContainerIP with nil settings = %q, want empty", got) + } +} + +func TestGetDefaultPortLowestExposed(t *testing.T) { + inspect := types.ContainerJSON{ + Config: &container.Config{ + ExposedPorts: nat.PortSet{ + "8080/tcp": struct{}{}, + "80/tcp": struct{}{}, + "443/tcp": struct{}{}, + "53/udp": struct{}{}, + }, + }, + } + for i := 0; i < 20; i++ { + if got := getDefaultPort(inspect); got != "80" { + t.Fatalf("getDefaultPort = %q, want 80 (lowest exposed TCP)", got) + } + } +} + +func TestGetDefaultPortFallsBackToBound(t *testing.T) { + inspect := types.ContainerJSON{ + Config: &container.Config{}, + NetworkSettings: &types.NetworkSettings{ + NetworkSettingsBase: types.NetworkSettingsBase{ + Ports: nat.PortMap{ + "3000/tcp": nil, + "2000/tcp": nil, + }, + }, + }, + } + if got := getDefaultPort(inspect); got != "2000" { + t.Errorf("getDefaultPort = %q, want 2000 (lowest bound TCP)", got) + } +} + +func TestGetDefaultPortDefault(t *testing.T) { + if got := getDefaultPort(types.ContainerJSON{Config: &container.Config{}}); got != "80" { + t.Errorf("getDefaultPort = %q, want 80", got) + } +} + +func TestGenerateTraefikConfigSingleHost(t *testing.T) { + cl := testLayer() + inspect := inspectWithIP("/myapp", "172.0.0.5") + info := ContainerInfo{Name: "myapp", VirtualHost: "myapp.loc", VirtualPort: "8080"} + + cfg := cl.generateTraefikConfig(inspect, info) + + // One HTTP and one HTTPS router for the single host. + if got := len(cfg.HTTP.Routers); got != 2 { + t.Fatalf("router count = %d, want 2", got) + } + httpRouter, ok := cfg.HTTP.Routers["myapp-0"] + if !ok { + t.Fatalf("missing http router myapp-0; got %v", cfg.HTTP.Routers) + } + if httpRouter.Rule != "Host(`myapp.loc`)" { + t.Errorf("http rule = %q, want Host(`myapp.loc`)", httpRouter.Rule) + } + tlsRouter, ok := cfg.HTTP.Routers["myapp-tls-0"] + if !ok { + t.Fatalf("missing tls router myapp-tls-0") + } + if tlsRouter.TLS == nil { + t.Error("tls router should have TLS config") + } + + svc, ok := cfg.HTTP.Services["myapp"] + if !ok { + t.Fatalf("missing service myapp") + } + if got := svc.LoadBalancer.Servers[0].URL; got != "http://172.0.0.5:8080" { + t.Errorf("server URL = %q, want http://172.0.0.5:8080", got) + } +} + +func TestGenerateTraefikConfigWildcardUsesHostRegexp(t *testing.T) { + cl := testLayer() + inspect := inspectWithIP("/wild", "172.0.0.6") + info := ContainerInfo{Name: "wild", VirtualHost: "*.wild.loc", VirtualPort: "80"} + + cfg := cl.generateTraefikConfig(inspect, info) + + router, ok := cfg.HTTP.Routers["wild-0"] + if !ok { + t.Fatalf("missing router wild-0") + } + if router.Rule != "HostRegexp(`^.*\\.wild\\.loc$`)" { + t.Errorf("wildcard rule = %q, want HostRegexp(`^.*\\.wild\\.loc$`)", router.Rule) + } +} + +func TestGenerateTraefikConfigMultiHost(t *testing.T) { + cl := testLayer() + inspect := inspectWithIP("/multi", "172.0.0.7") + info := ContainerInfo{Name: "multi", VirtualHost: "a.loc,b.loc", VirtualPort: "80"} + + cfg := cl.generateTraefikConfig(inspect, info) + + // Two hosts => two HTTP + two HTTPS routers, single shared service. + if got := len(cfg.HTTP.Routers); got != 4 { + t.Errorf("router count = %d, want 4", got) + } + if got := len(cfg.HTTP.Services); got != 1 { + t.Errorf("service count = %d, want 1", got) + } +} diff --git a/cmd/dns-server/main.go b/cmd/dns-server/main.go index e447beb..c41b183 100644 --- a/cmd/dns-server/main.go +++ b/cmd/dns-server/main.go @@ -17,6 +17,12 @@ import ( // DNS_UPSTREAM_TIMEOUT defines the timeout for DNS queries to upstream servers const DNS_UPSTREAM_TIMEOUT = 5 * time.Second +// defaultRecordTTL is the TTL (seconds) applied to generated A records. It is +// intentionally short: this is a local development resolver, so a low TTL lets +// a changed HTTP_PROXY_DNS_TARGET_IP propagate quickly instead of being cached +// by the OS stub resolver for an hour. +const defaultRecordTTL = 60 + type DNSServer struct { customDomains []string targetIP string @@ -60,6 +66,13 @@ func (s *DNSServer) forwardDNSQuery(r *dns.Msg) (*dns.Msg, error) { return nil, fmt.Errorf("all upstream servers failed") } +// writeMsg writes a DNS response, logging any write failure. +func (s *DNSServer) writeMsg(w dns.ResponseWriter, msg *dns.Msg) { + if err := w.WriteMsg(msg); err != nil { + s.logger.Error("Failed to write DNS response", "error", err) + } +} + // createRefusedResponse creates a REFUSED response for the given request func (s *DNSServer) createRefusedResponse(r *dns.Msg) *dns.Msg { msg := dns.Msg{} @@ -107,22 +120,30 @@ func (s *DNSServer) handleNonMatchingDomain(w dns.ResponseWriter, r *dns.Msg) { if err != nil { s.logger.Debug("Failed to forward query", "error", err) // If forwarding fails, return REFUSED - refusedResp := s.createRefusedResponse(r) - w.WriteMsg(refusedResp) + s.writeMsg(w, s.createRefusedResponse(r)) } else { - w.WriteMsg(response) + s.writeMsg(w, response) } } else { // Forwarding disabled - send REFUSED response s.logger.Debug("Sending REFUSED response (not matching configured domains)") - refusedResp := s.createRefusedResponse(r) - w.WriteMsg(refusedResp) + s.writeMsg(w, s.createRefusedResponse(r)) } } -// createARecord creates an A record for the given question -func (s *DNSServer) createARecord(question dns.Question) (dns.RR, error) { - return dns.NewRR(fmt.Sprintf("%s A %s", question.Name, s.targetIP)) +// createARecord creates an A record for the given question. The target IP is +// validated at startup, so it is constructed directly rather than parsed from a +// zone-file string on every query. +func (s *DNSServer) createARecord(question dns.Question) dns.RR { + return &dns.A{ + Hdr: dns.RR_Header{ + Name: question.Name, + Rrtype: dns.TypeA, + Class: dns.ClassINET, + Ttl: defaultRecordTTL, + }, + A: net.ParseIP(s.targetIP), + } } // handleQuestion processes a single DNS question and adds answers to the response @@ -132,13 +153,8 @@ func (s *DNSServer) handleQuestion(question dns.Question, msg *dns.Msg) { switch question.Qtype { case dns.TypeA: // Respond with our target IP for A records - rr, err := s.createARecord(question) - if err == nil { - msg.Answer = append(msg.Answer, rr) - s.logger.Info("Resolved A record", "name", name, "ip", s.targetIP) - } else { - s.logger.Error("Failed to create A record", "name", name, "error", err) - } + msg.Answer = append(msg.Answer, s.createARecord(question)) + s.logger.Info("Resolved A record", "name", name, "ip", s.targetIP) case dns.TypeAAAA: // For IPv6 queries, return empty response (no IPv6 support) s.logger.Debug("IPv6 query - returning empty response", "name", name) @@ -179,8 +195,7 @@ func (s *DNSServer) handleDNSRequest(w dns.ResponseWriter, r *dns.Msg) { } // All queries are for our domains - create and send response - response := s.createDNSResponse(r) - w.WriteMsg(response) + s.writeMsg(w, s.createDNSResponse(r)) } func main() { @@ -202,9 +217,10 @@ func main() { os.Exit(1) } - // Validate target IP - if net.ParseIP(cfg.DNSIP) == nil { - log.Error("Invalid target IP address", "ip", cfg.DNSIP) + // Validate target IP. The server answers A records only, so the target must + // be IPv4; an IPv6 address would be silently truncated into a 4-byte A record. + if ip := net.ParseIP(cfg.DNSIP); ip == nil || ip.To4() == nil { + log.Error("Invalid target IP address, must be IPv4", "ip", cfg.DNSIP) os.Exit(1) } diff --git a/cmd/dns-server/main_test.go b/cmd/dns-server/main_test.go new file mode 100644 index 0000000..74a1cb8 --- /dev/null +++ b/cmd/dns-server/main_test.go @@ -0,0 +1,30 @@ +package main + +import "testing" + +func TestIsDomainHandled(t *testing.T) { + s := &DNSServer{customDomains: []string{"loc", "spark.dev"}} + + tests := []struct { + name string + domain string + want bool + }{ + {"tld exact", "loc.", true}, + {"tld subdomain", "app.loc.", true}, + {"tld deep subdomain", "api.app.loc.", true}, + {"case insensitive", "APP.LOC.", true}, + {"specific domain exact", "spark.dev.", true}, + {"specific domain subdomain", "api.spark.dev.", true}, + {"unrelated", "example.com.", false}, + {"partial suffix not matched", "notspark.dev.", false}, + {"substring not matched", "myloc.", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := s.isDomainHandled(tt.domain); got != tt.want { + t.Errorf("isDomainHandled(%q) = %v, want %v", tt.domain, got, tt.want) + } + }) + } +} diff --git a/cmd/join-networks/main.go b/cmd/join-networks/main.go index 09220ce..763cc3f 100644 --- a/cmd/join-networks/main.go +++ b/cmd/join-networks/main.go @@ -93,7 +93,7 @@ func (nj *NetworkJoiner) HandleEvent(ctx context.Context, event events.Message) // Focuses on network connections to minimize API calls and provide network context. type ContainerInfo struct { ID string - Networks map[string]NetworkInfo + Networks NetworkSet } // NetworkOperation encapsulates a simple network management operation including @@ -118,14 +118,6 @@ func (ns NetworkSet) Add(networkID string) { ns[networkID] = true } -// NetworkInfo contains details about a network connection -type NetworkInfo struct { - ID string - Name string - Gateway string - IP string -} - // main parses command line arguments and runs the network join service func main() { containerName := flag.String("container-name", "http-proxy", "the name of this docker container") @@ -164,10 +156,7 @@ func (nj *NetworkJoiner) performInitialNetworkJoin(ctx context.Context, containe return fmt.Errorf("failed to get container info: %w", err) } - currentNetworks := make(NetworkSet) - for networkID := range containerInfo.Networks { - currentNetworks.Add(networkID) - } + currentNetworks := containerInfo.Networks bridgeNetworks, err := nj.getActiveBridgeNetworks(ctx, containerInfo.ID) if err != nil { @@ -266,14 +255,11 @@ func (nj *NetworkJoiner) getContainerInfo(ctx context.Context, containerName str return nil, fmt.Errorf("failed to inspect container %s: %w", containerName, err) } - networks := make(map[string]NetworkInfo) + networks := make(NetworkSet) - for networkName, networkData := range containerJSON.NetworkSettings.Networks { - networks[networkData.NetworkID] = NetworkInfo{ - ID: networkData.NetworkID, - Name: networkName, - Gateway: networkData.Gateway, - IP: networkData.IPAddress, + if containerJSON.NetworkSettings != nil { + for _, networkData := range containerJSON.NetworkSettings.Networks { + networks.Add(networkData.NetworkID) } } diff --git a/pkg/config/config.go b/pkg/config/config.go index ef9c1be..a2ce3cd 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -2,7 +2,6 @@ package config import ( "os" - "strconv" "strings" ) @@ -34,16 +33,6 @@ func GetEnvOrDefault(key, defaultValue string) string { return defaultValue } -// GetEnvOrDefaultInt returns an environment variable as an integer or a default -func GetEnvOrDefaultInt(key string, defaultValue int) int { - if value := os.Getenv(key); value != "" { - if intValue, err := strconv.Atoi(value); err == nil { - return intValue - } - } - return defaultValue -} - // GetEnvOrDefaultStringSlice returns an environment variable as a comma-separated slice or a default func GetEnvOrDefaultStringSlice(key string, defaultValue []string) []string { if value := os.Getenv(key); value != "" { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go new file mode 100644 index 0000000..d2d8cc9 --- /dev/null +++ b/pkg/config/config_test.go @@ -0,0 +1,63 @@ +package config + +import ( + "reflect" + "testing" +) + +func TestGetEnvOrDefault(t *testing.T) { + t.Run("returns default when unset", func(t *testing.T) { + if got := GetEnvOrDefault("HTTP_PROXY_TEST_UNSET", "fallback"); got != "fallback" { + t.Errorf("got %q, want fallback", got) + } + }) + t.Run("returns value when set", func(t *testing.T) { + t.Setenv("HTTP_PROXY_TEST_SET", "value") + if got := GetEnvOrDefault("HTTP_PROXY_TEST_SET", "fallback"); got != "value" { + t.Errorf("got %q, want value", got) + } + }) + t.Run("returns default when empty", func(t *testing.T) { + t.Setenv("HTTP_PROXY_TEST_EMPTY", "") + if got := GetEnvOrDefault("HTTP_PROXY_TEST_EMPTY", "fallback"); got != "fallback" { + t.Errorf("got %q, want fallback", got) + } + }) +} + +func TestGetEnvOrDefaultStringSlice(t *testing.T) { + def := []string{"loc"} + + t.Run("default when unset", func(t *testing.T) { + got := GetEnvOrDefaultStringSlice("HTTP_PROXY_TEST_SLICE_UNSET", def) + if !reflect.DeepEqual(got, def) { + t.Errorf("got %v, want %v", got, def) + } + }) + + t.Run("splits and trims", func(t *testing.T) { + t.Setenv("HTTP_PROXY_TEST_SLICE", " loc , dev ,test") + got := GetEnvOrDefaultStringSlice("HTTP_PROXY_TEST_SLICE", def) + want := []string{"loc", "dev", "test"} + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } + }) + + t.Run("drops empty entries", func(t *testing.T) { + t.Setenv("HTTP_PROXY_TEST_SLICE_EMPTY", "loc,,dev,") + got := GetEnvOrDefaultStringSlice("HTTP_PROXY_TEST_SLICE_EMPTY", def) + want := []string{"loc", "dev"} + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } + }) + + t.Run("default when only separators", func(t *testing.T) { + t.Setenv("HTTP_PROXY_TEST_SLICE_SEP", " , , ") + got := GetEnvOrDefaultStringSlice("HTTP_PROXY_TEST_SLICE_SEP", def) + if !reflect.DeepEqual(got, def) { + t.Errorf("got %v, want %v", got, def) + } + }) +} diff --git a/pkg/service/docker_event_service.go b/pkg/service/docker_event_service.go index fc07b72..beaba7a 100644 --- a/pkg/service/docker_event_service.go +++ b/pkg/service/docker_event_service.go @@ -35,12 +35,19 @@ type EventHandler interface { SetDependencies(client *client.Client, logger *logger.Logger) } +// eventSubscriber subscribes to the Docker event stream. It matches the +// signature of (*client.Client).Events and exists as a seam so the reconnect +// behavior of the event loop can be tested without a Docker daemon. +type eventSubscriber func(ctx context.Context, options events.ListOptions) (<-chan events.Message, <-chan error) + // Service represents a Docker-event-driven service type Service struct { - client *client.Client - logger *logger.Logger - handler EventHandler - serviceName string + client *client.Client + logger *logger.Logger + handler EventHandler + serviceName string + subscribe eventSubscriber + reconnectDelay time.Duration } // NewService creates a new Docker event-driven service @@ -69,10 +76,12 @@ func NewService(ctx context.Context, serviceName string, logLevel string, handle handler.SetDependencies(dockerClient, log) return &Service{ - client: dockerClient, - logger: log, - handler: handler, - serviceName: serviceName, + client: dockerClient, + logger: log, + handler: handler, + serviceName: serviceName, + subscribe: dockerClient.Events, + reconnectDelay: 5 * time.Second, }, nil } @@ -91,35 +100,22 @@ func (s *Service) Close() error { return s.client.Close() } -// Run starts the service with signal handling and event processing +// Run executes the event loop until the context is cancelled or the loop fails. +// Signal handling and lifecycle are owned by RunWithSignalHandling. func (s *Service) Run(ctx context.Context) error { s.logger.Info("Starting service", "name", s.serviceName) + return s.runEventLoop(ctx) +} - // Setup signal handling - sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM, syscall.SIGQUIT) - - // Start the event loop - errChan := make(chan error, 1) - go func() { - errChan <- s.runEventLoop(ctx) - }() - - // Wait for shutdown signal or error - select { - case <-sigChan: - s.logger.Info("Received shutdown signal") - if err := s.Close(); err != nil { - s.logger.Error("Error while closing service", "error", err) - } - return context.Canceled - case err := <-errChan: - if err != nil { - s.logger.Error("Service error", "error", err) - return err - } - s.logger.Info("Service completed successfully") - return nil +// containerEventOptions returns the Docker event-stream filters for the +// container start/die events the services react to. +func containerEventOptions() events.ListOptions { + return events.ListOptions{ + Filters: filters.NewArgs( + filters.Arg("type", "container"), + filters.Arg("event", "start"), + filters.Arg("event", "die"), + ), } } @@ -133,37 +129,54 @@ func (s *Service) runEventLoop(ctx context.Context) error { } // Listen for Docker events - eventsChan, errChan := s.client.Events(ctx, events.ListOptions{ - Filters: filters.NewArgs( - filters.Arg("type", "container"), - filters.Arg("event", "start"), - filters.Arg("event", "die"), - ), - }) + eventsChan, errChan := s.subscribe(ctx, containerEventOptions()) for { select { case <-ctx.Done(): return nil - case event := <-eventsChan: + case event, ok := <-eventsChan: + if !ok { + // The Docker daemon closed the stream (e.g. restart). Reconnect + // after a backoff instead of spinning on the closed channel. + if !s.backoffBeforeReconnect(ctx) { + return nil + } + eventsChan, errChan = s.subscribe(ctx, containerEventOptions()) + continue + } s.processEventSafely(ctx, event) - case err := <-errChan: + case err, ok := <-errChan: + if !ok { + if !s.backoffBeforeReconnect(ctx) { + return nil + } + eventsChan, errChan = s.subscribe(ctx, containerEventOptions()) + continue + } if err != nil { s.logger.Error("Docker events error", "error", err) - // Reconnect and continue - time.Sleep(5 * time.Second) - eventsChan, errChan = s.client.Events(ctx, events.ListOptions{ - Filters: filters.NewArgs( - filters.Arg("type", "container"), - filters.Arg("event", "start"), - filters.Arg("event", "die"), - ), - }) + if !s.backoffBeforeReconnect(ctx) { + return nil + } + eventsChan, errChan = s.subscribe(ctx, containerEventOptions()) } } } } +// backoffBeforeReconnect waits before reconnecting to the Docker event stream. +// It returns false if the context is cancelled during the wait, signalling the +// caller to stop instead of reconnecting. +func (s *Service) backoffBeforeReconnect(ctx context.Context) bool { + select { + case <-ctx.Done(): + return false + case <-time.After(s.reconnectDelay): + return true + } +} + // processEventSafely wraps event processing with proper error handling and logging func (s *Service) processEventSafely(ctx context.Context, event events.Message) { // Respect context cancellation @@ -207,7 +220,7 @@ func RunWithSignalHandling(ctx context.Context, serviceName string, logLevel str // Wait for shutdown signal or error select { case err := <-errChan: - if err != nil && err != context.Canceled { + if err != nil { service.GetLogger().Error("Service failed", "error", err) os.Exit(1) } @@ -219,7 +232,7 @@ func RunWithSignalHandling(ctx context.Context, serviceName string, logLevel str // Wait for graceful shutdown with timeout select { case err := <-errChan: - if err != nil && err != context.Canceled { + if err != nil { service.GetLogger().Error("Error during shutdown", "error", err) } case <-time.After(10 * time.Second): diff --git a/pkg/service/docker_event_service_test.go b/pkg/service/docker_event_service_test.go new file mode 100644 index 0000000..f22d413 --- /dev/null +++ b/pkg/service/docker_event_service_test.go @@ -0,0 +1,130 @@ +package service + +import ( + "context" + "errors" + "sync" + "testing" + "time" + + "github.com/docker/docker/api/types/events" + "github.com/docker/docker/client" + "github.com/sparkfabrik/http-proxy/pkg/logger" +) + +// fakeHandler is a no-op EventHandler for exercising the event loop. +type fakeHandler struct { + scanErr error +} + +func (f *fakeHandler) HandleInitialScan(ctx context.Context) error { return f.scanErr } +func (f *fakeHandler) HandleEvent(context.Context, events.Message) error { return nil } +func (f *fakeHandler) GetName() string { return "fake" } +func (f *fakeHandler) SetDependencies(*client.Client, *logger.Logger) {} + +func newTestService(h EventHandler, subscribe eventSubscriber) *Service { + return &Service{ + logger: logger.New("test"), + handler: h, + serviceName: "test", + subscribe: subscribe, + reconnectDelay: time.Millisecond, + } +} + +func waitSignal(t *testing.T, ch <-chan struct{}, msg string) { + t.Helper() + select { + case <-ch: + case <-time.After(2 * time.Second): + t.Fatal(msg) + } +} + +func TestRunEventLoopReconnectsOnClosedEventsChannel(t *testing.T) { + calls := make(chan struct{}, 10) + var mu sync.Mutex + var current chan events.Message + + subscribe := func(context.Context, events.ListOptions) (<-chan events.Message, <-chan error) { + ev := make(chan events.Message) + mu.Lock() + current = ev + mu.Unlock() + calls <- struct{}{} + return ev, make(chan error) + } + + s := newTestService(&fakeHandler{}, subscribe) + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- s.runEventLoop(ctx) }() + + waitSignal(t, calls, "event loop did not make the initial subscription") + + // Simulate the Docker daemon closing the stream. + mu.Lock() + close(current) + mu.Unlock() + + waitSignal(t, calls, "event loop did not reconnect after the stream closed") + + cancel() + select { + case err := <-done: + if err != nil { + t.Fatalf("runEventLoop returned error: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("event loop did not stop after context cancellation") + } +} + +func TestRunEventLoopReconnectsOnError(t *testing.T) { + calls := make(chan struct{}, 10) + var mu sync.Mutex + var currentErr chan error + + subscribe := func(context.Context, events.ListOptions) (<-chan events.Message, <-chan error) { + er := make(chan error, 1) + mu.Lock() + currentErr = er + mu.Unlock() + calls <- struct{}{} + return make(chan events.Message), er + } + + s := newTestService(&fakeHandler{}, subscribe) + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- s.runEventLoop(ctx) }() + + waitSignal(t, calls, "event loop did not make the initial subscription") + + // Deliver a stream error, which should trigger a reconnect. + mu.Lock() + currentErr <- errors.New("boom") + mu.Unlock() + + waitSignal(t, calls, "event loop did not reconnect after a stream error") + + cancel() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("event loop did not stop after context cancellation") + } +} + +func TestRunEventLoopReturnsInitialScanError(t *testing.T) { + wantErr := errors.New("scan failed") + subscribe := func(context.Context, events.ListOptions) (<-chan events.Message, <-chan error) { + t.Error("subscribe should not be called when the initial scan fails") + return nil, nil + } + + s := newTestService(&fakeHandler{scanErr: wantErr}, subscribe) + if err := s.runEventLoop(context.Background()); !errors.Is(err, wantErr) { + t.Fatalf("runEventLoop error = %v, want %v", err, wantErr) + } +} diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 9476363..631b759 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -176,6 +176,16 @@ func CheckContext(ctx context.Context) error { } } +// HasTraefikLabel reports whether any label key starts with "traefik." +func HasTraefikLabel(labels map[string]string) bool { + for label := range labels { + if strings.HasPrefix(label, "traefik.") { + return true + } + } + return false +} + // ShouldManageContainer checks if a container should be managed based on dinghy env vars or traefik labels // Returns true if the container has VIRTUAL_HOST environment variable or traefik labels func ShouldManageContainer(env []string, labels map[string]string) bool { @@ -184,14 +194,7 @@ func ShouldManageContainer(env []string, labels map[string]string) bool { return true } - // Check for traefik labels (any label starting with "traefik.") - for label := range labels { - if strings.HasPrefix(label, "traefik.") { - return true - } - } - - return false + return HasTraefikLabel(labels) } // HasManageableContainersInNetwork checks if a network has any manageable containers, @@ -231,13 +234,3 @@ func HasManageableContainersInNetwork(ctx context.Context, dockerClient *client. return false, nil } - -// SliceToSet converts a slice of strings to a map[string]struct{} for O(1) lookups -// This is useful for creating sets from slices where you only need to check existence -func SliceToSet(slice []string) map[string]struct{} { - set := make(map[string]struct{}) - for _, item := range slice { - set[item] = struct{}{} - } - return set -} diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go new file mode 100644 index 0000000..4545d03 --- /dev/null +++ b/pkg/utils/utils_test.go @@ -0,0 +1,92 @@ +package utils + +import "testing" + +func TestGetDockerEnvVar(t *testing.T) { + env := []string{"FOO=bar", "VIRTUAL_HOST=app.loc", "EMPTY="} + tests := []struct { + key string + want string + }{ + {"VIRTUAL_HOST", "app.loc"}, + {"FOO", "bar"}, + {"EMPTY", ""}, + {"MISSING", ""}, + } + for _, tt := range tests { + if got := GetDockerEnvVar(env, tt.key); got != tt.want { + t.Errorf("GetDockerEnvVar(%q) = %q, want %q", tt.key, got, tt.want) + } + } +} + +func TestHasTraefikLabel(t *testing.T) { + tests := []struct { + name string + labels map[string]string + want bool + }{ + {"nil", nil, false}, + {"none", map[string]string{"com.example": "x"}, false}, + {"enable", map[string]string{"traefik.enable": "true"}, true}, + {"nested", map[string]string{"traefik.http.routers.a.rule": "Host(`x`)"}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := HasTraefikLabel(tt.labels); got != tt.want { + t.Errorf("HasTraefikLabel(%v) = %v, want %v", tt.labels, got, tt.want) + } + }) + } +} + +func TestShouldManageContainer(t *testing.T) { + tests := []struct { + name string + env []string + labels map[string]string + want bool + }{ + {"neither", []string{"FOO=bar"}, nil, false}, + {"virtual host", []string{"VIRTUAL_HOST=app.loc"}, nil, true}, + {"traefik label", nil, map[string]string{"traefik.enable": "true"}, true}, + {"both", []string{"VIRTUAL_HOST=app.loc"}, map[string]string{"traefik.enable": "true"}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ShouldManageContainer(tt.env, tt.labels); got != tt.want { + t.Errorf("ShouldManageContainer = %v, want %v", got, tt.want) + } + }) + } +} + +func TestFormatDockerID(t *testing.T) { + tests := []struct { + in string + want string + }{ + {"abcdef0123456789", "abcdef012345"}, + {"short", "short"}, + {"", ""}, + {"abcdef012345", "abcdef012345"}, + } + for _, tt := range tests { + if got := FormatDockerID(tt.in); got != tt.want { + t.Errorf("FormatDockerID(%q) = %q, want %q", tt.in, got, tt.want) + } + } +} + +func TestValidateLogLevel(t *testing.T) { + for _, lvl := range []string{"debug", "info", "warn", "error"} { + if err := ValidateLogLevel(lvl); err != nil { + t.Errorf("ValidateLogLevel(%q) returned error: %v", lvl, err) + } + } + for _, lvl := range []string{"", "trace", "INFO", "warning"} { + if err := ValidateLogLevel(lvl); err == nil { + t.Errorf("ValidateLogLevel(%q) expected error, got nil", lvl) + } + } +}