Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion jobs/async-upload/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ See asterisks below table for details
| MODEL_SYNC_DESTINATION_OCI_REGISTRY | --destination-oci-registry | | ✅\+ | When --destination-type is "oci". Indicates which registry the creds belong to |
| MODEL_SYNC_DESTINATION_OCI_USERNAME | --destination-oci-username | | ✅\+ | " |
| MODEL_SYNC_DESTINATION_OCI_PASSWORD | --destination-oci-password | | ✅\+ | " |
| MODEL_SYNC_DESTINATION_OCI_BASE_IMAGE | --destination-oci-base-image | busybox:latest | | When --destination-type is "oci". The image to use when pushing to an OCI registry |
| MODEL_SYNC_DESTINATION_OCI_BASE_IMAGE | --destination-oci-base-image | busybox:latest | | When --destination-type is "oci". The base image to use when building the ModelCar OCI image |
| MODEL_SYNC_DESTINATION_OCI_BASE_IMAGE_TLS_VERIFY | --destination-oci-base-image-tls-verify | true | | When --destination-type is "oci". TLS verification when pulling the base image. Set to `false` for registries with self-signed certificates (e.g. disconnected clusters) |
| MODEL_SYNC_DESTINATION_OCI_BASE_IMAGE_CREDENTIALS_PATH | --destination-oci-base-image-credentials-path | | | When --destination-type is "oci". Path to an auth file (e.g. `.dockerconfigjson`) for pulling the base image from a private or mirrored registry |
| MODEL_SYNC_DESTINATION_OCI_ENABLE_TLS_VERIFY | --destination-oci-enable-tls-verify | true | | When --destination-type is "oci". Specifies whether to use TLS when pushing to registry |
| MODEL_SYNC_MODEL_UPLOAD_INTENT | --model-upload-intent | update_artifact | | The intent of the upload job. Options include `["create_model", "create_version", "update_artifact"]`. |
| MODEL_SYNC_MODEL_ID | --model-id | | ✅\% | The `RegisteredModel.id` |
Expand Down
4 changes: 4 additions & 0 deletions jobs/async-upload/job/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ def _parser() -> cap.ArgumentParser:
p.add_argument("--destination-oci-username")
p.add_argument("--destination-oci-password")
p.add_argument("--destination-oci-base-image", default="public.ecr.aws/docker/library/busybox:latest")
p.add_argument("--destination-oci-base-image-tls-verify", default=True, type=str2bool)
p.add_argument("--destination-oci-base-image-credentials-path", metavar="PATH")
# The `type` converter is needed here to support env-based booleans
# See: https://github.com/bw2/ConfigArgParse/tree/master?tab=readme-ov-file#special-values
p.add_argument("--destination-oci-enable-tls-verify", default=True, type=str2bool)
Expand Down Expand Up @@ -484,6 +486,8 @@ def get_config(argv: list[str] | None = None) -> AsyncUploadConfig:
password=args.destination_oci_password,
email=None,
base_image=args.destination_oci_base_image,
base_image_tls_verify=args.destination_oci_base_image_tls_verify,
base_image_credentials_path=args.destination_oci_base_image_credentials_path,
enable_tls_verify=args.destination_oci_enable_tls_verify,
credentials_path=args.destination_oci_credentials_path
)
Expand Down
2 changes: 2 additions & 0 deletions jobs/async-upload/job/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class OCIConfig(BaseModel):
password: str | None = None
email: str | None = None
base_image: str = "public.ecr.aws/docker/library/busybox:latest"
base_image_tls_verify: bool = True
base_image_credentials_path: str | None = None
enable_tls_verify: bool = True

