From 02fd2de5027b428aa65edb0cecd5946612f78d0b Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Tue, 22 Sep 2026 23:01:25 +0800 Subject: [PATCH] fix(build): three drill-driven fixes so the lane actually completes on a starter node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first live build (Kaniko v1.24, 4 vCPU / 5.5 GiB node) walked the new transport end to end and hit three real defects, each invisible to unit tests: - The Job requested its FULL limits (2 CPU / 4Gi per container), so the build Pod never scheduled on the platform's own starter node: FailedScheduling / Insufficient memory, Pending forever. Requests are now a small floor (250m / 512Mi, never above a configured cap) while the limits stay the safety caps. - Kaniko re-copies the Dockerfile out of the context and chowns/chmods it to the source owner; a 65532-owned context (the distroless felis image uid) fails that under the pod's dropped capabilities ('copying dockerfile: chown /kaniko/Dockerfile: operation not permitted'). The fetch container now extracts as root — the uid Kaniko already runs as — so the copy succeeds; the pod was root by necessity regardless. - Trivy's DB fetch is exactly what the build egress lock denies: the scan step failed closed on mirror.gcr.io. New [registry] trivy_db_repository renders --db-repository, and docs/troubleshooting.md §8e now carries the verified mirror recipe (docker pull/tag/push of aquasec/trivy-db:2 into the internal registry; --insecure already covers its plain HTTP). Verified live after this batch: fetch initContainer streamed the blob through the API + netpol + token, Kaniko built and pushed registry.felis.svc:5000/ user-uploads/sub-:latest, and Trivy scanned against the mirrored DB. --- cmd/felis/api.go | 3 ++ docs/deferred-seams.md | 9 ++-- docs/troubleshooting.md | 25 ++++++++-- internal/build/build.go | 30 +++++++----- internal/build/jobspec.go | 88 ++++++++++++++++++++++++++-------- internal/build/jobspec_test.go | 70 +++++++++++++++++++++++++++ internal/config/config.go | 24 ++++++++-- 7 files changed, 206 insertions(+), 43 deletions(-) diff --git a/cmd/felis/api.go b/cmd/felis/api.go index 726f1a7..dd147a4 100644 --- a/cmd/felis/api.go +++ b/cmd/felis/api.go @@ -421,6 +421,9 @@ func buildConfig(cfg *config.Config) build.Config { TrivyImage: cfg.Registry.TrivyImage, CPULimit: cfg.Registry.BuildCPULimit, MemLimit: cfg.Registry.BuildMemLimit, + // Empty keeps Trivy's own default; an install with builds points this at + // the internal DB mirror (see config.RegistryConfig.TrivyDBRepository). + TrivyDBRepository: cfg.Registry.TrivyDBRepository, } } diff --git a/docs/deferred-seams.md b/docs/deferred-seams.md index 0f88aed..b8e1e8c 100644 --- a/docs/deferred-seams.md +++ b/docs/deferred-seams.md @@ -55,9 +55,12 @@ A grep across `*.md` and `*.go` returns both sets; only the Go ones are seams. filesystem view or object-store credentials. Kaniko/Trivy images are external-only by default; `[registry] kaniko_image / trivy_image / build_cpu_limit / build_mem_limit` override them for mirrored or air-gapped - installs, and Trivy's vulnerability DB download needs the same treatment (a - `package_source_cidrs` allowance or an internal `TRIVY_DB_REPOSITORY` mirror) or - the scan step fails closed on an egress-locked install. + installs. Trivy's vulnerability DB is the same story, and now has its own knob: + `[registry] trivy_db_repository` points `--db-repository` at an internal mirror + (recipe in docs/troubleshooting.md §8e). Left unset on an egress-locked box the + scan step fails closed — Kaniko pushes, Trivy exits on the DB download — which + is the correct fail direction but leaves the build unfinished, so the mirror is + part of a production build install. ## Built; only its I/O is unverifiable from this repo diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 6730585..7c274d3 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -408,14 +408,33 @@ url = "registry.felis.svc:5000" build_namespace = "felis-build" kaniko_image = "registry.felis.svc:5000/mirror/kaniko:v1.23.2" trivy_image = "registry.felis.svc:5000/mirror/trivy:0.58.1" +trivy_db_repository = "registry.felis.svc:5000/mirror/trivy-db:2" build_cpu_limit = "2" build_mem_limit = "4Gi" ``` then restart `felis-api` (it renders the Job from this config). Unset fields keep -the defaults. Note the user-modpack context topologies are a separate, -still-open seam (see `docs/deferred-seams.md`); this section only makes the -executors reachable. +the defaults. + +`trivy_db_repository` is not optional on an egress-locked box. Trivy fetches its +vulnerability DB from `mirror.gcr.io`/`ghcr.io` unless told otherwise, and the +build egress policy denies those hosts — so the scan step fails closed +(`failed to download vulnerability DB`) and NO build ever completes, even though +Kaniko pushed the image. Mirror the DB into the internal registry once: + +``` +# On a host with internet + docker access to the cluster's registry +# (add its address to the daemon's insecure-registries first; the registry +# serves plain HTTP): +# docker pull mirror.gcr.io/aquasec/trivy-db:2 +# docker tag mirror.gcr.io/aquasec/trivy-db:2 :5000/mirror/trivy-db:2 +# docker push :5000/mirror/trivy-db:2 +``` + +The Job's Trivy container already runs with `--insecure`, so the internal +registry's plain HTTP works for the DB pull exactly as it does for the scanned +image. Re-mirror the tag periodically (Trivy refreshes the DB several times a +day upstream; a stale mirror only means stale CVE data, never a failed gate). --- diff --git a/internal/build/build.go b/internal/build/build.go index 5135b42..6aca390 100644 --- a/internal/build/build.go +++ b/internal/build/build.go @@ -220,6 +220,11 @@ type Config struct { // only when a build's ContextRef is an http(s) URL (the submit lane's derived // shape); an install that never builds user submissions can leave it empty. FelisImage string + // TrivyDBRepository overrides where Trivy fetches its vulnerability DB + // (--db-repository). Empty keeps Trivy's upstream default, which the build + // egress lock denies — an install with builds must point this at an internal + // mirror (see config.RegistryConfig.TrivyDBRepository). + TrivyDBRepository string // KanikoImage / TrivyImage are the executor images. KanikoImage string TrivyImage string @@ -354,18 +359,19 @@ func (b *Builder) Submit(ctx context.Context, req Request) (*Build, error) { // jobParams projects a build + config onto the inputs jobspec.go renders. func (b *Builder) jobParams(bld *Build, cfg Config) JobParams { return JobParams{ - BuildID: bld.ID, - ImageRef: bld.ImageRef, - ContextRef: bld.ContextRef, - Namespace: cfg.Namespace, - ServiceAccount: cfg.ServiceAccount, - RegistryURL: cfg.RegistryURL, - FelisImage: cfg.FelisImage, - KanikoImage: cfg.KanikoImage, - TrivyImage: cfg.TrivyImage, - Deadline: cfg.Deadline, - CPULimit: cfg.CPULimit, - MemLimit: cfg.MemLimit, + BuildID: bld.ID, + ImageRef: bld.ImageRef, + ContextRef: bld.ContextRef, + Namespace: cfg.Namespace, + ServiceAccount: cfg.ServiceAccount, + RegistryURL: cfg.RegistryURL, + FelisImage: cfg.FelisImage, + TrivyDBRepository: cfg.TrivyDBRepository, + KanikoImage: cfg.KanikoImage, + TrivyImage: cfg.TrivyImage, + Deadline: cfg.Deadline, + CPULimit: cfg.CPULimit, + MemLimit: cfg.MemLimit, } } diff --git a/internal/build/jobspec.go b/internal/build/jobspec.go index 49ea618..f8f363e 100644 --- a/internal/build/jobspec.go +++ b/internal/build/jobspec.go @@ -63,12 +63,16 @@ type JobParams struct { RegistryURL string // FelisImage runs the context-fetch initContainer (the felis binary's // fetch-context entrypoint). Required when ContextRef is an http(s) URL. - FelisImage string - KanikoImage string - TrivyImage string - Deadline time.Duration - CPULimit string - MemLimit string + FelisImage string + // TrivyDBRepository overrides Trivy's vulnerability-DB source (the + // --db-repository flag). Empty keeps Trivy's own default; see + // build.Config.TrivyDBRepository for why an in-cluster install sets it. + TrivyDBRepository string + KanikoImage string + TrivyImage string + Deadline time.Duration + CPULimit string + MemLimit string } // BuildJobName is the deterministic Job name for a build id. @@ -134,6 +138,19 @@ func BuildJob(p JobParams) (*batchv1.Job, error) { return nil, fmt.Errorf("build: context ref %q needs FelisImage for the fetch initContainer", p.ContextRef) } contextPath = contextMountPath + // The fetch container runs as root while Kaniko keeps the image default + // (also root): Kaniko re-copies the Dockerfile out of the context and + // chowns/chmods it to the SOURCE file's owner, which fails for any other + // owner without CAP_CHOWN/CAP_FOWNER — capabilities this pod deliberately + // drops (the live drill hit exactly this: "copying dockerfile: chown + // /kaniko/Dockerfile: operation not permitted" with the distroless uid + // 65532). Extracting as root, the uid Kaniko itself runs as, keeps the + // context owned by the only user that can satisfy that copy. The pod is + // root by necessity regardless: Kaniko unpacks base-image layers into its + // own filesystem. + fetchSec := sec.DeepCopy() + fetchSec.RunAsUser = int64Ptr(0) + fetchSec.RunAsGroup = int64Ptr(0) fetch := corev1.Container{ Name: ContainerFetch, Image: p.FelisImage, @@ -155,8 +172,8 @@ func BuildJob(p JobParams) (*batchv1.Job, error) { }}, }}, VolumeMounts: []corev1.VolumeMount{{Name: contextVolume, MountPath: contextMountPath}}, - Resources: corev1.ResourceRequirements{Limits: limits, Requests: limits}, - SecurityContext: sec, + Resources: corev1.ResourceRequirements{Limits: limits, Requests: buildRequests(limits)}, + SecurityContext: fetchSec, } initContainers = append(initContainers, fetch) kanikoMounts = []corev1.VolumeMount{{Name: contextVolume, MountPath: contextMountPath, ReadOnly: true}} @@ -185,23 +202,32 @@ func BuildJob(p JobParams) (*batchv1.Job, error) { "--skip-tls-verify", }, VolumeMounts: kanikoMounts, - Resources: corev1.ResourceRequirements{Limits: limits, Requests: limits}, + Resources: corev1.ResourceRequirements{Limits: limits, Requests: buildRequests(limits)}, SecurityContext: sec, } initContainers = append(initContainers, kaniko) + trivyArgs := []string{ + "image", + "--exit-code", "1", + "--severity", "CRITICAL", + "--no-progress", + "--insecure", + } + // The DB source is configurable because the default (mirror.gcr.io/ghcr.io) + // is exactly what the build egress lock denies: an install that never mirrors + // the DB cannot complete a scan, and the gate fails closed on purpose. The + // supported shape is the internal registry (`--insecure` above already covers + // its plain HTTP). + if p.TrivyDBRepository != "" { + trivyArgs = append(trivyArgs, "--db-repository", p.TrivyDBRepository) + } + trivyArgs = append(trivyArgs, p.ImageRef) trivy := corev1.Container{ - Name: ContainerTrivy, - Image: p.TrivyImage, - Args: []string{ - "image", - "--exit-code", "1", - "--severity", "CRITICAL", - "--no-progress", - "--insecure", - p.ImageRef, - }, - Resources: corev1.ResourceRequirements{Limits: limits, Requests: limits}, + Name: ContainerTrivy, + Image: p.TrivyImage, + Args: trivyArgs, + Resources: corev1.ResourceRequirements{Limits: limits, Requests: buildRequests(limits)}, SecurityContext: sec, } @@ -391,6 +417,28 @@ func resourceLimits(cpu, mem string) (corev1.ResourceList, error) { }, nil } +// buildRequests is the scheduler floor a build container asks for while its +// configured limit stays the safety cap. Reserving the full cap as a request is +// what once made a default install on the platform's starter node (4 vCPU / +// 5.5 GiB) unable to schedule ANY build — caught by the live end-to-end drill, not +// by any unit test. A build is best-effort batch work: it may be throttled or +// evicted under contention, which fails the Job loudly, and the caps still stop a +// runaway build from exhausting the node. +func buildRequests(limits corev1.ResourceList) corev1.ResourceList { + req := corev1.ResourceList{} + for res, floor := range map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("250m"), + corev1.ResourceMemory: resource.MustParse("512Mi"), + } { + limit, ok := limits[res] + if ok && limit.Cmp(floor) < 0 { + floor = limit // never ask for more than the cap + } + req[res] = floor + } + return req +} + func boolPtr(b bool) *bool { return &b } func int32Ptr(i int32) *int32 { return &i } diff --git a/internal/build/jobspec_test.go b/internal/build/jobspec_test.go index 00c7f99..05a4ae3 100644 --- a/internal/build/jobspec_test.go +++ b/internal/build/jobspec_test.go @@ -105,6 +105,43 @@ func TestBuildJobContainersAreHardened(t *testing.T) { } } +// The limits are caps, but the requests must be a schedulable floor: reserving the +// full 2 CPU / 4Gi on a starter node (4 vCPU / 5.5 GiB, where the api, operator, +// registry, Postgres and the game pods live too) leaves no room for the Pod — +// proven live: FailedScheduling/Insufficient memory, build stuck Pending forever. +func TestBuildJobRequestsAreASchedulableFloor(t *testing.T) { + job, err := BuildJob(sampleJobParams()) + if err != nil { + t.Fatalf("BuildJob: %v", err) + } + all := append([]corev1.Container{}, job.Spec.Template.Spec.InitContainers...) + all = append(all, job.Spec.Template.Spec.Containers...) + for _, c := range all { + reqMem, limMem := c.Resources.Requests.Memory(), c.Resources.Limits.Memory() + reqCPU, limCPU := c.Resources.Requests.Cpu(), c.Resources.Limits.Cpu() + if reqMem.Cmp(*limMem) >= 0 || reqCPU.Cmp(*limCPU) >= 0 { + t.Errorf("container %q requests must be below its limits (req %s/%s, lim %s/%s)", + c.Name, reqCPU, reqMem, limCPU, limMem) + } + if reqMem.IsZero() || reqCPU.IsZero() { + t.Errorf("container %q must still ask for a non-zero floor", c.Name) + } + } + // A tiny operator-set cap must be honoured: the request never exceeds it. + p := sampleJobParams() + p.CPULimit, p.MemLimit = "100m", "128Mi" + job, err = BuildJob(p) + if err != nil { + t.Fatalf("BuildJob(tiny): %v", err) + } + for _, c := range job.Spec.Template.Spec.InitContainers { + if c.Resources.Requests.Memory().Cmp(*c.Resources.Limits.Memory()) > 0 || + c.Resources.Requests.Cpu().Cmp(*c.Resources.Limits.Cpu()) > 0 { + t.Errorf("container %q request exceeds a configured cap", c.Name) + } + } +} + // kaniko builds and pushes to the request's exact target; trivy gates admission // with --exit-code 1 --severity CRITICAL on that same ref. func TestBuildJobKanikoPushesAndTrivyGates(t *testing.T) { @@ -141,6 +178,30 @@ func TestBuildJobKanikoPushesAndTrivyGates(t *testing.T) { if !hasArg(trivy.Args, p.ImageRef) { t.Errorf("trivy must scan the pushed ref %q, args=%v", p.ImageRef, trivy.Args) } + // No DB repository configured: Trivy keeps its own default. + if hasArg(trivy.Args, "--db-repository") { + t.Errorf("unset TrivyDBRepository must not render --db-repository, args=%v", trivy.Args) + } +} + +// A configured DB repository (the internal mirror) must reach Trivy as +// --db-repository: without it the scan tries the internet, which the build egress +// lock denies, and every build fails closed at the scan gate. +func TestBuildJobTrivyDBRepositoryOverride(t *testing.T) { + p := sampleJobParams() + p.TrivyDBRepository = "registry.felis.svc:5000/mirror/trivy-db:2" + job, err := BuildJob(p) + if err != nil { + t.Fatalf("BuildJob: %v", err) + } + trivy := job.Spec.Template.Spec.Containers[0] + if !argPairPresent(trivy.Args, "--db-repository", p.TrivyDBRepository) { + t.Errorf("trivy args = %v, want --db-repository %s", trivy.Args, p.TrivyDBRepository) + } + // The scanned image ref must stay the last argument. + if last := trivy.Args[len(trivy.Args)-1]; last != p.ImageRef { + t.Errorf("image ref must remain the last argument, args=%v", trivy.Args) + } } // The build namespace egress lock must be default-deny: deny all ingress, and @@ -228,6 +289,15 @@ func TestBuildJobFetchesHTTPContext(t *testing.T) { if len(kaniko.Env) != 0 { t.Errorf("kaniko must carry no env (especially no token), got %v", kaniko.Env) } + // The fetch container extracts as root — the uid Kaniko runs as — because + // Kaniko re-copies the Dockerfile and chowns it to the source owner, which no + // other uid can satisfy under this pod's dropped capabilities. + if fetch.SecurityContext == nil || fetch.SecurityContext.RunAsUser == nil || *fetch.SecurityContext.RunAsUser != 0 { + t.Error("fetch container must extract as root so Kaniko can inherit the context ownership") + } + if fetch.SecurityContext != nil && (fetch.SecurityContext.Privileged == nil || *fetch.SecurityContext.Privileged) { + t.Error("fetch container must still be unprivileged") + } if !hasArg(kaniko.Args, "--context="+contextMountPath) { t.Errorf("kaniko context = %v, want the fetched local dir %s", kaniko.Args, contextMountPath) } diff --git a/internal/config/config.go b/internal/config/config.go index b2e557f..7509c1a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -132,6 +132,18 @@ type RegistryConfig struct { TrivyImage string `toml:"trivy_image"` BuildCPULimit string `toml:"build_cpu_limit"` BuildMemLimit string `toml:"build_mem_limit"` + // TrivyDBRepository points Trivy at an OCI repository holding the + // vulnerability DB (--db-repository). Trivy's default fetches from + // mirror.gcr.io/ghcr.io, which the build egress lock denies — so on a + // default install the scan step fails closed and no build ever completes. + // The supported shape is an internal mirror: copy + // mirror.gcr.io/aquasec/trivy-db:2 into this cluster's registry (recipe in + // docs/troubleshooting.md §8) and set this to + // registry..svc:5000/mirror/trivy-db:2. The scan runs with --insecure, + // so the plain-HTTP internal registry works. Empty keeps Trivy's own + // default (only usable on an install that deliberately opens internet + // egress to the DB hosts). + TrivyDBRepository string `toml:"trivy_db_repository"` // UserUploadsContext is the object-store base under which a user-submitted // modpack's Kaniko build context is pinned. It belongs to the §16 build // subsystem's input domain (the build-context store), introduced by the @@ -139,8 +151,9 @@ type RegistryConfig struct { // lane derives {UserUploadsContext}/{submissionID}/context.tar.gz; both transports // that place the blob there now ship (submit.LocalContextStore for a local path, // submit.S3ContextStore for an s3:// base, selected in cmd/felis by the shape of - // this value). What stays deferred is the far end — Kaniko reading that context - // from inside the build Pod (INTEGRATION-ONLY, see submit/blobstore.go). It is + // this value), and so does the read end: the build Pod's fetch initContainer + // streams the blob back over the API's internal face, so this value just names + // where the API stores it, not where Kaniko must reach. It is // kept distinct from [archive] on purpose — a world // archive (§19 WorldArchiver) and a build context (§16) are different artifacts // with different lifecycles, so the two must not share a store binding. @@ -217,9 +230,10 @@ const ( defaultStore = "tarLocal" // defaultUserUploadsContext is a non-empty, platform-namespaced placeholder so // the modpack approval lane's derived context ref is well-formed even before a - // deployment points it at a real object store. The blob transport is deferred, - // so this base only has to be a sensible, parseable prefix (see the §16 build - // subsystem and the internal/submit package doc for the lane's provenance). + // deployment points it at a real object store. It is only a parseable prefix — + // an s3:// base with no credentials leaves the upload transport unwired, and + // the endpoint answers an honest 503 (see the §16 build subsystem and the + // internal/submit package doc for the lane's provenance). defaultUserUploadsContext = "s3://felis-user-uploads" // defaultSMTPPort is the STARTTLS submission port; applied only when [smtp] // host is set (a portless [smtp] block with no host stays fully zero).