containers / podman-compose

a script to run docker-compose.yml using podman
GNU General Public License v2.0
5.09k stars 484 forks source link

`yaml` converting the bool args into Python boolean types #1022

Open mahendrapaipuri opened 3 months ago

mahendrapaipuri commented 3 months ago

Describe the bug

When we use boolean types in compose file, Python will convert them into its native boolean types. This means a value set as test: true in compose file will be stored at {'value': True} once it is loaded. If this value test is used inside the Dockerfile it will be eventually set as True while we set the value as true on compose file and we expect it to be set as true in the container.

To Reproduce For instance for a compose file as follows:

services:
  test:
    container_name: 'test-app'
    build:
      context: ./config
      args:
        test: true

and in the config folder, a Dockerfile as follows:

FROM alpine:latest

ARG test=false

ENV TEST "${test}"

By starting the containers using podman-compose up -d and inspecting the container, we can see env var TEST will be set to True and not true.

Expected behavior Arguments must be set the value defined in compose file.

Actual behavior Boolean arguments getting capitalized.

Output


$ podman-compose version
podman-compose version 1.2.0
podman version 4.5.1

$ podman-compose up
...
STEP 1/3: FROM alpine:latest
Resolving "alpine" using unqualified-search registries (/home/paipuri/.config/containers/registries.conf)
Trying to pull docker.io/library/alpine:latest...
Getting image source signatures
Copying blob c6a83fedfae6 skipped: already exists  
Copying config 324bc02ae1 done  
Writing manifest to image destination
Storing signatures
STEP 2/3: ARG test=false
--> 8168508479d3
STEP 3/3: ENV TEST "${test}"
COMMIT bool_bug_test
--> 7de87c86cee5
Successfully tagged localhost/bool_bug_test:latest
7de87c86cee513da2f78db19b01f2b2f096f175aec0f5755bb9b7c0e5f3dd689
fa2cd312f440b67f0ad0bb0aa6830700ab0eff757fc637dfe49a22a13c3ed78b
405becfd2a257ef7c3c055f2baf5ad4ca3d9f7ebb37a50473bcec92a5e29aa8f

$ podman inspect test-app
[redacted output]
"Config": {
               "Env": [
                    "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
                    "TERM=xterm",
                    "container=podman",
                    "TEST=True",
                    "HOME=/root",
                    "HOSTNAME=405becfd2a25"
               ],
 }

**Environment:**
 - OS: Linux
 - podman version: 4.5.1
 - podman compose version: 1.2.0
shuckc commented 2 months ago

You are right, this is difference in behavior to docker-compose:

% docker inspect test-app | grep TEST
                "TEST=true"
% podman inspect test-app | grep TEST
                    "TEST=True",

One workaround is to quote your strings in the yaml file, e.g. as test: 'true'.

>>> import yaml
>>> yaml.safe_load("xyz: 'true'")
{'xyz': 'true'}
>>> yaml.safe_load("xyz: true")
{'xyz': True}

Another ugly hack is to clear out the implicit type conversions happening inside pyyaml's cache, although his might break other portions of the config. Ideally would be path-dependant to only happen inside environment tags:

% python
Python 3.9.19 (main, Mar 19 2024, 16:08:27) 
[Clang 15.0.0 (clang-1500.3.9.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import yaml
>>> sl = yaml.SafeLoader("xyz: true")
>>> del sl.yaml_implicit_resolvers['t']
>>> yaml.safe_load("xyz: true")
{'xyz': 'true'}

The implicit resolver cache is global, so this now affects all yaml parsing within the process. If you patch the sl = and del lines above into your podman-compose.py file, it seems to work (for this very limited case):

% podman rm test-app
% podman image rm localhost/t2_test 
% podman-compose --verbose up -d    
...
% podman inspect test-app | grep TEST                                                                
                    "TEST=true",
mahendrapaipuri commented 1 month ago

Hello @shuckc Cheers for the reply!!

One workaround is to quote your strings in the yaml file, e.g. as test: 'true'

Yes, this is what we ended up doing.

I agree that patching this globally with pyyaml can have a lot of broken cases. Maybe documenting this behaviour can help users.