bollard icon indicating copy to clipboard operation
bollard copied to clipboard

`ContainerConfig.entrypoint` is not deserializer correctly in some cases

Open milenkovicm opened this issue 2 years ago • 12 comments

In some cases, like when using podman ContainerConfig.entrypoint is a string, in those cases deserializer freaks out with

panicked at 'called `Result::unwrap()` on an `Err` value: Error("invalid type: string \"/entrypoint. ...

looking at documentation at generated model it looks like it is expected to get either string or string array

 /// The entry point for the container as a string or an array of strings.  If the array consists of exactly one empty string (`[\"\"]`) then the entry point is reset to system default (i.e., the entry point used by docker when there is no `ENTRYPOINT` instruction in the `Dockerfile`). 
    #[serde(rename = "Entrypoint")]
    #[serde(skip_serializing_if="Option::is_none")]
    pub entrypoint: Option<Vec<String>>,

https://github.com/fussybeaver/bollard/blob/79ea9ce81e98702040527a5923ceb33a7015ca37/codegen/target/generated-sources/src/models.rs#L377

I guess there should be custom deserializer to cover this case.

Something like

#[allow(dead_code)]
fn string_or_seq_string_optional<'de, D>(deserializer: D) -> Result<Option<Vec<String>>, D::Error>
    where D: serde::de::Deserializer<'de>
{
    struct StringOrVec(std::marker::PhantomData<Vec<String>>);

    impl<'de> serde::de::Visitor<'de> for StringOrVec {
        type Value = Option<Vec<String>>;

        fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
            formatter.write_str("string or list of strings")
        }

        fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
            where E: serde::de::Error
        {
            Ok(Some(vec![value.to_owned()]))
        }

        fn visit_seq<S>(self, visitor: S) -> Result<Self::Value, S::Error>
            where S: serde::de::SeqAccess<'de>
        {
            serde::de::Deserialize::deserialize(serde::de::value::SeqAccessDeserializer::new(visitor)).map(|result| Some(result))
        }
    }
    
    deserializer.deserialize_any(StringOrVec(std::marker::PhantomData))
}

might work.

or serde_with::OneOrMany if external dependency is preferred

milenkovicm avatar Jun 02 '22 14:06 milenkovicm

Thanks for the detailed report.

Did you find this problem only on Podman, or is this reproducible on vanilla Docker ?

At the moment the models are generated from the documentation, which currently don't specify a string:

https://github.com/moby/moby/blob/master/api/swagger.yaml#L1271-L1281

If this is reproducible in plain Docker, we should ask upstream to have something (like a swagger anyOf) added in the documentation at this location. Subsequently, we can build this deserializer into Bollard for this special case.

The other alternative is to build some sort of exception into the generator...

fussybeaver avatar Jun 06 '22 13:06 fussybeaver

Hey @fussybeaver, Thanks for your reply.

Vanilla docker has no such problem, this is problem with podman only (which has other problems e.g StopSignal as int instead of string), thus podman would probably be the location where this issue should go.

Feel free to close this issue as irrelevant if you agree

milenkovicm avatar Jun 06 '22 13:06 milenkovicm

Perhaps we should initially raise the issue on the Podman bug tracker to see whether the Podman developers are responsive around these issues.

fussybeaver avatar Jun 06 '22 14:06 fussybeaver

All right, I'll follow up with them in following day or so

milenkovicm avatar Jun 06 '22 14:06 milenkovicm

Opened issue on podman, lets see whats their opinion https://github.com/containers/podman/issues/14513

milenkovicm avatar Jun 07 '22 15:06 milenkovicm

Reading the thread on your ticket makes me realize how self-contained the podman system is. Perhaps we do need some sort of cargo feature in Bollard to toggle between these slight differences?

fussybeaver avatar Jun 14 '22 19:06 fussybeaver

@fussybeaver do you want to support podman as well, it is quite obvious that they have their own api?

milenkovicm avatar Jun 14 '22 21:06 milenkovicm

It would be nice to, but it needs some investigation at the best approach - I notice they have their own swagger API as well.

Perhaps initially it'd be good just to look at the diff between the two swagger API's and checking whether it's supportable at all.

fussybeaver avatar Jun 15 '22 09:06 fussybeaver

Not related to this issue (directly) but to the discussion of podman being different. I'm also facing a parsing issue on bollard's side because here podman can return initlialized when inspecting a container

https://github.com/containers/podman/blob/73713062806aa4c2db25dc62e2fff47406085dc8/libpod/define/containerstate.go#L43-L71

However, initialized is not part of the enum variants for bollard https://github.com/fussybeaver/bollard/blob/79ea9ce81e98702040527a5923ceb33a7015ca37/codegen/target/generated-sources/src/models.rs#L620-L637

chesedo avatar Jun 17 '22 13:06 chesedo

Perhaps we do need some sort of cargo feature in Bollard to toggle between these slight differences?

I asked the Podman team about their Docker compat layer seeming to be different from Docker in this discussion -> containers/podman#14641

Their response is that "the docker compat api should always return correct docker output, if not file a bug with a reproducer". So nothing special should be needed in terms of bollard parsing Podman's docker compat output.

chesedo avatar Jun 23 '22 05:06 chesedo

Root of my problem is that i want to use podman-docker to proxy requests, and as it has been said in https://github.com/containers/podman/issues/14513#issuecomment-1155283737 podman-docker is just a alias to podman, thus breaking schema breaks all tooling. Tools I use really on docker executable ... will leave podman for some other time

milenkovicm avatar Jun 23 '22 09:06 milenkovicm

I see... I use Podman exclusively. But in the rare event when I need to test a tool targeted for docker, I just set the DOCKER_HOST env variable to point to the podman compat socket. But so far this is only for bollard and docker-compose so I'm luckily to not need a tool which requires the docker executable

chesedo avatar Jun 23 '22 11:06 chesedo