@model_validator(mode='after')
Expand Down
9 changes: 8 additions & 1 deletion jobs/async-upload/job/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,18 @@ def _get_upload_params(config: AsyncUploadConfig) -> S3Params | OCIParams:
)
elif isinstance(destination_config, OCIStorageConfig):
push_args = []
pull_args = []
# Note: These are all skopeo args, see: https://github.com/containers/skopeo/blob/main/docs/skopeo-copy.1.md
if not destination_config.enable_tls_verify:
push_args.append("--dest-tls-verify=false")
if destination_config.credentials_path:
push_args.append("--authfile")
push_args.append(destination_config.credentials_path)
if not destination_config.base_image_tls_verify:
pull_args.append("--src-tls-verify=false")
if destination_config.base_image_credentials_path:
pull_args.append("--authfile")
pull_args.append(destination_config.base_image_credentials_path)

return OCIParams(
base_image=destination_config.base_image,
Expand All @@ -43,7 +49,8 @@ def _get_upload_params(config: AsyncUploadConfig) -> S3Params | OCIParams:
oci_password=destination_config.password,
# Same as the default backend, but with additional args included
custom_oci_backend=utils._get_skopeo_backend(
push_args=push_args
pull_args=pull_args,
push_args=push_args,
),
)
else:
Expand Down
6 changes: 6 additions & 0 deletions jobs/async-upload/samples/sample_job_s3_to_oci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ spec:
value: "/opt/creds/destination/.dockerconfigjson"
- name: MODEL_SYNC_DESTINATION_OCI_BASE_IMAGE
value: "public.ecr.aws/docker/library/busybox:latest"
# Uncomment to disable TLS verification when pulling the base image (e.g. disconnected clusters with self-signed certs)
# - name: MODEL_SYNC_DESTINATION_OCI_BASE_IMAGE_TLS_VERIFY
# value: "false"
# Uncomment to provide auth for pulling the base image from a private/mirrored registry
# - name: MODEL_SYNC_DESTINATION_OCI_BASE_IMAGE_CREDENTIALS_PATH
# value: "/opt/creds/base-image/.dockerconfigjson"
- name: MODEL_SYNC_DESTINATION_OCI_ENABLE_TLS_VERIFY
value: "false"

Expand Down
37 changes: 37 additions & 0 deletions jobs/async-upload/tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,3 +511,40 @@ def test_oci_credentials_file_loading(
assert config.destination.email == expected["email"]


def test_base_image_pull_flags_defaults(
source_s3_env_vars, destination_oci_env_vars, update_artifact_intent_env_vars
):
"""Test that base image pull flags default to secure values"""
config = get_config([])

assert isinstance(config.destination, OCIStorageConfig)
assert config.destination.base_image_tls_verify is True
assert config.destination.base_image_credentials_path is None


def test_base_image_pull_flags_from_env(
source_s3_env_vars, destination_oci_env_vars, update_artifact_intent_env_vars, monkeypatch
):
"""Test that base image pull flags can be set via environment variables"""
monkeypatch.setenv("MODEL_SYNC_DESTINATION_OCI_BASE_IMAGE_TLS_VERIFY", "false")
monkeypatch.setenv("MODEL_SYNC_DESTINATION_OCI_BASE_IMAGE_CREDENTIALS_PATH", "/etc/pull-secret/.dockerconfigjson")

config = get_config([])

assert isinstance(config.destination, OCIStorageConfig)
assert config.destination.base_image_tls_verify is False
assert config.destination.base_image_credentials_path == "/etc/pull-secret/.dockerconfigjson"


def test_base_image_pull_flags_from_params(
source_s3_env_vars, destination_oci_env_vars, update_artifact_intent_env_vars
):
"""Test that base image pull flags can be set via CLI parameters"""
config = get_config([
"--destination-oci-base-image-tls-verify", "false",
"--destination-oci-base-image-credentials-path", "/tmp/auth.json",
])

assert isinstance(config.destination, OCIStorageConfig)
assert config.destination.base_image_tls_verify is False
assert config.destination.base_image_credentials_path == "/tmp/auth.json"
104 changes: 104 additions & 0 deletions jobs/async-upload/tests/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,110 @@
UpdateArtifactIntent
)

class TestGetUploadParamsOCIPullArgs:
"""Test cases for base image pull_args passed to _get_skopeo_backend"""

