Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ContainerConfig.entrypoint is not deserializer correctly in some cases #238

Open
milenkovicm opened this issue Jun 2, 2022 · 12 comments
Open

Comments

@milenkovicm
Copy link

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>>,

pub entrypoint: Option<Vec<String>>,

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

@fussybeaver
Copy link
Owner

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...

@milenkovicm
Copy link
Author

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

@fussybeaver
Copy link
Owner

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

@milenkovicm
Copy link
Author

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

@milenkovicm
Copy link
Author

Opened issue on podman, lets see whats their opinion containers/podman#14513

@fussybeaver
Copy link
Owner

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?

@milenkovicm
Copy link
Author

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

@fussybeaver
Copy link
Owner

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.

@chesedo
Copy link

chesedo commented Jun 17, 2022

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

pub enum ContainerStateStatusEnum {
#[serde(rename = "")]
EMPTY,
#[serde(rename = "created")]
CREATED,
#[serde(rename = "running")]
RUNNING,
#[serde(rename = "paused")]
PAUSED,
#[serde(rename = "restarting")]
RESTARTING,
#[serde(rename = "removing")]
REMOVING,
#[serde(rename = "exited")]
EXITED,
#[serde(rename = "dead")]
DEAD,
}

@chesedo
Copy link

chesedo commented Jun 23, 2022

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.

@milenkovicm
Copy link
Author

Root of my problem is that i want to use podman-docker to proxy requests, and as it has been said in containers/podman#14513 (comment) 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

@chesedo
Copy link

chesedo commented Jun 23, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants