Skip to content

Commit

Permalink
Merge pull request #13 from siemens/develop
Browse files Browse the repository at this point in the history
refactor: (2nd attempt) remove pipe close monitoring, rely only on signals
  • Loading branch information
thediveo committed Dec 26, 2023
2 parents f012cff + d3ca0b6 commit 83e731d
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 252 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@
*.syso
coverage.*
__debug_bin
/cshargextcap
/cshargextcap.exe
*.log
2 changes: 1 addition & 1 deletion .goreleaser.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ builds:
- netgo
- osusergo
ldflags:
- 's -w -X main.version={{.Version}} -X main.commit={{.Commit}} -X main.date={{.Date}} -X main.builtBy=goreleaser'
- '-s -w -X main.version={{.Version}} -X main.commit={{.Commit}} -X main.date={{.Date}} -X main.builtBy=goreleaser'
hooks:
post:
- cmd: packaging/windows/post.sh {{ .Path }}
Expand Down
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ SHELL:=/bin/bash
GOGEN:=go generate .
BUILDTAGS:="osusergo,netgo"

.PHONY: help clean dist pkgsite report run vuln
.PHONY: help clean cshargextcap dist pkgsite report run vuln

help: ## list available targets
@# Derived from Gomega's Makefile (github.com/onsi/gomega) under MIT License
Expand All @@ -21,6 +21,12 @@ dist: ## build snapshot cshargextcap binary packages+archives in dist/ and test
@ls -lh dist/cshargextcap_*
@echo "🏁 done"

cshargextcap: ## build local extcap binary
go build \
-tags netgo,osusergo \
-ldflags "-s -w" \
./cmd/cshargextcap

clean: ## cleans up build and testing artefacts
rm -rf dist
find . -name __debug_bin -delete
Expand Down
63 changes: 48 additions & 15 deletions capturestream.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,36 @@
package cshargextcap

import (
"context"
"os"
"os/signal"
"strings"
"syscall"

"github.com/siemens/csharg"
"github.com/siemens/cshargextcap/cli/target"
"github.com/siemens/cshargextcap/cli/wireshark"
"github.com/siemens/cshargextcap/pipe"

log "github.com/sirupsen/logrus"
)

// Capture is the workhorse: it opens the pipe (fifo) offered by Wireshark, then
// starts a new Capture stream using the given SharkTank client and container
// target description. Then it lets csharg pump all packet Capture data arriving
// from the underlying websocket connected to the Capture service into the
// Wireshark pipe.
// Capture is the workhorse: it opens the named pipe (fifo) offered by
// Wireshark, then starts a new Capture stream using the given SharkTank client
// and container target description. Then it lets csharg pump all packet Capture
// data arriving from the underlying websocket connected to the capture service
// into the Wireshark pipe.
func Capture(st csharg.SharkTank) int {
// While Wireshark (and Tshark) currently send SIGTERM (and maybe SIGINT in
// some situations, maybe when using a control pipe which we don't) only on
// unix systems, there are developer discussions to in the future send
// events to a Windows extcap. As Go maps such events to its signal API
// we're already now unconditionally handling SIGINT and SIGTERM in the hope
// that we're future-proof.
defer func() {
signal.Reset(syscall.SIGINT, syscall.SIGTERM)
}()

// Open packet stream pipe to Wireshark to feed it jucy packets...
log.Debugf("fifo to Wireshark %s", wireshark.FifoPath)
log.Debugf("opening fifo to Wireshark %s", wireshark.FifoPath)
fifo, err := os.OpenFile(wireshark.FifoPath, os.O_WRONLY, 0)
if err != nil {
log.Errorf("cannot open fifo: %s", err.Error())
Expand Down Expand Up @@ -64,16 +74,39 @@ func Capture(st csharg.SharkTank) int {
log.Errorf("cannot start capture: %s", err.Error())
return 1
}
defer cs.Stop() // be overly careful

// Always keep an eye on the fifo getting closed by Wireshark: we then need
// to stop the capture stream. This is necessary because the capture stream
// might be idle for long times and thus we would otherwise not notice that
// Wireshark has already stopped capturing.
// Wireshark on unix systems sends SIGINT upon stopping a capture and
// SIGTERM upon wanting to quit. We here use Debug logs as otherwise
// Wireshark will report the logging as errors to the user. We only accept
// that in case of a fatal abort when catching one of the signals twice or
// one after the other.
sigs := make(chan os.Signal, 1)
go func() {
pipe.WaitTillBreak(context.Background(), fifo)
cs.Stop()
fatal := false
for sig := range sigs {
switch sig {
case syscall.SIGINT:
log.Debug("received SIGINT")
case syscall.SIGTERM:
log.Debug("received SIGTERM")
}
if fatal {
// twice a signal --> immediate abort
log.Fatal("aborting")
}
fatal = true
log.Debug("shutting down capture stream")
go func() {
cs.Stop() // blocks, and is also idempotent.
}()
}
}()
// As mentioned above, we unconditionally handle SIGINT and SIGTERM on all
// platforms. While this is currently not needed on Windows, some day it
// might become alive.
signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)

defer cs.Stop() // be overly careful
// ...and finally wait for the packet capture to terminate (or getting
// ex-term-inated).
cs.Wait()
Expand Down
65 changes: 0 additions & 65 deletions pipe/checker_notwin.go

This file was deleted.

93 changes: 0 additions & 93 deletions pipe/checker_notwin_test.go

This file was deleted.

52 changes: 0 additions & 52 deletions pipe/checker_windows.go

This file was deleted.

4 changes: 0 additions & 4 deletions pipe/doc.go

This file was deleted.

21 changes: 0 additions & 21 deletions pipe/package_test.go

This file was deleted.

0 comments on commit 83e731d

Please sign in to comment.