@patch("job.upload.utils._get_skopeo_backend")
def test_pull_args_with_tls_disabled_and_credentials(self, mock_get_backend):
mock_get_backend.return_value = Mock()

config = AsyncUploadConfig(
source=S3StorageConfig(
bucket="src-bucket", key="src-key",
access_key_id="id", secret_access_key="secret", region="us-east-1"
),
destination=OCIStorageConfig(
uri="registry.internal/model:v1", registry="registry.internal",
username="user", password="pass",
base_image="registry.internal/busybox:latest",
base_image_tls_verify=False,
base_image_credentials_path="/etc/pull-secret/.dockerconfigjson",
enable_tls_verify=False,
credentials_path="/tmp/push-creds"
),
model=ModelConfig(intent=UpdateArtifactIntent(artifact_id="123")),
storage=StorageConfig(path="/tmp/test"),
registry=RegistryConfig(server_address="test-server")
)

_get_upload_params(config)

mock_get_backend.assert_called_once()
call_kwargs = mock_get_backend.call_args
pull_args = call_kwargs.kwargs.get("pull_args") or call_kwargs[1].get("pull_args")
push_args = call_kwargs.kwargs.get("push_args") or call_kwargs[1].get("push_args")

assert "--src-tls-verify=false" in pull_args
assert "--authfile" in pull_args
assert "/etc/pull-secret/.dockerconfigjson" in pull_args
assert "--dest-tls-verify=false" in push_args
assert "--authfile" in push_args
assert "/tmp/push-creds" in push_args

@patch("job.upload.utils._get_skopeo_backend")
def test_pull_args_defaults_are_empty(self, mock_get_backend):
mock_get_backend.return_value = Mock()

config = AsyncUploadConfig(
source=S3StorageConfig(
bucket="src-bucket", key="src-key",
access_key_id="id", secret_access_key="secret", region="us-east-1"
),
destination=OCIStorageConfig(
uri="quay.io/org/model:v1", registry="quay.io",
username="user", password="pass",
base_image="quay.io/quay/busybox:latest",
),
model=ModelConfig(intent=UpdateArtifactIntent(artifact_id="123")),
storage=StorageConfig(path="/tmp/test"),
registry=RegistryConfig(server_address="test-server")
)

_get_upload_params(config)

mock_get_backend.assert_called_once()
call_kwargs = mock_get_backend.call_args
pull_args = call_kwargs.kwargs.get("pull_args") or call_kwargs[1].get("pull_args")
push_args = call_kwargs.kwargs.get("push_args") or call_kwargs[1].get("push_args")

assert pull_args == []
assert push_args == []

@patch("job.upload.utils._get_skopeo_backend")
def test_pull_args_independent_from_push_args(self, mock_get_backend):
"""Base image pull settings are independent from destination push settings"""
mock_get_backend.return_value = Mock()

config = AsyncUploadConfig(
source=S3StorageConfig(
bucket="src-bucket", key="src-key",
access_key_id="id", secret_access_key="secret", region="us-east-1"
),
destination=OCIStorageConfig(
uri="registry.internal/model:v1", registry="registry.internal",
username="user", password="pass",
base_image="registry.internal/busybox:latest",
base_image_tls_verify=False,
base_image_credentials_path="/etc/pull-secret/.dockerconfigjson",
enable_tls_verify=True,
credentials_path=None,
),
model=ModelConfig(intent=UpdateArtifactIntent(artifact_id="123")),
storage=StorageConfig(path="/tmp/test"),
registry=RegistryConfig(server_address="test-server")
)

_get_upload_params(config)

call_kwargs = mock_get_backend.call_args
pull_args = call_kwargs.kwargs.get("pull_args") or call_kwargs[1].get("pull_args")
push_args = call_kwargs.kwargs.get("push_args") or call_kwargs[1].get("push_args")

assert "--src-tls-verify=false" in pull_args
assert "--authfile" in pull_args
assert push_args == []


class TestGetUploadParams:
"""Test cases for _get_upload_params function"""

Expand Down
Loading