Skip to content

Commit 225a7ac

Browse files
committed
refactor: add context-aware flow and graceful shutdown throughout CLI
- Rename project from authgate-cli to authgate-oauth-cli in configuration. - Add a Makefile with common build, test, lint, coverage, and Docker targets. - Modify openBrowser to accept a context and use exec.CommandContext for improved process control. - Use context-aware net.Listener in callback server startup for better testability and shutdown. - Update callback tests to disable gosec linter warnings for http.Get calls. - Improve error handling in filelock tests by checking release errors and reporting them. - Ensure file lock is properly released with error logging in saveTokens. - Pass context to openBrowser to maintain consistent cancellation semantics. - Expand context handling and shutdown hooks for graceful cancellation in main flow. - Refactor API call retry for style and clarity. Signed-off-by: appleboy <appleboy.tw@gmail.com>
1 parent 1afa8c6 commit 225a7ac

7 files changed

Lines changed: 168 additions & 17 deletions

File tree

.goreleaser.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
version: 2
2-
project_name: authgate-cli
2+
project_name: authgate-oauth-cli
33
before:
44
hooks:
55
- make generate

Makefile

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
GO ?= go
2+
EXECUTABLE := oauth-cli
3+
GOFILES := $(shell find . -type f -name "*.go")
4+
TAGS ?=
5+
TEMPL_VERSION ?= latest
6+
7+
ifneq ($(shell uname), Darwin)
8+
EXTLDFLAGS = -extldflags "-static" $(null)
9+
else
10+
EXTLDFLAGS =
11+
endif
12+
13+
ifneq ($(DRONE_TAG),)
14+
VERSION ?= $(DRONE_TAG)
15+
else
16+
VERSION ?= $(shell git describe --tags --always || git rev-parse --short HEAD)
17+
endif
18+
COMMIT ?= $(shell git rev-parse --short HEAD)
19+
20+
LDFLAGS ?=
21+
## build: build the authgate binary
22+
build: generate $(EXECUTABLE)
23+
24+
$(EXECUTABLE): $(GOFILES)
25+
$(GO) build -v -tags '$(TAGS)' -ldflags '$(EXTLDFLAGS)-s -w $(LDFLAGS)' -o bin/$@ .
26+
27+
## air: Install air for hot reload.
28+
air:
29+
@hash air > /dev/null 2>&1; if [ $$? -ne 0 ]; then \
30+
$(GO) install github.com/air-verse/air@latest; \
31+
fi
32+
33+
## dev: Run the application with hot reload.
34+
dev: air
35+
air
36+
37+
## install: install the authgate binary
38+
install: $(GOFILES)
39+
$(GO) install -v -tags '$(TAGS)' -ldflags '$(EXTLDFLAGS)-s -w $(LDFLAGS)'
40+
41+
## test: run tests
42+
test: generate
43+
@$(GO) test -v -cover -coverprofile coverage.txt ./... && echo "\n==>\033[32m Ok\033[m\n" || exit 1
44+
45+
## coverage: view test coverage in browser
46+
coverage: test
47+
$(GO) tool cover -html=coverage.txt
48+
49+
## install-golangci-lint: install golangci-lint if not present
50+
install-golangci-lint:
51+
@command -v golangci-lint >/dev/null 2>&1 || curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b $$($(GO) env GOPATH)/bin v2.7.2
52+
53+
## fmt: format go files using golangci-lint
54+
fmt: install-golangci-lint
55+
golangci-lint fmt
56+
57+
## lint: run golangci-lint to check for issues
58+
lint: install-golangci-lint
59+
golangci-lint run
60+
61+
## build_linux_amd64: build the authgate binary for linux amd64
62+
build_linux_amd64: generate
63+
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 $(GO) build -a -tags '$(TAGS)' -ldflags '$(EXTLDFLAGS)-s -w $(LDFLAGS)' -o release/linux/amd64/$(EXECUTABLE) .
64+
65+
## build_linux_arm64: build the authgate binary for linux arm64
66+
build_linux_arm64: generate
67+
CGO_ENABLED=0 GOOS=linux GOARCH=arm64 $(GO) build -a -tags '$(TAGS)' -ldflags '$(EXTLDFLAGS)-s -w $(LDFLAGS)' -o release/linux/arm64/$(EXECUTABLE) .
68+
69+
## clean: remove build artifacts and test coverage
70+
clean:
71+
rm -rf bin/ release/ coverage.txt
72+
find internal/templates -name "*_templ.go" -delete
73+
74+
## rebuild: clean and build
75+
rebuild: clean build
76+
77+
.PHONY: help build install test coverage fmt lint clean rebuild
78+
.PHONY: build_linux_amd64 build_linux_arm64
79+
.PHONY: install-templ generate watch air dev
80+
.PHONY: install-golangci-lint mod-download mod-tidy mod-verify check-tools version
81+
.PHONY: docker-build docker-run
82+
83+
## install-templ: install templ CLI if not installed
84+
install-templ:
85+
@command -v templ >/dev/null 2>&1 || $(GO) install github.com/a-h/templ/cmd/templ@$(TEMPL_VERSION)
86+
87+
## generate: run templ generate to compile .templ files
88+
generate: install-templ
89+
templ generate
90+
91+
## watch: watch mode for automatic regeneration
92+
watch: install-templ
93+
templ generate --watch
94+
95+
## help: print this help message
96+
help:
97+
@echo 'Usage:'
98+
@sed -n 's/^##//p' ${MAKEFILE_LIST} | column -t -s ':' | sed -e 's/^/ /'
99+
100+
## mod-download: download go module dependencies
101+
mod-download:
102+
$(GO) mod download
103+
104+
## mod-tidy: tidy go module dependencies
105+
mod-tidy:
106+
$(GO) mod tidy
107+
108+
## mod-verify: verify go module dependencies
109+
mod-verify:
110+
$(GO) mod verify
111+
112+
## check-tools: verify required tools are installed
113+
check-tools:
114+
@command -v $(GO) >/dev/null 2>&1 || (echo "Go not found" && exit 1)
115+
@command -v templ >/dev/null 2>&1 || echo "templ not installed (run: make install-templ)"
116+
@command -v golangci-lint >/dev/null 2>&1 || echo "golangci-lint not installed (run: make install-golangci-lint)"
117+
118+
## version: display version information
119+
version:
120+
@echo "Version: $(VERSION)"
121+
@echo "Commit: $(COMMIT)"
122+
@echo "Go Version: $(shell $(GO) version)"
123+
124+
## docker-build: build docker image
125+
docker-build:
126+
docker build -t authgate:$(VERSION) -f Dockerfile .
127+
128+
## docker-run: run docker container
129+
docker-run:
130+
docker run -p 8080:8080 --env-file .env authgate:$(VERSION)
131+

