elastic / elastic/package-spec

[Change Proposal] Revisit linked-file-system handling in the mode-aware validator

Open
#1,185 0 comments 0 reactions 0 assignees View on GitHub
discuss
Dominant language
Go
Stars
20
Forks
93
Avg merge
15h 10m
Merged PRs (30d)
12

Description

**Origin:** PR #1177 (mode-aware validator API) — agreed to merge as-is and revisit.
**Anchor comment:** https://github.com/elastic/package-spec/pull/1177#discussion_r3362170814

## What problem is this solving?

The mode-aware validator decides whether `.link` files are **resolved** or **blocked**
by wrapping the filesystem in `linkedfiles.FS` vs `linkedfiles.BlockFS`, chosen per
validation mode. This logic is spread across `ValidateFromPath`, `ValidateFromZip`,
and `ValidateFromFS` in `code/go/pkg/validator/validator.go`, and `ValidateFromFS`
additionally runtime-type-switches on the concrete FS types to reject incompatible
FS/mode pairings.

The current shape has three interrelated problems:
- The per-mode wrapping decision is duplicated across three methods.
- `BlockFS` is an abstraction used in essentially one place.
- The type-switches are fragile: a pre-wrapped `*linkedfiles.FS` can bypass mode
semantics if a caller passes one unexpectedly.

It works and is fully internal, but the shape is not satisfying and should be cleaned
up before more modes/rules build on top of it.

## Where does this need to be implemented?

This is an internal refactor confined to `package-spec` only — no external consumers
are affected. The public API (`New`, `ValidateFrom*`) must remain unchanged.

Impacted files:
- `code/go/pkg/validator/validator.go` (`ValidateFromPath`, `ValidateFromZip`, `ValidateFromFS`)
- `code/go/internal/linkedfiles/fs.go` (`FS`, `BlockFS`, `NewFS`, `NewBlockFS`)

Behavior to preserve (covered by existing tests):
- **BuildMode:** `.link` files rejected.
- **SourceMode:** `.link` files resolved.
- **LegacyMode:** plain FS blocked; a caller-supplied `*linkedfiles.FS` is preserved.

## Options to evaluate

1. **Reject `.link` via a semantic rule** in build mode (like `_dev` / `external: ecs`
rejection) and drop `BlockFS` — *if* the spec always receives link files already
resolved. Open question: confirm whether that assumption holds.
2. **Push the decision into `linkedfiles`** — make `NewFS`/`NewBlockFS` idempotent /
aware of already-wrapped inputs so callers stop type-switching.
3. **Make `Mode` an interface** with a `wrapFS(location, fsys) fs.FS` method implemented
by `SourceMode`/`BuildMode`/`LegacyMode` — keeps FS wrapping but moves the per-mode
branch behind a polymorphic call.

## References

- Review threads: r3362170814, r3361725625, r3343192039, r3343206097, r3349318219, r3361751165

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.