feat: Phase 2b — ingest and ETL layer - #3
Closed
TheMeinerLP wants to merge 32 commits into
Closed
Conversation
Adds the WorldSource and WorldIngest CRDs (bluemap.onelitefeather.net/v1alpha1, namespaced) that Phase 2b's ingest layer builds on, plus the shared BundleRef model. Every nested group is initialised in its field declaration and initSpec()/initStatus() are overridden on both resources, avoiding the two Phase 2a traps that previously blocked parallel work. Also sets up the new ingest module with the AWS SDK v2 S3 client dependency.
Recognizes vanilla, bukkit, and nested (ZIP upload) directory layouts for an ingested world and resolves each dimension to its region directory. Falls back to a loud LayoutDetectionException, naming the paths actually found, instead of guessing when no known layout matches -- including when a forced layout kind does not fit the structure on disk.
Writes region files for every dimension first and the manifest last, so an interrupted write never leaves a manifest behind for an incomplete bundle version. The manifest carries the region list per dimension (read from r.<x>.<z>.mca file names), a SHA-256 checksum over the uploaded bytes, and serialises to/from JSON via a small self-contained codec (no new dependency). S3Client is a one-method facade over the AWS SDK v2 client, narrow enough to fake in tests.
… escapes LayoutDetector walks directory trees extracted from untrusted sources (Pterodactyl backups, user-uploaded ZIPs). Validate the world name against path separators and ".." segments, verify every candidate dimension path resolves (via toRealPath) inside the given root, and reject symlinks outright via NOFOLLOW_LINKS so a crafted archive can no longer point the bundle writer at files outside the working directory.
… pull sources Adds WorldSourceConnector/SourceVersion and two implementations: S3SourceConnector (delimiter-scoped object listing per prefix, zip/tar.gz-aware fetch) and PterodactylConnector (Client API backup listing + signed-URL download, verified against the panel's own source on GitHub). The Pterodactyl backup is a tar.gz of the whole server, so fetch() streams it exactly once through a hand-rolled tar reader and writes only the configured world paths, never buffering the archive. No archive or JSON library was available to this module's dependency set, so Archives/TarStreamReader and MinimalJson were added as small, purpose-scoped helpers rather than reaching for a new dependency.
…talog Three parallel agents left the ingest module without a JSON library and without the Testcontainers dependency, since none of them were allowed to touch build files. BundleManifest and connector/MinimalJson each grew their own hand-rolled JSON codec, and S3SourceConnectorTest drove MinIO through the raw docker CLI instead. Jackson is already pulled in transitively by fabric8's kubernetes-client for the operator module; this is the first place it's declared explicitly. Version 2.22.1 verified against Maven Central. Testcontainers catalog entries already existed for operator/runner -- just add them here too.
BundleManifest and PterodactylConnector each carried their own recursive-descent JSON parser because the module had no JSON library to share. Both now use jackson-databind: BundleManifest (de)serialises directly through its records via Jackson's built-in record support (no annotations needed), and PterodactylConnector reads panel responses as JsonNode instead of a hand-rolled Map<String,Object>. MinimalJson is gone. BundleManifest keeps the same IllegalArgumentException contract on bad input (non-object root, trailing content after the JSON value) by enabling DeserializationFeature.FAIL_ON_TRAILING_TOKENS and translating JsonProcessingException at the boundary. No test was weakened: the same round-trip, human-readable-field, null-handling and rejection tests pass unchanged against the new implementation.
S3SourceConnectorTest drove a MinIO container through ProcessBuilder and the raw docker CLI because the module had no Testcontainers dependency. Switch to org.testcontainers.containers.MinIOContainer, the same mechanism operator and runner already use for their own container-based tests. Testcontainers' Ryuk reaper removes the container even if a test crashes, which the previous docker run/docker stop pairing could not guarantee. Same real MinIO instance, same four tests, same assertions -- only the container lifecycle mechanism changed.
TarStreamReader handles PAX path overrides (typeflag 'x') alongside the GNU long-name mechanism (typeflag 'L'), but only the GNU case had a test. Both extend a tar entry's path past the classic 100-byte ustar name field, and a Pterodactyl backup of a server with a deep plugin/world tree routinely contains both -- add the missing PAX case, built the same way the GNU one already is. Also checked the security question this raises for real: TarStreamReader unpacks archives from untrusted sources (Pterodactyl backups, S3 objects), the same trust boundary LayoutDetector was recently hardened against for path traversal and symlink escapes. Archives.extractTar already resolves and contains every entry path (resolveSafely), and neither it nor TarStreamReader ever calls Files.createSymbolicLink, so a typeflag '2' entry lands as an inert regular file rather than a real symlink -- no fix was needed here. Added ArchivesTest as regression coverage for both: a '../' entry is rejected before anything is written, and a symlink-typed entry never becomes a real filesystem symlink.
…riter BundleWriter.write() had no way to fill manifest.source.type or manifest.minecraftVersion because task 3 (bundle writer) and task 4 (connectors) were built in parallel worktrees, neither aware of the other's concrete values. Only the orchestrator knows both, so extend write()'s signature to accept them and pass them straight into the manifest. Also make WorldLayout implement BundleWriter.WorldLayoutLike: task 2 and task 3 defined the same shape (kind()/dimensions()) independently so neither had to wait on the other, but nothing ever declared the record to actually satisfy the interface, which only surfaces once something (the ingest orchestrator) tries to pass a WorldLayout to BundleWriter directly.
IngestMain wires the pieces built in tasks 1-4 into a runnable Kubernetes Job, the ingest-side analog of runner/: read and validate every environment variable up front (fail with a clear message and a non-zero exit before anything is fetched), pick the S3 or Pterodactyl connector, fetch into a work directory, detect the layout, then write the bundle. Progress goes to stdout as periodic, throttled lines plus phase markers -- no HTTP server, since the job is short-lived and its Kubernetes Job/Pod status already gives a reconciler the coarse state it needs; see ingest/README.md's "Design notes" for the full reasoning, including why the Minecraft version comes from APUS_MC_VERSION rather than parsing level.dat. Also split S3SourceConnectorTest into its own :ingest:integrationTest task, matching the runner/operator convention: container-backed tests must not be part of the routine build/check.
…ingest reconcilers WorldSourceReconciler needs to evaluate WorldSource.spec.poll (a Cron expression) and to call the ingest connectors' discover() directly; WorldIngestReconciler's retention enforcement needs to list/delete objects in the bundle bucket. Adds cron-utils 9.2.1 to the version catalog (verified against Maven Central; its only non-test runtime dependency is slf4j-api) instead of hand-rolling a cron parser, and wires the operator module to the ingest module and the AWS SDK S3 client.
Closes the loop from a configured WorldSource to a rendered map: WorldSourceReconciler evaluates spec.poll on a Cron schedule (CronSchedule, backed by cron-utils), calls the matching WorldSourceConnector.discover(), and creates one deterministically-named WorldIngest per configured world when a new source version appears -- idempotent by construction, so a retried reconcile never double-triggers an ingest. WorldIngestReconciler builds the ingest Job via IngestJobBuilder, enforces that only one ingest per WorldSource runs at a time via an optimistic lock on WorldSource.status.activeIngest (mirroring BlueMapRenderReconciler's map-level lock), mirrors best-effort progress parsed from the ingest pod's log lines (IngestLogProgress) into status, and on success writes WorldSource.status.latestBundle before enforcing spec.retention.keepVersions -- never deleting a bundle version still referenced by any BlueMapRender (AwsBundleStore / BundleStore). Both reconcilers follow the phase 2a patterns: name+UID ownership checks before adopting an existing resource (WorldIngestReconciler on the Job, WorldSourceReconciler on a colliding WorldIngest name), client.supports()-free since neither CRD is foreign, and the shared Labels class (extended with SOURCE/SOURCE_UID) for every created resource. WorldSourceStatus gains an ActiveIngest lock field (mirroring BlueMapMapStatus.latestRender) and OperatorConfig gains the operator-wide ingest image and bundle-destination settings (bundleBucket, bundleS3Endpoint, bundleS3Region, bundleCredentialsSecretName) neither CRD carries a field for. Both reconcilers are registered in ApusOperator.
…tion tests Widens the entry point's visibility from package-private to public so a container-based integration test in another module can drive the real fetch -> detect -> write orchestration instead of reimplementing it by hand. Also corrects two stale forward-references (IngestMainTest javadoc, ingest/README.md) that assumed this end-to-end test would live in :ingest:integrationTest -- it lives in :runner:integrationTest instead, next to the render machinery it also needs.
Adds IngestRenderContractTest to 🏃integrationTest: builds a Bukkit-layout world (sibling world/world_nether/world_the_end folders) from testdata/mini-world's real region files, runs the real IngestMain.run entry point against a Testcontainers MinIO, then asserts the resulting manifest is complete with the three normalised logical dimension names and a region list matching what was actually written. Cross-checks the manifest's claims against a real S3 object listing -- including that manifest.json's own lastModified is never older than any region file's, proving it really is the bundle's last-written object against a real backend, not just a fake client's call log. Finally takes the bundle's own overworld dimension path, builds the same bundleUrl shape a BlueMapRender carries, and starts the real apus/runner:dev image (phase 1) against it, asserting a genuine rendered tile lands in the map bucket -- the actual proof that the contract between the ingest and render halves of phase 2b holds, not just that each half works in isolation. Reuses MinioFixtures's MinIO/network/ runner-container helpers rather than duplicating them. Depends on :ingest from :runner's test source set, the same "depend on the module instead of duplicating its logic" pattern operator/ build.gradle.kts already uses.
…oi, record true source ref BundleWriter wrote bundles under <tenant>/<worldId>/<version> with no source-scoping segment, so two WorldSources ingesting a world with the same id (e.g. the vanilla default "world") would silently share a bucket prefix; a retention pass for one could delete the other's still-referenced bundle. BundlePath is now the single place this path is built (previously duplicated in BundleWriter, AwsBundleStore and WorldIngestReconciler), and every caller includes the owning source's name. Also: the writer only ever copied *.mca region files, dropping level.dat, entities/ and poi/ even though the bundle spec lists all three as bundle content; and manifest.source.ref held the bundle's own version identifier instead of the source's, permanently losing where a bundle actually came from (e.g. a Pterodactyl backup UUID). Both fixed in the same write() pass: level.dat is read from the overworld dimension's region directory's parent, entities/poi are included per-dimension where present, and the caller now supplies the real source version id for source.ref.
…ve limits APUS_BUNDLE_SOURCE_NAME is now a mandatory env var (see the bundle-path commit) so IngestConfig always has it available for BundleWriter. Also adds APUS_MAX_ARCHIVE_TOTAL_BYTES/APUS_MAX_ARCHIVE_ENTRIES, both optional with generous defaults, threaded into each connector's source config map so Archives can enforce them during extraction.
Archives.extractZip/extractTar wrote every entry with no cap on count, per-entry size, or total volume. The ingest job mounts no volume and sets no resource limit for its work directory, so extraction lands on the container's writable layer backed by the node's own disk -- a crafted "archive bomb" (or just an unexpectedly large legitimate archive) could fill it and degrade every other pod scheduled on that node. Both limits are now enforced against bytes/entries actually produced during extraction (a counting OutputStream wrapper for zip and for tar's transferTo), not a declared/expected size -- entry metadata is attacker-controlled input and, for zip in particular, not always even present. Exceeding either limit aborts extraction with a clear IOException. Limits default to generous values (5 GiB / 200k entries) and are configurable via IngestConfig (previous commit); the existing unbounded overloads are kept so callers that intentionally want no cap (tests) are unaffected. S3SourceConnector.fetch now passes Archives.limitsFrom(config) through to extraction; PterodactylConnector's wiring is in the next commit alongside its other hardening.
…ion, no URL leakage
Several independent problems in the same connector:
- No connect or request timeout on its HttpClient, so a panel that accepts
a connection and never responds could hang the calling thread forever.
discover() runs directly inside WorldSourceReconciler, whose JOSDK worker
pool is shared across all five reconcilers, so a hung panel would starve
unrelated render/ingest reconciliation too. Both timeouts are now set
(30s), including on the archive download request itself -- that timeout
only bounds receiving the response, not the full multi-GB body transfer.
- A failure streaming the backup archive embedded the signed download URL
directly in the exception message, which lands on stderr and therefore in
log aggregation. The signed URL is a short-lived but fully-privileged
credential for the entire backup; the message now names only the host.
- discover() assumed the backups-list response always has the
{"object":"list","data":[...]} envelope without checking. A response that
doesn't match (different API version, a proxy error page returned with a
2xx, ...) made root.path("data") resolve to an empty Jackson node, which
iterates as zero backups -- reported as a perfectly healthy "no versions
available yet" instead of a visible failure. Now validated explicitly and
rejected with a clear error if the envelope doesn't match.
- fetch() now also passes Archives.limitsFrom(config) through to
extractTar, completing the archive-extraction bound from the previous
commit for this connector too.
BundleStore.listVersions/deleteVersion took only tenant+worldId, matching the pre-fix bundle path shape. Now takes sourceName too and uses the shared BundlePath (ingest module) to build prefixes, keeping AwsBundleStore's listing/deletion scoped to one source's own bundles -- see the bundle-path commit for why worldId alone was never enough to keep two sources from colliding.
…urce ownership Four related correctness bugs in WorldIngestReconciler, found together in the same review pass: - applyProgress() copied the ingest pod's log phase verbatim into status.phase, including terminal values. IngestMain logs "phase=Succeeded" immediately before its process exits -- strictly before the Job controller can have observed the pod's exit and set status.succeeded. A reconcile landing in that window would adopt Succeeded from the log, and reconcile() treats any already-terminal ingest as a permanent no-op on every future call: onJobSucceeded (which fills WorldSource.status.latestBundle and WorldIngest.status.bundle) then never runs, even once the Job genuinely succeeds. Terminality now belongs exclusively to isJobSucceeded/isJobFailed, evaluated against the Job's own status; applyProgress only ever advances to a non-terminal phase. - isJobFailed() also treated status.failed > 0 (any failed pod attempt) as the whole Job having failed, even though backoffLimit exists precisely so a transient failure gets retried. The first failed attempt would end the ingest terminally while the Job kept running underneath it and could still write a complete bundle on a later attempt -- which then has nowhere to be registered, since a terminal ingest is never reconciled again. Now only the Job's own Failed condition (set once backoffLimit is exhausted) counts. - The reconciler resolved WorldSource purely by spec.sourceRef.name and then wrote its status and drove retention (including deletions) against it, without checking the labels WorldSourceReconciler stamps on every WorldIngest it creates. A hand-written or stale WorldIngest -- e.g. its original source was deleted and a different source created under the same name, giving it a different UID -- could control a source it never triggered. Applies the same name+UID owner-check pattern already used elsewhere in this module: a mismatch reports ResourceConflict instead of proceeding. - applyRetention now also protects every WorldSource's recorded status.latestBundle in the namespace, not just the source the current pass belongs to -- a safety net alongside (not instead of) the source-scoped bundle path from the BundlePath commit. New/updated tests in WorldIngestReconcilerTest cover each of the four (logReportingSucceededBeforeTheJobStatusDoesNotEndTheIngestPrematurely, aSingleFailedPodAttemptDoesNotEndTheIngestWhileTheJobStillHasRetriesLeft, anIngestWithoutTheSourceOwnerLabelsIsReportedAsAConflictAndNeverTouchesTheSource, twoSourcesWithTheSameWorldNameNeverCollideOnTheSameBundlePath, retentionNeverDeletesAVersionRecordedAsAnotherSourcesLatestBundle); existing tests updated for the new source-scoped bundle path and the Job Failed condition.
…inally failed Same mistake as WorldIngestReconciler's isJobFailed (previous commit): BlueMapRenderReconciler counted status.failed > 0 (any failed pod attempt) as the whole Job having failed, even though backoffLimit exists so a transient failure gets retried. Now only the Job's Failed condition counts. New test: aSingleFailedPodAttemptDoesNotEndTheRenderWhileTheJobStillHasRetriesLeft.
…ephemeral storage manifest.minecraftVersion was a mandatory bundle-spec field the operator never filled: IngestJobBuilder never set APUS_MC_VERSION, so it stayed permanently empty. Reading it from level.dat was considered and rejected -- this project deliberately carries no NBT-parsing dependency, and level.dat is genuinely absent for connectors that only fetch specific region files. Instead WorldSource.spec.worlds[].minecraftVersion is a new optional field the tenant sets directly (they already know which version they run); IngestJobBuilder reads it for the matching world selector and passes it through as APUS_MC_VERSION, same as it already does for layout. Also sets APUS_BUNDLE_SOURCE_NAME (see the bundle-path commit) and gives the ingest container an ephemeral-storage request/limit: it mounts no volume for its work directory or the archive it extracts, so even a legitimate large world could otherwise starve the node's disk. This is defense in depth alongside (not instead of) Archives' own configurable extraction limits.
Updates the environment-variable contract table and the "Minecraft version" design note for APUS_BUNDLE_SOURCE_NAME (now mandatory) and the new APUS_MAX_ARCHIVE_TOTAL_BYTES/APUS_MAX_ARCHIVE_ENTRIES variables added in prior commits.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 35899484 | Triggered | Generic High Entropy Secret | c1d2ef4 | ingest/src/main/java/net/onelitefeather/apus/ingest/IngestConfig.java | View secret |
| 35899483 | Triggered | Generic High Entropy Secret | 1ab1a89 | operator/src/main/java/net/onelitefeather/apus/operator/ingest/IngestJobBuilder.java | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
…rdcoding them Same fix as feat/phase-1-render-kern's MinioFixtures: S3SourceConnectorTest hardcoded the well-known MinIO default access/secret key pair for its throwaway Testcontainers instance. Generate a fresh, random pair per test run instead, long enough to satisfy MinIO's own minimum key lengths.
…onfig The strings the secret scanner flagged were not only in old commits: the phase 1 plan carried them in a code sample, and .gitguardian.yaml listed them in plaintext, which made the exemption file a finding of its own. The plan sample now shows a placeholder, and the exemptions are SHA256 digests.
This was referenced Aug 9, 2026
Contributor
Author
This was referenced Aug 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2b of Apus: the ingest/ETL layer. Stacked on PR #2 (phase 2a), which must land first.
A
WorldSourceis now configured once and Apus takes it from there: it polls for newworld data, normalises whatever layout it finds into a versioned World Bundle in S3, and
the render path picks it up without knowing where the world came from.
The contract, proven end to end
The whole point of this layer is that the render path stays ignorant of world origins.
That was a claim until now.
IngestRenderContractTestruns the real ingest against aBukkit-layout world in real MinIO, then renders the resulting bundle with the phase 1
runner image — and a tile lands in the map bucket. No production code had to change
for the contract to hold.
What this delivers
LayoutDetectoroverworld/the_nether/the_endBundleWriterS3SourceConnector,PterodactylConnectorWorldSource/WorldIngestCRDs + reconcilersingestcontainer imageAdding a source later costs one connector:
discover()andfetch(). Transform andload are shared.
Security findings fixed in this branch
This is the layer that handles untrusted input — unpacked server backups and
uploaded archives — so the findings here matter more than elsewhere.
LayoutDetectorfollowed symlinks and did not confine paths to the work root. Acrafted archive with
world/region -> /etcwould have put system files into a bundle.Paths are now resolved and confined, symlinks rejected, world names validated.
evict unrelated pods. Bounded by total size and entry count, and the job now declares
ephemeral-storage.are short-lived credentials for an entire server backup.
WorldIngestcould drive a foreignWorldSource— writing its status anddeleting its bundles — because only the name was checked. Ownership is now verified by
name and UID.
Two critical correctness bugs, both caught by review
An ingest could report success while its bundle stayed invisible. The reconciler took
the terminal phase from the pod log, but
IngestMainlogsphase=Succeededjust beforeexiting — before the Job controller sets
succeeded. A reconcile landing in that windowmarked the resource terminal, so the completion handler never ran: no
latestBundle, noretention, no retry, and a complete bundle sitting in S3 that nothing referenced.
Terminality now comes only from Job status.
Retention could delete another source's live bundle. Bundle paths were keyed by
tenant and world name, so two sources with a world called
world— the Minecraft default— shared a prefix, and pruning crossed the boundary. Paths are now scoped by source, and
every source's
latestBundlein the namespace is protected.Known gaps
not panel source. It is now rejected if it does not match, instead of silently looking
like "no backups available".
minecraftVersioncomes from a new field on the world selector rather than fromlevel.dat— the project carries no NBT dependency by design.Testing
160 operator tests, 65 ingest tests, plus integration tests against real MinIO and k3s.
Container-based tests stay out of
build:./gradlew :ingest:integrationTest,./gradlew :runner:integrationTest.A re-review of the final fix wave is still outstanding — the two critical fixes were
verified directly, the remainder rests on the fix agent's own test evidence.