refactor: simplify schemas, manifest builder, and registry error hand…

.changeset/yellow-pens-retire.md

@@ -0,0 +1,5 @@
+---
+"docker-tar-pusher": patch
+---
+
+Fix layer accumulation bug when pushing images with multiple RepoTags. Refactor ManifestBuilder into a pure function, consolidate option schemas with defaults via v.parse, and use isAxiosError consistently in registry error handling.

.env.test

@@ -1 +1 @@
-REGISTRY_URL=http://localhost:5001
+REGISTRY_URL=http://localhost:15000

.github/workflows/check-pr.yml

@@ -52,8 +52,6 @@ jobs:
         run: pnpm build
 
       - name: Run tests
-        env:
-          REGISTRY_URL: 'http://localhost:5000'
         run: pnpm test --coverage
 
       - name: Report Coverage

compose.yaml

@@ -0,0 +1,6 @@
+services:
+  registry:
+    image: registry:2
+    ports:
+      - "15000:5000"
+    restart: always

package.json

@@ -28,7 +28,7 @@
     "docker image",
     "docker tar"
   ],
-  "author": "Károly Pákozdi <karoly.pakozdi.150@gmail.com>",
+  "author": "Károly Pákozdi <karoly@pkzd.hu>",
   "license": "MIT",
   "devDependencies": {
     "@biomejs/biome": "2.2.6",
@@ -48,5 +48,5 @@
     "tar": "^6.1.11",
     "valibot": "^1.1.0"
   },
-  "packageManager": "pnpm@10.18.3+sha512.bbd16e6d7286fd7e01f6b3c0b3c932cda2965c06a908328f74663f10a9aea51f1129eea615134bf992831b009eabe167ecb7008b597f40ff9bc75946aadfb08d"
+  "packageManager": "pnpm@10.28.2+sha512.41872f037ad22f7348e3b1debbaf7e867cfd448f2726d9cf74c08f19507c31d2c8e7a11525b983febc2df640b5438dee6023ebb1f84ed43cc2d654d2bc326264"
 }

src/dtp/DockerRegistryService.ts

@@ -2,7 +2,7 @@ import { createHash } from "node:crypto";
 import { createReadStream } from "node:fs";
 import { stat } from "node:fs/promises";
 import { join } from "node:path";
-import type { AxiosInstance } from "axios";
+import { type AxiosInstance, isAxiosError } from "axios";
 import { createInstance } from "../config/axios";
 import RegistryError from "../errors/RegistryError";
 import UploadError from "../errors/UploadError";
