aws-nitro-enclaves-cli icon indicating copy to clipboard operation
aws-nitro-enclaves-cli copied to clipboard

Null fields in docker config cause build-enclave to fail

Open jalil-salame opened this issue 1 year ago • 4 comments

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:

  • If either Entrypoint or Cmd is null then it fails with UnsupportedEntryPoint

  • If both Entrypoint and Cmd are not null and Env is null then it panics here:

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

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:

  • If Entrypoint is null, then use Cmd
  • If Cmd is null, then use Entrypoint

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

jalil-salame avatar Mar 05 '24 10:03 jalil-salame

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 avatar Mar 07 '24 10:03 jalil-salame

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.

jalil-salame avatar Mar 07 '24 10:03 jalil-salame

Thank you! Downgrading to Docker 24.0.5 fixed my issue

jplock avatar Mar 15 '24 00:03 jplock

#595 will fix the issue.

meerd avatar Apr 05 '24 15:04 meerd

#595 has been merged!

jalil-salame avatar Apr 13 '24 13:04 jalil-salame