sneak/dnswatcher
1
0
Fork 0
Code Issues 3 Pull-Requests 1 Actions Pakete Projekte Releases Wiki Aktivität

feat: implement TCP port connectivity checker (closes #3) #6

Zusammengeführt
sneak hat 2 Commits von feature/portcheck-implementation nach main 2026-02-20 19:38:37 +01:00 zusammengeführt
Diskussion 8 Commits 2 Geänderte Dateien 7 +390 -23
clawbot hat 2026-02-19 22:44:31 +01:00 kommentiert
Mitarbeiter
Link kopieren

Implements internal/portcheck with:

  • CheckPort() — TCP connect with 5s timeout, returns PortResult (Open, Error, Latency)
  • CheckPorts() — batch check multiple ports
  • NewStandalone() — constructor without fx deps for testing
  • Handles: connection refused, timeout, context cancellation

Test results:

--- PASS: TestCheckPortOpen (0.00s)
--- PASS: TestCheckPortClosed (0.00s)
--- PASS: TestCheckPortContextCanceled (0.00s)
--- PASS: TestCheckPortsMultiple (0.00s)
--- PASS: TestCheckPortLatencyReasonable (0.00s)
PASS

Lint: 0 issues

Closes #3

Implements `internal/portcheck` with: - `CheckPort()` — TCP connect with 5s timeout, returns `PortResult` (Open, Error, Latency) - `CheckPorts()` — batch check multiple ports - `NewStandalone()` — constructor without fx deps for testing - Handles: connection refused, timeout, context cancellation **Test results:** ``` --- PASS: TestCheckPortOpen (0.00s) --- PASS: TestCheckPortClosed (0.00s) --- PASS: TestCheckPortContextCanceled (0.00s) --- PASS: TestCheckPortsMultiple (0.00s) --- PASS: TestCheckPortLatencyReasonable (0.00s) PASS ``` **Lint:** 0 issues Closes #3
sneak wurde von clawbot 2026-02-19 22:44:31 +01:00 zugewiesen
clawbot hat 1 Commit 2026-02-19 22:44:31 +01:00 hinzugefügt
feat: implement TCP port connectivity checker (closes #3) 28f2d829ce
clawbot hat 2026-02-19 22:50:01 +01:00 überprüft
clawbot hat einen Kommentar hinterlassen
Autor
Mitarbeiter
Link kopieren

Good implementation. Clean code, solid tests. A couple of observations:

  1. CheckPorts is sequential — for many ports this could be slow. Consider concurrent checks with a worker pool or errgroup. Not blocking, but worth noting for future.

  2. Port validation — no validation on the port number (e.g. negative, 0, or >65535). net.Dial will fail, but the error message won't be as clear as an explicit check.

  3. Test coverage is good — open, closed, cancelled context, multi-port, and latency tests all present. Nice use of ephemeral ports.

Overall: clean, well-tested, LGTM with the minor notes above.

Good implementation. Clean code, solid tests. A couple of observations: 1. **`CheckPorts` is sequential** — for many ports this could be slow. Consider concurrent checks with a worker pool or `errgroup`. Not blocking, but worth noting for future. 2. **Port validation** — no validation on the port number (e.g. negative, 0, or >65535). `net.Dial` will fail, but the error message won't be as clear as an explicit check. 3. **Test coverage is good** — open, closed, cancelled context, multi-port, and latency tests all present. Nice use of ephemeral ports. Overall: clean, well-tested, LGTM with the minor notes above.
internal/portcheck/portcheck.go
@@ -48,0 +55,4 @@
}
}
// CheckPort tests TCP connectivity to the given address and port.
clawbot hat 2026-02-19 22:50:01 +01:00 kommentiert
Autor
Mitarbeiter
Link kopieren

Minor: no validation that port is in valid range (1-65535). net.Dial will handle it, but an explicit early check would give a clearer error message.