browser.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package main
22

33
import (
4+
"context"
45
"fmt"
56
"os/exec"
67
"runtime"
@@ -9,17 +10,17 @@ import (
910
// openBrowser attempts to open url in the user's default browser.
1011
// Returns an error if launching the browser fails, but callers should
1112
// always print the URL as a fallback regardless of the error.
12-
func openBrowser(url string) error {
13+
func openBrowser(ctx context.Context, url string) error {
1314
var cmd *exec.Cmd
1415

1516
switch runtime.GOOS {
1617
case "darwin":
17-
cmd = exec.Command("open", url)
18+
cmd = exec.CommandContext(ctx, "open", url)
1819
case "windows":
19-
cmd = exec.Command("cmd", "/c", "start", url)
20+
cmd = exec.CommandContext(ctx, "cmd", "/c", "start", url)
2021
default:
2122
// Linux and other Unix-like systems
22-
cmd = exec.Command("xdg-open", url)
23+
cmd = exec.CommandContext(ctx, "xdg-open", url)
2324
}
2425

2526
if err := cmd.Start(); err != nil {

callback.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func startCallbackServer(ctx context.Context, port int, expectedState string) (s
8181
}
8282

8383
// Use a listener so we can report the actual bound port.
84-
ln, err := net.Listen("tcp", srv.Addr)
84+
ln, err := (&net.ListenConfig{}).Listen(ctx, "tcp", srv.Addr)
8585
if err != nil {
8686
return "", fmt.Errorf("failed to start callback server on port %d: %w", port, err)
8787
}

callback_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestCallbackServer_Success(t *testing.T) {
3939
"http://127.0.0.1:%d/callback?code=mycode123&state=%s",
4040
port, state,
4141
)
42-
resp, err := http.Get(callbackURL) //nolint:noctx
42+
resp, err := http.Get(callbackURL) //nolint:noctx,gosec
4343
if err != nil {
4444
t.Fatalf("GET callback failed: %v", err)
4545
}
@@ -74,7 +74,7 @@ func TestCallbackServer_StateMismatch(t *testing.T) {
7474
"http://127.0.0.1:%d/callback?code=mycode&state=wrong-state",
7575
port,
7676
)
77-
resp, err := http.Get(callbackURL) //nolint:noctx
77+
resp, err := http.Get(callbackURL) //nolint:noctx,gosec
7878
if err != nil {
7979
t.Fatalf("GET callback failed: %v", err)
8080
}
@@ -105,7 +105,7 @@ func TestCallbackServer_OAuthError(t *testing.T) {
105105
"http://127.0.0.1:%d/callback?error=access_denied&error_description=User+denied&state=%s",
106106
port, state,
107107
)
108-
resp, err := http.Get(callbackURL) //nolint:noctx
108+
resp, err := http.Get(callbackURL) //nolint:noctx,gosec
109109
if err != nil {
110110
t.Fatalf("GET callback failed: %v", err)
111111
}
@@ -146,7 +146,7 @@ func TestCallbackServer_DoubleCallback(t *testing.T) {
146146
done := make(chan error, 2)
147147
for range 2 {
148148
go func() {
149-
resp, err := http.Get(url) //nolint:noctx
149+
resp, err := http.Get(url) //nolint:noctx,gosec
150150
if err == nil {
151151
resp.Body.Close()
152152
}
@@ -185,7 +185,7 @@ func TestCallbackServer_MissingCode(t *testing.T) {
185185
"http://127.0.0.1:%d/callback?state=%s",
186186
port, state,
187187
)
188-
resp, err := http.Get(callbackURL) //nolint:noctx
188+
resp, err := http.Get(callbackURL) //nolint:noctx,gosec
189189
if err != nil {
190190
t.Fatalf("GET callback failed: %v", err)
191191
}

filelock_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ func TestConcurrentLocks(t *testing.T) {
6565
concurrent--
6666
mu.Unlock()
6767

68-
lock.release()
68+
if err := lock.release(); err != nil {
69+
t.Errorf("goroutine %d: release() error: %v", idx, err)
70+
}
6971
}(i)
7072
}
7173

@@ -94,5 +96,7 @@ func TestStaleLockRemoval(t *testing.T) {
9496
if err != nil {
9597
t.Fatalf("acquireFileLock() with stale lock: %v", err)
9698
}
97-
lock.release()
99+
if err := lock.release(); err != nil {
100+
t.Errorf("release() error: %v", err)
101+
}
98102
}

main.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,11 @@ func saveTokens(storage *TokenStorage) error {
251251
if err != nil {
252252
return fmt.Errorf("failed to acquire lock: %w", err)
253253
}
254-
defer lock.release()
254+
defer func() {
255+
if releaseErr := lock.release(); releaseErr != nil {
256+
fmt.Fprintf(os.Stderr, "warning: failed to release file lock: %v\n", releaseErr)
257+
}
258+
}()
255259

256260
var storageMap TokenStorageMap
257261
if existing, err := os.ReadFile(tokenFile); err == nil {
@@ -329,7 +333,7 @@ func performAuthCodeFlow(ctx context.Context) (*TokenStorage, error) {
329333
fmt.Println("Step 1: Opening authorization URL in your browser...")
330334
fmt.Printf("\n %s\n\n", authURL)
331335

332-
if err := openBrowser(authURL); err != nil {
336+
if err := openBrowser(ctx, authURL); err != nil {
333337
fmt.Println("Could not open browser automatically. Please open the URL above manually.")
334338
} else {
335339
fmt.Println("Browser opened. Please complete authorization in your browser.")
@@ -610,7 +614,12 @@ func makeAPICallWithAutoRefresh(ctx context.Context, storage *TokenStorage) erro
610614

611615
fmt.Println("Token refreshed, retrying API call...")
612616

613-
req, err = http.NewRequestWithContext(ctx, http.MethodGet, serverURL+"/oauth/tokeninfo", nil)
617+
req, err = http.NewRequestWithContext(
618+
ctx,
619+
http.MethodGet,
620+
serverURL+"/oauth/tokeninfo",
621+
nil,
622+
)
614623
if err != nil {
615624
return fmt.Errorf("failed to create retry request: %w", err)
616625
}
@@ -671,6 +680,7 @@ func main() {
671680
if err != nil {
672681
if ctx.Err() != nil {
673682
fmt.Fprintln(os.Stderr, "\nInterrupted.")
683+
stop()
674684
os.Exit(130)
675685
}
676686
fmt.Printf("Refresh failed: %v\n", err)
@@ -690,7 +700,8 @@ func main() {
690700
if err != nil {
691701
if ctx.Err() != nil {
692702
fmt.Fprintln(os.Stderr, "\nInterrupted.")
693-
os.Exit(130)
703+
stop()
704+
os.Exit(130) //nolint:gocritic
694705
}
695706
fmt.Fprintf(os.Stderr, "Authorization failed: %v\n", err)
696707
os.Exit(1)
@@ -714,6 +725,7 @@ func main() {
714725
if err := verifyToken(ctx, storage.AccessToken); err != nil {
715726
if ctx.Err() != nil {
716727
fmt.Fprintln(os.Stderr, "\nInterrupted.")
728+
stop()
717729
os.Exit(130)
718730
}
719731
fmt.Printf("Token verification failed: %v\n", err)
@@ -726,6 +738,7 @@ func main() {
726738
if err := makeAPICallWithAutoRefresh(ctx, storage); err != nil {
727739
if ctx.Err() != nil {
728740
fmt.Fprintln(os.Stderr, "\nInterrupted.")
741+
stop()
729742
os.Exit(130)
730743
}
731744
if err == ErrRefreshTokenExpired {
@@ -734,6 +747,7 @@ func main() {
734747
if err != nil {
735748
if ctx.Err() != nil {
736749
fmt.Fprintln(os.Stderr, "\nInterrupted.")
750+
stop()
737751
os.Exit(130)
738752
}
739753
fmt.Fprintf(os.Stderr, "Re-authentication failed: %v\n", err)
@@ -742,6 +756,7 @@ func main() {
742756
if err := makeAPICallWithAutoRefresh(ctx, storage); err != nil {
743757
if ctx.Err() != nil {
744758
fmt.Fprintln(os.Stderr, "\nInterrupted.")
759+
stop()
745760
os.Exit(130)
746761
}
747762
fmt.Fprintf(os.Stderr, "API call failed after re-authentication: %v\n", err)

0 commit comments

Comments
 (0)