Skip to content

add dockerRegistrysAuth#1193

Open
alexcos20 wants to merge 8 commits intomainfrom
feature/docker_registry_auth
Open

add dockerRegistrysAuth#1193
alexcos20 wants to merge 8 commits intomainfrom
feature/docker_registry_auth

Conversation

@alexcos20
Copy link
Member

@alexcos20 alexcos20 commented Feb 5, 2026

Fixes #1180

Add Docker Registry Authentication Support

Summary

This PR adds support for authenticating with private Docker/OCI registries when validating and pulling Docker images. The implementation allows nodes to access private container registries (Docker Hub, GHCR, GitLab Container Registry, etc.) by configuring registry credentials.

Changes

Configuration

  • New environment variable: DOCKER_REGISTRY_AUTHS - JSON object mapping registry URLs to authentication credentials
  • New config property: dockerRegistryAuth - Parsed from DOCKER_REGISTRY_AUTHS environment variable
  • Schema validation: Added DockerRegistryAuthSchema and DockerRegistryAuthsSchema to validate auth configuration

Core Implementation

  1. Config Schema (src/utils/config/schemas.ts):

    • Added DockerRegistryAuthSchema with username, password, and auth fields
    • Added DockerRegistryAuthsSchema as a record mapping registry URLs to auth objects
    • Integrated into OceanNodeConfigSchema with JSON parsing support
  2. Base Class (src/components/c2d/compute_engine_base.ts):

    • Added getDockerRegistryAuth(registry: string) method to retrieve auth credentials for a registry
    • Added static c2dEnginesInstance reference for static method access
  3. Docker Engine (src/components/c2d/compute_engine_docker.ts):

    • getDockerManifest(): Updated to use registry authentication

      • Retrieves auth credentials for the image's registry
      • Adds Basic auth header to initial manifest request if credentials exist
      • Adds Basic auth header to token request (on 401) if credentials exist
      • Supports both username/password and pre-encoded auth string formats
    • pullImage(): Updated to use registry authentication

      • Parses image to extract registry URL
      • Retrieves auth credentials for the registry
      • Builds Dockerode authconfig object with credentials
      • Passes authconfig to docker.pull() for authenticated image pulls
  4. C2D Engines (src/components/c2d/compute_engines.ts):

    • Added getDockerRegistryAuth() public method
    • Sets static c2dEnginesInstance reference when creating engines
    • Loads dockerRegistryAuth from config

Documentation

  • docs/env.md: Added comprehensive documentation for DOCKER_REGISTRY_AUTHS environment variable
    • Configuration examples for Docker Hub, GHCR, and GitLab
    • Field descriptions and usage notes
    • Registry-specific authentication guidance

Testing

  • src/test/integration/dockerRegistryAuth.test.ts: New integration test suite
    • Tests public registry access (no credentials)
    • Tests registry auth configuration (username/password and auth string)
    • Tests multiple registry configurations
    • Tests error handling for invalid images

Configuration Example

export DOCKER_REGISTRY_AUTHS='{
  "https://registry-1.docker.io": {
    "username": "myuser",
    "password": "mypassword"
  },
  "https://ghcr.io": {
    "username": "myuser",
    "password": "ghp_..."
  },
  "https://registry.gitlab.com": {
    "auth": "base64encodedtoken"
  }
}'

Or in config.json:

{
  "dockerRegistryAuth": {
    "https://registry-1.docker.io": {
      "username": "myuser",
      "password": "mypassword"
    }
  }
}

Authentication Methods Supported

  1. Username + Password: Provide username and password fields
  2. Auth String: Provide pre-encoded base64 auth string (alternative to username/password)

Backward Compatibility

  • ✅ Fully backward compatible - if no credentials are configured, the node attempts unauthenticated access (works for public images)
  • ✅ No breaking changes to existing APIs
  • ✅ Existing functionality remains unchanged

Testing

  • ✅ Integration tests added for registry authentication
  • ✅ Tests cover public images, authenticated images, and error cases
  • ✅ All linter errors resolved

Related Issues

Fixes: [Add support for private Docker registry authentication]

@alexcos20 alexcos20 self-assigned this Feb 5, 2026
@alexcos20 alexcos20 linked an issue Feb 5, 2026 that may be closed by this pull request
1 task
@alexcos20
Copy link
Member Author

/run-security-scan

Copy link
Member Author

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This pull request introduces the capability to authenticate with private Docker/OCI registries when validating and pulling images for Compute-to-Data (C2D) operations. It defines a new environment variable DOCKER_REGISTRY_AUTHS to store registry credentials (username/password or auth token). The changes include updating data types and configuration schemas, modifying the C2DEngineDocker to integrate authentication for getDockerManifest and docker.pull methods, and converting relevant static methods to instance methods. Comprehensive documentation for the new configuration is added, along with new integration tests to verify the authentication logic.

