Introducing logger interface

[intel352]

Introducing support for logger interface. Preserved compatibility with existing behavior so that we still support zap logger while supporting users that don't use zap logger.

The integration test needs to be executed to ensure no regression added.
There's no existing unit tests (and I don't have the patience to add those throughout right now), so we don't have confidence for this change without integration test execution.

Fixes #48

[zeevmoney]

Branch is not rebased onto current master -- merging will revert features

Problem: This branch is forked from an old commit (a926395) and has not been rebased against current master. The diff between this branch and master shows 91 files changed with 2300 deletions, including:

• Removal of FactsSyncTimeoutPolicy type and GetFactsSyncTimeoutPolicy() method (added in PR #55)
• Removal of WaitForSyncOptions struct and the options parameter from WaitForSync() (added/fixed in PR #59)
• Removal of the else branch in WaitForSync that handles non-nil timeout values
• Deletion of .github/workflows/ci.yml (added in PR #60)
• Reversion of model changes from multiple merged PRs (#64, #65, #67, #69, #70)
• Removal of ListTenantUsers functionality (commit 9d29cff)

Merging this PR as-is will silently revert all of these features.

Suggestion: Rebase this branch onto current master and resolve conflicts before review can proceed. The logger interface changes themselves are relatively small, but the stale branch state makes this PR dangerous to merge.

[zeevmoney]

zapLoggerAdapter methods panic on odd number of params or non-string keys

The Debug, Info, Warn, and Error methods iterate over params by i += 2 and blindly cast params[i].(string). Two panic scenarios:

1. If a caller passes an odd number of variadic args (e.g., logger.Debug("msg", "key_without_value")), the loop will read params[i+1] out of bounds.
2. If a caller passes a non-string value in a key position (e.g., logger.Info("msg", 123, "value")), the params[i].(string) assertion panics.

Since the Logger interface accepts ...interface{}, callers have no compile-time protection against misuse.

Suggestion: Add bounds checking and a safe type assertion before accessing params. For example:

for i := 0; i < len(params)-1; i += 2 {
    key, ok := params[i].(string)
    if !ok {
        key = fmt.Sprintf("%v", params[i])
    }
    zFields = append(zFields, zap.Any(key, params[i+1]))
}
// Handle trailing odd param
if len(params)%2 != 0 {
    zFields = append(zFields, zap.Any("EXTRA_VALUE", params[len(params)-1]))
}

[zeevmoney]

Asymmetric Error signature breaks the key-value pair contract

The Error method has signature Error(msg string, err error, params ...interface{}) while all other methods have (msg string, params ...interface{}). This creates two problems:

1. The zapLoggerAdapter.Error implementation treats the second arg as a dedicated error, then iterates remaining params as key-value pairs. But Debug/Info/Warn treat ALL variadic args as key-value pairs. This asymmetry is a footgun -- callers must remember the different calling conventions.

2. There is no way to log an error at Debug/Info/Warn levels (e.g., for non-fatal warnings with an attached error). Callers would need to pass the error as a value in a key-value pair: logger.Warn("retrying", "error", err), which loses the zap zap.Error() special handling.

Suggestion: Consider a uniform signature where all levels use (msg string, params ...interface{}) and the adapter detects error values in params. Alternatively, adopt a more idiomatic Go approach using log/slog-style patterns: Error(msg string, args ...any) where errors are passed as "error", err key-value pairs, consistent across all levels.

[zeevmoney]

Structured logging context silently dropped

Problem: The original code logged structured fields:

r.logger.Debug("role created",
    zap.String("type", "role"),
    zap.String("key", role.GetKey()),
    zap.String("id", role.Id),
)

Now it is just r.logger.Debug("role created") with no context. This is a regression in observability -- operators lose the ability to identify which role was created in logs. The same issue applies to Users.Create, Tenants.Create, and ResourceInstances.Create.

Suggestion: Pass the context through the new interface:

r.logger.Debug("role created", "type", "role", "key", role.GetKey(), "id", role.Id)

The zapLoggerAdapter already supports key-value pairs in the variadic params.

[zeevmoney]

No builder method for custom log.Logger implementations

The builder only has WithLogger(*zap.Logger), which wraps the zap logger in the adapter. Users who want to supply their own log.Logger implementation (the primary goal of this PR) have no builder method to do so. They must bypass the builder entirely and use NewPermitConfigWithLogger() directly, losing access to all builder-pattern configuration (WithPdpUrl, WithDebug, WithTimeout, etc.).

Suggestion: Add a WithCustomLogger(logger log.Logger) method to the builder:

func (c *PermitConfig) WithCustomLogger(logger log.Logger) *PermitConfig {
    c.Logger = logger
    return c
}

[zeevmoney]

newLoggerFromZap does not guard against nil *zap.Logger

newLoggerFromZap(nil) returns a *zapLoggerAdapter{zapLogger: nil}, and any subsequent call to Debug/Info/Warn/Error will panic on z.zapLogger.Debug(...) because zapLogger is nil.

This is reachable in practice: NewPermitConfig(apiUrl, token, pdpUrl, debug, ctx, nil) passes nil through to newLoggerFromZap.

Suggestion: Either guard against nil in newLoggerFromZap by returning a no-op logger, or document and validate that the zap logger must not be nil. A nil guard is safer:

func newLoggerFromZap(zapLogger *zap.Logger) log.Logger {
    if zapLogger == nil {
        return nil // let Build() handle the default
    }
    return &zapLoggerAdapter{zapLogger: zapLogger}
}

[zeevmoney]

Exported key constants couple the log package to specific callers

Problem: UserKey, ResponseBodyKey, and CountKey are generic logging key constants exported from the log package. This package should define the logger interface only -- it should not be aware of domain-specific key names used by callers in the api or enforcement packages. As more callers add new keys, this package will accumulate unrelated constants.

Suggestion: Move these constants to the packages that use them (pkg/api, pkg/enforcement), or remove them entirely and use string literals at the call sites. The log package should remain a pure interface definition.

[zeevmoney]

No documentation on the key-value pair contract for the variadic params

Problem: The Logger interface methods accept params ...interface{} but there is no documentation specifying that params must be passed as alternating key-value pairs (key1, val1, key2, val2, ...). This contract is implicit and only enforced at runtime by the zapLoggerAdapter implementation (which panics on violations -- see separate comment).

Suggestion: Add a doc comment to the interface explaining the expected calling convention:

// Logger defines a structured logging interface.
// The params arguments are expected as alternating key-value pairs:
//   logger.Info("message", "key1", value1, "key2", value2)
// Keys must be strings. An odd number of params will result in
// the last value being logged with a synthetic key.
type Logger interface {

Alternatively, consider a strongly-typed approach similar to log/slog.Attr to prevent misuse at compile time.