@@ -29,26 +29,24 @@ export default class DockerRegistryService {
 
   public async upload(cwd: string, image: string, file: string) {
     const uploadUrl = await this.initiateUpload(image);
-    const chunkMetaData = await this.pushFileInChunks(cwd, uploadUrl, file);
-    return chunkMetaData;
+
+    return await this.pushFileInChunks(cwd, uploadUrl, file);
   }
 
   public async pushManifest(
     manifest: RegistryManifest,
     image: string,
     tag: string,
   ): Promise<void> {
-    const headers = {
-      [RequestHeaders.CONTENT_TYPE]: ContentTypes.APPLICATION_MANIFEST,
-    };
     const url = `${this.config.registryUrl}/v2/${image}/manifests/${tag}`;
     try {
-      await this.axios.put(url, manifest, { headers });
+      await this.axios.put(url, manifest, {
+        headers: {
+          [RequestHeaders.CONTENT_TYPE]: ContentTypes.APPLICATION_MANIFEST,
+        },
+      });
     } catch (e) {
-      const statusCode =
-        e instanceof Error && "response" in e
-          ? (e as Error & { response?: { status?: number } }).response?.status
-          : undefined;
+      const statusCode = isAxiosError(e) ? e.response?.status : undefined;
       throw new RegistryError(
         `Failed to push manifest for ${image}:${tag}`,
         statusCode,
@@ -118,10 +116,7 @@ export default class DockerRegistryService {
       const { headers } = await this.axios.post(startUploadUrl);
       return headers.location || ""; // FIXME: quickfix for axios' breaking API change
     } catch (e) {
-      const statusCode =
-        e instanceof Error && "response" in e
-          ? (e as Error & { response?: { status?: number } }).response?.status
-          : undefined;
+      const statusCode = isAxiosError(e) ? e.response?.status : undefined;
       throw new RegistryError(
         `Failed to initiate upload for image: ${image}`,
         statusCode,

src/dtp/DockerTarPusher.ts

@@ -6,11 +6,12 @@ import * as v from "valibot";
 import ManifestError from "../errors/ManifestError";
 import {
   type ApplicationConfiguration,
-  type DockerTarPusherOptionsSchema,
+  type ChunkMetaData,
+  DockerTarPusherOptionsSchema,
   ManifestSchema,
 } from "../types";
 import DockerRegistryService from "./DockerRegistryService";
-import ManifestBuilder from "./ManifestBuilder";
+import { buildManifest } from "./ManifestBuilder";
 
 export type DockerTarPusherOptions = v.InferInput<
   typeof DockerTarPusherOptionsSchema
@@ -21,11 +22,7 @@ export default class DockerTarPusher {
   private readonly dockerRegistryService: DockerRegistryService;
 
   constructor(options: DockerTarPusherOptions) {
-    this.config = {
-      sslVerify: true,
-      chunkSize: 10 * 1024 * 1024,
-      ...options,
-    };
+    this.config = v.parse(DockerTarPusherOptionsSchema, options);
 
     this.dockerRegistryService = new DockerRegistryService({
       chunkSize: this.config.chunkSize,
@@ -36,49 +33,45 @@ export default class DockerTarPusher {
   }
 
   async pushToRegistry() {
-    const manifestBuilder = new ManifestBuilder();
-    const tempDir: string | null = null;
+    const tempDir = await mkdtemp(join(tmpdir(), "dtp-"));
     try {
-      const workDir = await mkdtemp(join(tmpdir(), "dtp-"));
-      await extract({ file: this.config.tarball, cwd: workDir });
+      await extract({ file: this.config.tarball, cwd: tempDir });
 
-      const { RepoTags, Layers, Config } = await this.readManifest(workDir);
+      const { repoTags, config, layers } = await this.readManifest(tempDir);
 
-      for (const repoTag of RepoTags) {
+      for (const repoTag of repoTags) {
         const [image, tag] = this.config.image
           ? [this.config.image.name, this.config.image.version]
           : repoTag.split(":");
-        const layerPromises = Layers.map(async (layer, index) => {
-          this.config.onProgress?.({
-            type: "layer",
-            current: index + 1,
-            total: Layers.length,
-            bytesUploaded: 0,
-            totalBytes: 0,
-            item: layer,
-          });
-          return this.dockerRegistryService.upload(workDir, image, layer);
-        });
 
-        const layerResults = await Promise.all(layerPromises);
-        for (const result of layerResults) {
-          manifestBuilder.addLayer(result);
-        }
+        const layerResults = await Promise.all(
+          layers.map((layer, index) => {
+            this.config.onProgress?.({
+              type: "layer",
+              current: index + 1,
+              total: layers.length,
+              bytesUploaded: 0,
+              totalBytes: 0,
+              item: layer,
+            });
+            return this.dockerRegistryService.upload(tempDir, image, layer);
+          }),
+        );
 
         this.config.onProgress?.({
           type: "config",
           current: 1,
           total: 1,
           bytesUploaded: 0,
           totalBytes: 0,
-          item: Config,
+          item: config,
         });
+
         const configResult = await this.dockerRegistryService.upload(
-          workDir,
+          tempDir,
           image,
-          Config,
+          config,
         );
-        manifestBuilder.setConfig(configResult);
 
         this.config.onProgress?.({
           type: "manifest",
@@ -89,13 +82,11 @@ export default class DockerTarPusher {
           item: `${image}:${tag}`,
         });
 
-        const manifest = manifestBuilder.buildManifest();
+        const manifest = buildManifest(layerResults, configResult);
         await this.dockerRegistryService.pushManifest(manifest, image, tag);
       }
     } finally {
-      if (tempDir) {
-        await rm(tempDir, { recursive: true, force: true });
-      }
+      await rm(tempDir, { recursive: true, force: true });
     }
   }
 

src/dtp/ManifestBuilder.ts

@@ -1,44 +1,18 @@
-import ManifestError from "../errors/ManifestError";
-import type { ChunkMetaData, Config, Layer, RegistryManifest } from "../types";
+import type { ChunkMetaData } from "../types";
 import { ContentTypes } from "../types";
 
-export default class ManifestBuilder {
-  private layers?: Layer[];
-  private config?: Config;
-
-  public buildManifest(): RegistryManifest {
-    if (!this.config || !this.layers) {
-      throw new ManifestError(
-        "Cannot build manifest: config or layers are not set",
-        {
-          operation: "build",
-        },
-      );
-    }
-    return {
-      config: this.config,
-      layers: this.layers,
-      schemaVersion: 2,
-      mediaType: ContentTypes.APPLICATION_MANIFEST,
-    };
-  }
-
-  public addLayer({ digest, size }: ChunkMetaData): void {
-    if (!this.layers) {
-      this.layers = [];
-    }
-    this.layers.push({
-      digest,
-      size,
-      mediaType: ContentTypes.APPLICATION_LAYER,
-    });
-  }
-
-  public setConfig({ digest, size }: ChunkMetaData): void {
-    this.config = {
-      digest,
-      size,
-      mediaType: ContentTypes.APPLICATION_CONFIG,
-    };
-  }
-}
+export const buildManifest = (
+  layerChunks: ChunkMetaData[],
+  config: ChunkMetaData,
+) => ({
+  config: {
+    ...config,
+    mediaType: ContentTypes.APPLICATION_CONFIG,
+  },
+  layers: layerChunks.map((layerChunk) => ({
+    ...layerChunk,
+    mediaType: ContentTypes.APPLICATION_LAYER,
+  })),
+  schemaVersion: 2,
+  mediaType: ContentTypes.APPLICATION_MANIFEST,
+});

src/index.test.ts

@@ -1,33 +1,36 @@
 import { execSync } from "node:child_process";
-import { beforeAll, describe, expect, test } from "vitest";
+import { rmSync } from "node:fs";
+import { afterAll, beforeAll, describe, expect, test } from "vitest";
 import { DockerTarPusher } from "./index";
 
-const image = "busybox";
-const tarball = "/tmp/image.tar.gz";
-const registryUrl = process.env.REGISTRY_URL || "http://localhost:5000";
+const images = ["busybox", "alpine", "nginx"];
+const registryUrl = process.env.REGISTRY_URL || "http://localhost:15000";
 
 beforeAll(() => {
-  execSync(`docker pull ${image}:latest`);
-  execSync(`docker save ${image}:latest | gzip > ${tarball}`);
+  for (const image of images) {
+    execSync(`docker pull ${image}:latest`);
+    execSync(`docker save ${image}:latest | gzip > /tmp/${image}.tar.gz`);
+  }
+}, 120_000);
+
+afterAll(() => {
+  for (const image of images) {
+    rmSync(`/tmp/${image}.tar.gz`, { force: true });
+  }
 });
 
 describe("DockerTarPusher", () => {
-  test("should upload image to registry", async () => {
+  test.each(images)("should upload %s to registry", async (image) => {
     const dtp = new DockerTarPusher({
-      tarball,
+      tarball: `/tmp/${image}.tar.gz`,
       registryUrl,
     });
 
-    await expect(dtp.pushToRegistry()).resolves.not.toThrow();
-
-    const result = await fetch(`${registryUrl}/v2/_catalog`, {
-      method: "GET",
-    });
+    await dtp.pushToRegistry();
 
+    const result = await fetch(`${registryUrl}/v2/_catalog`);
     const json = (await result.json()) as { repositories: string[] };
 
-    expect(json.repositories).toEqual(
-      expect.arrayContaining([expect.stringContaining(image)]),
-    );
+    expect(json.repositories).toContain(image);
   });
 });