Skip to content
This repository was archived by the owner on Aug 14, 2020. It is now read-only.

Handle Entrypoint and Cmd correctly #20

Merged
merged 1 commit into from
Feb 13, 2015
Merged

Handle Entrypoint and Cmd correctly #20

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions lib/docker2aci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,7 @@ func generateManifest(layerData DockerImageData, dockerURL *ParsedDockerURL) (*s
genManifest.Labels = labels

if dockerConfig != nil {
var exec types.Exec
if len(dockerConfig.Cmd) > 0 {
exec = types.Exec(dockerConfig.Cmd)
} else if len(dockerConfig.Entrypoint) > 0 {
exec = types.Exec(dockerConfig.Entrypoint)
}
exec := getExecCommand(dockerConfig.Entrypoint, dockerConfig.Cmd)
if exec != nil {
user, group := parseDockerUser(dockerConfig.User)
app := &types.App{Exec: exec, User: user, Group: group}
Expand All @@ -447,6 +442,20 @@ func generateManifest(layerData DockerImageData, dockerURL *ParsedDockerURL) (*s
return genManifest, nil
}

func getExecCommand(entrypoint []string, cmd []string) types.Exec {
var command []string
if entrypoint == nil && cmd == nil {
return nil
}
command = append(entrypoint, cmd...)
// non-absolute paths are not allowed, fallback to "/bin/sh -c command"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting to feel like we should have a means to provide warnings during the conversion process when things like this happen, thoughts? (In the real world this would probably very rarely cause problems since having /bin/sh in Docker images is a really common pattern, but would be good practise to give people some hint that we are doing this kind of translation and the produced image is potentially non-functional)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could warn with something like Exec is not an absolute path, using "/bin/sh -c EXEC". The resulting ACI could be non-functional.

This will be printed for each layer that we convert so it might be too verbose, especially if we're squashing. We could accumulate the warnings and print only one of each at the end. Something like:

Warnings found when converting the image. The resulting ACI could be non-fuctional:
        Exec is not an absolute path, using "/bin/sh -c EXEC"
        Docker User is empty, assuming root

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, printing is less than ideal since this is a library. Maybe we could allow callers to pass in something to record warnings. Not sure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's follow up with that later, filed #24

if !filepath.IsAbs(command[0]) {
command_prefix := []string{"/bin/sh", "-c"}
command = append(command_prefix, strings.Join(command, " "))
}
return command
}

func parseDockerUser(dockerUser string) (string, string) {
// if the docker user is empty assume root user and group
if dockerUser == "" {
Expand Down