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

Review config for cmd/entrypoint after building a stage #348

Merged
merged 5 commits into from
Sep 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions integration/dockerfiles/Dockerfile_test_cmd
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
FROM gcr.io/distroless/base@sha256:628939ac8bf3f49571d05c6c76b8688cb4a851af6c7088e599388259875bde20 AS first
CMD ["mycmd"]

FROM first
ENTRYPOINT ["myentrypoint"] # This should clear out CMD in the config metadata
2 changes: 0 additions & 2 deletions pkg/commands/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/google/go-containerregistry/pkg/v1"
"github.com/moby/buildkit/frontend/dockerfile/instructions"
"github.com/sirupsen/logrus"
)

type CmdCommand struct {
Expand All @@ -48,7 +47,6 @@ func (c *CmdCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
newCommand = c.cmd.CmdLine
}

logrus.Infof("Replacing CMD in config with %v", newCommand)
config.Cmd = newCommand
config.ArgsEscaped = true
return nil
Expand Down
2 changes: 0 additions & 2 deletions pkg/commands/entrypoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/google/go-containerregistry/pkg/v1"
"github.com/moby/buildkit/frontend/dockerfile/instructions"
"github.com/sirupsen/logrus"
)

type EntrypointCommand struct {
Expand All @@ -47,7 +46,6 @@ func (e *EntrypointCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerf
newCommand = e.cmd.CmdLine
}

logrus.Infof("Replacing Entrypoint in config with %v", newCommand)
config.Entrypoint = newCommand
return nil
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,13 @@ const (
S3BuildContextPrefix = "s3://"
LocalDirBuildContextPrefix = "dir://"

HOME = "HOME"
// DefaultHOMEValue is the default value Docker sets for $HOME
HOME = "HOME"
DefaultHOMEValue = "/root"

// Docker command names
Cmd = "cmd"
Entrypoint = "entrypoint"
)

// KanikoBuildFiles is the list of files required to build kaniko
Expand Down
21 changes: 21 additions & 0 deletions pkg/executor/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) {
if err := sb.build(opts); err != nil {
return nil, errors.Wrap(err, "error building stage")
}
reviewConfig(stage, &sb.cf.Config)
sourceImage, err := mutate.Config(sb.image, sb.cf.Config)
if err != nil {
return nil, err
Expand Down Expand Up @@ -277,3 +278,23 @@ func resolveOnBuild(stage *config.KanikoStage, config *v1.Config) error {
config.OnBuild = nil
return nil
}

// reviewConfig makes sure the value of CMD is correct after building the stage
// If ENTRYPOINT was set in this stage but CMD wasn't, then CMD should be cleared out
// See Issue #346 for more info
func reviewConfig(stage config.KanikoStage, config *v1.Config) {
entrypoint := false
cmd := false

for _, c := range stage.Commands {
if c.Name() == constants.Cmd {
cmd = true
}
if c.Name() == constants.Entrypoint {
entrypoint = true
}
}
if entrypoint && !cmd {
config.Cmd = nil
}
}
76 changes: 76 additions & 0 deletions pkg/executor/build_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
Copyright 2018 Google LLC

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package executor

import (
"testing"

"github.com/GoogleContainerTools/kaniko/pkg/config"
"github.com/GoogleContainerTools/kaniko/pkg/dockerfile"
"github.com/GoogleContainerTools/kaniko/testutil"
"github.com/google/go-containerregistry/pkg/v1"
)

func Test_reviewConfig(t *testing.T) {
tests := []struct {
name string
dockerfile string
originalCmd []string
originalEntrypoint []string
expectedCmd []string
}{
{
name: "entrypoint and cmd declared",
dockerfile: `
FROM scratch
CMD ["mycmd"]
ENTRYPOINT ["myentrypoint"]`,
originalEntrypoint: []string{"myentrypoint"},
Copy link
Member

Choose a reason for hiding this comment

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

by originalCmd, do you mean command from previous stage?

Copy link
Member

Choose a reason for hiding this comment

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

use different values for dockerfile.CMD and dockerfile.ENTRYPOINT.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

by originalCmd, I mean the original value of Cmd in the config file (it should match the value specified in the Dockerfile)

The function reviews the Dockerfile and then clears the Cmd field if it isn't declared in the Dockerfile.

originalCmd: []string{"mycmd"},
expectedCmd: []string{"mycmd"},
},
{
name: "only entrypoint declared",
dockerfile: `
FROM scratch
ENTRYPOINT ["myentrypoint"]`,
originalEntrypoint: []string{"myentrypoint"},
originalCmd: []string{"mycmd"},
expectedCmd: nil,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
config := &v1.Config{
Cmd: test.originalCmd,
Entrypoint: test.originalEntrypoint,
}
reviewConfig(stage(t, test.dockerfile), config)
testutil.CheckErrorAndDeepEqual(t, false, nil, test.expectedCmd, config.Cmd)
})
}
}

func stage(t *testing.T, d string) config.KanikoStage {
stages, err := dockerfile.Parse([]byte(d))
if err != nil {
t.Fatalf("error parsing dockerfile: %v", err)
}
return config.KanikoStage{
Stage: stages[0],
}
}