elastic / elastic/beats

[refactor-opportunist] Extract shared file-auth transport for CEL and HTTPJSON inputs

Open
#49,602 1 comment 0 reactions 0 assignees View on GitHub
needs_team
Dominant language
Go
Stars
12.7k
Forks
5k
Avg merge
2d 15m
Merged PRs (30d)
385

Description

## 🏗️ Refactor Proposal

**Summary:** Consolidate duplicated file-based auth transport logic into `x-pack/filebeat/input/internal/fileauth` and migrate HTTP inputs incrementally.

## Problem
`cel` and `httpjson` each maintain nearly identical file-auth transport implementations (permission checks, token reload, header injection), creating two maintenance points for the same behavior.

Concrete duplication evidence:
- `x-pack/filebeat/input/cel/file_auth.go:17-117` contains full file-auth transport lifecycle (`newFileAuthTransport`, `RoundTrip`, refresh logic).
- Before this POC, `x-pack/filebeat/input/httpjson/file_auth.go` contained the same logic; this POC removes that duplicate block and replaces it with a shared call.
- `x-pack/filebeat/input/cel/config_auth.go:141-185` and `x-pack/filebeat/input/httpjson/config_auth.go:83-127` also carry mirrored file-auth config helpers (`headerName`, `refreshInterval`).

This duplication increases drift risk when changing auth-file permission policy, refresh behavior, or request header handling.

## Proposed Approach
Create a shared internal auth transport package and migrate one input at a time:
- Introduce `x-pack/filebeat/input/internal/fileauth` with shared `Config` + `Transport`.
- Keep per-input config schema and wiring unchanged.
- Replace package-local transport implementations with thin adapters to shared code.
- Migrate `httpjson` first (done in POC), then `cel`, then optionally consolidate repeated config helper methods.

## Proof of Concept
I partially implemented this refactor on one representative slice to verify viability.

**Files changed:**
- `x-pack/filebeat/input/internal/fileauth/transport.go` (new shared implementation)
- `x-pack/filebeat/input/httpjson/file_auth.go` (now thin adapter to shared transport)
- `x-pack/filebeat/input/httpjson/file_auth_test.go` (test adjusted to black-box refresh timing)

**Before → After:**
- Before: `httpjson/file_auth.go` had a full local transport implementation (~113 lines of logic).
- After: `httpjson/file_auth.go` delegates to:
- `fileauth.NewTransport(fileauth.Config{...}, base)` (`x-pack/filebeat/input/httpjson/file_auth.go:13-24`)
- shared logic in `x-pack/filebeat/input/internal/fileauth/transport.go:40-119`.

**Verification:**
- `go test ./x-pack/filebeat/input/httpjson ./x-pack/filebeat/input/cel` ✅
- `go test ./x-pack/filebeat/input/internal/fileauth` ✅ (`[no test files]`, package compiles)
- Existing targeted tests pass after extraction.

## Incremental Rollout Plan
This refactor can be completed incrementally:
1. Migrate `httpjson` to shared file-auth transport (completed in POC).
2. Migrate `cel` to the same shared transport and remove `x-pack/filebeat/input/cel/file_auth.go` duplicate logic.
3. Extract shared file-auth config helper methods (`headerName`, `refreshInterval`) used by both inputs.

## Risks and Mitigations
- Behavior drift in auth header/refresh semantics: preserve existing tests and add shared package tests during `cel` migration.
- Import-cycle risk from `internal` package boundaries: keep shared package dependency-free (only stdlib).
- Timing-sensitive tests becoming flaky: prefer black-box assertions with bounded waits over internal clock field mutation.

## Evidence
- Duplicate transport implementation:
- `x-pack/filebeat/input/cel/file_auth.go:17-117`
- (previously mirrored in `x-pack/filebeat/input/httpjson/file_auth.go`, replaced in POC)
- Current shared implementation:
- `x-pack/filebeat/input/internal/fileauth/transport.go:40-119`
- Current HTTPJSON adapter:
- `x-pack/filebeat/input/httpjson/file_auth.go:13-24`
- Repeated config helpers still present:
- `x-pack/filebeat/input/cel/config_auth.go:173-185`
- `x-pack/filebeat/input/httpjson/config_auth.go:115-127`

This is distinct from #49402 (OAuth2 validation duplication) and #49084 (tracer path-validation helpers); it targets file-auth transport/config consolidation in CEL + HTTPJSON.

---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Refactor Opportunist](https://github.com/elastic/beats/actions/runs/23442389513)

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

Contributor guide

Open the contributing guide

Research direction

Start with x-pack/filebeat/input/cel/file_auth.go and the shared implementation in x-pack/filebeat/input/internal/fileauth/transport.go, then compare the HTTPJSON adapter in x-pack/filebeat/input/httpjson/file_auth.go. Migrate CEL while preserving its config wiring, add shared-package coverage for refresh and header behavior, and run go test ./x-pack/filebeat/input/httpjson ./x-pack/filebeat/input/cel plus the internal package.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.