diff --git a/jobs/async-upload/README.md b/jobs/async-upload/README.md index d0dd5e385c..ea972047f0 100644 --- a/jobs/async-upload/README.md +++ b/jobs/async-upload/README.md @@ -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` | diff --git a/jobs/async-upload/job/config.py b/jobs/async-upload/job/config.py index 83bb71ec06..9e4255b953 100644 --- a/jobs/async-upload/job/config.py +++ b/jobs/async-upload/job/config.py @@ -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) @@ -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 ) diff --git a/jobs/async-upload/job/models.py b/jobs/async-upload/job/models.py index 815ad3abe8..66339b48f7 100644 --- a/jobs/async-upload/job/models.py +++ b/jobs/async-upload/job/models.py @@ -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') diff --git a/jobs/async-upload/job/upload.py b/jobs/async-upload/job/upload.py index 1efb97a5a1..bb858a5fa5 100644 --- a/jobs/async-upload/job/upload.py +++ b/jobs/async-upload/job/upload.py @@ -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, @@ -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: diff --git a/jobs/async-upload/samples/sample_job_s3_to_oci.yaml b/jobs/async-upload/samples/sample_job_s3_to_oci.yaml index 291194cd0e..fde420d520 100644 --- a/jobs/async-upload/samples/sample_job_s3_to_oci.yaml +++ b/jobs/async-upload/samples/sample_job_s3_to_oci.yaml @@ -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" diff --git a/jobs/async-upload/tests/test_config.py b/jobs/async-upload/tests/test_config.py index 414fecc403..d95a64b1a4 100644 --- a/jobs/async-upload/tests/test_config.py +++ b/jobs/async-upload/tests/test_config.py @@ -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" diff --git a/jobs/async-upload/tests/test_upload.py b/jobs/async-upload/tests/test_upload.py index e3cfe60d3e..b7e96417cd 100644 --- a/jobs/async-upload/tests/test_upload.py +++ b/jobs/async-upload/tests/test_upload.py @@ -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"""