Comments:
• [ERROR][bug] The dockerRegistryAuth interface defines username, password, and auth as required string properties. However, the documentation (in docs/env.md) explicitly states that 'Each registry entry must provide either username+password or auth.' This implies that these fields should be optional (e.g., username?: string). Please update the interface to reflect this optionality to avoid type mismatches and allow flexible configuration where not all three fields are present.
• [WARNING][style] The check if (!this.dockerRegistryAuths) is redundant. According to the OceanNodeConfigSchema, dockerRegistrysAuth defaults to an empty object {} if not explicitly provided. This means this.dockerRegistryAuths will always be an object and never null or undefined. You can safely remove this check.
• [WARNING][bug] The pullOptions.authconfig object is constructed to always include username, password, and auth, even if some of these fields are empty strings or undefined from the dockerRegistryAuth object. It's generally better practice to only include the fields that are actually provided and necessary for dockerode's authconfig. For example, if auth is present, username and password might not be needed by dockerode, or vice-versa. Consider conditionally adding username and password if they exist and auth is not used, to avoid sending redundant or empty fields to the Docker daemon. A more precise construction might look like:

pullOptions.authconfig = {
  serveraddress,
  ...(dockerRegistryAuth.auth ? { auth: authString } : { username: dockerRegistryAuth.username, password: dockerRegistryAuth.password })
};

(assuming username and password are guaranteed to be present if auth is not).
• [INFO][style] The private dockerRegistryAuths: dockerRegistrysAuth property in C2DEngines appears to be redundant. The dockerRegistrysAuth is already part of the OceanNodeConfig and is directly passed to the C2DEngineDocker constructor when engines are created. C2DEngines acts as a factory and doesn't seem to store or use this property directly itself after initialization. Consider removing it.
• [ERROR][bug] In the imageCleanup.test.ts file, null is passed for dockerRegistryAuths in the C2DEngineDocker constructor. This is inconsistent with the OceanNodeConfigSchema which defaults dockerRegistrysAuth to an empty object {}. The C2DEngineDocker constructor also expects dockerRegistrysAuth: dockerRegistrysAuth. Passing null could lead to runtime type errors if methods attempt to access properties of dockerRegistryAuths assuming it's an object. Please change null to {} or an appropriate mock dockerRegistrysAuth object to align with the schema and type expectations.
• [ERROR][bug] The DockerRegistryAuthSchema defines username, password, and auth as required z.string() properties. This directly conflicts with the documentation in docs/env.md which states that a registry entry must provide 'either username+password or auth'. This schema will cause validation to fail if users configure only auth or only username/password. Please adjust the schema to correctly reflect this 'either/or' condition, for example, by making username, password, and auth optional (z.string().optional()) and then using z.refine to enforce the 'at least one' or 'either/or' logic.

@alexcos20
Copy link
Member Author

/run-security-scan

Copy link
Member Author

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI automated code review (Gemini 3).

Overall risk: low

Summary:
This pull request introduces support for authenticating with private Docker registries when pulling images for C2D compute jobs. It achieves this by adding a new configuration option DOCKER_REGISTRY_AUTHS which accepts a JSON object mapping registry URLs to authentication credentials (username/password or a pre-encoded auth token). The changes include: updating type definitions, modifying the C2D engine base and Docker implementations to consume and apply these credentials during manifest fetching and image pulling, adding detailed documentation for the new configuration, and introducing robust Zod schemas for validation. A comprehensive integration test suite has been added to cover various authentication scenarios and schema validation.

Comments:
• [INFO][style] The // eslint-disable-next-line require-await comment is added for public abstract checkDockerImage. While it's common to disable this rule for abstract methods where the implementation decides async status, it's worth a quick thought if there's a reason the abstract method must return a Promise but doesn't always require await in its implementation. In this specific case, it seems reasonable given the Promise<ValidateParams> return type, implying potential asynchronous operations.
• [INFO][performance] Changing static parseImage to private parseImage is a good refactoring, making it an instance method and allowing access to this.dockerRegistryAuths indirectly. This is a positive change for the design, aligning image parsing with the engine's specific configuration rather than a global static utility.
• [WARNING][security] When constructing the Basic authentication string Buffer.from(${dockerRegistryAuth.username}:${dockerRegistryAuth.password}).toString('base64'), ensure that dockerRegistryAuth.username and dockerRegistryAuth.password are guaranteed to be non-empty strings if present. The Zod schema (DockerRegistryAuthSchema in src/utils/config/schemas.ts) already handles this by requiring both to be non-empty if auth is not provided. This is good, but it's a critical point for security that these values are validated before reaching this point to avoid encoding empty credentials.
• [INFO][performance] Similar to the getDockerManifest method, the serveraddress extraction for pullOptions.authconfig is well-handled by parsing the registry URL. This correctly accounts for potential ports in the registry URL, which is important for more complex registry setups.
• [INFO][testing] The integration tests for DockerRegistryAuthSchema are excellent. They cover all critical validation paths, including required combinations (auth OR username+password) and rejection of incomplete or empty credentials. This robust testing of the schema itself significantly increases confidence in the overall feature's stability and security implications.
• [INFO][security] The refine logic in DockerRegistryAuthSchema is very well implemented. It accurately enforces the rule that either auth must be present and non-empty, OR both username and password must be present and non-empty. This is crucial for preventing misconfigurations that could lead to failed authentication attempts or security vulnerabilities from partially supplied credentials.

@alexcos20
Copy link
Member Author

Do not merge, because #1197 will be merged here before

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow configuration of Registry credentials to avoid Rate Limiting

1 participant