311217f Merge pull request #13 from bnema/fix/copilot-review-pr11 · gordon

M internal/adapters/in/cli/auth.go +2 -3


@@ -6,7 +6,6 @@ import (
6	6		"fmt"
7	7		"os"
8	8		"strings"
9		-	"syscall"
10	9		"time"
11	10
12	11		"github.com/bnema/zerowrap"

@@ -272,8 +271,8 @@ func runTokenRevoke(tokenID, configPath string) error {
272	271		func runPasswordHash() error {
273	272		fmt.Print("Enter password: ")
274	273
275		-	// Read password without echo
276		-	passwordBytes, err := term.ReadPassword(syscall.Stdin)
	274	+	// Read password without echo (use os.Stdin.Fd() for better compatibility)
	275	+	passwordBytes, err := term.ReadPassword(int(os.Stdin.Fd()))
277	276		if err != nil {
278	277		// Fallback for non-terminal input
279	278		reader := bufio.NewReader(os.Stdin)

 	"fmt"
 	"os"
 	"strings"
-	"syscall"
 	"time"
 	"github.com/bnema/zerowrap"
 func runPasswordHash() error {
 	fmt.Print("Enter password: ")
-	// Read password without echo
+	// Read password without echo (use os.Stdin.Fd() for better compatibility)
-	passwordBytes, err := term.ReadPassword(syscall.Stdin)
+	passwordBytes, err := term.ReadPassword(int(os.Stdin.Fd()))
 	if err != nil {
 		// Fallback for non-terminal input
 		reader := bufio.NewReader(os.Stdin)

M internal/app/logs.go +53 -17


@@ -6,7 +6,9 @@ import (
6	6		"fmt"
7	7		"os"
8	8		"os/exec"
	9	+	"os/signal"
9	10		"path/filepath"
	11	+	"syscall"
10	12		"time"
11	13
12	14		"github.com/spf13/viper"

@@ -91,37 +93,59 @@ func showFileLog(logPath string, follow bool, lines int) error {
91	93		return followFile(logPath)
92	94		}
93	95
94		-	// tailFile reads the last N lines from a file.
	96	+	// tailFile reads the last N lines from a file using a ring buffer.
	97	+	// This is memory-efficient as it only keeps the last N lines in memory,
	98	+	// regardless of total file size.
95	99		func tailFile(path string, lines int) error {
	100	+	if lines <= 0 {
	101	+	return nil
	102	+	}
	103	+
96	104		file, err := os.Open(path)
97	105		if err != nil {
98	106		return fmt.Errorf("failed to open log file: %w", err)
99	107		}
100	108		defer file.Close()
101	109
102		-	// Read all lines into a buffer
103		-	var allLines []string
104	110		scanner := bufio.NewScanner(file)
	111	+
	112	+	// Use a ring buffer to store only the last N lines in memory
	113	+	buffer := make([]string, lines)
	114	+	index := 0
	115	+	total := 0
	116	+
105	117		for scanner.Scan() {
106		-	allLines = append(allLines, scanner.Text())
	118	+	buffer[index] = scanner.Text()
	119	+	index = (index + 1) % lines
	120	+	total++
107	121		}
108	122		if err := scanner.Err(); err != nil {
109	123		return fmt.Errorf("failed to read log file: %w", err)
110	124		}
111	125
112		-	// Print last N lines
113		-	start := 0
114		-	if len(allLines) > lines {
115		-	start = len(allLines) - lines
	126	+	if total == 0 {
	127	+	return nil
116	128		}
117		-	for _, line := range allLines[start:] {
118		-	fmt.Println(line)
	129	+
	130	+	// Print lines in correct order
	131	+	if total <= lines {
	132	+	// Fewer than N lines read; buffer is partially filled, in order from index 0
	133	+	for i := 0; i < total; i++ {
	134	+	fmt.Println(buffer[i])
	135	+	}
	136	+	} else {
	137	+	// More than N lines read; buffer is full ring starting at index
	138	+	for i := 0; i < lines; i++ {
	139	+	pos := (index + i) % lines
	140	+	fmt.Println(buffer[pos])
	141	+	}
119	142		}
120	143
121	144		return nil
122	145		}
123	146
124	147		// followFile watches a file for new content and prints it.
	148	+	// It handles interrupt signals (Ctrl+C) gracefully to ensure proper cleanup.
125	149		func followFile(path string) error {
126	150		file, err := os.Open(path)
127	151		if err != nil {

@@ -135,17 +159,29 @@ func followFile(path string) error {
135	159		return fmt.Errorf("failed to seek to end of file: %w", err)
136	160		}
137	161
	162	+	// Set up signal handling for graceful shutdown
	163	+	sigChan := make(chan os.Signal, 1)
	164	+	signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM)
	165	+	defer signal.Stop(sigChan)
	166	+
138	167		reader := bufio.NewReader(file)
139	168
140		-	// Poll for new content
	169	+	// Poll for new content with signal checking
141	170		for {
142		-	line, err := reader.ReadString('\n')
143		-	if err != nil {
144		-	// No new content, wait and retry
145		-	time.Sleep(100 * time.Millisecond)
146		-	continue
	171	+	select {
	172	+	case <-sigChan:
	173	+	// Graceful exit on interrupt
	174	+	fmt.Println() // Print newline for cleaner terminal output
	175	+	return nil
	176	+	default:
	177	+	line, err := reader.ReadString('\n')
	178	+	if err != nil {
	179	+	// No new content, wait and retry
	180	+	time.Sleep(100 * time.Millisecond)
	181	+	continue
	182	+	}
	183	+	fmt.Print(line)
147	184		}
148		-	fmt.Print(line)
149	185		}
150	186		}
151	187


@@ -6,7 +6,9 @@ import (
6		"fmt"	6		"fmt"
7		"os"	7		"os"
8		"os/exec"	8		"os/exec"
			9	+	"os/signal"
9		"path/filepath"	10		"path/filepath"
			11	+	"syscall"
10		"time"	12		"time"
11			13
12		"github.com/spf13/viper"	14		"github.com/spf13/viper"

@@ -91,37 +93,59 @@ func showFileLog(logPath string, follow bool, lines int) error {
91		return followFile(logPath)	93		return followFile(logPath)
92		}	94		}
93			95
94	-	// tailFile reads the last N lines from a file.	96	+	// tailFile reads the last N lines from a file using a ring buffer.
			97	+	// This is memory-efficient as it only keeps the last N lines in memory,
			98	+	// regardless of total file size.
95		func tailFile(path string, lines int) error {	99		func tailFile(path string, lines int) error {
			100	+	if lines <= 0 {
			101	+	return nil
			102	+	}
			103	+
96		file, err := os.Open(path)	104		file, err := os.Open(path)
97		if err != nil {	105		if err != nil {
98		return fmt.Errorf("failed to open log file: %w", err)	106		return fmt.Errorf("failed to open log file: %w", err)
99		}	107		}
100		defer file.Close()	108		defer file.Close()
101			109
102	-	// Read all lines into a buffer
103	-	var allLines []string
104		scanner := bufio.NewScanner(file)	110		scanner := bufio.NewScanner(file)
			111	+
			112	+	// Use a ring buffer to store only the last N lines in memory
			113	+	buffer := make([]string, lines)
			114	+	index := 0
			115	+	total := 0
			116	+
105		for scanner.Scan() {	117		for scanner.Scan() {
106	-	allLines = append(allLines, scanner.Text())	118	+	buffer[index] = scanner.Text()
			119	+	index = (index + 1) % lines
			120	+	total++
107		}	121		}
108		if err := scanner.Err(); err != nil {	122		if err := scanner.Err(); err != nil {
109		return fmt.Errorf("failed to read log file: %w", err)	123		return fmt.Errorf("failed to read log file: %w", err)
110		}	124		}
111			125
112	-	// Print last N lines	126	+	if total == 0 {
113	-	start := 0	127	+	return nil
114	-	if len(allLines) > lines {
115	-	start = len(allLines) - lines
116		}	128		}
117	-	for _, line := range allLines[start:] {	129	+
118	-	fmt.Println(line)	130	+	// Print lines in correct order
			131	+	if total <= lines {
			132	+	// Fewer than N lines read; buffer is partially filled, in order from index 0
			133	+	for i := 0; i < total; i++ {
			134	+	fmt.Println(buffer[i])
			135	+	}
			136	+	} else {
			137	+	// More than N lines read; buffer is full ring starting at index
			138	+	for i := 0; i < lines; i++ {
			139	+	pos := (index + i) % lines
			140	+	fmt.Println(buffer[pos])
			141	+	}
119		}	142		}
120			143
121		return nil	144		return nil
122		}	145		}
123			146
124		// followFile watches a file for new content and prints it.	147		// followFile watches a file for new content and prints it.
			148	+	// It handles interrupt signals (Ctrl+C) gracefully to ensure proper cleanup.
125		func followFile(path string) error {	149		func followFile(path string) error {
126		file, err := os.Open(path)	150		file, err := os.Open(path)
127		if err != nil {	151		if err != nil {

@@ -135,17 +159,29 @@ func followFile(path string) error {
135		return fmt.Errorf("failed to seek to end of file: %w", err)	159		return fmt.Errorf("failed to seek to end of file: %w", err)
136		}	160		}
137			161
			162	+	// Set up signal handling for graceful shutdown
			163	+	sigChan := make(chan os.Signal, 1)
			164	+	signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM)
			165	+	defer signal.Stop(sigChan)
			166	+
138		reader := bufio.NewReader(file)	167		reader := bufio.NewReader(file)
139			168
140	-	// Poll for new content	169	+	// Poll for new content with signal checking
141		for {	170		for {
142	-	line, err := reader.ReadString('\n')	171	+	select {
143	-	if err != nil {	172	+	case <-sigChan:
144	-	// No new content, wait and retry	173	+	// Graceful exit on interrupt
145	-	time.Sleep(100 * time.Millisecond)	174	+	fmt.Println() // Print newline for cleaner terminal output
146	-	continue	175	+	return nil
			176	+	default:
			177	+	line, err := reader.ReadString('\n')
			178	+	if err != nil {
			179	+	// No new content, wait and retry
			180	+	time.Sleep(100 * time.Millisecond)
			181	+	continue
			182	+	}
			183	+	fmt.Print(line)
147		}	184		}
148	-	fmt.Print(line)
149		}	185		}
150		}	186		}
151			187

M internal/app/run.go +8 -1


@@ -701,11 +701,18 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha
701	701		}
702	702
703	703		// runServers starts the HTTP servers and waits for shutdown.
	704	+	// Signal handling notes:
	705	+	// - SIGINT/SIGTERM: Triggers graceful shutdown via signal.NotifyContext
	706	+	// - SIGUSR1: Triggers config reload without restart
	707	+	// The deferred signal.Stop(reloadChan) ensures the signal handler is properly
	708	+	// cleaned up before program exit, preventing signal handler leaks.
704	709		func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler http.Handler, containerSvc *container.Service, eventBus out.EventBus, log zerowrap.Logger) error {
705	710		ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM)
706	711		defer cancel()
707	712
708		-	// Set up SIGUSR1 for reload
	713	+	// Set up SIGUSR1 for reload.
	714	+	// Note: signal.Stop must be called (via defer) to release the channel
	715	+	// and prevent signal handler leaks when the function returns.
709	716		reloadChan := make(chan os.Signal, 1)
710	717		signal.Notify(reloadChan, syscall.SIGUSR1)
711	718		defer signal.Stop(reloadChan)

 }
 // runServers starts the HTTP servers and waits for shutdown.
+// Signal handling notes:
+// - SIGINT/SIGTERM: Triggers graceful shutdown via signal.NotifyContext
+// - SIGUSR1: Triggers config reload without restart
+// The deferred signal.Stop(reloadChan) ensures the signal handler is properly
+// cleaned up before program exit, preventing signal handler leaks.
 func runServers(ctx context.Context, cfg Config, registryHandler, proxyHandler http.Handler, containerSvc *container.Service, eventBus out.EventBus, log zerowrap.Logger) error {
 	ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM)
 	defer cancel()
-	// Set up SIGUSR1 for reload
+	// Set up SIGUSR1 for reload.
+	// Note: signal.Stop must be called (via defer) to release the channel
+	// and prevent signal handler leaks when the function returns.
 	reloadChan := make(chan os.Signal, 1)
 	signal.Notify(reloadChan, syscall.SIGUSR1)
 	defer signal.Stop(reloadChan)

M internal/usecase/container/service.go +5 -0


@@ -910,6 +910,11 @@ func normalizeImageRef(image string) string {
910	910		// user/repo:tag -> docker.io/user/repo
911	911		// localhost:5000/image:tag -> localhost:5000/image
912	912		// registry.example.com/image:tag -> registry.example.com/image
	913	+	//
	914	+	// Note: This function assumes valid Docker image references. Edge cases like
	915	+	// "registry.com:5000" (registry with port but no image name) are not valid
	916	+	// image references per Docker's naming conventions, which require at least
	917	+	// one path component after the registry (e.g., "registry.com:5000/image").
913	918
914	919		// Find the tag separator: the last colon that isn't part of a port number.
915	920		// A colon is part of a port if there's no slash after it until the next colon.


@@ -910,6 +910,11 @@ func normalizeImageRef(image string) string {
910	// user/repo:tag -> docker.io/user/repo	910		// user/repo:tag -> docker.io/user/repo
911	// localhost:5000/image:tag -> localhost:5000/image	911		// localhost:5000/image:tag -> localhost:5000/image
912	// registry.example.com/image:tag -> registry.example.com/image	912		// registry.example.com/image:tag -> registry.example.com/image
		913	+	//
		914	+	// Note: This function assumes valid Docker image references. Edge cases like
		915	+	// "registry.com:5000" (registry with port but no image name) are not valid
		916	+	// image references per Docker's naming conventions, which require at least
		917	+	// one path component after the registry (e.g., "registry.com:5000/image").
913		918
914	// Find the tag separator: the last colon that isn't part of a port number.	919		// Find the tag separator: the last colon that isn't part of a port number.
915	// A colon is part of a port if there's no slash after it until the next colon.	920		// A colon is part of a port if there's no slash after it until the next colon.