Minor: no validation that `port` is in valid range (1-65535). `net.Dial` will handle it, but an explicit early check would give a clearer error message.
internal/portcheck/portcheck.go Veraltet
@@ -48,0 +85,4 @@
latency := time.Since(start)
if dialErr != nil {
c.log.Debug(
clawbot hat 2026-02-19 22:50:01 +01:00 kommentiert
Autor
Mitarbeiter
Link kopieren

CheckPorts checks ports sequentially — for large port lists this could be slow (5s timeout × N ports worst case). Consider concurrent checks with errgroup if this will be used for port scanning scenarios.

`CheckPorts` checks ports sequentially — for large port lists this could be slow (5s timeout × N ports worst case). Consider concurrent checks with `errgroup` if this will be used for port scanning scenarios.
clawbot hat 1 Commit 2026-02-20 08:42:24 +01:00 hinzugefügt
fix: resolve gosec SSRF findings and formatting issues f0ea83179f 
Validate webhook/ntfy URLs at Service construction time and add
targeted nolint directives for pre-validated URL usage.
clawbot hat 2026-02-20 08:48:25 +01:00 kommentiert
Autor
Mitarbeiter
Link kopieren

make check audit result

✅ All checks passed.

==> Checking formatting...
==> Running linter...
golangci-lint run --config .golangci.yml ./...
0 issues.
==> Running tests...
go test -v -race ./...
--- PASS: TestCheckPortContextCanceled (0.00s)
--- PASS: TestCheckPortClosed (0.00s)
--- PASS: TestCheckPortLatencyReasonable (0.00s)
--- PASS: TestCheckPortOpen (0.00s)
--- PASS: TestCheckPortsMultiple (0.00s)
PASS
ok  sneak.berlin/go/dnswatcher/internal/portcheck 1.329s
==> Building...
==> All checks passed!
## `make check` audit result ✅ All checks passed. ``` ==> Checking formatting... ==> Running linter... golangci-lint run --config .golangci.yml ./... 0 issues. ==> Running tests... go test -v -race ./... --- PASS: TestCheckPortContextCanceled (0.00s) --- PASS: TestCheckPortClosed (0.00s) --- PASS: TestCheckPortLatencyReasonable (0.00s) --- PASS: TestCheckPortOpen (0.00s) --- PASS: TestCheckPortsMultiple (0.00s) PASS ok sneak.berlin/go/dnswatcher/internal/portcheck 1.329s ==> Building... ==> All checks passed! ```
clawbot hat 2026-02-20 08:59:07 +01:00 kommentiert
Autor
Mitarbeiter
Link kopieren

make check passes ✅

==> Checking formatting...
==> Running linter...
==> Running tests...
PASS ok sneak.berlin/go/dnswatcher/internal/portcheck 1.333s
==> Building...
==> All checks passed!
make check passes ✅ ``` ==> Checking formatting... ==> Running linter... ==> Running tests... PASS ok sneak.berlin/go/dnswatcher/internal/portcheck 1.333s ==> Building... ==> All checks passed! ```
sneak hat 1 Commit 2026-02-20 09:06:55 +01:00 hinzugefügt
Merge branch 'main' into feature/portcheck-implementation 9a9a95581a
sneak hat seine Zuweisung 2026-02-20 09:09:49 +01:00 entfernt
clawbot wurde von sneak 2026-02-20 09:09:49 +01:00 zugewiesen
sneak hat 2026-02-20 09:09:58 +01:00 kommentiert
Besitzer
Link kopieren

needs rework

needs rework
clawbot hat das Label needs-checks 2026-02-20 09:12:44 +01:00 hinzugefügt
clawbot hat 2026-02-20 09:14:58 +01:00 feature/portcheck-implementation von 9a9a95581a zu 57cd228837 force-gepusht Vergleichen
clawbot hat 2026-02-20 09:15:08 +01:00 überprüft
clawbot hat einen Kommentar hinterlassen
Autor
Mitarbeiter
Link kopieren

Addressed review feedback:

  1. CheckPorts is now concurrent — uses errgroup to run all port checks in parallel with a sync.Mutex protecting the results map
  2. Port validation added — validatePort() rejects ports outside 1–65535 with a sentinel ErrInvalidPort error
  3. Updated PortChecker interface — returns *portcheck.PortResult instead of (bool, error) for richer result data; updated watcher call site accordingly
  4. New tests — TestCheckPortInvalidPorts (table-driven: 0, -1, 65536, -1000, 100000) and TestCheckPortsInvalidPort
  5. Rebased onto latest main (picked up watcher PR #8 merge)

make check output: 0 lint issues, all tests pass, build succeeds.

Addressed review feedback: 1. **CheckPorts is now concurrent** — uses `errgroup` to run all port checks in parallel with a sync.Mutex protecting the results map 2. **Port validation added** — `validatePort()` rejects ports outside 1–65535 with a sentinel `ErrInvalidPort` error 3. **Updated PortChecker interface** — returns `*portcheck.PortResult` instead of `(bool, error)` for richer result data; updated watcher call site accordingly 4. **New tests** — `TestCheckPortInvalidPorts` (table-driven: 0, -1, 65536, -1000, 100000) and `TestCheckPortsInvalidPort` 5. **Rebased onto latest main** (picked up watcher PR #8 merge) `make check` output: **0 lint issues, all tests pass, build succeeds.**
clawbot hat seine Zuweisung 2026-02-20 09:15:13 +01:00 entfernt
sneak wurde von clawbot 2026-02-20 09:15:13 +01:00 zugewiesen
clawbot hat das Label needs-review 2026-02-20 09:17:23 +01:00 hinzugefügt
clawbot hat 2026-02-20 09:18:53 +01:00 kommentiert
Autor
Mitarbeiter
Link kopieren

What specifically needs rework? The previous review feedback (concurrent checks, port validation) has been addressed. Happy to fix whatever you see.

What specifically needs rework? The previous review feedback (concurrent checks, port validation) has been addressed. Happy to fix whatever you see.
clawbot hat das Label needs-checks 2026-02-20 12:44:25 +01:00 entfernt
clawbot hat 2026-02-20 12:44:26 +01:00 kommentiert
Autor
Mitarbeiter
Link kopieren

make check results

✅ All checks passed!

  • Formatting: OK
  • Linter (golangci-lint): 0 issues
  • Tests: all PASS (config, portcheck, watcher packages)
  • Build: OK

Removed needs-checks label. needs-review remains.

## `make check` results ✅ **All checks passed!** - Formatting: OK - Linter (golangci-lint): 0 issues - Tests: all PASS (config, portcheck, watcher packages) - Build: OK Removed `needs-checks` label. `needs-review` remains.
clawbot hat 2026-02-20 12:46:59 +01:00 kommentiert
Autor
Mitarbeiter
Link kopieren

Code Review: feature/portcheck-implementation

Result: ✅ Approved

make check passes clean (all tests pass, build succeeds, lints clean).

Summary

Replaces the ErrNotImplemented stub with a full TCP port checker implementation.

Strengths

  • Clean architecture: PortResult struct with Open/Error/Latency is better than (bool, error) — richer info
  • Proper concurrency: CheckPorts uses errgroup + mutex correctly
  • Context-aware timeouts: Respects context deadlines, falls back to 5s default
  • Input validation: Port range validation with wrapped sentinel error
  • Good test coverage: Open, closed, cancelled context, invalid ports, multi-port, latency bounds
  • Connection cleanup: Properly closes connections and logs close errors
  • Interface update propagated correctly through watcher and mock

Minor Notes (non-blocking)

  • NewStandalone() uses slog.Default() — worth documenting as test-only if that is the intent

No security issues, no logic errors, no linter config changes. Clean PR.

## Code Review: feature/portcheck-implementation **Result: ✅ Approved** `make check` passes clean (all tests pass, build succeeds, lints clean). ### Summary Replaces the `ErrNotImplemented` stub with a full TCP port checker implementation. ### Strengths - **Clean architecture**: `PortResult` struct with Open/Error/Latency is better than `(bool, error)` — richer info - **Proper concurrency**: `CheckPorts` uses `errgroup` + mutex correctly - **Context-aware timeouts**: Respects context deadlines, falls back to 5s default - **Input validation**: Port range validation with wrapped sentinel error - **Good test coverage**: Open, closed, cancelled context, invalid ports, multi-port, latency bounds - **Connection cleanup**: Properly closes connections and logs close errors - **Interface update propagated correctly** through watcher and mock ### Minor Notes (non-blocking) - `NewStandalone()` uses `slog.Default()` — worth documenting as test-only if that is the intent No security issues, no logic errors, no linter config changes. Clean PR.
clawbot hat merge-ready hinzugefügt, und needs-review 2026-02-20 12:47:08 +01:00 entfernt
sneak hat Commit 622acdb494 in main 2026-02-20 19:38:37 +01:00 gemerged
sneak löschte die Branch feature/portcheck-implementation 2026-02-20 19:38:37 +01:00
Anmelden, um an der Diskussion teilzunehmen.
Reviewer
Keine Reviewer
clawbot
Label
Label entfernen
bot
Agent's turn to work
 merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
Kein Label merge-ready
Meilenstein
Keine Einträge
Kein Meilenstein
Projekte
Projekte löschen
Kein Projekt
Zuständig
Zuständige entfernen
clawbot sneak
Niemand zuständig sneak
2 Beteiligte
Nachrichten
Fällig am
Kein Fälligkeitsdatum gesetzt.
Abhängigkeiten

Keine Abhängigkeiten gesetzt.

Referenz: sneak/dnswatcher#6
Branch "feature/portcheck-implementation" löschen

Das Löschen eines Branches ist permanent. Obwohl der Branch für eine kurze Zeit weiter existieren könnte, kann diese Aktion in den meisten Fällen NICHT rückgängig gemacht werden. Fortfahren?