Skip to content

Commit 495b11d

Browse files
committed
feat: Add exception for edge cases and add unit tests
1 parent 527971f commit 495b11d

File tree

2 files changed

+258
-53
lines changed

2 files changed

+258
-53
lines changed

podman/domain/containers_create.py

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
logger = logging.getLogger("podman.containers")
1919

20-
NAMED_VOLUME_PATTERN = re.compile(r'[a-zA-Z0-9][a-zA-Z0-9_.-]*')
20+
NAMED_VOLUME_PATTERN = re.compile(r"[a-zA-Z0-9][a-zA-Z0-9_.-]*")
2121

2222

2323
class CreateMixin: # pylint: disable=too-few-public-methods
@@ -375,7 +375,9 @@ def create(
375375
payload = api.prepare_body(payload)
376376

377377
response = self.client.post(
378-
"/containers/create", headers={"content-type": "application/json"}, data=payload
378+
"/containers/create",
379+
headers={"content-type": "application/json"},
380+
data=payload,
379381
)
380382
response.raise_for_status(not_found=ImageNotFound)
381383

@@ -396,14 +398,34 @@ def _convert_env_list_to_dict(env_list):
396398
Raises:
397399
ValueError: If any environment variable is not in the correct format
398400
"""
401+
if not isinstance(env_list, list):
402+
raise TypeError(f"Expected list, got {type(env_list).__name__}")
403+
399404
env_dict = {}
405+
400406
for env_var in env_list:
401-
if '=' not in env_var:
407+
if not isinstance(env_var, str):
408+
raise TypeError(
409+
f"Environment variable must be a string, "
410+
f"got {type(env_var).__name__}: {repr(env_var)}"
411+
)
412+
413+
# Handle empty strings
414+
if not env_var.strip():
415+
raise ValueError(f"Environment variable cannot be empty")
416+
if "=" not in env_var:
402417
raise ValueError(
403418
f"Environment variable '{env_var}' is not in the correct format. "
404419
"Expected format: 'KEY=value'"
405420
)
406-
key, value = env_var.split('=', 1) # Split on first '=' only
421+
key, value = env_var.split("=", 1) # Split on first '=' only
422+
423+
# Validate key is not empty
424+
if not key.strip():
425+
raise ValueError(
426+
f"Environment variable at index {i} has empty key: '{env_var}'"
427+
)
428+
407429
env_dict[key] = value
408430
return env_dict
409431

@@ -507,9 +529,9 @@ def to_bytes(size: Union[int, str, None]) -> Union[int, None]:
507529
try:
508530
return int(size)
509531
except ValueError as bad_size:
510-
mapping = {'b': 0, 'k': 1, 'm': 2, 'g': 3}
511-
mapping_regex = ''.join(mapping.keys())
512-
search = re.search(rf'^(\d+)([{mapping_regex}])$', size.lower())
532+
mapping = {"b": 0, "k": 1, "m": 2, "g": 3}
533+
mapping_regex = "".join(mapping.keys())
534+
search = re.search(rf"^(\d+)([{mapping_regex}])$", size.lower())
513535
if search:
514536
return int(search.group(1)) * (1024 ** mapping[search.group(2)])
515537
raise TypeError(
@@ -532,7 +554,9 @@ def to_bytes(size: Union[int, str, None]) -> Union[int, None]:
532554
"cni_networks": [pop("network")],
533555
"command": args.pop("command", args.pop("cmd", None)),
534556
"conmon_pid_file": pop("conmon_pid_file"), # TODO document, podman only
535-
"containerCreateCommand": pop("containerCreateCommand"), # TODO document, podman only
557+
"containerCreateCommand": pop(
558+
"containerCreateCommand"
559+
), # TODO document, podman only
536560
"devices": [],
537561
"dns_option": pop("dns_opt"),
538562
"dns_search": pop("dns_search"),
@@ -581,7 +605,9 @@ def to_bytes(size: Union[int, str, None]) -> Union[int, None]:
581605
"rootfs_propagation": pop("rootfs_propagation"),
582606
"sdnotifyMode": pop("sdnotifyMode"), # TODO document, podman only
583607
"seccomp_policy": pop("seccomp_policy"), # TODO document, podman only
584-
"seccomp_profile_path": pop("seccomp_profile_path"), # TODO document, podman only
608+
"seccomp_profile_path": pop(
609+
"seccomp_profile_path"
610+
), # TODO document, podman only
585611
"secrets": [], # TODO document, podman only
586612
"selinux_opts": pop("security_opt"),
587613
"shm_size": to_bytes(pop("shm_size")),
@@ -597,7 +623,9 @@ def to_bytes(size: Union[int, str, None]) -> Union[int, None]:
597623
"unified": pop("unified"), # TODO document, podman only
598624
"unmask": pop("unmasked_paths"), # TODO document, podman only
599625
"use_image_hosts": pop("use_image_hosts"), # TODO document, podman only
600-
"use_image_resolve_conf": pop("use_image_resolve_conf"), # TODO document, podman only
626+
"use_image_resolve_conf": pop(
627+
"use_image_resolve_conf"
628+
), # TODO document, podman only
601629
"user": pop("user"),
602630
"version": pop("version"),
603631
"volumes": [],
@@ -619,9 +647,15 @@ def to_bytes(size: Union[int, str, None]) -> Union[int, None]:
619647
params["log_configuration"]["driver"] = args["log_config"].get("Type")
620648

621649
if "Config" in args["log_config"]:
622-
params["log_configuration"]["path"] = args["log_config"]["Config"].get("path")
623-
params["log_configuration"]["size"] = args["log_config"]["Config"].get("size")
624-
params["log_configuration"]["options"] = args["log_config"]["Config"].get("options")
650+
params["log_configuration"]["path"] = args["log_config"]["Config"].get(
651+
"path"
652+
)
653+
params["log_configuration"]["size"] = args["log_config"]["Config"].get(
654+
"size"
655+
)
656+
params["log_configuration"]["options"] = args["log_config"][
657+
"Config"
658+
].get("options")
625659
args.pop("log_config")
626660

627661
for item in args.pop("mounts", []):
@@ -648,7 +682,7 @@ def to_bytes(size: Union[int, str, None]) -> Union[int, None]:
648682
if _k in bool_options and v is True:
649683
options.append(option_name)
650684
elif _k in regular_options:
651-
options.append(f'{option_name}={v}')
685+
options.append(f"{option_name}={v}")
652686
elif _k in simple_options:
653687
options.append(v)
654688

@@ -676,7 +710,9 @@ def parse_host_port(_container_port, _protocol, _host):
676710
result.append(port_map)
677711
elif isinstance(_host, list):
678712
for host_list in _host:
679-
host_list_result = parse_host_port(_container_port, _protocol, host_list)
713+
host_list_result = parse_host_port(
714+
_container_port, _protocol, host_list
715+
)
680716
result.extend(host_list_result)
681717
elif isinstance(_host, dict):
682718
_host_port = _host.get("port")
@@ -750,12 +786,12 @@ def parse_host_port(_container_port, _protocol, _host):
750786

751787
for item in args.pop("volumes", {}).items():
752788
key, value = item
753-
extended_mode = value.get('extended_mode', [])
789+
extended_mode = value.get("extended_mode", [])
754790
if not isinstance(extended_mode, list):
755791
raise ValueError("'extended_mode' value should be a list")
756792

757793
options = extended_mode
758-
mode = value.get('mode')
794+
mode = value.get("mode")
759795
if mode is not None:
760796
if not isinstance(mode, str):
761797
raise ValueError("'mode' value should be a str")
@@ -770,10 +806,10 @@ def parse_host_port(_container_port, _protocol, _host):
770806
params["volumes"].append(volume)
771807
else:
772808
mount_point = {
773-
"destination": value['bind'],
809+
"destination": value["bind"],
774810
"options": options,
775811
"source": key,
776-
"type": 'bind',
812+
"type": "bind",
777813
}
778814
params["mounts"].append(mount_point)
779815

@@ -818,7 +854,8 @@ def parse_host_port(_container_port, _protocol, _host):
818854

819855
if len(args) > 0:
820856
raise TypeError(
821-
"Unknown keyword argument(s): " + " ,".join(f"'{k}'" for k in args.keys())
857+
"Unknown keyword argument(s): "
858+
+ " ,".join(f"'{k}'" for k in args.keys())
822859
)
823860

824861
return params

0 commit comments

Comments
 (0)