lukaszraczylo/kportal
1
0
Fork 0
mirror of https://github.com/lukaszraczylo/kportal.git synced 2026-07-09 06:44:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

style: Extract UI constants and refactor main view rendering (#30)

Browse Source 
- [x] Add golangci-lint configuration with gocritic ifElseChain disabled
- [x] Rename error variables to avoid shadowing (createErr, watcherErr, watchErr, etc.)
- [x] Replace `interface{}` with `any` type alias throughout codebase
- [x] Add package-level documentation comments to all internal packages
- [x] Reorder struct fields alphabetically for consistency
- [x] Extract UI constants (terminal dimensions, column widths, colors) to constants.go
- [x] Refactor BubbleTeaUI main view rendering into smaller helper functions
- [x] Simplify nested conditionals and improve code clarity
- [x] Add `isForwardDisabled()` helper method to BubbleTeaUI
- [x] Update file permissions from 0644 to 0600 in config tests
- [x] Add `#nosec` comments and error suppression where appropriate
- [x] Improve test table struct field ordering for readability
- [x] Fix resource parsing in AddForward using strings.SplitN
- [x] Add comprehensive tests for new UI helper functions and constants
This commit is contained in:
Lukasz Raczylo
2026-01-13 09:37:45 +00:00
committed by GitHub
parent 3d71f64901
commit 96ae1d45e0
54 changed files with 1319 additions and 730 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/config/config.go
+38 -19
View File
@@ -1,3 +1,24 @@
// Package config provides configuration loading, validation, watching, and
// mutation for kportal. It handles parsing the .kportal.yaml configuration
// file and provides hot-reload support via file watching.
//
// The configuration structure supports multiple Kubernetes contexts, each
// with namespaces containing port-forward definitions. Additional settings
// for health checks, reliability, and mDNS hostname publishing are also
// supported.
//
// Basic usage:
//
// cfg, err := config.Load("~/.kportal.yaml")
// if err != nil {
// log.Fatal(err)
// }
//
// For hot-reload support, use the ConfigWatcher:
//
// watcher, err := config.NewConfigWatcher(path, func(cfg *config.Config) {
// // Handle configuration changes
// })
package config
import (
@@ -36,10 +57,10 @@ const (
// Config represents the root configuration structure from .kportal.yaml
type Config struct {
Contexts []Context `yaml:"contexts"`
HealthCheck *HealthCheckSpec `yaml:"healthCheck,omitempty"`
Reliability *ReliabilitySpec `yaml:"reliability,omitempty"`
MDNS *MDNSSpec `yaml:"mdns,omitempty"`
Contexts []Context `yaml:"contexts"`
}
// MDNSSpec configures mDNS (multicast DNS) hostname publishing
@@ -59,10 +80,10 @@ type HealthCheckSpec struct {
// ReliabilitySpec configures connection reliability features
type ReliabilitySpec struct {
TCPKeepalive string `yaml:"tcpKeepalive,omitempty"` // e.g., "30s" - OS-level keepalive
DialTimeout string `yaml:"dialTimeout,omitempty"` // e.g., "30s" - connection dial timeout
RetryOnStale bool `yaml:"retryOnStale,omitempty"` // Auto-reconnect on stale detection
WatchdogPeriod string `yaml:"watchdogPeriod,omitempty"` // e.g., "30s" - goroutine watchdog interval
TCPKeepalive string `yaml:"tcpKeepalive,omitempty"`
DialTimeout string `yaml:"dialTimeout,omitempty"`
WatchdogPeriod string `yaml:"watchdogPeriod,omitempty"`
RetryOnStale bool `yaml:"retryOnStale,omitempty"`
}
// parseDurationOrDefault parses a duration string and returns the default if empty or invalid.
@@ -167,11 +188,11 @@ type Namespace struct {
// HTTPLogSpec configures HTTP traffic logging for a forward
type HTTPLogSpec struct {
Enabled bool `yaml:"enabled"` // Enable HTTP logging
LogFile string `yaml:"logFile,omitempty"` // Output file (empty = stdout)
MaxBodySize int `yaml:"maxBodySize,omitempty"` // Max body size to log (default 1MB)
IncludeHeaders bool `yaml:"includeHeaders,omitempty"` // Include headers in log
FilterPath string `yaml:"filterPath,omitempty"` // Optional glob filter for paths
LogFile string `yaml:"logFile,omitempty"`
FilterPath string `yaml:"filterPath,omitempty"`
MaxBodySize int `yaml:"maxBodySize,omitempty"`
Enabled bool `yaml:"enabled"`
IncludeHeaders bool `yaml:"includeHeaders,omitempty"`
}
// UnmarshalYAML implements custom unmarshaling to support both bool and struct formats
@@ -196,17 +217,15 @@ func (h *HTTPLogSpec) UnmarshalYAML(unmarshal func(interface{}) error) error {
// Forward represents a single port-forward configuration
type Forward struct {
Resource string `yaml:"resource"` // e.g., "pod/my-app", "service/postgres", "pod"
Selector string `yaml:"selector"` // Label selector for pod resolution (e.g., "app=nginx,env=prod")
Protocol string `yaml:"protocol"` // tcp or udp
Port int `yaml:"port"` // Remote port
LocalPort int `yaml:"localPort"` // Local port
Alias string `yaml:"alias,omitempty"` // Optional human-readable alias for this forward
HTTPLog *HTTPLogSpec `yaml:"httpLog,omitempty"` // Optional HTTP traffic logging
// Runtime fields (not in YAML)
HTTPLog *HTTPLogSpec `yaml:"httpLog,omitempty"`
Resource string `yaml:"resource"`
Selector string `yaml:"selector"`
Protocol string `yaml:"protocol"`
Alias string `yaml:"alias,omitempty"`
contextName string
namespaceName string
Port int `yaml:"port"`
LocalPort int `yaml:"localPort"`
}
// ID returns a unique identifier for this forward configuration.
internal/config/config_getters_test.go
+12 -12
View File
@@ -40,8 +40,8 @@ func TestParseDurationOrDefault(t *testing.T) {
// TestConfig_GetHealthCheckIntervalOrDefault tests health check interval getter
func TestConfig_GetHealthCheckIntervalOrDefault(t *testing.T) {
tests := []struct {
name string
config *Config
name string
expected time.Duration
}{
{
@@ -83,8 +83,8 @@ func TestConfig_GetHealthCheckIntervalOrDefault(t *testing.T) {
// TestConfig_GetHealthCheckTimeoutOrDefault tests health check timeout getter
func TestConfig_GetHealthCheckTimeoutOrDefault(t *testing.T) {
tests := []struct {
name string
config *Config
name string
expected time.Duration
}{
{
@@ -162,8 +162,8 @@ func TestConfig_GetHealthCheckMethod(t *testing.T) {
// TestConfig_GetMaxConnectionAge tests max connection age getter
func TestConfig_GetMaxConnectionAge(t *testing.T) {
tests := []struct {
name string
config *Config
name string
expected time.Duration
}{
{
@@ -198,8 +198,8 @@ func TestConfig_GetMaxConnectionAge(t *testing.T) {
// TestConfig_GetMaxIdleTime tests max idle time getter
func TestConfig_GetMaxIdleTime(t *testing.T) {
tests := []struct {
name string
config *Config
name string
expected time.Duration
}{
{
@@ -234,8 +234,8 @@ func TestConfig_GetMaxIdleTime(t *testing.T) {
// TestConfig_GetTCPKeepalive tests TCP keepalive getter
func TestConfig_GetTCPKeepalive(t *testing.T) {
tests := []struct {
name string
config *Config
name string
expected time.Duration
}{
{
@@ -270,8 +270,8 @@ func TestConfig_GetTCPKeepalive(t *testing.T) {
// TestConfig_GetRetryOnStale tests retry on stale getter
func TestConfig_GetRetryOnStale(t *testing.T) {
tests := []struct {
name string
config *Config
name string
expected bool
}{
{
@@ -306,8 +306,8 @@ func TestConfig_GetRetryOnStale(t *testing.T) {
// TestConfig_GetWatchdogPeriod tests watchdog period getter
func TestConfig_GetWatchdogPeriod(t *testing.T) {
tests := []struct {
name string
config *Config
name string
expected time.Duration
}{
{
@@ -342,8 +342,8 @@ func TestConfig_GetWatchdogPeriod(t *testing.T) {
// TestConfig_GetDialTimeout tests dial timeout getter
func TestConfig_GetDialTimeout(t *testing.T) {
tests := []struct {
name string
config *Config
name string
expected time.Duration
}{
{
@@ -378,8 +378,8 @@ func TestConfig_GetDialTimeout(t *testing.T) {
// TestConfig_IsMDNSEnabled tests mDNS enabled getter
func TestConfig_IsMDNSEnabled(t *testing.T) {
tests := []struct {
name string
config *Config
name string
expected bool
}{
{
@@ -509,8 +509,8 @@ func TestForward_GetHTTPLogMaxBodySize(t *testing.T) {
func TestForward_GetMDNSAlias(t *testing.T) {
tests := []struct {
name string
forward Forward
expected string
forward Forward
}{
{
name: "explicit alias",
@@ -591,7 +591,7 @@ func TestLoadConfig_FileTooLarge(t *testing.T) {
largeData[i] = 'a'
}
err := os.WriteFile(configPath, largeData, 0644)
err := os.WriteFile(configPath, largeData, 0600)
require.NoError(t, err)
cfg, err := LoadConfig(configPath)
@@ -628,7 +628,7 @@ mdns:
enabled: true
`
err := os.WriteFile(configPath, []byte(yaml), 0644)
err := os.WriteFile(configPath, []byte(yaml), 0600)
require.NoError(t, err)
cfg, err := LoadConfig(configPath)
internal/config/config_test.go
+8 -10
View File
@@ -39,7 +39,7 @@ func TestLoadConfig_ValidYAML(t *testing.T) {
localPort: 8081
`
err := os.WriteFile(configPath, []byte(validYAML), 0644)
err := os.WriteFile(configPath, []byte(validYAML), 0600)
assert.NoError(t, err, "should write temp config file")
// Load the config
@@ -82,7 +82,7 @@ func TestLoadConfig_InvalidYAML(t *testing.T) {
forwards: [this is invalid yaml syntax
`
err := os.WriteFile(configPath, []byte(invalidYAML), 0644)
err := os.WriteFile(configPath, []byte(invalidYAML), 0600)
assert.NoError(t, err, "should write temp config file")
// Load the config
@@ -103,8 +103,8 @@ func TestLoadConfig_FileNotFound(t *testing.T) {
func TestForward_ID(t *testing.T) {
tests := []struct {
name string
forward Forward
expectedID string
forward Forward
}{
{
name: "pod with explicit name",
@@ -165,8 +165,8 @@ func TestForward_ID(t *testing.T) {
func TestForward_String(t *testing.T) {
tests := []struct {
name string
forward Forward
expectedString string
forward Forward
}{
{
name: "pod without selector",
@@ -389,10 +389,8 @@ func TestHTTPLogSpec_UnmarshalYAML(t *testing.T) {
if tt.expected {
assert.NotNil(t, fwd.HTTPLog, "HTTPLog should not be nil")
assert.True(t, fwd.HTTPLog.Enabled, "HTTPLog.Enabled should be true")
} else {
if fwd.HTTPLog != nil {
assert.False(t, fwd.HTTPLog.Enabled, "HTTPLog.Enabled should be false")
}
} else if fwd.HTTPLog != nil {
assert.False(t, fwd.HTTPLog.Enabled, "HTTPLog.Enabled should be false")
}
})
}
@@ -407,8 +405,8 @@ func TestNewEmptyConfig(t *testing.T) {
func TestConfig_IsEmpty(t *testing.T) {
tests := []struct {
name string
config *Config
name string
expected bool
}{
{
@@ -505,7 +503,7 @@ func TestCreateEmptyConfigFile_AlreadyExists(t *testing.T) {
configPath := filepath.Join(tmpDir, ".kportal.yaml")
// Create existing file
err := os.WriteFile(configPath, []byte("existing content"), 0644)
err := os.WriteFile(configPath, []byte("existing content"), 0600)
assert.NoError(t, err)
// Try to create config file - should fail
internal/config/mutator_test.go
+1 -1
View File
@@ -648,7 +648,7 @@ func TestMutator_Concurrent(t *testing.T) {
}
// Some will succeed, some will fail due to validation
// The important thing is no race condition
mutator.AddForward("dev", "default", fwd)
_ = mutator.AddForward("dev", "default", fwd)
}(i)
}
internal/config/validator.go
+8 -10
View File
@@ -17,9 +17,9 @@ func IsValidPort(port int) bool {
// ValidationError represents a configuration validation error with context.
type ValidationError struct {
Field string // The field that failed validation
Message string // Error message
Context map[string]string // Additional context information
Context map[string]string
Field string
Message string
}
// Validator validates configuration files.
@@ -199,14 +199,12 @@ func (v *Validator) validateResource(fwd *Forward) []ValidationError {
Message: fmt.Sprintf("Pod name cannot be empty for forward %s", fwd.ID()),
})
}
} else {
} else if fwd.Selector == "" {
// pod (no name) - must have selector
if fwd.Selector == "" {
errs = append(errs, ValidationError{
Field: "selector",
Message: fmt.Sprintf("Forward %s uses generic 'pod' resource and must have a selector", fwd.ID()),
})
}
errs = append(errs, ValidationError{
Field: "selector",
Message: fmt.Sprintf("Forward %s uses generic 'pod' resource and must have a selector", fwd.ID()),
})
}
}
internal/config/validator_test.go
+11 -11
View File
@@ -11,10 +11,10 @@ func TestValidator_ValidateConfig(t *testing.T) {
validator := NewValidator()
tests := []struct {
name string
config *Config
expectErrors bool
name string
errorContains []string
expectErrors bool
}{
{
name: "valid config",
@@ -227,9 +227,9 @@ func TestValidator_ValidateResourceFormat(t *testing.T) {
tests := []struct {
name string
errorContains []string
forward Forward
expectErrors bool
errorContains []string
}{
{
name: "valid pod with name",
@@ -370,10 +370,10 @@ func TestValidator_CheckDuplicatePorts(t *testing.T) {
validator := NewValidator()
tests := []struct {
name string
config *Config
expectErrors bool
name string
errorContains []string
expectErrors bool
}{
{
name: "no duplicate ports",
@@ -552,8 +552,8 @@ func TestFormatValidationErrors(t *testing.T) {
tests := []struct {
name string
errors []ValidationError
expectEmpty bool
expectContains []string
expectEmpty bool
}{
{
name: "no errors",
@@ -625,10 +625,10 @@ func TestValidator_ValidateStructure(t *testing.T) {
validator := NewValidator()
tests := []struct {
name string
config *Config
expectErrors bool
name string
errorContains []string
expectErrors bool
}{
{
name: "empty context name",
@@ -697,10 +697,10 @@ func TestValidator_ValidateMDNS(t *testing.T) {
validator := NewValidator()
tests := []struct {
name string
config *Config
expectErrors bool
name string
errorContains []string
expectErrors bool
}{
{
name: "mDNS disabled - no validation",
@@ -968,8 +968,8 @@ func TestValidator_ValidateConfigWithOptions(t *testing.T) {
validator := NewValidator()
tests := []struct {
name string
config *Config
name string
allowEmpty bool
expectErrors bool
}{
internal/config/watcher.go
+6 -6
View File
@@ -16,13 +16,13 @@ type ReloadCallback func(*Config) error
// Watcher watches a configuration file for changes and triggers hot-reload.
type Watcher struct {
configPath string
callback ReloadCallback
watcher *fsnotify.Watcher
done chan struct{}
configPath string
wg sync.WaitGroup
stopOnce sync.Once
verbose bool
wg sync.WaitGroup // Ensures watch goroutine exits before Stop returns
stopOnce sync.Once // Ensures Stop is safe to call multiple times
}
// NewWatcher creates a new file watcher for the given config file.
@@ -34,7 +34,7 @@ func NewWatcher(configPath string, callback ReloadCallback, verbose bool) (*Watc
absPath, err := filepath.Abs(configPath)
if err != nil {
_ = watcher.Close()
_ = watcher.Close() // Cleanup on error path; already returning error
return nil, fmt.Errorf("failed to resolve absolute path: %w", err)
}
@@ -42,7 +42,7 @@ func NewWatcher(configPath string, callback ReloadCallback, verbose bool) (*Watc
// (many editors delete and recreate files on save)
dir := filepath.Dir(absPath)
if err := watcher.Add(dir); err != nil {
_ = watcher.Close()
_ = watcher.Close() // Cleanup on error path; already returning error
return nil, fmt.Errorf("failed to watch directory %s: %w", dir, err)
}
@@ -66,7 +66,7 @@ func (w *Watcher) Start() {
func (w *Watcher) Stop() {
w.stopOnce.Do(func() {
close(w.done)
_ = w.watcher.Close()
_ = w.watcher.Close() // Best-effort cleanup during shutdown
})
w.wg.Wait() // Wait for watch goroutine to exit
}
internal/config/watcher_test.go
+20 -18
View File
@@ -27,7 +27,7 @@ func TestNewWatcher(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
callback := func(cfg *Config) error { return nil }
@@ -57,7 +57,7 @@ func TestNewWatcher_Verbose(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
callback := func(cfg *Config) error { return nil }
@@ -85,13 +85,15 @@ func TestNewWatcher_RelativePath(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
// Change to tmpDir and use relative path
originalDir, _ := os.Getwd()
defer os.Chdir(originalDir)
os.Chdir(tmpDir)
originalDir, err := os.Getwd()
require.NoError(t, err)
defer func() { _ = os.Chdir(originalDir) }()
err = os.Chdir(tmpDir)
require.NoError(t, err)
callback := func(cfg *Config) error { return nil }
@@ -119,7 +121,7 @@ func TestWatcher_StartStop(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
callback := func(cfg *Config) error { return nil }
@@ -161,7 +163,7 @@ func TestWatcher_DetectsFileChange(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
var mu sync.Mutex
@@ -199,7 +201,7 @@ func TestWatcher_DetectsFileChange(t *testing.T) {
port: 9090
localPort: 9090
`
err = os.WriteFile(configPath, []byte(updated), 0644)
err = os.WriteFile(configPath, []byte(updated), 0600)
require.NoError(t, err)
// Wait for callback with timeout
@@ -239,7 +241,7 @@ func TestWatcher_IgnoresInvalidConfig(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
callbackCount := 0
@@ -267,7 +269,7 @@ func TestWatcher_IgnoresInvalidConfig(t *testing.T) {
- name: default
forwards: [this is invalid
`
err = os.WriteFile(configPath, []byte(invalid), 0644)
err = os.WriteFile(configPath, []byte(invalid), 0600)
require.NoError(t, err)
// Wait a bit
@@ -294,7 +296,7 @@ func TestWatcher_IgnoresValidationErrors(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
callbackCount := 0
@@ -328,7 +330,7 @@ func TestWatcher_IgnoresValidationErrors(t *testing.T) {
port: 9090
localPort: 8080
`
err = os.WriteFile(configPath, []byte(invalid), 0644)
err = os.WriteFile(configPath, []byte(invalid), 0600)
require.NoError(t, err)
// Wait a bit
@@ -356,7 +358,7 @@ func TestWatcher_IgnoresOtherFiles(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
callbackCount := 0
@@ -378,7 +380,7 @@ func TestWatcher_IgnoresOtherFiles(t *testing.T) {
time.Sleep(100 * time.Millisecond)
// Write to a different file
err = os.WriteFile(otherPath, []byte("some content"), 0644)
err = os.WriteFile(otherPath, []byte("some content"), 0600)
require.NoError(t, err)
// Wait a bit
@@ -405,7 +407,7 @@ func TestWatcher_HandleReload_LoadError(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
callbackCalled := false
@@ -445,7 +447,7 @@ func TestWatcher_DoubleStop(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
callback := func(cfg *Config) error { return nil }
@@ -479,7 +481,7 @@ func TestWatcher_StopWithoutStart(t *testing.T) {
port: 8080
localPort: 8080
`
err := os.WriteFile(configPath, []byte(initial), 0644)
err := os.WriteFile(configPath, []byte(initial), 0600)
require.NoError(t, err)
callback := func(cfg *Config) error { return nil }