diff --git a/cmd/main.go b/cmd/main.go index b8188608d..c4888ff90 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -21,17 +21,18 @@ import ( "crypto/tls" "flag" "fmt" + "log/slog" "os" "path/filepath" "time" - "sigs.k8s.io/controller-runtime/pkg/metrics/filters" + "github.com/spf13/pflag" intController "github.com/splunk/splunk-operator/internal/controller" "github.com/splunk/splunk-operator/internal/controller/debug" "github.com/splunk/splunk-operator/pkg/config" + "github.com/splunk/splunk-operator/pkg/logging" "github.com/splunk/splunk-operator/pkg/splunk/enterprise/validation" - "sigs.k8s.io/controller-runtime/pkg/certwatcher" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. @@ -45,14 +46,15 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" _ "k8s.io/client-go/plugin/pkg/client/auth" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/certwatcher" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/metrics/filters" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" enterpriseApiV3 "github.com/splunk/splunk-operator/api/v3" enterpriseApi "github.com/splunk/splunk-operator/api/v4" - "github.com/splunk/splunk-operator/internal/controller" //+kubebuilder:scaffold:imports //extapi "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -76,8 +78,11 @@ func main() { var enableLeaderElection bool var probeAddr string var pprofActive bool - var logEncoder string - var logLevel int + + // Structured logging flags + var logLevel string + var logFormat string + var logAddSource bool var leaseDuration time.Duration var renewDeadline time.Duration @@ -86,27 +91,49 @@ func main() { var tlsOpts []func(*tls.Config) - // TLS certificate configuration for metrics var metricsCertPath, metricsCertName, metricsCertKey string - flag.StringVar(&logEncoder, "log-encoder", "json", "log encoding ('json' or 'console')") - flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") - flag.BoolVar(&enableLeaderElection, "leader-elect", false, + pflag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") + pflag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") - flag.BoolVar(&pprofActive, "pprof", true, "Enable pprof endpoint") - flag.IntVar(&logLevel, "log-level", int(zapcore.InfoLevel), "set log level") - flag.IntVar(&leaseDurationSecond, "lease-duration", leaseDurationSecond, "manager lease duration in seconds") - flag.IntVar(&renewDeadlineSecond, "renew-duration", renewDeadlineSecond, "manager renew duration in seconds") - flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metrics endpoint binds to. "+ + pflag.BoolVar(&pprofActive, "pprof", true, "Enable pprof endpoint") + + // Structured logging flags (can also be set via LOG_LEVEL, LOG_FORMAT, LOG_ADD_SOURCE env vars) + pflag.StringVar(&logLevel, "log-level", "", "log level: debug, info, warn, error (overrides LOG_LEVEL env var)") + pflag.StringVar(&logFormat, "log-format", "", "log output format: json, text (overrides LOG_FORMAT env var)") + pflag.BoolVar(&logAddSource, "log-add-source", false, "add source file:line to log output (overrides LOG_ADD_SOURCE env var)") + pflag.IntVar(&leaseDurationSecond, "lease-duration", leaseDurationSecond, "manager lease duration in seconds") + pflag.IntVar(&renewDeadlineSecond, "renew-duration", renewDeadlineSecond, "manager renew duration in seconds") + pflag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metrics endpoint binds to. "+ "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") - flag.BoolVar(&secureMetrics, "metrics-secure", false, + pflag.BoolVar(&secureMetrics, "metrics-secure", false, "If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.") // TLS certificate flags for metrics server - flag.StringVar(&metricsCertPath, "metrics-cert-path", "", "The directory that contains the metrics server certificate.") - flag.StringVar(&metricsCertName, "metrics-cert-name", "tls.crt", "The name of the metrics server certificate file.") - flag.StringVar(&metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.") + pflag.StringVar(&metricsCertPath, "metrics-cert-path", "", "The directory that contains the metrics server certificate.") + pflag.StringVar(&metricsCertName, "metrics-cert-name", "tls.crt", "The name of the metrics server certificate file.") + pflag.StringVar(&metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.") + + config.DefaultMutableFeatureGate.AddFlag(pflag.CommandLine) + + opts := zap.Options{ + Development: true, + TimeEncoder: zapcore.RFC3339NanoTimeEncoder, + } + opts.BindFlags(flag.CommandLine) + pflag.CommandLine.AddGoFlagSet(flag.CommandLine) + pflag.Parse() + + ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + + if allGates := config.DefaultMutableFeatureGate.GetAll(); len(allGates) > 0 { + effectiveStates := make(map[string]bool, len(allGates)) + for gate := range allGates { + effectiveStates[string(gate)] = config.DefaultMutableFeatureGate.Enabled(gate) + } + setupLog.Info("Feature gates initialized", "gates", effectiveStates) + } // Metrics endpoint is enabled in 'config/default/kustomization.yaml'. The Metrics options configure the server. // More info: @@ -147,15 +174,20 @@ func main() { renewDeadline = time.Duration(renewDeadlineSecond) * time.Second } - opts := zap.Options{ - Development: true, - TimeEncoder: zapcore.RFC3339NanoTimeEncoder, + // Initialize structured logging infrastructure + // Flags take precedence over environment variables + var addSourcePtr *bool + if logAddSource { + addSourcePtr = &logAddSource } - opts.BindFlags(flag.CommandLine) - flag.Parse() + logCfg := logging.LoadConfigWithFlags(logLevel, logFormat, addSourcePtr) + _ = logging.SetupLogger(logCfg) - // Logging setup - ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + // Log startup information using slog + slog.Info("splunk Operator starting", + slog.String("log_level", logging.LevelToString(logCfg.Level)), + slog.String("log_format", logCfg.Format), + slog.Bool("log_add_source", logCfg.AddSource)) // Configure metrics certificate watcher if metrics certs are provided var metricsCertWatcher *certwatcher.CertWatcher @@ -264,7 +296,7 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "Standalone") os.Exit(1) } - if err := (&controller.IngestorClusterReconciler{ + if err := (&intController.IngestorClusterReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), Recorder: mgr.GetEventRecorderFor("ingestorcluster-controller"), @@ -280,10 +312,11 @@ func main() { os.Exit(1) } - // Setup centralized validation webhook server (opt-in via ENABLE_VALIDATION_WEBHOOK env var, defaults to false) - enableWebhooks := os.Getenv("ENABLE_VALIDATION_WEBHOOK") - if enableWebhooks == "true" { - // Parse optional timeout configurations from environment + if _, ok := os.LookupEnv("ENABLE_VALIDATION_WEBHOOK"); ok { + setupLog.Info("DEPRECATED: ENABLE_VALIDATION_WEBHOOK env var is deprecated and will be removed in a future release; use --feature-gates=ValidationWebhook=true instead") + } + + if config.DefaultMutableFeatureGate.Enabled(config.ValidationWebhook) { readTimeout := 10 * time.Second if val := os.Getenv("WEBHOOK_READ_TIMEOUT"); val != "" { if d, err := time.ParseDuration(val); err == nil { @@ -306,16 +339,15 @@ func main() { Client: mgr.GetClient(), }) - // Add webhook server as a runnable to the manager if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error { return webhookServer.Start(ctx) })); err != nil { setupLog.Error(err, "unable to add webhook server to manager") os.Exit(1) } - setupLog.Info("Validation webhook enabled via ENABLE_VALIDATION_WEBHOOK=true") + setupLog.Info("Validation webhook enabled") } else { - setupLog.Info("Validation webhook disabled (set ENABLE_VALIDATION_WEBHOOK=true to enable)") + setupLog.Info("Validation webhook disabled (set --feature-gates=ValidationWebhook=true to enable)") } //+kubebuilder:scaffold:builder diff --git a/config/default-with-webhook/kustomization-cluster.yaml b/config/default-with-webhook/kustomization-cluster.yaml index c596f0c68..f4ddabdb2 100644 --- a/config/default-with-webhook/kustomization-cluster.yaml +++ b/config/default-with-webhook/kustomization-cluster.yaml @@ -1,7 +1,7 @@ # Adds namespace to all resources. # Cluster-scoped deployment WITH webhook enabled (opt-in) # Requires cert-manager to be installed in the cluster -namespace: splunk-operator +namespace: splunk-operator # Value of this field is prepended to the # names of all resources, e.g. a deployment named @@ -115,7 +115,7 @@ patches: patch: |- - op: add path: /spec/template/spec/containers/0/env - value: + value: - name: WATCH_NAMESPACE value: WATCH_NAMESPACE_VALUE - name: RELATED_IMAGE_SPLUNK_ENTERPRISE @@ -124,12 +124,13 @@ patches: value: splunk-operator - name: SPLUNK_GENERAL_TERMS value: SPLUNK_GENERAL_TERMS_VALUE - - name: ENABLE_VALIDATION_WEBHOOK - value: "true" - name: POD_NAME valueFrom: fieldRef: fieldPath: metadata.name + - op: add + path: /spec/template/spec/containers/0/args/- + value: --feature-gates=ValidationWebhook=true # [METRICS] The following patch will enable the metrics endpoint using HTTPS and the port :8443. # More info: https://book.kubebuilder.io/reference/metrics - path: manager_metrics_patch.yaml diff --git a/config/default-with-webhook/kustomization-namespace.yaml b/config/default-with-webhook/kustomization-namespace.yaml index 193791601..2bc1845c6 100644 --- a/config/default-with-webhook/kustomization-namespace.yaml +++ b/config/default-with-webhook/kustomization-namespace.yaml @@ -1,7 +1,7 @@ # Adds namespace to all resources. # Namespace-scoped deployment WITH webhook enabled (opt-in) # Requires cert-manager to be installed in the cluster -namespace: splunk-operator +namespace: splunk-operator # Value of this field is prepended to the # names of all resources, e.g. a deployment named @@ -115,7 +115,7 @@ patches: patch: |- - op: add path: /spec/template/spec/containers/0/env - value: + value: - name: WATCH_NAMESPACE valueFrom: fieldRef: @@ -126,12 +126,13 @@ patches: value: splunk-operator - name: SPLUNK_GENERAL_TERMS value: SPLUNK_GENERAL_TERMS_VALUE - - name: ENABLE_VALIDATION_WEBHOOK - value: "true" - name: POD_NAME valueFrom: fieldRef: fieldPath: metadata.name + - op: add + path: /spec/template/spec/containers/0/args/- + value: --feature-gates=ValidationWebhook=true # [METRICS] The following patch will enable the metrics endpoint using HTTPS and the port :8443. # More info: https://book.kubebuilder.io/reference/metrics - path: manager_metrics_patch.yaml diff --git a/config/default-with-webhook/kustomization.yaml b/config/default-with-webhook/kustomization.yaml index 5ba87fec1..f4ddabdb2 100644 --- a/config/default-with-webhook/kustomization.yaml +++ b/config/default-with-webhook/kustomization.yaml @@ -1,7 +1,7 @@ # Adds namespace to all resources. # Cluster-scoped deployment WITH webhook enabled (opt-in) # Requires cert-manager to be installed in the cluster -namespace: splunk-operator +namespace: splunk-operator # Value of this field is prepended to the # names of all resources, e.g. a deployment named @@ -115,7 +115,7 @@ patches: patch: |- - op: add path: /spec/template/spec/containers/0/env - value: + value: - name: WATCH_NAMESPACE value: WATCH_NAMESPACE_VALUE - name: RELATED_IMAGE_SPLUNK_ENTERPRISE @@ -123,13 +123,14 @@ patches: - name: OPERATOR_NAME value: splunk-operator - name: SPLUNK_GENERAL_TERMS - value: WATCH_NAMESPACE_VALUE - - name: ENABLE_VALIDATION_WEBHOOK - value: "true" + value: SPLUNK_GENERAL_TERMS_VALUE - name: POD_NAME valueFrom: fieldRef: fieldPath: metadata.name + - op: add + path: /spec/template/spec/containers/0/args/- + value: --feature-gates=ValidationWebhook=true # [METRICS] The following patch will enable the metrics endpoint using HTTPS and the port :8443. # More info: https://book.kubebuilder.io/reference/metrics - path: manager_metrics_patch.yaml diff --git a/docs/FeatureGates.md b/docs/FeatureGates.md new file mode 100644 index 000000000..6661ab055 --- /dev/null +++ b/docs/FeatureGates.md @@ -0,0 +1,110 @@ +# Feature Gates + +The Splunk Operator uses the Kubernetes [FeatureGate](https://pkg.go.dev/k8s.io/component-base/featuregate) pattern to control rollout of new functionality. Feature gates allow new code to be merged to the main branch without activating in production, giving teams a safe, per-environment opt-in mechanism. + +## Usage + +Enable or disable feature gates at operator startup: + +```bash +/manager --feature-gates=ValidationWebhook=true +``` + +## Maturity Lifecycle + +| Stage | Default | Can Override | Next Step | +|-----------|---------|-------------|-----------------------------------------| +| **Alpha** | off | Yes | Promote to Beta after validation | +| **Beta** | on | Yes | Promote to GA after sustained stability | +| **GA** | on | No | Remove gate in a future release | + +## Current Feature Gates + +| Gate | Default | Stage | Since | Description | +|-----------------------|---------|-------|---------|----------------------------------------------------------| +| `ValidationWebhook` | `false` | Alpha | v3.2.0 | Centralized validation webhook server for CR admission | + +## Adding a New Feature Gate + +Follow these steps: + +### 1. Register the gate in `pkg/config/featuregates.go` + +Add a constant and an entry in `defaultFeatureGates`: + +```go +const ( + MyNewFeature featuregate.Feature = "MyNewFeature" +) + +var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ + // existing gates … + MyNewFeature: {Default: false, PreRelease: featuregate.Alpha}, +} +``` + +### 2. Guard the code path + +Check the gate wherever the feature-specific logic runs: + +```go +if config.DefaultMutableFeatureGate.Enabled(config.MyNewFeature) { + // feature-specific logic +} +``` + +This can guard anything — a reconciler code path, a helper function, a webhook handler, an HTTP endpoint, etc. + +### Example: Gating a New Controller (CRD) + +When the feature gate introduces an entirely new CRD and controller, there are additional steps beyond the basic gate check. All three steps below are **mandatory** for any new CRD behind a feature gate. + +#### a. Gate controller registration in `cmd/main.go` + +Wrap the `SetupWithManager` call so the controller only starts when the gate is on: + +```go +if config.DefaultMutableFeatureGate.Enabled(config.MyNewFeature) { + if err = (&controller.MyNewReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "MyNew") + os.Exit(1) + } +} +``` + +#### b. Add a validating webhook for the gated CRD group + +A validating webhook **must** reject CR creation when the gate is off. Without this, users can create resources that no controller will reconcile, leading to silent failures: + +```go +func (v *MyNewValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + if !config.DefaultMutableFeatureGate.Enabled(config.MyNewFeature) { + return nil, fmt.Errorf( + "the MyNewFeature feature is not enabled; "+ + "set --feature-gates=MyNewFeature=true to activate") + } + return nil, nil +} +``` + +#### c. Label the CRD manifests + +Every gated CRD **must** carry maturity annotations and labels in `config/crd/bases/`. These signal to operators and tooling which gate controls the CRD and its current stability level: + +```yaml +metadata: + annotations: + splunk.com/feature-gate: MyNewFeature + splunk.com/feature-stage: Alpha + labels: + splunk.com/feature-stage: alpha +``` + +## Promoting a Gate + +- **Alpha → Beta**: Change `Default: false` to `Default: true` in `featuregates.go`; update the CRD label to `beta` +- **Beta → GA**: Set `LockToDefault: true` in the `FeatureSpec`; update the CRD label to `ga` +- **GA → Removed**: Delete the constant and `FeatureSpec` entry; remove the `if` guard in `cmd/main.go`; remove the CRD annotations/labels and the validating webhook diff --git a/docs/LoggingAndEvents.md b/docs/LoggingAndEvents.md new file mode 100644 index 000000000..3926fcd55 --- /dev/null +++ b/docs/LoggingAndEvents.md @@ -0,0 +1,277 @@ +--- +title: Logging and Events +parent: Reference +nav_order: 6 +--- + +# Logging & Events + +## Logging + +The operator uses Go's `log/slog` package for structured logging. The global logger is configured once at startup in `cmd/main.go` via `logging.SetupLogger()` and set as the default with `slog.SetDefault()`. + +### Getting a Logger + +| Where | How | +|---|---| +| **Controller `Reconcile`** | `logger := slog.Default().With("controller", "Name", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx))` | +| **Business logic (pkg)** | `logger := logging.FromContext(ctx).With("func", "FunctionName")` | + +Controllers inject the logger into context so downstream code can retrieve it: + +```go +logger := slog.Default().With("controller", "Standalone", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) +ctx = logging.WithLogger(ctx, logger) +``` + +The `reconcileID` is a unique identifier assigned by controller-runtime to each reconcile invocation. It is automatically propagated to all downstream log calls via context, making it possible to correlate every log line produced during a single reconcile pass — even across deeply nested function calls. + +### Log Levels + +| Level | Use for | +|---|---| +| `Debug` | Verbose diagnostics (state dumps, intermediate values) | +| `Info` | Normal operations (reconcile start, requeue, status changes) | +| `Warn` | Recoverable issues (deprecated config, fallback behavior) | +| `Error` | Failures that need attention (API errors, broken invariants) | + +Configure at runtime via `LOG_LEVEL` env var or `--log-level` flag. Values: `debug`, `info`, `warn`, `error`. + +### Writing Log Messages + +**Message format:** short, lowercase, past tense or present participle. Describe *what happened*, not the function name. Use **PascalCase** for CRD type names in messages — `ClusterManager`, `IndexerCluster`, `SearchHeadCluster`, `LicenseManager`, `LicenseMaster`, `ClusterMaster`, `MonitoringConsole`, `Standalone`. + +```go +// Good +logger.InfoContext(ctx, "ClusterManager types not found in namespace", "error", err) +logger.InfoContext(ctx, "scaling up IndexerCluster", "replicas", replicas) + +// Bad - inconsistent casing +logger.InfoContext(ctx, "clusterManager types not found") +logger.InfoContext(ctx, "scaling up indexer cluster") +``` + +```go +// Good +logger.InfoContext(ctx, "statefulset updated", "replicas", replicas) +logger.ErrorContext(ctx, "failed to fetch secret", "error", err, "secret", name) + +// Bad - don't restate the function name or use generic messages +logger.InfoContext(ctx, "ApplyStandalone") +logger.InfoContext(ctx, "error occurred") +``` + +**Rules:** + +1. Always use `*Context(ctx, ...)` variants (`InfoContext`, `ErrorContext`, etc.). +2. Always pass `"error", err` as a key-value pair — never as the message string. +3. Use consistent key names across the codebase. Key names **must** be `camelCase` — no spaces, no Title Case, no special characters like parentheses. + +```go +// Good +logger.InfoContext(ctx, "start", "crVersion", instance.GetResourceVersion()) +logger.InfoContext(ctx, "Requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) +logger.InfoContext(ctx, "Getting Apps list", "bucket", client.BucketName) + +// Bad - spaces and inconsistent casing in keys +logger.InfoContext(ctx, "start", "CR version", instance.GetResourceVersion()) +logger.InfoContext(ctx, "Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) +logger.InfoContext(ctx, "Getting Apps list", "AWS S3 Bucket", client.BucketName) +``` + +Common key names: + +| Key | Meaning | +|---|---| +| `"error"` | The `error` value | +| `"name"` | CR or resource name | +| `"namespace"` | Kubernetes namespace | +| `"controller"` | Controller name | +| `"func"` | Function name (in pkg layer) | +| `"reconcileID"` | Unique ID per reconcile pass (set by controller) | +| `"replicas"` | Replica count | +| `"phase"` | CR phase | +| `"podName"` | Pod name | +| `"appName"` | App name | +| `"bucket"` | Storage bucket name (S3/GCS/Azure) | +| `"crVersion"` | CR resource version | +| `"periodSeconds"` | Requeue delay in seconds | + +4. **Don't log and return the same error.** controller-runtime automatically logs every non-nil error returned from `Reconcile()` via `log.Error(err, "Reconciler error")`. If you also log it in the business logic, the same error appears twice. Instead, wrap the error with context using `fmt.Errorf("description: %w", err)` so the single controller-runtime log line is descriptive. + +### Error Wrapping + +The `Apply*` functions in `pkg/splunk/enterprise/` wrap validation errors before returning them. This adds context about which resource type failed, producing a clear chain when controller-runtime logs the final error: + +``` +validate clustermanager spec: license not accepted, please adjust SPLUNK_GENERAL_TERMS ... +``` + +**Pattern:** + +```go +err = validateClusterManagerSpec(ctx, client, cr) +if err != nil { + eventPublisher.Warning(ctx, EventReasonValidateSpecFailed, + fmt.Sprintf("Spec validation failed for %s — check operator logs", cr.GetName())) + return result, fmt.Errorf("validate clustermanager spec: %w", err) +} +``` + +**Rules:** + +- Use `%w` (not `%v`) so callers can inspect the original error with `errors.Is` / `errors.Unwrap`. +- The prefix should be lowercase and describe the operation that failed, e.g. `"validate standalone spec"`, `"apply splunk config"`. +- Don't wrap and log the same error — pick one. Wrapping is preferred because it keeps context in the error chain and avoids double-logging. + +```go +// Good — wrap and return, no explicit log needed +return result, fmt.Errorf("validate standalone spec: %w", err) + +// Bad — double-logged: once here, once by controller-runtime +scopedLog.Error(err, "Failed to validate standalone spec") +return result, err +``` + +**When writing tests** that assert on wrapped errors, use `strings.Contains` rather than an exact match so the test doesn't break if the wrapping prefix changes: + +```go +// Good — resilient to wrapping changes +if !strings.Contains(err.Error(), "license not accepted") { + t.Errorf("Unexpected error: %v", err) +} + +// Bad — brittle, breaks if wrapping prefix is renamed +if err.Error() != "validate standalone spec: license not accepted, ..." { + t.Errorf("Unexpected error: %v", err) +} +``` +5. Sensitive data (passwords, tokens, secrets) is automatically redacted by the handler. + +### Configuration + +| Env Var | Flag | Default | Description | +|---|---|---|---| +| `LOG_LEVEL` | `--log-level` | `info` | Minimum log level | +| `LOG_FORMAT` | `--log-format` | `json` | `json` or `text` | +| `LOG_ADD_SOURCE` | `--log-add-source` | `false` | Include source file/line (auto-enabled at debug) | + +Flags take precedence over env vars. + +--- + +## Kubernetes Events + +Events are user-facing signals visible via `kubectl describe`. Use them for significant state changes that an operator user should see — not for internal debugging. + +### How It Works + +1. Controllers pass the event recorder into context: + ```go + ctx = context.WithValue(ctx, splcommon.EventRecorderKey, r.Recorder) + ``` +2. Business logic retrieves a publisher: + ```go + eventPublisher := GetEventPublisher(ctx, cr) + ``` +3. Emit events using constants from `pkg/splunk/enterprise/event_reasons.go`: + ```go + eventPublisher.Normal(ctx, EventReasonScaledUp, + fmt.Sprintf("Successfully scaled %s from %d to %d replicas", cr.GetName(), old, new)) + eventPublisher.Warning(ctx, EventReasonValidateSpecFailed, + fmt.Sprintf("Spec validation failed for %s — check operator logs", cr.GetName())) + ``` + +### Event Reason Constants + +All event reasons are defined as constants in `pkg/splunk/enterprise/event_reasons.go`. **Never use string literals for event reasons** — always use the `EventReason*` constants. This ensures consistency across controllers and makes reasons searchable. + +**Normal reasons:** + +| Constant | Value | Use for | +|---|---|---| +| `EventReasonScaledUp` | `ScaledUp` | Successful scale up | +| `EventReasonScaledDown` | `ScaledDown` | Successful scale down | +| `EventReasonClusterInitialized` | `ClusterInitialized` | Cluster first becomes ready | +| `EventReasonClusterQuorumRestored` | `ClusterQuorumRestored` | Quorum recovered | +| `EventReasonPasswordSyncCompleted` | `PasswordSyncCompleted` | Secret sync finished | + +**Warning reasons (common):** + +| Constant | Value | Use for | +|---|---|---| +| `EventReasonValidateSpecFailed` | `ValidateSpecFailed` | CR spec validation errors | +| `EventReasonApplySplunkConfigFailed` | `ApplySplunkConfigFailed` | General config apply failures | +| `EventReasonAppFrameworkInitFailed` | `AppFrameworkInitFailed` | App framework init errors | +| `EventReasonApplyServiceFailed` | `ApplyServiceFailed` | Service create/update failures | +| `EventReasonStatefulSetFailed` | `StatefulSetFailed` | StatefulSet get failures | +| `EventReasonStatefulSetUpdateFailed` | `StatefulSetUpdateFailed` | StatefulSet update failures | +| `EventReasonDeleteFailed` | `DeleteFailed` | CR deletion failures | +| `EventReasonSecretMissing` | `SecretMissing` | Required secret not found | +| `EventReasonUpgradeCheckFailed` | `UpgradeCheckFailed` | Upgrade path validation errors | + +See [`event_reasons.go`](https://github.com/splunk/splunk-operator/blob/main/pkg/splunk/enterprise/event_reasons.go) for the full list. + +To add a new event reason: add a constant to `event_reasons.go`, then use it in the code. + +### When to Use Events vs Logs + +| Scenario | Log | Event | +|---|---|---| +| Reconcile start/requeue | Yes | No | +| API call failed (transient) | Yes | No | +| Spec validation failed | Yes | Yes (Warning) | +| Successful scale up/down | Yes | Yes (Normal) | +| Phase transition | Yes | Yes (Normal) | +| Security-related failure | Yes | Yes (Warning) | + +### Writing Event Messages + +**Reason:** always use an `EventReason*` constant — e.g. `EventReasonScaledUp`, `EventReasonValidateSpecFailed`. + +**Message:** sentence case, user-friendly, include the resource name. **Never include raw `err.Error()` in events** — error details belong in logs only. Events are visible to any user with `kubectl describe` access and may leak internal paths, secret names, or stack traces. Instead, write a user-actionable summary and point to logs for details. + +```go +// Good — uses constant, event gives a user-actionable summary, log has full error +logger.ErrorContext(ctx, "smartstore volume key validation failed", "error", err) +eventPublisher.Warning(ctx, EventReasonRemoteVolumeKeyCheckFailed, + fmt.Sprintf("Remote volume key change check failed for %s — check operator logs", cr.GetName())) + +eventPublisher.Normal(ctx, EventReasonScaledUp, + fmt.Sprintf("Successfully scaled %s from %d to %d replicas", cr.GetName(), oldCount, newCount)) + +// Bad — string literal instead of constant +eventPublisher.Warning(ctx, "ValidationFailed", "...") + +// Bad — leaking err.Error() into events +eventPublisher.Warning(ctx, EventReasonValidateSpecFailed, + fmt.Sprintf("Invalid smartstore config: %s", err.Error())) + +// Bad — too vague, no context +eventPublisher.Warning(ctx, "Error", "something went wrong") +``` + +**Log + Event combo for errors:** + +```go +err = validateStandaloneSpec(ctx, client, cr) +if err != nil { + // Log: full error for operator developers / debugging + logger.ErrorContext(ctx, "spec validation failed", "error", err) + + // Event: user-actionable summary visible in kubectl describe + eventPublisher.Warning(ctx, EventReasonValidateSpecFailed, + fmt.Sprintf("Spec validation failed for %s — check operator logs", cr.GetName())) + + return result, err +} +``` + +**Rules:** + +1. Always use `EventReason*` constants — never string literals for event reasons. +2. Use `Normal` for successful operations the user should know about. +3. Use `Warning` for failures or degraded states that may need user intervention. +4. Always include the resource name in the message. +5. Never pass `err.Error()` into event messages — log the full error, keep events clean. +6. Don't emit events for routine internal operations — keep the event stream actionable. diff --git a/docs/ValidationWebhook.md b/docs/ValidationWebhook.md index a3ae4c2de..f17ad7535 100644 --- a/docs/ValidationWebhook.md +++ b/docs/ValidationWebhook.md @@ -85,23 +85,25 @@ If you prefer not to use cert-manager, you can provide your own TLS certificates #### Option 1: Use the Webhook-Enabled Kustomize Overlay -Deploy using the `config/default-with-webhook` overlay which includes all necessary webhook components: +Deploy using the `config/default-with-webhook` overlay which includes all necessary webhook components and enables the `ValidationWebhook` feature gate automatically: ```bash -# Build and apply the webhook-enabled configuration -kustomize build config/default-with-webhook | kubectl apply -f - +make deploy IMG= ENVIRONMENT=default-with-webhook \ + SPLUNK_GENERAL_TERMS="--accept-sgt-current-at-splunk-com" ``` -#### Option 2: Enable Webhook on Existing Deployment +This uses the same `make deploy` target as the standard deployment, which substitutes the `WATCH_NAMESPACE`, `SPLUNK_ENTERPRISE_IMAGE`, and `SPLUNK_GENERAL_TERMS` placeholder values before running `kustomize build`. -If you already have the operator deployed, you can enable the webhook by setting the `ENABLE_VALIDATION_WEBHOOK` environment variable: +#### Option 2: Enable via Feature Gate on Existing Deployment + +If you already have the operator deployed with the webhook Kubernetes resources (Service, ValidatingWebhookConfiguration, TLS certificates), enable the feature gate by patching the container args: ```bash -kubectl set env deployment/splunk-operator-controller-manager \ - ENABLE_VALIDATION_WEBHOOK=true -n splunk-operator +kubectl patch deployment splunk-operator-controller-manager -n splunk-operator \ + --type='json' -p='[{"op": "add", "path": "/spec/template/spec/containers/0/args/-", "value": "--feature-gates=ValidationWebhook=true"}]' ``` -**Note:** This option also requires the webhook service, ValidatingWebhookConfiguration, and TLS certificates to be deployed. Use Option 1 for a complete deployment. +**Note:** This requires the webhook service, ValidatingWebhookConfiguration, and TLS certificates to already be deployed. Use Option 1 for a complete deployment. #### Option 3: Modify Default Kustomization @@ -119,6 +121,14 @@ Then deploy: make deploy IMG= SPLUNK_GENERAL_TERMS="--accept-sgt-current-at-splunk-com" ``` +### Legacy: ENABLE_VALIDATION_WEBHOOK Environment Variable + +> **Deprecated:** The `ENABLE_VALIDATION_WEBHOOK` environment variable is deprecated and will be removed in a future release. Use the `--feature-gates=ValidationWebhook=true` flag instead. + +For backwards compatibility, setting `ENABLE_VALIDATION_WEBHOOK=true` as an environment variable on the operator container will still enable the validation webhook. The operator logs a deprecation warning when this method is used. + +When both the `--feature-gates=ValidationWebhook=...` CLI flag and the `ENABLE_VALIDATION_WEBHOOK` env var are set, the **CLI flag takes precedence**. The env var is applied at startup before flag parsing, so the CLI value overwrites it. + ## Validated Fields The webhook validates the following spec fields: @@ -290,7 +300,7 @@ kubectl get validatingwebhookconfiguration splunk-operator-validating-webhook-co ```bash kubectl logs -n splunk-operator deployment/splunk-operator-controller-manager | grep -i webhook -# Look for: "Validation webhook enabled via ENABLE_VALIDATION_WEBHOOK=true" +# Look for: "Validation webhook enabled" # Look for: "Starting webhook server" {"port": 9443} ``` @@ -362,7 +372,7 @@ kubectl logs -n splunk-operator deployment/splunk-operator-controller-manager | If you see "Validation webhook disabled" in the logs, ensure: -1. The `ENABLE_VALIDATION_WEBHOOK` environment variable is set to `true` +1. The `--feature-gates=ValidationWebhook=true` flag is set on the operator container args (or the legacy `ENABLE_VALIDATION_WEBHOOK=true` env var is set) 2. You're using the correct kustomize overlay (`config/default-with-webhook`) ## Architecture @@ -457,7 +467,7 @@ var GVR = schema.GroupVersionResource{ // Add to DefaultValidators map var DefaultValidators = map[schema.GroupVersionResource]Validator{ // ... existing validators ... - + GVR: &GenericValidator[*enterpriseApi.]{ ValidateCreateFunc: ValidateCreate, ValidateUpdateFunc: ValidateUpdate, @@ -503,12 +513,7 @@ If your CRD doesn't need context-aware validation, you can omit `ValidateCreateW ## Disabling the Webhook -To disable the webhook after it has been enabled: - -```bash -kubectl set env deployment/splunk-operator-controller-manager \ - ENABLE_VALIDATION_WEBHOOK=false -n splunk-operator -``` +To disable the webhook after it has been enabled, remove the `--feature-gates=ValidationWebhook=true` flag from the container args (or remove the `ENABLE_VALIDATION_WEBHOOK` env var if using the legacy method). Or redeploy using the default kustomization (without webhook): diff --git a/go.mod b/go.mod index 8b5adb30c..0ff959e8b 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( github.com/onsi/gomega v1.39.1 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.22.0 + github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.11.1 github.com/wk8/go-ordered-map/v2 v2.1.7 go.uber.org/zap v1.27.0 @@ -31,6 +32,7 @@ require ( k8s.io/apiextensions-apiserver v0.33.0 k8s.io/apimachinery v0.33.0 k8s.io/client-go v0.33.0 + k8s.io/component-base v0.33.0 k8s.io/kubectl v0.26.2 sigs.k8s.io/controller-runtime v0.21.0 ) @@ -113,7 +115,6 @@ require ( github.com/rs/xid v1.2.1 // indirect github.com/sirupsen/logrus v1.9.3 // indirect github.com/spf13/cobra v1.8.1 // indirect - github.com/spf13/pflag v1.0.5 // indirect github.com/stoewer/go-strcase v1.3.0 // indirect github.com/stretchr/objx v0.5.2 // indirect github.com/x448/float16 v0.8.4 // indirect @@ -152,7 +153,6 @@ require ( gopkg.in/ini.v1 v1.66.4 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apiserver v0.33.0 // indirect - k8s.io/component-base v0.33.0 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20250318190949-c8a335a9a2ff // indirect k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 // indirect diff --git a/internal/controller/clustermanager_controller.go b/internal/controller/clustermanager_controller.go index 2a844bd0a..70396c8ba 100644 --- a/internal/controller/clustermanager_controller.go +++ b/internal/controller/clustermanager_controller.go @@ -18,10 +18,12 @@ package controller import ( "context" + "log/slog" "time" enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/splunk/splunk-operator/internal/controller/common" + "github.com/splunk/splunk-operator/pkg/logging" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" "github.com/pkg/errors" @@ -37,7 +39,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -79,8 +80,8 @@ func (r *ClusterManagerReconciler) Reconcile(ctx context.Context, req ctrl.Reque metrics.ReconcileCounters.With(metrics.GetPrometheusLabels(req, "ClusterManager")).Inc() defer recordInstrumentionData(time.Now(), req, "controller", "ClusterManager") - reqLogger := log.FromContext(ctx) - reqLogger = reqLogger.WithValues("clustermanager", req.NamespacedName) + logger := slog.Default().With("controller", "ClusterManager", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) + ctx = logging.WithLogger(ctx, logger) // Fetch the ClusterManager instance := &enterpriseApi.ClusterManager{} @@ -105,14 +106,14 @@ func (r *ClusterManagerReconciler) Reconcile(ctx context.Context, req ctrl.Reque } } - reqLogger.Info("start", "CR version", instance.GetResourceVersion()) + logger.InfoContext(ctx, "start", "crVersion", instance.GetResourceVersion()) // Pass event recorder through context ctx = context.WithValue(ctx, splcommon.EventRecorderKey, r.Recorder) result, err := ApplyClusterManager(ctx, r.Client, instance, nil) if result.Requeue && result.RequeueAfter != 0 { - reqLogger.Info("Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) + logger.InfoContext(ctx, "requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) } return result, err diff --git a/internal/controller/clustermaster_controller.go b/internal/controller/clustermaster_controller.go index 4e2b5b94a..5580edda2 100644 --- a/internal/controller/clustermaster_controller.go +++ b/internal/controller/clustermaster_controller.go @@ -18,10 +18,12 @@ package controller import ( "context" + "log/slog" "time" enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/splunk/splunk-operator/internal/controller/common" + "github.com/splunk/splunk-operator/pkg/logging" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" "github.com/pkg/errors" @@ -37,7 +39,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -79,8 +80,8 @@ func (r *ClusterMasterReconciler) Reconcile(ctx context.Context, req ctrl.Reques metrics.ReconcileCounters.With(metrics.GetPrometheusLabels(req, "ClusterMaster")).Inc() defer recordInstrumentionData(time.Now(), req, "controller", "ClusterMaster") - reqLogger := log.FromContext(ctx) - reqLogger = reqLogger.WithValues("clustermaster", req.NamespacedName) + logger := slog.Default().With("controller", "ClusterMaster", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) + ctx = logging.WithLogger(ctx, logger) // Fetch the ClusterMaster instance := &enterpriseApiV3.ClusterMaster{} @@ -105,14 +106,14 @@ func (r *ClusterMasterReconciler) Reconcile(ctx context.Context, req ctrl.Reques } } - reqLogger.Info("start", "CR version", instance.GetResourceVersion()) + logger.InfoContext(ctx, "start", "crVersion", instance.GetResourceVersion()) // Pass event recorder through context ctx = context.WithValue(ctx, splcommon.EventRecorderKey, r.Recorder) result, err := ApplyClusterMaster(ctx, r.Client, instance) if result.Requeue && result.RequeueAfter != 0 { - reqLogger.Info("Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) + logger.InfoContext(ctx, "requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) } return result, err diff --git a/internal/controller/indexercluster_controller.go b/internal/controller/indexercluster_controller.go index ddc8b17c9..4c1f3803e 100644 --- a/internal/controller/indexercluster_controller.go +++ b/internal/controller/indexercluster_controller.go @@ -18,10 +18,12 @@ package controller import ( "context" + "log/slog" "time" enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/splunk/splunk-operator/internal/controller/common" + "github.com/splunk/splunk-operator/pkg/logging" "github.com/pkg/errors" enterpriseApiV3 "github.com/splunk/splunk-operator/api/v3" @@ -38,7 +40,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -79,8 +80,8 @@ func (r *IndexerClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque metrics.ReconcileCounters.With(metrics.GetPrometheusLabels(req, "IndexerCluster")).Inc() defer recordInstrumentionData(time.Now(), req, "controller", "IndexerCluster") - reqLogger := log.FromContext(ctx) - reqLogger = reqLogger.WithValues("indexercluster", req.NamespacedName) + logger := slog.Default().With("controller", "IndexerCluster", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) + ctx = logging.WithLogger(ctx, logger) // Fetch the IndexerCluster instance := &enterpriseApi.IndexerCluster{} @@ -105,14 +106,14 @@ func (r *IndexerClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque } } - reqLogger.Info("start", "CR version", instance.GetResourceVersion()) + logger.InfoContext(ctx, "start", "crVersion", instance.GetResourceVersion()) // Pass event recorder through context ctx = context.WithValue(ctx, splcommon.EventRecorderKey, r.Recorder) result, err := ApplyIndexerCluster(ctx, r.Client, instance) if result.Requeue && result.RequeueAfter != 0 { - reqLogger.Info("Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) + logger.InfoContext(ctx, "requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) } return result, err diff --git a/internal/controller/ingestorcluster_controller.go b/internal/controller/ingestorcluster_controller.go index 2725d32a6..389aea9e1 100644 --- a/internal/controller/ingestorcluster_controller.go +++ b/internal/controller/ingestorcluster_controller.go @@ -16,6 +16,7 @@ package controller import ( "context" + "log/slog" "time" appsv1 "k8s.io/api/apps/v1" @@ -28,13 +29,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/pkg/errors" enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/splunk/splunk-operator/internal/controller/common" + "github.com/splunk/splunk-operator/pkg/logging" metrics "github.com/splunk/splunk-operator/pkg/splunk/client/metrics" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" enterprise "github.com/splunk/splunk-operator/pkg/splunk/enterprise" @@ -68,8 +69,8 @@ func (r *IngestorClusterReconciler) Reconcile(ctx context.Context, req ctrl.Requ metrics.ReconcileCounters.With(metrics.GetPrometheusLabels(req, "IngestorCluster")).Inc() defer recordInstrumentionData(time.Now(), req, "controller", "IngestorCluster") - reqLogger := log.FromContext(ctx) - reqLogger = reqLogger.WithValues("ingestorcluster", req.NamespacedName) + logger := slog.Default().With("controller", "IngestorCluster", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) + ctx = logging.WithLogger(ctx, logger) // Fetch the IngestorCluster instance := &enterpriseApi.IngestorCluster{} @@ -94,14 +95,14 @@ func (r *IngestorClusterReconciler) Reconcile(ctx context.Context, req ctrl.Requ } } - reqLogger.Info("start", "CR version", instance.GetResourceVersion()) + logger.InfoContext(ctx, "start", "crVersion", instance.GetResourceVersion()) // Pass event recorder through context ctx = context.WithValue(ctx, splcommon.EventRecorderKey, r.Recorder) result, err := ApplyIngestorCluster(ctx, r.Client, instance) if result.Requeue && result.RequeueAfter != 0 { - reqLogger.Info("Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) + logger.InfoContext(ctx, "requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) } return result, err diff --git a/internal/controller/licensemanager_controller.go b/internal/controller/licensemanager_controller.go index 27e39a7f5..e0c750f0d 100644 --- a/internal/controller/licensemanager_controller.go +++ b/internal/controller/licensemanager_controller.go @@ -18,10 +18,12 @@ package controller import ( "context" + "log/slog" "time" enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/splunk/splunk-operator/internal/controller/common" + "github.com/splunk/splunk-operator/pkg/logging" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" "github.com/pkg/errors" @@ -36,7 +38,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -77,8 +78,8 @@ func (r *LicenseManagerReconciler) Reconcile(ctx context.Context, req ctrl.Reque metrics.ReconcileCounters.With(metrics.GetPrometheusLabels(req, "LicenseManager")).Inc() defer recordInstrumentionData(time.Now(), req, "controller", "LicenseManager") - reqLogger := log.FromContext(ctx) - reqLogger = reqLogger.WithValues("licensemanager", req.NamespacedName) + logger := slog.Default().With("controller", "LicenseManager", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) + ctx = logging.WithLogger(ctx, logger) // Fetch the LicenseManager instance := &enterpriseApi.LicenseManager{} @@ -103,14 +104,14 @@ func (r *LicenseManagerReconciler) Reconcile(ctx context.Context, req ctrl.Reque } } - reqLogger.Info("start", "CR version", instance.GetResourceVersion()) + logger.InfoContext(ctx, "start", "crVersion", instance.GetResourceVersion()) // Pass event recorder through context ctx = context.WithValue(ctx, splcommon.EventRecorderKey, r.Recorder) result, err := ApplyLicenseManager(ctx, r.Client, instance) if result.Requeue && result.RequeueAfter != 0 { - reqLogger.Info("Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) + logger.InfoContext(ctx, "requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) } return result, err diff --git a/internal/controller/licensemaster_controller.go b/internal/controller/licensemaster_controller.go index c413cab50..106042e1e 100644 --- a/internal/controller/licensemaster_controller.go +++ b/internal/controller/licensemaster_controller.go @@ -18,9 +18,11 @@ package controller import ( "context" + "log/slog" "time" "github.com/splunk/splunk-operator/internal/controller/common" + "github.com/splunk/splunk-operator/pkg/logging" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" "github.com/pkg/errors" @@ -37,7 +39,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -78,8 +79,8 @@ func (r *LicenseMasterReconciler) Reconcile(ctx context.Context, req ctrl.Reques metrics.ReconcileCounters.With(metrics.GetPrometheusLabels(req, "LicenseMaster")).Inc() defer recordInstrumentionData(time.Now(), req, "controller", "LicenseMaster") - reqLogger := log.FromContext(ctx) - reqLogger = reqLogger.WithValues("licensemaster", req.NamespacedName) + logger := slog.Default().With("controller", "LicenseMaster", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) + ctx = logging.WithLogger(ctx, logger) // Fetch the LicenseMaster instance := &enterpriseApiV3.LicenseMaster{} @@ -104,14 +105,14 @@ func (r *LicenseMasterReconciler) Reconcile(ctx context.Context, req ctrl.Reques } } - reqLogger.Info("start", "CR version", instance.GetResourceVersion()) + logger.InfoContext(ctx, "start", "crVersion", instance.GetResourceVersion()) // Pass event recorder through context ctx = context.WithValue(ctx, splcommon.EventRecorderKey, r.Recorder) result, err := ApplyLicenseMaster(ctx, r.Client, instance) if result.Requeue && result.RequeueAfter != 0 { - reqLogger.Info("Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) + logger.InfoContext(ctx, "requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) } return result, err diff --git a/internal/controller/monitoringconsole_controller.go b/internal/controller/monitoringconsole_controller.go index 571e0f9f8..4f7db35ae 100644 --- a/internal/controller/monitoringconsole_controller.go +++ b/internal/controller/monitoringconsole_controller.go @@ -18,10 +18,12 @@ package controller import ( "context" + "log/slog" "time" enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/splunk/splunk-operator/internal/controller/common" + "github.com/splunk/splunk-operator/pkg/logging" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" "github.com/pkg/errors" @@ -37,7 +39,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -77,8 +78,9 @@ type MonitoringConsoleReconciler struct { func (r *MonitoringConsoleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { metrics.ReconcileCounters.With(metrics.GetPrometheusLabels(req, "MonitoringConsole")).Inc() defer recordInstrumentionData(time.Now(), req, "controller", "MonitoringConsole") - reqLogger := log.FromContext(ctx) - reqLogger = reqLogger.WithValues("monitoringconsole", req.NamespacedName) + + logger := slog.Default().With("controller", "MonitoringConsole", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) + ctx = logging.WithLogger(ctx, logger) // Fetch the MonitoringConsole instance := &enterpriseApi.MonitoringConsole{} @@ -103,14 +105,14 @@ func (r *MonitoringConsoleReconciler) Reconcile(ctx context.Context, req ctrl.Re } } - reqLogger.Info("start", "CR version", instance.GetResourceVersion()) + logger.InfoContext(ctx, "start", "crVersion", instance.GetResourceVersion()) // Pass event recorder through context ctx = context.WithValue(ctx, splcommon.EventRecorderKey, r.Recorder) result, err := ApplyMonitoringConsole(ctx, r.Client, instance) if result.Requeue && result.RequeueAfter != 0 { - reqLogger.Info("Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) + logger.InfoContext(ctx, "requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) } return result, err diff --git a/internal/controller/searchheadcluster_controller.go b/internal/controller/searchheadcluster_controller.go index 9ff2eca36..77688bbf8 100644 --- a/internal/controller/searchheadcluster_controller.go +++ b/internal/controller/searchheadcluster_controller.go @@ -18,10 +18,12 @@ package controller import ( "context" + "log/slog" "time" enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/splunk/splunk-operator/internal/controller/common" + "github.com/splunk/splunk-operator/pkg/logging" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" "github.com/pkg/errors" @@ -36,7 +38,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -77,8 +78,8 @@ func (r *SearchHeadClusterReconciler) Reconcile(ctx context.Context, req ctrl.Re metrics.ReconcileCounters.With(metrics.GetPrometheusLabels(req, "SearchHeadCluster")).Inc() defer recordInstrumentionData(time.Now(), req, "controller", "SearchHeadCluster") - reqLogger := log.FromContext(ctx) - reqLogger = reqLogger.WithValues("searchheadcluster", req.NamespacedName) + logger := slog.Default().With("controller", "SearchHeadCluster", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) + ctx = logging.WithLogger(ctx, logger) // Fetch the SearchHeadCluster instance := &enterpriseApi.SearchHeadCluster{} @@ -103,14 +104,14 @@ func (r *SearchHeadClusterReconciler) Reconcile(ctx context.Context, req ctrl.Re } } - reqLogger.Info("start", "CR version", instance.GetResourceVersion()) + logger.InfoContext(ctx, "start", "crVersion", instance.GetResourceVersion()) // Pass event recorder through context ctx = context.WithValue(ctx, splcommon.EventRecorderKey, r.Recorder) result, err := ApplySearchHeadCluster(ctx, r.Client, instance) if result.Requeue && result.RequeueAfter != 0 { - reqLogger.Info("Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) + logger.InfoContext(ctx, "requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) } return result, err diff --git a/internal/controller/standalone_controller.go b/internal/controller/standalone_controller.go index bb7106f05..63693fb2c 100644 --- a/internal/controller/standalone_controller.go +++ b/internal/controller/standalone_controller.go @@ -18,10 +18,12 @@ package controller import ( "context" + "log/slog" "time" enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/splunk/splunk-operator/internal/controller/common" + "github.com/splunk/splunk-operator/pkg/logging" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" appsv1 "k8s.io/api/apps/v1" @@ -39,7 +41,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/log" ) const ( @@ -82,8 +83,8 @@ func (r *StandaloneReconciler) Reconcile(ctx context.Context, req ctrl.Request) metrics.ReconcileCounters.With(metrics.GetPrometheusLabels(req, "Standalone")).Inc() defer recordInstrumentionData(time.Now(), req, "controller", "Standalone") - reqLogger := log.FromContext(ctx) - reqLogger = reqLogger.WithValues("standalone", req.NamespacedName) + logger := slog.Default().With("controller", "Standalone", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) + ctx = logging.WithLogger(ctx, logger) // Fetch the Standalone instance := &enterpriseApi.Standalone{} @@ -108,14 +109,14 @@ func (r *StandaloneReconciler) Reconcile(ctx context.Context, req ctrl.Request) } } - reqLogger.Info("start", "CR version", instance.GetResourceVersion()) + logger.InfoContext(ctx, "start", "crVersion", instance.GetResourceVersion()) // Pass event recorder through context ctx = context.WithValue(ctx, splcommon.EventRecorderKey, r.Recorder) result, err := ApplyStandalone(ctx, r.Client, instance) if result.Requeue && result.RequeueAfter != 0 { - reqLogger.Info("Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) + logger.InfoContext(ctx, "requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) } return result, err diff --git a/internal/controller/telemetry_controller.go b/internal/controller/telemetry_controller.go index 7214f67cc..63935740a 100644 --- a/internal/controller/telemetry_controller.go +++ b/internal/controller/telemetry_controller.go @@ -18,19 +18,18 @@ package controller import ( "context" "fmt" - enterprise "github.com/splunk/splunk-operator/pkg/splunk/enterprise" - ctrl "sigs.k8s.io/controller-runtime" + "log/slog" "time" + "github.com/splunk/splunk-operator/pkg/logging" metrics "github.com/splunk/splunk-operator/pkg/splunk/client/metrics" - + enterprise "github.com/splunk/splunk-operator/pkg/splunk/enterprise" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" - + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" ) @@ -55,14 +54,14 @@ func (r *TelemetryReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( metrics.ReconcileCounters.With(metrics.GetPrometheusLabels(req, "Telemetry")).Inc() defer recordInstrumentionData(time.Now(), req, "controller", "Telemetry") - reqLogger := log.FromContext(ctx) - reqLogger = reqLogger.WithValues("telemetry", req.NamespacedName) + logger := slog.Default().With("controller", "Telemetry", "name", req.Name, "namespace", req.Namespace, "reconcileID", controller.ReconcileIDFromContext(ctx)) + ctx = logging.WithLogger(ctx, logger) - reqLogger.Info("Reconciling telemetry") + logger.InfoContext(ctx, "reconciling telemetry") defer func() { if rec := recover(); rec != nil { - reqLogger.Error(fmt.Errorf("panic: %v", rec), "Recovered from panic in TelemetryReconciler.Reconcile") + logger.ErrorContext(ctx, "recovered from panic in TelemetryReconciler.Reconcile", "error", fmt.Errorf("panic: %v", rec)) } }() @@ -71,27 +70,27 @@ func (r *TelemetryReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( err := r.Get(ctx, req.NamespacedName, cm) if err != nil { if k8serrors.IsNotFound(err) { - reqLogger.Info("telemetry configmap not found; requeueing", "period(seconds)", int(telemetryRetryDelay/time.Second)) + logger.InfoContext(ctx, "telemetry configmap not found; requeueing", "periodSeconds", int(telemetryRetryDelay/time.Second)) return ctrl.Result{Requeue: true, RequeueAfter: telemetryRetryDelay}, nil } - reqLogger.Error(err, "could not load telemetry configmap; requeueing", "period(seconds)", int(telemetryRetryDelay/time.Second)) + logger.ErrorContext(ctx, "could not load telemetry configmap; requeueing", "error", err, "periodSeconds", int(telemetryRetryDelay/time.Second)) return ctrl.Result{Requeue: true, RequeueAfter: telemetryRetryDelay}, nil } if len(cm.Data) == 0 { - reqLogger.Info("telemetry configmap has no data keys") + logger.InfoContext(ctx, "telemetry configmap has no data keys") return ctrl.Result{Requeue: true, RequeueAfter: telemetryRetryDelay}, nil } - reqLogger.Info("start", "Telemetry configmap version", cm.GetResourceVersion()) + logger.InfoContext(ctx, "start", "configmapVersion", cm.GetResourceVersion()) result, err := applyTelemetryFn(ctx, r.Client, cm) if err != nil { - reqLogger.Error(err, "Failed to send telemetry") + logger.ErrorContext(ctx, "failed to send telemetry", "error", err) return ctrl.Result{Requeue: true, RequeueAfter: telemetryRetryDelay}, nil } if result.Requeue && result.RequeueAfter != 0 { - reqLogger.Info("Requeued", "period(seconds)", int(result.RequeueAfter/time.Second)) + logger.InfoContext(ctx, "requeued", "periodSeconds", int(result.RequeueAfter/time.Second)) } return result, err diff --git a/pkg/config/featuregates.go b/pkg/config/featuregates.go new file mode 100644 index 000000000..934807511 --- /dev/null +++ b/pkg/config/featuregates.go @@ -0,0 +1,67 @@ +// Copyright (c) 2018-2026 Splunk Inc. All rights reserved. + +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "fmt" + "os" + + "k8s.io/component-base/featuregate" +) + +// Feature gate constants. Add new feature gates here following the checklist +// in docs/FeatureGates.md. +// +// Lifecycle: +// +// Alpha – off by default, opt-in via --feature-gates==true +// Beta – on by default, opt-out via --feature-gates==false +// GA – on, locked; remove the gate in a subsequent release +const ( + // ValidationWebhook gates the centralized validation webhook server. + // When enabled, the operator runs a validating webhook that enforces + // CR schema rules at admission time. + // Replaces the legacy ENABLE_VALIDATION_WEBHOOK env var. + ValidationWebhook featuregate.Feature = "ValidationWebhook" +) + +// defaultFeatureGates is the authoritative registry of all feature gates and +// their default state / maturity. Each entry here automatically becomes +// available via --feature-gates on the operator binary. +var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ + ValidationWebhook: {Default: false, PreRelease: featuregate.Alpha}, +} + +var DefaultMutableFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate() + +func init() { + if err := DefaultMutableFeatureGate.Add(defaultFeatureGates); err != nil { + panic(err) + } + applyLegacyValidationWebhookEnv(DefaultMutableFeatureGate) +} + +// applyLegacyValidationWebhookEnv preserves backwards compatibility for +// deployments using the ENABLE_VALIDATION_WEBHOOK env var instead of +// --feature-gates=ValidationWebhook=true. +// Remove once the ENABLE_VALIDATION_WEBHOOK deprecation period ends. +func applyLegacyValidationWebhookEnv(fg featuregate.MutableFeatureGate) { + if os.Getenv("ENABLE_VALIDATION_WEBHOOK") == "true" { + if err := fg.SetFromMap(map[string]bool{string(ValidationWebhook): true}); err != nil { + fmt.Fprintf(os.Stderr, "WARNING: failed to apply legacy env var ENABLE_VALIDATION_WEBHOOK: %v\n", err) + } + } +} diff --git a/pkg/config/featuregates_test.go b/pkg/config/featuregates_test.go new file mode 100644 index 000000000..03f9d9779 --- /dev/null +++ b/pkg/config/featuregates_test.go @@ -0,0 +1,91 @@ +// Copyright (c) 2018-2026 Splunk Inc. All rights reserved. + +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "os" + "testing" + + "k8s.io/component-base/featuregate" +) + +func TestValidationWebhookRegistered(t *testing.T) { + all := DefaultMutableFeatureGate.GetAll() + spec, ok := all[ValidationWebhook] + if !ok { + t.Fatal("ValidationWebhook gate not registered") + } + if spec.Default != false { + t.Errorf("ValidationWebhook default: got %v, want false", spec.Default) + } + if spec.PreRelease != featuregate.Alpha { + t.Errorf("ValidationWebhook prerelease: got %v, want Alpha", spec.PreRelease) + } +} + +func TestValidationWebhookOffByDefault(t *testing.T) { + if DefaultMutableFeatureGate.Enabled(ValidationWebhook) { + t.Error("ValidationWebhook should be disabled by default (Alpha)") + } +} + +func TestLegacyEnvVarEnablesGate(t *testing.T) { + fg := featuregate.NewFeatureGate() + if err := fg.Add(map[featuregate.Feature]featuregate.FeatureSpec{ + ValidationWebhook: {Default: false, PreRelease: featuregate.Alpha}, + }); err != nil { + t.Fatalf("Add: %v", err) + } + + t.Setenv("ENABLE_VALIDATION_WEBHOOK", "true") + applyLegacyValidationWebhookEnv(fg) + + if !fg.Enabled(ValidationWebhook) { + t.Error("ValidationWebhook should be enabled when ENABLE_VALIDATION_WEBHOOK=true") + } +} + +func TestLegacyEnvVarIgnoredWhenUnset(t *testing.T) { + fg := featuregate.NewFeatureGate() + if err := fg.Add(map[featuregate.Feature]featuregate.FeatureSpec{ + ValidationWebhook: {Default: false, PreRelease: featuregate.Alpha}, + }); err != nil { + t.Fatalf("Add: %v", err) + } + + os.Unsetenv("ENABLE_VALIDATION_WEBHOOK") + applyLegacyValidationWebhookEnv(fg) + + if fg.Enabled(ValidationWebhook) { + t.Error("ValidationWebhook should remain disabled when env var is not set") + } +} + +func TestLegacyEnvVarIgnoredWhenNotTrue(t *testing.T) { + fg := featuregate.NewFeatureGate() + if err := fg.Add(map[featuregate.Feature]featuregate.FeatureSpec{ + ValidationWebhook: {Default: false, PreRelease: featuregate.Alpha}, + }); err != nil { + t.Fatalf("Add: %v", err) + } + + t.Setenv("ENABLE_VALIDATION_WEBHOOK", "false") + applyLegacyValidationWebhookEnv(fg) + + if fg.Enabled(ValidationWebhook) { + t.Error("ValidationWebhook should remain disabled when ENABLE_VALIDATION_WEBHOOK=false") + } +} diff --git a/pkg/logging/slog.go b/pkg/logging/slog.go new file mode 100644 index 000000000..152d41d8d --- /dev/null +++ b/pkg/logging/slog.go @@ -0,0 +1,201 @@ +// Copyright (c) 2018-2026 Splunk Inc. All rights reserved. + +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package logging + +import ( + "context" + "io" + "log/slog" + "os" + "strings" + "time" +) + +type slogContextKey struct{} + +// WithLogger returns a new context with the given slog.Logger attached. +func WithLogger(ctx context.Context, l *slog.Logger) context.Context { + return context.WithValue(ctx, slogContextKey{}, l) +} + +// FromContext extracts the slog.Logger from the context. +// Falls back to slog.Default() if none is set. +func FromContext(ctx context.Context) *slog.Logger { + if l, ok := ctx.Value(slogContextKey{}).(*slog.Logger); ok { + return l + } + return slog.Default() +} + +const ( + EnvLogLevel = "LOG_LEVEL" + EnvLogFormat = "LOG_FORMAT" + EnvLogAddSource = "LOG_ADD_SOURCE" + + FormatJSON = "json" + FormatText = "text" + + DefaultLogLevel = slog.LevelInfo + DefaultLogFormat = FormatJSON +) + +// Config holds the logging configuration +type Config struct { + Level slog.Level + Format string + AddSource bool +} + +// sensitiveKeys contains field names that should be redacted +var sensitiveKeys = []string{ + "password", + "token", + "secret", + "apikey", + "api_key", + "credential", + "auth", +} + +// LoadConfig loads logging configuration from environment variables +// Environment variables take precedence over defaults +func LoadConfig() Config { + cfg := Config{ + Level: DefaultLogLevel, + Format: DefaultLogFormat, + AddSource: false, + } + + if level := os.Getenv(EnvLogLevel); level != "" { + cfg.Level = LevelFromString(level) + } + + if format := os.Getenv(EnvLogFormat); format != "" { + cfg.Format = strings.ToLower(format) + } + + if addSource := os.Getenv(EnvLogAddSource); addSource != "" { + cfg.AddSource = strings.ToLower(addSource) == "true" + } + + if cfg.Level == slog.LevelDebug && os.Getenv(EnvLogAddSource) == "" { + cfg.AddSource = true + } + + return cfg +} + +// LoadConfigWithFlags loads configuration with command-line flag overrides +// Flags take precedence over environment variables +func LoadConfigWithFlags(levelFlag, formatFlag string, addSourceFlag *bool) Config { + cfg := LoadConfig() + + if levelFlag != "" { + cfg.Level = LevelFromString(levelFlag) + } + + if formatFlag != "" { + cfg.Format = strings.ToLower(formatFlag) + } + + if addSourceFlag != nil { + cfg.AddSource = *addSourceFlag + } + + if cfg.Level == slog.LevelDebug && addSourceFlag == nil && os.Getenv(EnvLogAddSource) == "" { + cfg.AddSource = true + } + + return cfg +} + +// NewHandler creates a new slog.Handler based on the configuration +func NewHandler(cfg Config) slog.Handler { + return NewHandlerWithWriter(cfg, os.Stdout) +} + +// NewHandlerWithWriter creates a new slog.Handler that writes to the specified writer +func NewHandlerWithWriter(cfg Config, w io.Writer) slog.Handler { + opts := &slog.HandlerOptions{ + Level: cfg.Level, + AddSource: cfg.AddSource, + ReplaceAttr: redactSensitiveData, + } + + if cfg.Format == FormatText { + return slog.NewTextHandler(w, opts) + } + + return slog.NewJSONHandler(w, opts) +} + +// redactSensitiveData replaces sensitive field values with [REDACTED] +func redactSensitiveData(groups []string, a slog.Attr) slog.Attr { + keyLower := strings.ToLower(a.Key) + for _, sensitive := range sensitiveKeys { + if strings.Contains(keyLower, sensitive) { + return slog.String(a.Key, "[REDACTED]") + } + } + + if a.Key == slog.TimeKey { + if t, ok := a.Value.Any().(time.Time); ok { + return slog.String(slog.TimeKey, t.Format(time.RFC3339)) + } + } + + return a +} + +// SetupLogger initializes the global slog logger with the given configuration +// and optional attributes, then returns the configured logger +func SetupLogger(cfg Config, attrs ...slog.Attr) *slog.Logger { + handler := NewHandler(cfg) + logger := slog.New(handler) + for _, attr := range attrs { + logger = logger.With(attr) + } + slog.SetDefault(logger) + return logger +} + +// LevelFromString converts a string to slog.Level +func LevelFromString(s string) slog.Level { + switch strings.ToLower(s) { + case "debug": + return slog.LevelDebug + case "warn", "warning": + return slog.LevelWarn + case "error": + return slog.LevelError + default: + return slog.LevelInfo + } +} + +// LevelToString converts slog.Level to string +func LevelToString(level slog.Level) string { + switch level { + case slog.LevelDebug: + return "debug" + case slog.LevelWarn: + return "warn" + case slog.LevelError: + return "error" + default: + return "info" + } +} diff --git a/pkg/logging/slog_test.go b/pkg/logging/slog_test.go new file mode 100644 index 000000000..5887392d8 --- /dev/null +++ b/pkg/logging/slog_test.go @@ -0,0 +1,768 @@ +// Copyright (c) 2018-2026 Splunk Inc. All rights reserved. + +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package logging_test + +import ( + "bytes" + "log/slog" + "os" + "testing" + + "github.com/splunk/splunk-operator/pkg/logging" + "github.com/stretchr/testify/assert" +) + +const ( + envLogLevel = "LOG_LEVEL" + envLogFormat = "LOG_FORMAT" + envLogAddSource = "LOG_ADD_SOURCE" +) + +func TestLoadConfig(t *testing.T) { + tests := []struct { + name string + envVars map[string]string + expectedLevel slog.Level + expectedFormat string + expectedSource bool + }{ + { + name: "default values", + envVars: map[string]string{}, + expectedLevel: slog.LevelInfo, + expectedFormat: "json", + expectedSource: false, + }, + { + name: "debug level with auto source", + envVars: map[string]string{ + envLogLevel: "debug", + }, + expectedLevel: slog.LevelDebug, + expectedFormat: "json", + expectedSource: true, + }, + { + name: "warn level with text format", + envVars: map[string]string{ + envLogLevel: "warn", + envLogFormat: "text", + }, + expectedLevel: slog.LevelWarn, + expectedFormat: "text", + expectedSource: false, + }, + { + name: "error level with explicit source", + envVars: map[string]string{ + envLogLevel: "error", + envLogAddSource: "true", + }, + expectedLevel: slog.LevelError, + expectedFormat: "json", + expectedSource: true, + }, + { + name: "debug level with source disabled", + envVars: map[string]string{ + envLogLevel: "debug", + envLogAddSource: "false", + }, + expectedLevel: slog.LevelDebug, + expectedFormat: "json", + expectedSource: false, + }, + { + name: "invalid level defaults to info", + envVars: map[string]string{ + envLogLevel: "invalid", + }, + expectedLevel: slog.LevelInfo, + expectedFormat: "json", + expectedSource: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clearEnv() + for k, v := range tt.envVars { + os.Setenv(k, v) + } + + cfg := logging.LoadConfig() + + assert.Equal(t, tt.expectedLevel, cfg.Level) + assert.Equal(t, tt.expectedFormat, cfg.Format) + assert.Equal(t, tt.expectedSource, cfg.AddSource) + + clearEnv() + }) + } +} + +func TestLoadConfigWithFlags(t *testing.T) { + clearEnv() + os.Setenv(envLogLevel, "info") + os.Setenv(envLogFormat, "json") + defer clearEnv() + + addSource := true + cfg := logging.LoadConfigWithFlags("debug", "text", &addSource) + + assert.Equal(t, slog.LevelDebug, cfg.Level) + assert.Equal(t, "text", cfg.Format) + assert.True(t, cfg.AddSource) +} + +func TestLevelFromString(t *testing.T) { + tests := []struct { + input string + expected slog.Level + }{ + {"debug", slog.LevelDebug}, + {"DEBUG", slog.LevelDebug}, + {"info", slog.LevelInfo}, + {"INFO", slog.LevelInfo}, + {"warn", slog.LevelWarn}, + {"WARN", slog.LevelWarn}, + {"warning", slog.LevelWarn}, + {"error", slog.LevelError}, + {"ERROR", slog.LevelError}, + {"invalid", slog.LevelInfo}, + {"", slog.LevelInfo}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + result := logging.LevelFromString(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestLevelToString(t *testing.T) { + tests := []struct { + level slog.Level + expected string + }{ + {slog.LevelDebug, "debug"}, + {slog.LevelInfo, "info"}, + {slog.LevelWarn, "warn"}, + {slog.LevelError, "error"}, + {slog.Level(100), "info"}, // unknown level defaults to info + } + + for _, tt := range tests { + t.Run(tt.expected, func(t *testing.T) { + result := logging.LevelToString(tt.level) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestNewHandler(t *testing.T) { + tests := []struct { + name string + config logging.Config + }{ + { + name: "JSON handler", + config: logging.Config{ + Level: slog.LevelInfo, + Format: "json", + AddSource: false, + }, + }, + { + name: "Text handler", + config: logging.Config{ + Level: slog.LevelDebug, + Format: "text", + AddSource: true, + }, + }, + { + name: "Invalid format defaults to JSON", + config: logging.Config{ + Level: slog.LevelInfo, + Format: "invalid_format", + AddSource: false, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handler := logging.NewHandler(tt.config) + assert.NotNil(t, handler) + }) + } +} + +func TestSetupLogger(t *testing.T) { + cfg := logging.Config{ + Level: slog.LevelInfo, + Format: "json", + AddSource: false, + } + + logger := logging.SetupLogger(cfg) + assert.NotNil(t, logger) + assert.Equal(t, logger, slog.Default()) +} + +func TestSetupLoggerWithAttrs(t *testing.T) { + cfg := logging.Config{ + Level: slog.LevelInfo, + Format: "json", + AddSource: false, + } + + logger := logging.SetupLogger(cfg, + slog.String("component", "splunk-operator"), + slog.String("version", "1.0.0"), + slog.String("build", "abc123")) + assert.NotNil(t, logger) + assert.Equal(t, logger, slog.Default()) +} + +func TestSetupLoggerWithEmptyAttrs(t *testing.T) { + cfg := logging.Config{ + Level: slog.LevelInfo, + Format: "json", + AddSource: false, + } + + logger := logging.SetupLogger(cfg) + assert.NotNil(t, logger) + + // Test logging still works + var buf bytes.Buffer + handler := slog.NewJSONHandler(&buf, &slog.HandlerOptions{Level: slog.LevelInfo}) + testLogger := slog.New(handler) + testLogger.Info("test message") + assert.NotEmpty(t, buf.String()) +} + +func TestSetupLogger_ConcurrentAccess(t *testing.T) { + cfg := logging.Config{ + Level: slog.LevelInfo, + Format: "json", + AddSource: false, + } + + logger := logging.SetupLogger(cfg) + + done := make(chan bool, 10) + for i := 0; i < 10; i++ { + go func(id int) { + defer func() { + if r := recover(); r != nil { + t.Errorf("Panic in goroutine %d: %v", id, r) + } + done <- true + }() + + for j := 0; j < 100; j++ { + logger.Info("concurrent test", + slog.Int("goroutine", id), + slog.Int("iteration", j)) + } + }(i) + } + + for i := 0; i < 10; i++ { + <-done + } +} + +func TestLoadConfigWithFlags_EdgeCases(t *testing.T) { + tests := []struct { + name string + envLevel string + envFormat string + flagLevel string + flagFormat string + flagAddSource *bool + expectedLevel slog.Level + expectedFormat string + expectedSource bool + }{ + { + name: "empty flags use env vars", + envLevel: "warn", + envFormat: "text", + flagLevel: "", + flagFormat: "", + flagAddSource: nil, + expectedLevel: slog.LevelWarn, + expectedFormat: "text", + expectedSource: false, + }, + { + name: "flags override env vars", + envLevel: "info", + envFormat: "json", + flagLevel: "error", + flagFormat: "text", + flagAddSource: boolPtr(true), + expectedLevel: slog.LevelError, + expectedFormat: "text", + expectedSource: true, + }, + { + name: "invalid flag level defaults to info", + envLevel: "warn", + envFormat: "json", + flagLevel: "invalid", + flagFormat: "", + flagAddSource: nil, + expectedLevel: slog.LevelInfo, + expectedFormat: "json", + expectedSource: false, + }, + { + name: "nil addSource pointer uses env/default", + envLevel: "info", + envFormat: "json", + flagLevel: "", + flagFormat: "", + flagAddSource: nil, + expectedLevel: slog.LevelInfo, + expectedFormat: "json", + expectedSource: false, + }, + { + name: "false addSource pointer explicitly disables", + envLevel: "debug", + envFormat: "json", + flagLevel: "", + flagFormat: "", + flagAddSource: boolPtr(false), + expectedLevel: slog.LevelDebug, + expectedFormat: "json", + expectedSource: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clearEnv() + if tt.envLevel != "" { + os.Setenv(envLogLevel, tt.envLevel) + } + if tt.envFormat != "" { + os.Setenv(envLogFormat, tt.envFormat) + } + + cfg := logging.LoadConfigWithFlags(tt.flagLevel, tt.flagFormat, tt.flagAddSource) + + assert.Equal(t, tt.expectedLevel, cfg.Level) + assert.Equal(t, tt.expectedFormat, cfg.Format) + assert.Equal(t, tt.expectedSource, cfg.AddSource) + + clearEnv() + }) + } +} + +func TestLoadConfig_InvalidEnvironmentValues(t *testing.T) { + tests := []struct { + name string + envVars map[string]string + expectedLevel slog.Level + expectedFormat string + expectedSource bool + }{ + { + name: "garbage log level", + envVars: map[string]string{ + envLogLevel: "notavalidlevel", + }, + expectedLevel: slog.LevelInfo, + expectedFormat: "json", + expectedSource: false, + }, + { + name: "numeric log level (invalid)", + envVars: map[string]string{ + envLogLevel: "123", + }, + expectedLevel: slog.LevelInfo, + expectedFormat: "json", + expectedSource: false, + }, + { + name: "special characters in log level", + envVars: map[string]string{ + envLogLevel: "!@#$%", + }, + expectedLevel: slog.LevelInfo, + expectedFormat: "json", + expectedSource: false, + }, + { + name: "invalid format stored as-is", + envVars: map[string]string{ + envLogFormat: "xml", + }, + expectedLevel: slog.LevelInfo, + expectedFormat: "xml", + expectedSource: false, + }, + { + name: "invalid add_source value", + envVars: map[string]string{ + envLogAddSource: "yes", + }, + expectedLevel: slog.LevelInfo, + expectedFormat: "json", + expectedSource: false, + }, + { + name: "mixed case add_source", + envVars: map[string]string{ + envLogAddSource: "TRUE", + }, + expectedLevel: slog.LevelInfo, + expectedFormat: "json", + expectedSource: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clearEnv() + for k, v := range tt.envVars { + os.Setenv(k, v) + } + + cfg := logging.LoadConfig() + + assert.Equal(t, tt.expectedLevel, cfg.Level) + assert.Equal(t, tt.expectedFormat, cfg.Format) + assert.Equal(t, tt.expectedSource, cfg.AddSource) + + clearEnv() + }) + } +} + +func TestLogOutput_LevelFiltering(t *testing.T) { + tests := []struct { + name string + handlerLevel slog.Level + logLevel slog.Level + shouldContain bool + }{ + {"debug log at info level", slog.LevelInfo, slog.LevelDebug, false}, + {"info log at info level", slog.LevelInfo, slog.LevelInfo, true}, + {"warn log at info level", slog.LevelInfo, slog.LevelWarn, true}, + {"error log at info level", slog.LevelInfo, slog.LevelError, true}, + {"info log at error level", slog.LevelError, slog.LevelInfo, false}, + {"error log at error level", slog.LevelError, slog.LevelError, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + opts := &slog.HandlerOptions{Level: tt.handlerLevel} + handler := slog.NewTextHandler(&buf, opts) + logger := slog.New(handler) + + switch tt.logLevel { + case slog.LevelDebug: + logger.Debug("test message") + case slog.LevelInfo: + logger.Info("test message") + case slog.LevelWarn: + logger.Warn("test message") + case slog.LevelError: + logger.Error("test message") + } + + hasOutput := buf.Len() > 0 + assert.Equal(t, tt.shouldContain, hasOutput) + }) + } +} + +func TestSensitiveDataRedaction(t *testing.T) { + var buf bytes.Buffer + cfg := logging.Config{ + Level: slog.LevelInfo, + Format: "text", + AddSource: false, + } + + handler := logging.NewHandlerWithWriter(cfg, &buf) + logger := slog.New(handler) + + logger.Info("test", + slog.String("password", "secret123"), + slog.String("token", "abc123"), + slog.String("username", "admin")) + + output := buf.String() + + // Verify sensitive data is redacted + assert.NotContains(t, output, "secret123", "password should be redacted") + assert.NotContains(t, output, "abc123", "token should be redacted") + assert.Contains(t, output, "[REDACTED]", "redaction marker should be present") + assert.Contains(t, output, "admin", "non-sensitive data should not be redacted") +} + +func clearEnv() { + os.Unsetenv(envLogLevel) + os.Unsetenv(envLogFormat) + os.Unsetenv(envLogAddSource) +} + +func boolPtr(b bool) *bool { + return &b +} + +func TestLevelConversions_Roundtrip(t *testing.T) { + levels := []slog.Level{ + slog.LevelDebug, + slog.LevelInfo, + slog.LevelWarn, + slog.LevelError, + } + + for _, level := range levels { + str := logging.LevelToString(level) + result := logging.LevelFromString(str) + assert.Equal(t, level, result, "Roundtrip failed for level %v", level) + } +} + +func TestNewHandler_OutputFormat(t *testing.T) { + t.Run("JSON format produces JSON output", func(t *testing.T) { + cfg := logging.Config{ + Level: slog.LevelInfo, + Format: "json", + AddSource: false, + } + handler := logging.NewHandler(cfg) + assert.NotNil(t, handler) + }) + + t.Run("Text format produces text output", func(t *testing.T) { + cfg := logging.Config{ + Level: slog.LevelInfo, + Format: "text", + AddSource: false, + } + handler := logging.NewHandler(cfg) + assert.NotNil(t, handler) + }) +} + +func TestSensitiveDataRedactionAllKeys(t *testing.T) { + var buf bytes.Buffer + cfg := logging.Config{ + Level: slog.LevelInfo, + Format: "text", + AddSource: false, + } + + handler := logging.NewHandlerWithWriter(cfg, &buf) + logger := slog.New(handler) + + // Test all sensitive key patterns + logger.Info("test", + slog.String("password", "pass1"), + slog.String("token", "tok1"), + slog.String("secret", "sec1"), + slog.String("apikey", "key1"), + slog.String("api_key", "key2"), + slog.String("credential", "cred1"), + slog.String("auth", "auth1"), + slog.String("normal_field", "visible")) + + output := buf.String() + + // Verify ALL sensitive values are redacted + sensitiveValues := []string{"pass1", "tok1", "sec1", "key1", "key2", "cred1", "auth1"} + for _, val := range sensitiveValues { + assert.NotContains(t, output, val, "sensitive value %q should be redacted", val) + } + + // Verify normal value is NOT redacted + assert.Contains(t, output, "visible", "normal field should not be redacted") +} + +func TestConfigStruct(t *testing.T) { + cfg := logging.Config{ + Level: slog.LevelDebug, + Format: "text", + AddSource: true, + } + + assert.Equal(t, slog.LevelDebug, cfg.Level) + assert.Equal(t, "text", cfg.Format) + assert.True(t, cfg.AddSource) +} + +func TestRedactionThroughHandler(t *testing.T) { + tests := []struct { + name string + key string + value string + shouldRedact bool + }{ + {"password field", "password", "secret123", true}, + {"Password field", "Password", "secret456", true}, + {"admin_password field", "admin_password", "secret789", true}, + {"token field", "token", "tok123", true}, + {"auth_token field", "auth_token", "tok456", true}, + {"secret field", "secret", "sec123", true}, + {"apikey field", "apikey", "key123", true}, + {"api_key field", "api_key", "key456", true}, + {"credential field", "credential", "cred123", true}, + {"auth field", "auth", "auth123", true}, + {"normal field", "namespace", "default", false}, + {"name field", "name", "myapp", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + cfg := logging.Config{ + Level: slog.LevelInfo, + Format: "text", + AddSource: false, + } + handler := logging.NewHandlerWithWriter(cfg, &buf) + logger := slog.New(handler) + + logger.Info("test", slog.String(tt.key, tt.value)) + output := buf.String() + + if tt.shouldRedact { + assert.NotContains(t, output, tt.value, "value should be redacted") + assert.Contains(t, output, "[REDACTED]") + } else { + assert.Contains(t, output, tt.value, "value should NOT be redacted") + } + }) + } +} + +func TestDebugLevelAutoEnablesSource(t *testing.T) { + clearEnv() + os.Setenv(envLogLevel, "debug") + defer clearEnv() + + cfg := logging.LoadConfig() + + assert.Equal(t, slog.LevelDebug, cfg.Level) + assert.True(t, cfg.AddSource, "Debug level should auto-enable source") +} + +func TestDebugLevelSourceCanBeDisabled(t *testing.T) { + clearEnv() + os.Setenv(envLogLevel, "debug") + os.Setenv(envLogAddSource, "false") + defer clearEnv() + + cfg := logging.LoadConfig() + + assert.Equal(t, slog.LevelDebug, cfg.Level) + assert.False(t, cfg.AddSource, "Source should be disabled when explicitly set to false") +} + +func TestWarningAlias(t *testing.T) { + // "warning" should be treated same as "warn" + result := logging.LevelFromString("warning") + assert.Equal(t, slog.LevelWarn, result) +} + +func TestCaseInsensitiveLevel(t *testing.T) { + tests := []string{"DEBUG", "Debug", "dEbUg", "debug"} + for _, input := range tests { + result := logging.LevelFromString(input) + assert.Equal(t, slog.LevelDebug, result, "Input: %s", input) + } +} + +func TestFormatCaseInsensitive(t *testing.T) { + clearEnv() + os.Setenv(envLogFormat, "JSON") + defer clearEnv() + + cfg := logging.LoadConfig() + assert.Equal(t, "json", cfg.Format) +} + +func TestAddSourceCaseInsensitive(t *testing.T) { + clearEnv() + os.Setenv(envLogAddSource, "TRUE") + defer clearEnv() + + cfg := logging.LoadConfig() + assert.True(t, cfg.AddSource) +} + +func TestFlagsOverrideEnvVars(t *testing.T) { + clearEnv() + os.Setenv(envLogLevel, "info") + os.Setenv(envLogFormat, "json") + os.Setenv(envLogAddSource, "false") + defer clearEnv() + + addSource := true + cfg := logging.LoadConfigWithFlags("debug", "text", &addSource) + + assert.Equal(t, slog.LevelDebug, cfg.Level, "Flag should override env var for level") + assert.Equal(t, "text", cfg.Format, "Flag should override env var for format") + assert.True(t, cfg.AddSource, "Flag should override env var for addSource") +} + +func TestEmptyFlagsUseEnvVars(t *testing.T) { + clearEnv() + os.Setenv(envLogLevel, "warn") + os.Setenv(envLogFormat, "text") + os.Setenv(envLogAddSource, "true") + defer clearEnv() + + cfg := logging.LoadConfigWithFlags("", "", nil) + + assert.Equal(t, slog.LevelWarn, cfg.Level) + assert.Equal(t, "text", cfg.Format) + assert.True(t, cfg.AddSource) +} + +func TestNilAddSourcePointer(t *testing.T) { + clearEnv() + os.Setenv(envLogAddSource, "true") + defer clearEnv() + + cfg := logging.LoadConfigWithFlags("", "", nil) + assert.True(t, cfg.AddSource, "nil pointer should use env var value") +} + +func TestExplicitFalseAddSource(t *testing.T) { + clearEnv() + os.Setenv(envLogAddSource, "true") + defer clearEnv() + + addSource := false + cfg := logging.LoadConfigWithFlags("", "", &addSource) + assert.False(t, cfg.AddSource, "explicit false should override env var") +} diff --git a/pkg/splunk/client/awss3client.go b/pkg/splunk/client/awss3client.go index b35d989c8..3c9eb1013 100644 --- a/pkg/splunk/client/awss3client.go +++ b/pkg/splunk/client/awss3client.go @@ -32,7 +32,7 @@ import ( "github.com/aws/aws-sdk-go-v2/credentials" "github.com/aws/aws-sdk-go-v2/feature/s3/manager" "github.com/aws/aws-sdk-go-v2/service/s3" - "sigs.k8s.io/controller-runtime/pkg/log" + "github.com/splunk/splunk-operator/pkg/logging" ) // blank assignment to verify that AWSS3Client implements RemoteDataClient @@ -83,8 +83,7 @@ func InitAWSClientWrapper(ctx context.Context, region, accessKeyID, secretAccess // InitAWSClientConfig initializes and returns a client config object func InitAWSClientConfig(ctx context.Context, regionWithEndpoint, accessKeyID, secretAccessKey string) SplunkAWSS3Client { - reqLogger := log.FromContext(ctx) - scopedLog := reqLogger.WithName("InitAWSClientConfig") + scopedLog := logging.FromContext(ctx).With("func", "InitAWSClientConfig") // Enforcing minimum version TLS1.2 tr := &http.Transport{ @@ -105,8 +104,8 @@ func InitAWSClientConfig(ctx context.Context, regionWithEndpoint, accessKeyID, s // Extract region and endpoint regEndSl := strings.Split(regionWithEndpoint, awsRegionEndPointDelimiter) if len(regEndSl) != 2 || strings.Count(regionWithEndpoint, awsRegionEndPointDelimiter) != 1 { - scopedLog.Error(err, "Unable to extract region and endpoint correctly for AWS client", - "regWithEndpoint", regionWithEndpoint) + scopedLog.ErrorContext(ctx, "unable to extract region and endpoint correctly for AWS client", + "regWithEndpoint", regionWithEndpoint, "error", err) return nil } region = regEndSl[0] @@ -123,7 +122,7 @@ func InitAWSClientConfig(ctx context.Context, regionWithEndpoint, accessKeyID, s "")), // token ) } else { - scopedLog.Info("No valid access/secret keys. Attempt to connect without them") + scopedLog.InfoContext(ctx, "no valid access/secret keys. Attempt to connect without them") cfg, err = config.LoadDefaultConfig(ctx, config.WithRegion(region), config.WithRetryMaxAttempts(3), @@ -131,7 +130,7 @@ func InitAWSClientConfig(ctx context.Context, regionWithEndpoint, accessKeyID, s ) } if err != nil { - scopedLog.Error(err, "Failed to initialize an AWS S3 config.") + scopedLog.ErrorContext(ctx, "failed to initialize an AWS S3 config", "error", err) return nil } s3Client := s3.NewFromConfig(cfg, func(o *s3.Options) { @@ -144,7 +143,7 @@ func InitAWSClientConfig(ctx context.Context, regionWithEndpoint, accessKeyID, s tlsVersion = getTLSVersion(tr) } - scopedLog.Info("AWS Client Config initialization successful.", "region", region, "TLS Version", tlsVersion) + scopedLog.InfoContext(ctx, "AWS client config initialization successful", "region", region, "tlsVersion", tlsVersion) return s3Client } @@ -211,10 +210,9 @@ func getTLSVersion(tr *http.Transport) string { // GetAppsList get the list of apps from remote storage func (awsclient *AWSS3Client) GetAppsList(ctx context.Context) (RemoteDataListResponse, error) { - reqLogger := log.FromContext(ctx) - scopedLog := reqLogger.WithName("GetAppsList") + scopedLog := logging.FromContext(ctx).With("func", "GetAppsList") - scopedLog.Info("Getting Apps list", "AWS S3 Bucket", awsclient.BucketName) + scopedLog.InfoContext(ctx, "getting Apps list", "bucket", awsclient.BucketName) remoteDataClientResponse := RemoteDataListResponse{} options := &s3.ListObjectsV2Input{ @@ -228,24 +226,24 @@ func (awsclient *AWSS3Client) GetAppsList(ctx context.Context) (RemoteDataListRe client := awsclient.Client resp, err := client.ListObjectsV2(ctx, options) if err != nil { - scopedLog.Error(err, "Unable to list items in bucket", "AWS S3 Bucket", awsclient.BucketName, "endpoint", awsclient.Endpoint) + scopedLog.ErrorContext(ctx, "unable to list items in bucket", "bucket", awsclient.BucketName, "endpoint", awsclient.Endpoint, "error", err) return remoteDataClientResponse, err } if resp.Contents == nil { - scopedLog.Info("empty objects list in bucket. No apps to install", "bucketName", awsclient.BucketName) + scopedLog.InfoContext(ctx, "empty objects list in bucket. No apps to install", "bucketName", awsclient.BucketName) return remoteDataClientResponse, nil } tmp, err := json.Marshal(resp.Contents) if err != nil { - scopedLog.Error(err, "Failed to marshal s3 response", "AWS S3 Bucket", awsclient.BucketName) + scopedLog.ErrorContext(ctx, "failed to marshal s3 response", "bucket", awsclient.BucketName, "error", err) return remoteDataClientResponse, err } err = json.Unmarshal(tmp, &(remoteDataClientResponse.Objects)) if err != nil { - scopedLog.Error(err, "Failed to unmarshal s3 response", "AWS S3 Bucket", awsclient.BucketName) + scopedLog.ErrorContext(ctx, "failed to unmarshal s3 response", "bucket", awsclient.BucketName, "error", err) return remoteDataClientResponse, err } @@ -254,14 +252,13 @@ func (awsclient *AWSS3Client) GetAppsList(ctx context.Context) (RemoteDataListRe // DownloadApp downloads the app from remote storage to local file system func (awsclient *AWSS3Client) DownloadApp(ctx context.Context, downloadRequest RemoteDataDownloadRequest) (bool, error) { - reqLogger := log.FromContext(ctx) - scopedLog := reqLogger.WithName("DownloadApp").WithValues("remoteFile", downloadRequest.RemoteFile, "localFile", + scopedLog := logging.FromContext(ctx).With("func", "DownloadApp", "remoteFile", downloadRequest.RemoteFile, "localFile", downloadRequest.LocalFile, "etag", downloadRequest.Etag) var numBytes int64 file, err := os.Create(downloadRequest.LocalFile) if err != nil { - scopedLog.Error(err, "Unable to open local file") + scopedLog.ErrorContext(ctx, "unable to open local file", "error", err) return false, err } defer file.Close() @@ -274,12 +271,12 @@ func (awsclient *AWSS3Client) DownloadApp(ctx context.Context, downloadRequest R IfMatch: aws.String(downloadRequest.Etag), }) if err != nil { - scopedLog.Error(err, "Unable to download item", "RemoteFile", downloadRequest.RemoteFile) + scopedLog.ErrorContext(ctx, "unable to download item", "RemoteFile", downloadRequest.RemoteFile, "error", err) os.Remove(downloadRequest.RemoteFile) return false, err } - scopedLog.Info("File downloaded", "numBytes: ", numBytes) + scopedLog.InfoContext(ctx, "file downloaded", "numBytes", numBytes) return true, err } diff --git a/pkg/splunk/client/azureblobclient.go b/pkg/splunk/client/azureblobclient.go index c74036269..f0bbc9679 100644 --- a/pkg/splunk/client/azureblobclient.go +++ b/pkg/splunk/client/azureblobclient.go @@ -26,7 +26,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/blob" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/container" - "sigs.k8s.io/controller-runtime/pkg/log" + "github.com/splunk/splunk-operator/pkg/logging" ) var _ RemoteDataClient = &AzureBlobClient{} @@ -122,10 +122,9 @@ func NewAzureBlobClient( endpoint string, // Custom endpoint (optional) initFunc GetInitFunc, // Initialization function ) (RemoteDataClient, error) { // Matches GetRemoteDataClient signature - reqLogger := log.FromContext(ctx) - scopedLog := reqLogger.WithName("NewAzureBlobClient") + scopedLog := logging.FromContext(ctx).With("func", "NewAzureBlobClient") - scopedLog.Info("Initializing AzureBlobClient") + scopedLog.InfoContext(ctx, "initializing AzureBlobClient") // Execute the initialization function if provided. if initFunc != nil { @@ -152,12 +151,12 @@ func NewAzureBlobClient( if secretAccessKey != "" { // Use Shared Key authentication. - scopedLog.Info("Using Shared Key authentication") + scopedLog.InfoContext(ctx, "using Shared Key authentication") // Create a Shared Key Credential. sharedKeyCredential, err := azblob.NewSharedKeyCredential(storageAccountName, secretAccessKey) if err != nil { - scopedLog.Error(err, "Failed to create SharedKeyCredential") + scopedLog.ErrorContext(ctx, "failed to create SharedKeyCredential", "error", err) return nil, fmt.Errorf("failed to create SharedKeyCredential: %w", err) } @@ -168,7 +167,7 @@ func NewAzureBlobClient( nil, ) if err != nil { - scopedLog.Error(err, "Failed to create ContainerClient with SharedKeyCredential") + scopedLog.ErrorContext(ctx, "failed to create ContainerClient with SharedKeyCredential", "error", err) return nil, fmt.Errorf("failed to create ContainerClient with SharedKeyCredential: %w", err) } @@ -178,7 +177,7 @@ func NewAzureBlobClient( credentialType = CredentialTypeSharedKey } else { // Use Azure AD authentication. - scopedLog.Info("Using Azure AD authentication") + scopedLog.InfoContext(ctx, "using Azure AD authentication") // Create a Token Credential using DefaultAzureCredential. // The Azure SDK uses environment variables to configure authentication when using DefaultAzureCredential. @@ -200,7 +199,7 @@ func NewAzureBlobClient( tokenCredential, err := azidentity.NewDefaultAzureCredential(nil) if err != nil { - scopedLog.Error(err, "Failed to create DefaultAzureCredential") + scopedLog.ErrorContext(ctx, "failed to create DefaultAzureCredential", "error", err) return nil, fmt.Errorf("failed to create DefaultAzureCredential: %w", err) } @@ -211,7 +210,7 @@ func NewAzureBlobClient( nil, ) if err != nil { - scopedLog.Error(err, "Failed to create ContainerClient with TokenCredential") + scopedLog.ErrorContext(ctx, "failed to create ContainerClient with TokenCredential", "error", err) return nil, fmt.Errorf("failed to create ContainerClient with TokenCredential: %w", err) } @@ -221,7 +220,7 @@ func NewAzureBlobClient( credentialType = CredentialTypeAzureAD } - scopedLog.Info("AzureBlobClient initialized successfully", + scopedLog.InfoContext(ctx, "azureBlobClient initialized successfully", "CredentialType", credentialType, "BucketName", bucketName, "StorageAccountName", storageAccountName, @@ -240,10 +239,9 @@ func NewAzureBlobClient( // GetAppsList retrieves a list of blobs (apps) from the Azure Blob container. func (client *AzureBlobClient) GetAppsList(ctx context.Context) (RemoteDataListResponse, error) { - reqLogger := log.FromContext(ctx) - scopedLog := reqLogger.WithName("AzureBlob:GetAppsList").WithValues("Bucket", client.BucketName) + scopedLog := logging.FromContext(ctx).With("func", "AzureBlob:GetAppsList", "Bucket", client.BucketName) - scopedLog.Info("Fetching list of apps") + scopedLog.InfoContext(ctx, "fetching list of apps") // Define options for listing blobs. options := &container.ListBlobsFlatOptions{ @@ -262,7 +260,7 @@ func (client *AzureBlobClient) GetAppsList(ctx context.Context) (RemoteDataListR for pager.More() { resp, err := pager.NextPage(ctx) if err != nil { - scopedLog.Error(err, "Error listing blobs") + scopedLog.ErrorContext(ctx, "error listing blobs", "error", err) return RemoteDataListResponse{}, fmt.Errorf("error listing blobs: %w", err) } @@ -282,21 +280,20 @@ func (client *AzureBlobClient) GetAppsList(ctx context.Context) (RemoteDataListR } } - scopedLog.Info("Successfully fetched list of apps", "TotalBlobs", len(blobs)) + scopedLog.InfoContext(ctx, "successfully fetched list of apps", "TotalBlobs", len(blobs)) return RemoteDataListResponse{Objects: blobs}, nil } // DownloadApp downloads a specific blob from Azure Blob Storage to a local file. func (client *AzureBlobClient) DownloadApp(ctx context.Context, downloadRequest RemoteDataDownloadRequest) (bool, error) { - reqLogger := log.FromContext(ctx) - scopedLog := reqLogger.WithName("AzureBlob:DownloadApp").WithValues( + scopedLog := logging.FromContext(ctx).With("func", "AzureBlob:DownloadApp", "Bucket", client.BucketName, "RemoteFile", downloadRequest.RemoteFile, "LocalFile", downloadRequest.LocalFile, ) - scopedLog.Info("Initiating blob download") + scopedLog.InfoContext(ctx, "initiating blob download") // Create a blob client for the specific blob. blobClient := client.ContainerClient.NewBlobClient(downloadRequest.RemoteFile) @@ -304,7 +301,7 @@ func (client *AzureBlobClient) DownloadApp(ctx context.Context, downloadRequest // Download the blob content. get, err := blobClient.DownloadStream(ctx, nil) if err != nil { - scopedLog.Error(err, "Failed to download blob") + scopedLog.ErrorContext(ctx, "failed to download blob", "error", err) return false, fmt.Errorf("failed to download blob: %w", err) } defer get.Body.Close() @@ -312,7 +309,7 @@ func (client *AzureBlobClient) DownloadApp(ctx context.Context, downloadRequest // Create or truncate the local file. localFile, err := os.Create(downloadRequest.LocalFile) if err != nil { - scopedLog.Error(err, "Failed to create local file") + scopedLog.ErrorContext(ctx, "failed to create local file", "error", err) return false, fmt.Errorf("failed to create local file: %w", err) } defer localFile.Close() @@ -320,11 +317,11 @@ func (client *AzureBlobClient) DownloadApp(ctx context.Context, downloadRequest // Write the content to the local file. _, err = io.Copy(localFile, get.Body) if err != nil { - scopedLog.Error(err, "Failed to write blob content to local file") + scopedLog.ErrorContext(ctx, "failed to write blob content to local file", "error", err) return false, fmt.Errorf("failed to write blob content to local file: %w", err) } - scopedLog.Info("Blob downloaded successfully") + scopedLog.InfoContext(ctx, "blob downloaded successfully") return true, nil } diff --git a/pkg/splunk/client/enterprise.go b/pkg/splunk/client/enterprise.go index 9291484cb..dc0c6e1d5 100644 --- a/pkg/splunk/client/enterprise.go +++ b/pkg/splunk/client/enterprise.go @@ -17,6 +17,7 @@ package client import ( "bytes" + "context" "crypto/tls" "encoding/json" "fmt" @@ -27,7 +28,7 @@ import ( "strings" "time" - "github.com/go-logr/logr" + "github.com/splunk/splunk-operator/pkg/logging" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" ) @@ -1035,23 +1036,25 @@ func (c *SplunkClient) RestartSplunk() error { // Updates conf files and their properties // See https://help.splunk.com/en/splunk-enterprise/leverage-rest-apis/rest-api-reference/10.0/configuration-endpoints/configuration-endpoint-descriptions -func (c *SplunkClient) UpdateConfFile(scopedLog logr.Logger, fileName, property string, propertyKVList [][]string) error { +func (c *SplunkClient) UpdateConfFile(ctx context.Context, fileName, property string, propertyKVList [][]string) error { + logger := logging.FromContext(ctx).With("func", "UpdateConfFile") + // Creates an object in a conf file if it doesn't exist endpoint := fmt.Sprintf("%s/servicesNS/nobody/system/configs/conf-%s", c.ManagementURI, fileName) body := fmt.Sprintf("name=%s", property) - scopedLog.Info("Creating conf file object if it does not exist", "fileName", fileName, "property", property) + logger.InfoContext(ctx, "creating conf file object if it does not exist", "fileName", fileName, "property", property) request, err := http.NewRequest("POST", endpoint, strings.NewReader(body)) if err != nil { - scopedLog.Error(err, "Failed to create conf file object if it does not exist", "fileName", fileName, "property", property) + logger.ErrorContext(ctx, "failed to create conf file object if it does not exist", "fileName", fileName, "property", property, "error", err.Error()) return err } - scopedLog.Info("Validating conf file object creation", "fileName", fileName, "property", property) + logger.InfoContext(ctx, "validating conf file object creation", "fileName", fileName, "property", property) expectedStatus := []int{200, 201, 409} err = c.Do(request, expectedStatus, nil) if err != nil { - scopedLog.Error(err, fmt.Sprintf("Status not in %v for conf file object creation", expectedStatus), "fileName", fileName, "property", property) + logger.ErrorContext(ctx, fmt.Sprintf("Status not in %v for conf file object creation", expectedStatus), "fileName", fileName, "property", property, "error", err.Error()) return err } @@ -1065,18 +1068,18 @@ func (c *SplunkClient) UpdateConfFile(scopedLog logr.Logger, fileName, property body = body[:len(body)-1] } - scopedLog.Info("Updating conf file object", "fileName", fileName, "property", property, "body", body) + logger.InfoContext(ctx, "updating conf file object", "fileName", fileName, "property", property) request, err = http.NewRequest("POST", endpoint, strings.NewReader(body)) if err != nil { - scopedLog.Error(err, "Failed to update conf file object", "fileName", fileName, "property", property, "body", body) + logger.ErrorContext(ctx, "failed to update conf file object", "fileName", fileName, "property", property, "error", err.Error()) return err } - scopedLog.Info("Validating conf file object update", "fileName", fileName, "property", property) + logger.InfoContext(ctx, "validating conf file object update", "fileName", fileName, "property", property) expectedStatus = []int{200, 201} err = c.Do(request, expectedStatus, nil) if err != nil { - scopedLog.Error(err, fmt.Sprintf("Status not in %v for conf file object update", expectedStatus), "fileName", fileName, "property", property, "body", body) + logger.ErrorContext(ctx, fmt.Sprintf("Status not in %v for conf file object update", expectedStatus), "fileName", fileName, "property", property, "error", err.Error()) } return err } diff --git a/pkg/splunk/client/enterprise_test.go b/pkg/splunk/client/enterprise_test.go index 2c902d537..16f2c04d8 100644 --- a/pkg/splunk/client/enterprise_test.go +++ b/pkg/splunk/client/enterprise_test.go @@ -25,7 +25,6 @@ import ( "testing" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" - "sigs.k8s.io/controller-runtime/pkg/log" spltest "github.com/splunk/splunk-operator/pkg/splunk/test" ) @@ -311,7 +310,7 @@ func TestGetclusterManagerInfo(t *testing.T) { } return nil } - body = splcommon.TestGetCMInfoEmpty + body = loadFixture(t, "get_cm_info_empty.json") splunkClientTester(t, "TestGetclusterManagerInfo", 200, body, wantRequest, test) // test error code @@ -331,7 +330,7 @@ func TestGetIndexerClusterPeerInfo(t *testing.T) { } return nil } - body := splcommon.TestGetIndexerClusterPeerInfo + body := loadFixture(t, "get_indexer_cluster_peer_info.json") splunkClientTester(t, "TestGetIndexerClusterPeerInfo", 200, body, wantRequest, test) // test body with no entries @@ -342,7 +341,7 @@ func TestGetIndexerClusterPeerInfo(t *testing.T) { } return nil } - body = splcommon.TestGetIndexerClusterPeerInfoEmpty + body = loadFixture(t, "get_indexer_cluster_peer_info_empty.json") splunkClientTester(t, "TestGetIndexerClusterPeerInfo", 200, body, wantRequest, test) // test error code @@ -383,7 +382,7 @@ func TestGetClusterManagerPeers(t *testing.T) { } return nil } - body := splcommon.TestGetCMPeers + body := loadFixture(t, "get_cm_peers.json") splunkClientTester(t, "TestGetClusterManagerPeers", 200, body, wantRequest, test) // test error response @@ -433,14 +432,14 @@ func TestAutomateMCApplyChanges(t *testing.T) { var wantRequests []*http.Request wantRequests = []*http.Request(append(wantRequests, request1, request2, request3, request4, request5, request6, request7, request8, request9, request10)) body := []string{ - `{"links":{},"origin":"https://localhost:8089/services/server/info","updated":"2020-09-24T06:38:53+00:00","generator":{"build":"a1a6394cc5ae","version":"8.0.5"},"entry":[{"name":"server-info","id":"https://localhost:8089/services/server/info/server-info","updated":"1970-01-01T00:00:00+00:00","links":{"alternate":"/services/server/info/server-info","list":"/services/server/info/server-info"},"author":"system","acl":{"app":"","can_list":true,"can_write":true,"modifiable":false,"owner":"system","perms":{"read":["*"],"write":[]},"removable":false,"sharing":"system"},"fields":{"required":[],"optional":[],"wildcard":[]},"content":{"activeLicenseGroup":"Trial","activeLicenseSubgroup":"Production","build":"a1a6394cc5ae","cluster_label":["idxc_label"],"cpu_arch":"x86_64","eai:acl":null,"fips_mode":false,"guid":"0F93F33C-4BDA-4A74-AD9F-3FCE26C6AFF0","health_info":"green","health_version":1,"host":"splunk-default-monitoring-console-86bc9b7c8c-d96x2","host_fqdn":"splunk-default-monitoring-console-86bc9b7c8c-d96x2","host_resolved":"splunk-default-monitoring-console-86bc9b7c8c-d96x2","isForwarding":true,"isFree":false,"isTrial":true,"kvStoreStatus":"ready","licenseKeys":["5C52DA5145AD67B8188604C49962D12F2C3B2CF1B82A6878E46F68CA2812807B"],"licenseSignature":"139bf73ec92c84121c79a9b8307a6724","licenseState":"OK","license_labels":["Splunk Enterprise Splunk Analytics for Hadoop Download Trial"],"master_guid":"0F93F33C-4BDA-4A74-AD9F-3FCE26C6AFF0","master_uri":"self","max_users":4294967295,"mode":"normal","numberOfCores":1,"numberOfVirtualCores":2,"os_build":"#1 SMP Thu Sep 3 19:04:44 UTC 2020","os_name":"Linux","os_name_extended":"Linux","os_version":"4.14.193-149.317.amzn2.x86_64","physicalMemoryMB":7764,"product_type":"enterprise","rtsearch_enabled":true,"serverName":"splunk-default-monitoring-console-86bc9b7c8c-d96x2","server_roles":["license_master","cluster_search_head","search_head"],"startup_time":1600928786,"staticAssetId":"CFE3D41EE2CCD1465E8C8453F83E4ECFFF540780B4490E84458DD4A3694CE4D1","version":"8.0.5"}}],"paging":{"total":1,"perPage":30,"offset":0},"messages":[]}`, - splcommon.TestMCApplyChanges, + loadFixture(t, "get_server_info.json"), + loadFixture(t, "mc_apply_changes.json"), "", "", "", - `{"links":{"create":"/servicesNS/nobody/splunk_monitoring_console/data/ui/nav/_new","_reload":"/servicesNS/nobody/splunk_monitoring_console/data/ui/nav/_reload","_acl":"/servicesNS/nobody/splunk_monitoring_console/data/ui/nav/_acl"},"origin":"https://localhost:8089/servicesNS/nobody/splunk_monitoring_console/data/ui/nav","updated":"2020-09-23T23:50:25+00:00","generator":{"build":"a1a6394cc5ae","version":"8.0.5"},"entry":[{"name":"default.distributed","id":"https://localhost:8089/servicesNS/nobody/splunk_monitoring_console/data/ui/nav/default.distributed","updated":"1970-01-01T00:00:00+00:00","links":{"alternate":"/servicesNS/nobody/splunk_monitoring_console/data/ui/nav/default.distributed","list":"/servicesNS/nobody/splunk_monitoring_console/data/ui/nav/default.distributed","_reload":"/servicesNS/nobody/splunk_monitoring_console/data/ui/nav/default.distributed/_reload","edit":"/servicesNS/nobody/splunk_monitoring_console/data/ui/nav/default.distributed"},"author":"nobody","acl":{"app":"splunk_monitoring_console","can_change_perms":true,"can_list":true,"can_share_app":true,"can_share_global":true,"can_share_user":false,"can_write":true,"modifiable":true,"owner":"nobody","perms":{"read":["admin"],"write":["admin"]},"removable":false,"sharing":"app"},"fields":{"required":["eai:data"],"optional":[],"wildcard":[]},"content":{"disabled":false,"eai:acl":null,"eai:appName":"splunk_monitoring_console","eai:data":"