aws / aws-nitro-enclaves-cli

Tooling for Nitro Enclave Management
Apache License 2.0
116 stars 78 forks source link

Null fields in docker config cause build-enclave to fail #591

Closed jalil-salame closed 2 months ago

jalil-salame commented 4 months ago

I use dockerTools.buildLayeredImage to build docker images. It sets Env, Entrypoint, and Cmd to null if not set, this causes nitro-cli build-enclave to fail:

I think the panic is because of a logic bug:

https://github.com/aws/aws-nitro-enclaves-cli/blob/bc9c0b8362287698c0348ffd1df1fcaa9f0ccce3/enclave_build/src/docker.rs#L328-L337

Should instead be:

match self.docker.images().get(&self.docker_image).inspect().await {
    Ok(image) => Ok((
        image.config.entrypoint.unwrap(),
        image.config.env.unwrap_or_else(Vec::<String>::new), // <-- here
    )),
    Err(e) => {
        error!("{:?}", e);
        Err(DockerError::InspectError)
    }
}

I don't know how docker handles it, but my assumption is:

I have tested the images directly with docker and they work as expected.

jalil-salame commented 4 months ago

I am having other issues with images created by dockerTools which can be attributed to shiplift (the docker library used). Shiplift hasn't been updated in 3 years and assumes things that are no longer true, I suggest migrating away from it in favor of bollard which is more up to date.

Specifically shiplift expects the virtual_size field to be present, but it is not guaranteed to be:

virtual_size: Total size of the image including all layers it is composed of. Deprecated: this field is omitted in API v1.44, but kept for backward compatibility. Use Size instead

jalil-salame commented 4 months ago

For anyone coming across this issue, you can temporarily fix it by rolling back docker (sudo dnf downgrade docker) to version 24 which still sends the deprecated field.

jplock commented 3 months ago

Thank you! Downgrading to Docker 24.0.5 fixed my issue

meerd commented 3 months ago

595 will fix the issue.

jalil-salame commented 2 months ago

595 has been merged!