diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 67aca6d..9905db8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,6 +24,14 @@ jobs: with: go-version-file: go.mod + # The unit tests under ./internal/... were not covered by any workflow, so + # they silently stopped compiling as the packages they test were + # refactored. Run them first and let a failure fail the job. + - name: Run unit tests + env: + CGO_LDFLAGS: -lm + run: go test ./internal/... + - name: Run tests in short mode env: CGO_LDFLAGS: -lm diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go index a651331..5abbf08 100644 --- a/internal/cache/cache_test.go +++ b/internal/cache/cache_test.go @@ -294,28 +294,34 @@ func TestCache_Stats(t *testing.T) { t.Errorf("expected empty stats after clear, got total=%d", total) } - // Use short-lived cache for this test - shortCache := New(WithTTL(150 * time.Millisecond)) - - // Use unique keys + // Use unique keys so a leftover store from another run cannot affect counts key1 := fmt.Sprintf("stats-key1-%d", time.Now().UnixNano()) key2 := fmt.Sprintf("stats-key2-%d", time.Now().UnixNano()) - // Add entries quickly - _ = shortCache.Set(key1, map[string]string{"K": "V"}) - _ = shortCache.Set(key2, map[string]string{"K": "V"}) + // Freshly written entries are valid. The TTL has to be comfortably longer + // than two keyring writes take: each Set shells out to the OS keyring, which + // can cost hundreds of milliseconds, and a tight TTL expires key1 while key2 + // is still being written. + validCache := New(WithTTL(5 * time.Minute)) + _ = validCache.Set(key1, map[string]string{"K": "V"}) + _ = validCache.Set(key2, map[string]string{"K": "V"}) - total, valid, expired = shortCache.Stats() + total, valid, expired = validCache.Stats() if valid != 2 { t.Errorf("expected 2 valid entries immediately after set, got valid=%d, expired=%d", valid, expired) } - // Wait for expiration - time.Sleep(200 * time.Millisecond) + _ = c.Clear() + + // A negative TTL writes entries whose ExpiresAt is already in the past, so + // expiry is asserted without sleeping on a real clock. + expiredCache := New(WithTTL(-1 * time.Minute)) + _ = expiredCache.Set(key1, map[string]string{"K": "V"}) + _ = expiredCache.Set(key2, map[string]string{"K": "V"}) - total, valid, expired = shortCache.Stats() + total, valid, expired = expiredCache.Stats() if expired != 2 { - t.Errorf("expected 2 expired entries after TTL, got valid=%d, expired=%d", valid, expired) + t.Errorf("expected 2 expired entries, got valid=%d, expired=%d", valid, expired) } // Clean up @@ -342,8 +348,9 @@ func TestCache_IsAvailable(t *testing.T) { func TestCache_KeyringNotAvailable(t *testing.T) { cache := New() - // Force keyring to be disabled - cache.keyringTested = true + // Force keyring to be disabled. Consuming keyringOnce first stops + // isKeyringAvailable from probing the real keyring and overwriting this. + cache.keyringOnce.Do(func() {}) cache.keyringDisabled = true // All operations should gracefully handle unavailable keyring diff --git a/internal/provider/gcsm/gcsm_test.go b/internal/provider/gcsm/gcsm_test.go index 62947e1..a591d9e 100644 --- a/internal/provider/gcsm/gcsm_test.go +++ b/internal/provider/gcsm/gcsm_test.go @@ -75,7 +75,9 @@ func TestParseConfig(t *testing.T) { "project_id": "", "secret_id": "my-secret", }, - wantErr: false, // parseConfig doesn't validate, Fetch does + wantProjectID: "", + wantSecretID: "my-secret", + wantErr: false, // parseConfig doesn't validate, Fetch does }, { name: "config with missing project_id field", diff --git a/internal/provider/vault/vault_test.go b/internal/provider/vault/vault_test.go index 23b5281..871ac12 100644 --- a/internal/provider/vault/vault_test.go +++ b/internal/provider/vault/vault_test.go @@ -30,7 +30,7 @@ func TestParseConfigWithAuthOptions(t *testing.T) { name: "config with explicit token auth", config: map[string]interface{}{ "path": "myapp/secret", - "auth": "token", + "auth": map[string]interface{}{"method": "token"}, }, wantAuth: "token", wantAuthMount: "", @@ -41,8 +41,7 @@ func TestParseConfigWithAuthOptions(t *testing.T) { name: "config with oidc auth", config: map[string]interface{}{ "path": "myapp/secret", - "auth": "oidc", - "role": "my-role", + "auth": map[string]interface{}{"method": "oidc", "role": "my-role"}, }, wantAuth: "oidc", wantAuthMount: "", @@ -52,10 +51,12 @@ func TestParseConfigWithAuthOptions(t *testing.T) { { name: "config with jwt auth and custom mount", config: map[string]interface{}{ - "path": "myapp/secret", - "auth": "jwt", - "authMount": "custom-jwt", - "role": "app-role", + "path": "myapp/secret", + "auth": map[string]interface{}{ + "method": "jwt", + "mount": "custom-jwt", + "role": "app-role", + }, }, wantAuth: "jwt", wantAuthMount: "custom-jwt", @@ -75,14 +76,15 @@ func TestParseConfigWithAuthOptions(t *testing.T) { return } - if cfg.Auth != tt.wantAuth { - t.Errorf("parseConfig() Auth = %v, want %v", cfg.Auth, tt.wantAuth) + method, mount, role, _ := authFields(cfg) + if method != tt.wantAuth { + t.Errorf("parseConfig() Auth.Method = %v, want %v", method, tt.wantAuth) } - if cfg.AuthMount != tt.wantAuthMount { - t.Errorf("parseConfig() AuthMount = %v, want %v", cfg.AuthMount, tt.wantAuthMount) + if mount != tt.wantAuthMount { + t.Errorf("parseConfig() Auth.Mount = %v, want %v", mount, tt.wantAuthMount) } - if cfg.Role != tt.wantRole { - t.Errorf("parseConfig() Role = %v, want %v", cfg.Role, tt.wantRole) + if role != tt.wantRole { + t.Errorf("parseConfig() Auth.Role = %v, want %v", role, tt.wantRole) } }) } @@ -91,8 +93,7 @@ func TestParseConfigWithAuthOptions(t *testing.T) { func TestParseConfigWithSSOTokens(t *testing.T) { config := map[string]interface{}{ "path": "myapp/secret", - "auth": "oidc", - "role": "my-role", + "auth": map[string]interface{}{"method": "oidc", "role": "my-role"}, "_sso_access_token": "test-access-token-123", "_sso_id_token": "test-id-token-456", } @@ -123,18 +124,17 @@ func TestVaultProvider_Fetch_OIDCAuthValidation(t *testing.T) { name: "oidc auth without role", config: map[string]interface{}{ "path": "myapp/secret", - "auth": "oidc", + "auth": map[string]interface{}{"method": "oidc"}, "_sso_access_token": "test-token", }, wantErr: true, - errMsg: "requires 'role' field", + errMsg: "requires 'auth.role' field", }, { name: "oidc auth without SSO token", config: map[string]interface{}{ "path": "myapp/secret", - "auth": "oidc", - "role": "my-role", + "auth": map[string]interface{}{"method": "oidc", "role": "my-role"}, }, wantErr: true, errMsg: "no SSO token available", @@ -143,17 +143,17 @@ func TestVaultProvider_Fetch_OIDCAuthValidation(t *testing.T) { name: "jwt auth without role", config: map[string]interface{}{ "path": "myapp/secret", - "auth": "jwt", + "auth": map[string]interface{}{"method": "jwt"}, "_sso_id_token": "test-token", }, wantErr: true, - errMsg: "requires 'role' field", + errMsg: "requires 'auth.role' field", }, { name: "unsupported auth method", config: map[string]interface{}{ "path": "myapp/secret", - "auth": "invalid-method", + "auth": map[string]interface{}{"method": "invalid-method"}, }, wantErr: true, errMsg: "unsupported auth method", @@ -286,8 +286,9 @@ func TestParseConfig(t *testing.T) { if cfg.Address != tt.wantAddress { t.Errorf("parseConfig() Address = %v, want %v", cfg.Address, tt.wantAddress) } - if cfg.Token != tt.wantToken { - t.Errorf("parseConfig() Token = %v, want %v", cfg.Token, tt.wantToken) + // A top-level `token:` is folded into Auth.Token for backward compatibility + if _, _, _, token := authFields(cfg); token != tt.wantToken { + t.Errorf("parseConfig() Auth.Token = %v, want %v", token, tt.wantToken) } if cfg.Mount != tt.wantMount { t.Errorf("parseConfig() Mount = %v, want %v", cfg.Mount, tt.wantMount) @@ -381,8 +382,8 @@ func TestVaultProvider_ConfigFields(t *testing.T) { if cfg.Address != "https://custom-vault.example.com:8200" { t.Errorf("Config.Address = %v, want %v", cfg.Address, "https://custom-vault.example.com:8200") } - if cfg.Token != "custom-token-123" { - t.Errorf("Config.Token = %v, want %v", cfg.Token, "custom-token-123") + if _, _, _, token := authFields(cfg); token != "custom-token-123" { + t.Errorf("Config.Auth.Token = %v, want %v", token, "custom-token-123") } if cfg.Mount != "custom-secret-engine" { t.Errorf("Config.Mount = %v, want %v", cfg.Mount, "custom-secret-engine") @@ -406,8 +407,8 @@ func TestVaultProvider_ConfigWithOptionalFields(t *testing.T) { if cfg.Address != "" { t.Errorf("Config.Address = %v, want empty string", cfg.Address) } - if cfg.Token != "" { - t.Errorf("Config.Token = %v, want empty string", cfg.Token) + if _, _, _, token := authFields(cfg); token != "" { + t.Errorf("Config.Auth.Token = %v, want empty string", token) } if cfg.Mount != "" { t.Errorf("Config.Mount = %v, want empty string", cfg.Mount) @@ -453,3 +454,12 @@ func containsSubstring(s, substr string) bool { return false } + +// authFields flattens the nested auth config so tests can assert on it without +// nil-checking VaultConfig.Auth at every call site. +func authFields(cfg *VaultConfig) (method, mount, role, token string) { + if cfg == nil || cfg.Auth == nil { + return "", "", "", "" + } + return cfg.Auth.Method, cfg.Auth.Mount, cfg.Auth.Role, cfg.Auth.Token +}