From 9b3dbf97b5b7e9e8f3dde1b8966b43d106b08f03 Mon Sep 17 00:00:00 2001 From: Timofey Kirillov Date: Fri, 24 Jun 2022 18:50:09 +0300 Subject: [PATCH] fix(docker-instructions): exactOptionValues option to fix docker-server backend options evaluation Signed-off-by: Timofey Kirillov Signed-off-by: Timofey Kirillov --- pkg/build/stage/docker_instructions.go | 12 +- pkg/config/docker.go | 2 + pkg/config/raw_docker.go | 3 + pkg/container_backend/legacy_interface.go | 1 + pkg/container_backend/legacy_stage_image.go | 13 +- .../legacy_stage_image_container.go | 10 +- .../legacy_stage_image_container_options.go | 100 ++++++++--- ...gacy_stage_image_container_options_test.go | 170 ++++++++++++++++++ pkg/container_backend/suite_test.go | 13 ++ 9 files changed, 291 insertions(+), 33 deletions(-) create mode 100644 pkg/container_backend/legacy_stage_image_container_options_test.go create mode 100644 pkg/container_backend/suite_test.go diff --git a/pkg/build/stage/docker_instructions.go b/pkg/build/stage/docker_instructions.go index 97c22ae8b8..a82bb6a507 100644 --- a/pkg/build/stage/docker_instructions.go +++ b/pkg/build/stage/docker_instructions.go @@ -30,9 +30,15 @@ type DockerInstructionsStage struct { instructions *config.Docker } -func (s *DockerInstructionsStage) GetDependencies(_ context.Context, _ Conveyor, _ container_backend.ContainerBackend, _, _ *StageImage) (string, error) { +func (s *DockerInstructionsStage) GetDependencies(_ context.Context, c Conveyor, backend container_backend.ContainerBackend, _, _ *StageImage) (string, error) { var args []string + // FIXME: add to digest if option set, keep current compatible digest otherwise + + if c.UseLegacyStapelBuilder(backend) && s.instructions.ExactValues { + args = append(args, "exact-values:::") + } + args = append(args, s.instructions.Volume...) args = append(args, s.instructions.Expose...) args = append(args, mapToSortedArgs(s.instructions.Env)...) @@ -61,6 +67,10 @@ func mapToSortedArgs(h map[string]string) (result []string) { } func (s *DockerInstructionsStage) PrepareImage(ctx context.Context, c Conveyor, cr container_backend.ContainerBackend, prevBuiltImage, stageImage *StageImage) error { + if c.UseLegacyStapelBuilder(cr) { + stageImage.Image.SetCommitChangeOptions(container_backend.LegacyCommitChangeOptions{ExactValues: s.instructions.ExactValues}) + } + if err := s.BaseStage.PrepareImage(ctx, c, cr, prevBuiltImage, stageImage); err != nil { return err } diff --git a/pkg/config/docker.go b/pkg/config/docker.go index ab157fbbc9..627754d397 100644 --- a/pkg/config/docker.go +++ b/pkg/config/docker.go @@ -11,6 +11,8 @@ type Docker struct { Entrypoint string HealthCheck string + ExactValues bool + raw *rawDocker } diff --git a/pkg/config/raw_docker.go b/pkg/config/raw_docker.go index 4d7f780039..712c12bbaa 100644 --- a/pkg/config/raw_docker.go +++ b/pkg/config/raw_docker.go @@ -16,6 +16,8 @@ type rawDocker struct { Entrypoint interface{} `yaml:"ENTRYPOINT,omitempty"` HealthCheck string `yaml:"HEALTHCHECK,omitempty"` + ExactValues bool `yaml:"exactValues,omitempty"` + rawStapelImage *rawStapelImage `yaml:"-"` // parent UnsupportedAttributes map[string]interface{} `yaml:",inline"` @@ -76,6 +78,7 @@ func (c *rawDocker) toDirective() (docker *Docker, err error) { } docker.HealthCheck = c.HealthCheck + docker.ExactValues = c.ExactValues docker.raw = c diff --git a/pkg/container_backend/legacy_interface.go b/pkg/container_backend/legacy_interface.go index 5fd7be8897..44695d4605 100644 --- a/pkg/container_backend/legacy_interface.go +++ b/pkg/container_backend/legacy_interface.go @@ -37,6 +37,7 @@ type LegacyImageInterface interface { SetStageDescription(stage *image.StageDescription) GetStageDescription() *image.StageDescription + SetCommitChangeOptions(opts LegacyCommitChangeOptions) GetCopy() LegacyImageInterface } diff --git a/pkg/container_backend/legacy_stage_image.go b/pkg/container_backend/legacy_stage_image.go index 0510d04821..68fef45ed7 100644 --- a/pkg/container_backend/legacy_stage_image.go +++ b/pkg/container_backend/legacy_stage_image.go @@ -15,10 +15,11 @@ import ( type LegacyStageImage struct { *legacyBaseImage - fromImage *LegacyStageImage - container *LegacyStageImageContainer - buildImage *legacyBaseImage - builtID string + fromImage *LegacyStageImage + container *LegacyStageImageContainer + buildImage *legacyBaseImage + builtID string + commitChangeOptions LegacyCommitChangeOptions } func NewLegacyStageImage(fromImage *LegacyStageImage, name string, containerBackend ContainerBackend) *LegacyStageImage { @@ -42,6 +43,10 @@ func (i *LegacyStageImage) GetCopy() LegacyImageInterface { return ni } +func (i *LegacyStageImage) SetCommitChangeOptions(opts LegacyCommitChangeOptions) { + i.commitChangeOptions = opts +} + func (i *LegacyStageImage) BuilderContainer() LegacyBuilderContainer { return &LegacyStageImageBuilderContainer{i} } diff --git a/pkg/container_backend/legacy_stage_image_container.go b/pkg/container_backend/legacy_stage_image_container.go index bf3b47c1fd..77b3aaed65 100644 --- a/pkg/container_backend/legacy_stage_image_container.go +++ b/pkg/container_backend/legacy_stage_image_container.go @@ -40,7 +40,7 @@ func (c *LegacyStageImageContainer) Name() string { } func (c *LegacyStageImageContainer) UserCommitChanges() []string { - return c.commitChangeOptions.toCommitChanges() + return c.commitChangeOptions.toCommitChanges(c.image.commitChangeOptions) } func (c *LegacyStageImageContainer) UserRunCommands() []string { @@ -201,13 +201,13 @@ func (c *LegacyStageImageContainer) prepareIntrospectOptions(ctx context.Context return c.prepareRunOptions(ctx) } -func (c *LegacyStageImageContainer) prepareCommitChanges(ctx context.Context) ([]string, error) { +func (c *LegacyStageImageContainer) prepareCommitChanges(ctx context.Context, opts LegacyCommitChangeOptions) ([]string, error) { commitOptions, err := c.prepareCommitOptions(ctx) if err != nil { return nil, err } - commitChanges, err := commitOptions.prepareCommitChanges(ctx) + commitChanges, err := commitOptions.prepareCommitChanges(ctx, opts) if err != nil { return nil, err } @@ -324,11 +324,13 @@ func IsStartContainerErr(err error) bool { func (c *LegacyStageImageContainer) commit(ctx context.Context) (string, error) { _ = c.image.ContainerBackend.(*DockerServerBackend) - commitChanges, err := c.prepareCommitChanges(ctx) + commitChanges, err := c.prepareCommitChanges(ctx, c.image.commitChangeOptions) if err != nil { return "", err } + fmt.Printf("!!! LegacyStageImageContainer.commit Changes:%#v\n", commitChanges) + commitOptions := types.ContainerCommitOptions{Changes: commitChanges} id, err := docker.ContainerCommit(ctx, c.name, commitOptions) if err != nil { diff --git a/pkg/container_backend/legacy_stage_image_container_options.go b/pkg/container_backend/legacy_stage_image_container_options.go index 2ea46aef99..4703a6507d 100644 --- a/pkg/container_backend/legacy_stage_image_container_options.go +++ b/pkg/container_backend/legacy_stage_image_container_options.go @@ -3,13 +3,20 @@ package container_backend import ( "context" "fmt" + "sort" "github.com/hashicorp/go-version" "github.com/werf/werf/pkg/docker" ) +type LegacyCommitChangeOptions struct { + ExactValues bool +} + type LegacyStageImageContainerOptions struct { + dockerServerVersion string + Volume []string VolumesFrom []string Expose []string @@ -132,6 +139,19 @@ func (co *LegacyStageImageContainerOptions) merge(co2 *LegacyStageImageContainer return mergedCo } +func getKeys(m map[string]string) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + return keys +} + +func sortStrings(arr []string) []string { + sort.Strings(arr) + return arr +} + func (co *LegacyStageImageContainerOptions) toRunArgs() ([]string, error) { var args []string @@ -143,12 +163,12 @@ func (co *LegacyStageImageContainerOptions) toRunArgs() ([]string, error) { args = append(args, fmt.Sprintf("--volumes-from=%s", volumesFrom)) } - for key, value := range co.Env { - args = append(args, fmt.Sprintf("--env=%s=%v", key, value)) + for _, k := range sortStrings(getKeys(co.Env)) { + args = append(args, fmt.Sprintf("--env=%s=%v", k, co.Env[k])) } - for key, value := range co.Label { - args = append(args, fmt.Sprintf("--label=%s=%v", key, value)) + for _, k := range sortStrings(getKeys(co.Label)) { + args = append(args, fmt.Sprintf("--label=%s=%v", k, co.Label[k])) } if co.User != "" { @@ -166,23 +186,23 @@ func (co *LegacyStageImageContainerOptions) toRunArgs() ([]string, error) { return args, nil } -func (co *LegacyStageImageContainerOptions) toCommitChanges() []string { +func (co *LegacyStageImageContainerOptions) toCommitChanges(opts LegacyCommitChangeOptions) []string { var args []string for _, volume := range co.Volume { - args = append(args, fmt.Sprintf("VOLUME %s", volume)) + args = append(args, fmt.Sprintf("VOLUME %s", escapeVolume(volume, opts.ExactValues))) } for _, expose := range co.Expose { - args = append(args, fmt.Sprintf("EXPOSE %s", expose)) + args = append(args, fmt.Sprintf("EXPOSE %s", defaultEscape(expose, opts.ExactValues))) } - for key, value := range co.Env { - args = append(args, fmt.Sprintf("ENV %s=%v", key, value)) + for _, k := range sortStrings(getKeys(co.Env)) { + args = append(args, fmt.Sprintf("ENV %s=%s", k, defaultEscape(co.Env[k], opts.ExactValues))) } - for key, value := range co.Label { - args = append(args, fmt.Sprintf("LABEL %s=%v", key, value)) + for _, k := range sortStrings(getKeys(co.Label)) { + args = append(args, fmt.Sprintf("LABEL %s=%s", k, defaultEscape(co.Label[k], opts.ExactValues))) } if co.Cmd != "" { @@ -208,23 +228,25 @@ func (co *LegacyStageImageContainerOptions) toCommitChanges() []string { return args } -func (co *LegacyStageImageContainerOptions) prepareCommitChanges(ctx context.Context) ([]string, error) { +func (co *LegacyStageImageContainerOptions) prepareCommitChanges(ctx context.Context, opts LegacyCommitChangeOptions) ([]string, error) { var args []string + fmt.Printf("-- LegacyStageImageContainerOptions.prepareCommitChanges opts.ExactValues=%v\n", opts.ExactValues) + for _, volume := range co.Volume { - args = append(args, fmt.Sprintf("VOLUME %s", volume)) + args = append(args, fmt.Sprintf("VOLUME %s", escapeVolume(volume, opts.ExactValues))) } for _, expose := range co.Expose { - args = append(args, fmt.Sprintf("EXPOSE %s", expose)) + args = append(args, fmt.Sprintf("EXPOSE %s", defaultEscape(expose, opts.ExactValues))) } - for key, value := range co.Env { - args = append(args, fmt.Sprintf("ENV %s=%v", key, value)) + for _, k := range sortStrings(getKeys(co.Env)) { + args = append(args, fmt.Sprintf("ENV %s=%s", k, defaultEscape(co.Env[k], opts.ExactValues))) } - for key, value := range co.Label { - args = append(args, fmt.Sprintf("LABEL %s=%v", key, value)) + for _, k := range sortStrings(getKeys(co.Label)) { + args = append(args, fmt.Sprintf("LABEL %s=%s", k, defaultEscape(co.Label[k], opts.ExactValues))) } if co.Workdir != "" { @@ -240,7 +262,7 @@ func (co *LegacyStageImageContainerOptions) prepareCommitChanges(ctx context.Con if co.Entrypoint != "" { entrypoint = co.Entrypoint } else { - entrypoint, err = getEmptyEntrypointInstructionValue(ctx) + entrypoint, err = getEmptyEntrypointInstructionValue(ctx, co.dockerServerVersion) if err != nil { return nil, fmt.Errorf("container options preparing failed: %w", err) } @@ -261,13 +283,20 @@ func (co *LegacyStageImageContainerOptions) prepareCommitChanges(ctx context.Con return args, nil } -func getEmptyEntrypointInstructionValue(ctx context.Context) (string, error) { - v, err := docker.ServerVersion(ctx) - if err != nil { - return "", err +func getEmptyEntrypointInstructionValue(ctx context.Context, dockerServerVersion string) (string, error) { + var rawServerVersion string + + if dockerServerVersion == "" { + v, err := docker.ServerVersion(ctx) + if err != nil { + return "", fmt.Errorf("unable to query docker server version: %w", err) + } + rawServerVersion = v.Version + } else { + rawServerVersion = dockerServerVersion } - serverVersion, err := version.NewVersion(v.Version) + serverVersion, err := version.NewVersion(rawServerVersion) if err != nil { return "", err } @@ -283,3 +312,26 @@ func getEmptyEntrypointInstructionValue(ctx context.Context) (string, error) { return "[\"\"]", nil } + +// FIXME: remove escaping in 2.0 or 1.3 +func escapeVolume(volume string, exactValues bool) string { + if exactValues { + return fmt.Sprintf("[%s]", quoteValue(volume)) + } + return volume +} + +func defaultEscape(value string, exactValues bool) string { + return doEscape(value, exactValues, quoteValue) +} + +func doEscape(value string, exactValues bool, escaper func(string) string) string { + if exactValues { + return escaper(value) + } + return value +} + +func quoteValue(value string) string { + return fmt.Sprintf("%q", value) +} diff --git a/pkg/container_backend/legacy_stage_image_container_options_test.go b/pkg/container_backend/legacy_stage_image_container_options_test.go new file mode 100644 index 0000000000..c26f85e0d2 --- /dev/null +++ b/pkg/container_backend/legacy_stage_image_container_options_test.go @@ -0,0 +1,170 @@ +package container_backend + +import ( + "bytes" + "context" + "fmt" + "strings" + + "github.com/moby/buildkit/frontend/dockerfile/instructions" + "github.com/moby/buildkit/frontend/dockerfile/parser" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +type commitChangesAndRunArgsTest struct { + CommitChangeOptions LegacyCommitChangeOptions + Options *LegacyStageImageContainerOptions + + ExpectedCommitChanges []string + ExpectedPrepareCommitChanges []string +} + +var _ = Describe("LegacyStageImageContainerOptions", func() { + DescribeTable("Docker server commit changes and run args generator", func(tst commitChangesAndRunArgsTest) { + ctx := context.Background() + + commitChanges := tst.Options.toCommitChanges(tst.CommitChangeOptions) + + prepareCommitChanges, err := tst.Options.prepareCommitChanges(ctx, tst.CommitChangeOptions) + Expect(err).To(Succeed()) + + fmt.Printf("Commit changes result: %v\n", commitChanges) + fmt.Printf("Prepare changes result: %v\n", prepareCommitChanges) + + dockerfile, err := parser.Parse(bytes.NewBufferString(strings.Join(prepareCommitChanges, "\n"))) + Expect(err).To(Succeed()) + + for _, n := range dockerfile.AST.Children { + _, err = instructions.ParseCommand(n) + Expect(err).To(Succeed()) + } + + Expect(commitChanges).To(Equal(tst.ExpectedCommitChanges)) + Expect(prepareCommitChanges).To(Equal(tst.ExpectedPrepareCommitChanges)) + }, + + Entry("empty", commitChangesAndRunArgsTest{ + Options: &LegacyStageImageContainerOptions{ + dockerServerVersion: "20.10.16", + }, + ExpectedPrepareCommitChanges: []string{`ENTRYPOINT [""]`, `CMD []`}, + ExpectedCommitChanges: nil, + }), + + Entry("without exact interpretation (legacy)", commitChangesAndRunArgsTest{ + Options: &LegacyStageImageContainerOptions{ + dockerServerVersion: "20.10.16", + Volume: []string{"/volume1", "/volume2", `/my\ volume\ with\ spaces`}, + VolumesFrom: []string{"container-1", "container-2"}, + Expose: []string{"80/tcp", "449/upd"}, + Env: map[string]string{ + "MYVAR": "V1", + "MYVAR2": `"value with spaces"`, + }, + Label: map[string]string{ + "version": "v1.2.3", + "maintainer": "vasilyi.ivanovich@gmail.com", + "mylabel": `"my value with spaces"`, + }, + Cmd: "server run", + Workdir: `"/work dir"`, + User: "1000:1000", + Entrypoint: `"/dir with spaces/bin/bash"`, + HealthCheck: "--interval=30s --timeout=3s CMD curl -f http://localhost/ || exit 1", + }, + ExpectedCommitChanges: []string{ + `VOLUME /volume1`, + `VOLUME /volume2`, + `VOLUME /my\ volume\ with\ spaces`, + `EXPOSE 80/tcp`, + `EXPOSE 449/upd`, + `ENV MYVAR=V1`, + `ENV MYVAR2="value with spaces"`, + `LABEL maintainer=vasilyi.ivanovich@gmail.com`, + `LABEL mylabel="my value with spaces"`, + `LABEL version=v1.2.3`, + `CMD server run`, + `WORKDIR "/work dir"`, + `USER 1000:1000`, + `ENTRYPOINT "/dir with spaces/bin/bash"`, + `HEALTHCHECK --interval=30s --timeout=3s CMD curl -f http://localhost/ || exit 1`, + }, + ExpectedPrepareCommitChanges: []string{ + `VOLUME /volume1`, + `VOLUME /volume2`, + `VOLUME /my\ volume\ with\ spaces`, + `EXPOSE 80/tcp`, + `EXPOSE 449/upd`, + `ENV MYVAR=V1`, + `ENV MYVAR2="value with spaces"`, + `LABEL maintainer=vasilyi.ivanovich@gmail.com`, + `LABEL mylabel="my value with spaces"`, + `LABEL version=v1.2.3`, + `WORKDIR "/work dir"`, + `USER 1000:1000`, + `ENTRYPOINT "/dir with spaces/bin/bash"`, + `CMD server run`, + `HEALTHCHECK --interval=30s --timeout=3s CMD curl -f http://localhost/ || exit 1`, + }, + }), + + Entry("with exact interpretation", commitChangesAndRunArgsTest{ + CommitChangeOptions: LegacyCommitChangeOptions{ExactValues: true}, + Options: &LegacyStageImageContainerOptions{ + dockerServerVersion: "20.10.16", + Volume: []string{"/volume1", "/volume2", "/my volume with spaces"}, + VolumesFrom: []string{"container-1", "container-2"}, + Expose: []string{"80/tcp", "449/upd"}, + Env: map[string]string{ + "MYVAR": "V1", + "MYVAR2": "value with spaces", + }, + Label: map[string]string{ + "version": "v1.2.3", + "maintainer": "vasilyi.ivanovich@gmail.com", + "mylabel": "my value with spaces", + }, + Cmd: "server run", + Workdir: "/work dir", + User: "1000:1000", + Entrypoint: "/dir with spaces/bin/bash", + HealthCheck: "--interval=30s --timeout=3s CMD curl -f http://localhost/ || exit 1", + }, + ExpectedCommitChanges: []string{ + `VOLUME ["/volume1"]`, + `VOLUME ["/volume2"]`, + `VOLUME ["/my volume with spaces"]`, + `EXPOSE "80/tcp"`, + `EXPOSE "449/upd"`, + `ENV MYVAR="V1"`, + `ENV MYVAR2="value with spaces"`, + `LABEL maintainer="vasilyi.ivanovich@gmail.com"`, + `LABEL mylabel="my value with spaces"`, + `LABEL version="v1.2.3"`, + `CMD server run`, + `WORKDIR /work dir`, + `USER 1000:1000`, + `ENTRYPOINT /dir with spaces/bin/bash`, + `HEALTHCHECK --interval=30s --timeout=3s CMD curl -f http://localhost/ || exit 1`, + }, + ExpectedPrepareCommitChanges: []string{ + `VOLUME ["/volume1"]`, + `VOLUME ["/volume2"]`, + `VOLUME ["/my volume with spaces"]`, + `EXPOSE "80/tcp"`, + `EXPOSE "449/upd"`, + `ENV MYVAR="V1"`, + `ENV MYVAR2="value with spaces"`, + `LABEL maintainer="vasilyi.ivanovich@gmail.com"`, + `LABEL mylabel="my value with spaces"`, + `LABEL version="v1.2.3"`, + `WORKDIR /work dir`, + `USER 1000:1000`, + `ENTRYPOINT /dir with spaces/bin/bash`, + `CMD server run`, + `HEALTHCHECK --interval=30s --timeout=3s CMD curl -f http://localhost/ || exit 1`, + }, + }), + ) +}) diff --git a/pkg/container_backend/suite_test.go b/pkg/container_backend/suite_test.go new file mode 100644 index 0000000000..251bdc2405 --- /dev/null +++ b/pkg/container_backend/suite_test.go @@ -0,0 +1,13 @@ +package container_backend + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestStage(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Stage Suite") +}