diff --git a/docs/spec.schema.json b/docs/spec.schema.json index 48c2a24c7..2a484de44 100644 --- a/docs/spec.schema.json +++ b/docs/spec.schema.json @@ -1106,16 +1106,38 @@ "description": "Spec is the specification for a package build." }, "SymlinkTarget": { + "oneOf": [ + { + "required": [ + "path" + ], + "title": "path" + }, + { + "required": [ + "paths" + ], + "title": "paths" + } + ], "properties": { "path": { "type": "string", - "description": "Path is the path where the symlink should be placed" + "description": "Path is the path where the symlink should be placed\n\nDeprecated: This is here for backward compatibility. Use `Paths` instead." + }, + "paths": { + "items": { + "type": "string" + }, + "type": "array", + "description": "Path is a list of `newpath`s that will all point to the same `oldpath`." } }, "additionalProperties": false, "type": "object", "required": [ - "path" + "path", + "paths" ], "description": "SymlinkTarget specifies the properties of a symlink" }, diff --git a/frontend/azlinux/handle_container.go b/frontend/azlinux/handle_container.go index d6f6bf2ab..005d46c35 100644 --- a/frontend/azlinux/handle_container.go +++ b/frontend/azlinux/handle_container.go @@ -79,7 +79,7 @@ func specToContainerLLB(w worker, spec *dalec.Spec, targetKey string, rpmDir llb dalec.WithConstraints(opts...), ).AddMount(workPath, rootfs) - if post := spec.GetImagePost(targetKey); post != nil && len(post.Symlinks) > 0 { + if post := spec.GetImagePost(targetKey); post != nil { rootfs = builderImg. Run(dalec.WithConstraints(opts...), dalec.InstallPostSymlinks(post, workPath)). AddMount(workPath, rootfs) diff --git a/frontend/windows/handle_container.go b/frontend/windows/handle_container.go index 88d56a204..17bd1cc58 100644 --- a/frontend/windows/handle_container.go +++ b/frontend/windows/handle_container.go @@ -6,6 +6,7 @@ import ( "fmt" "path" "runtime" + "sort" "sync" "github.com/Azure/dalec" @@ -198,16 +199,19 @@ func copySymlinks(post *dalec.PostInstall) llb.StateOption { return s } - lm := post.Symlinks - if len(lm) == 0 { + if len(post.Symlinks) == 0 { return s } - keys := dalec.SortMapKeys(lm) - for _, srcPath := range keys { - l := lm[srcPath] - dstPath := l.Path - s = s.File(llb.Mkdir(path.Dir(dstPath), 0755, llb.WithParents(true))) - s = s.File(llb.Copy(s, srcPath, dstPath)) + + sortedKeys := dalec.SortMapKeys(post.Symlinks) + for _, oldpath := range sortedKeys { + newpaths := post.Symlinks[oldpath].Paths + sort.Strings(newpaths) + + for _, newpath := range newpaths { + s = s.File(llb.Mkdir(path.Dir(newpath), 0755, llb.WithParents(true))) + s = s.File(llb.Copy(s, oldpath, newpath)) + } } return s diff --git a/helpers.go b/helpers.go index 1891beb15..6e0a84ab8 100644 --- a/helpers.go +++ b/helpers.go @@ -383,9 +383,15 @@ func InstallPostSymlinks(post *PostInstall, rootfsPath string) llb.RunOption { buf := bytes.NewBuffer(nil) buf.WriteString("set -ex\n") - for src, tgt := range post.Symlinks { - fmt.Fprintf(buf, "mkdir -p %q\n", filepath.Join(rootfsPath, filepath.Dir(tgt.Path))) - fmt.Fprintf(buf, "ln -s %q %q\n", src, filepath.Join(rootfsPath, tgt.Path)) + sortedKeys := SortMapKeys(post.Symlinks) + for _, oldpath := range sortedKeys { + newpaths := post.Symlinks[oldpath].Paths + sort.Strings(newpaths) + + for _, newpath := range newpaths { + fmt.Fprintf(buf, "mkdir -p %q\n", filepath.Join(rootfsPath, filepath.Dir(newpath))) + fmt.Fprintf(buf, "ln -s %q %q\n", oldpath, filepath.Join(rootfsPath, newpath)) + } } const name = "tmp.dalec.symlink.sh" diff --git a/image.go b/image.go index f2e483fea..ae710d6b0 100644 --- a/image.go +++ b/image.go @@ -145,46 +145,53 @@ func MergeImageConfig(dst *DockerImageConfig, src *ImageConfig) error { return nil } -func (s *ImageConfig) validate() error { - if s == nil { +func (i *ImageConfig) validate() error { + if i == nil { return nil } var errs []error - if s.Base != "" && len(s.Bases) > 0 { + if i.Base != "" && len(i.Bases) > 0 { errs = append(errs, errors.New("cannot specify both image.base and image.bases")) } - for i, base := range s.Bases { + for i, base := range i.Bases { if err := base.validate(); err != nil { errs = append(errs, errors.Wrapf(err, "bases[%d]", i)) } } + + if err := i.Post.validate(); err != nil { + errs = append(errs, errors.Wrap(err, "postinstall")) + } + return goerrors.Join(errs...) } -func (s *ImageConfig) fillDefaults() { - if s == nil { +func (i *ImageConfig) fillDefaults() { + if i == nil { return } // s.Bases is a superset of s.Base, so migrate s.Base to s.Bases - if s.Base != "" { - s.Bases = append(s.Bases, BaseImage{ + if i.Base != "" { + i.Bases = append(i.Bases, BaseImage{ Rootfs: Source{ DockerImage: &SourceDockerImage{ - Ref: s.Base, + Ref: i.Base, }, }, }) - s.Base = "" + i.Base = "" } - for _, bi := range s.Bases { + for _, bi := range i.Bases { bi.fillDefaults() } + + i.Post.normalizeSymlinks() } func (s *BaseImage) validate() error { @@ -199,10 +206,42 @@ func (s *BaseImage) validate() error { return nil } +func (p *PostInstall) validate() error { + if p == nil { + return nil + } + + var errs []error + + if err := validateSymlinks(p.Symlinks); err != nil { + errs = append(errs, err) + } + + return errors.Wrap(goerrors.Join(errs...), "symlink") +} + func (s *BaseImage) fillDefaults() { fillDefaults(&s.Rootfs) } +func (p *PostInstall) normalizeSymlinks() { + if p == nil { + return + } + + // validation has already taken place + for oldpath := range p.Symlinks { + cfg := p.Symlinks[oldpath] + if cfg.Path == "" { + continue + } + + cfg.Paths = append(cfg.Paths, cfg.Path) + cfg.Path = "" + p.Symlinks[oldpath] = cfg + } +} + func (bi *BaseImage) ResolveImageConfig(ctx context.Context, sOpt SourceOpts, opt sourceresolver.Opt) ([]byte, error) { // In the future, *BaseImage may support other source types, but for now it only supports Docker images. // diff --git a/load.go b/load.go index 2be576713..23f4d3f7e 100644 --- a/load.go +++ b/load.go @@ -324,6 +324,8 @@ func (s *Spec) FillDefaults() { t.fillDefaults() s.Targets[k] = t } + + s.Image.fillDefaults() } func (s Spec) Validate() error { @@ -386,8 +388,13 @@ func (s Spec) Validate() error { } } + if err := s.Image.validate(); err != nil { + errs = append(errs, errors.Wrap(err, "image")) + } + return goerrors.Join(errs...) } + func validatePatch(patch PatchSpec, patchSrc Source) error { if SourceIsDir(patchSrc) { // Patch sources that use directory-backed sources require a subpath in the @@ -452,3 +459,80 @@ func (b *ArtifactBuild) processBuildArgs(lex *shell.Lex, args map[string]string, return goerrors.Join(errs...) } + +func validateSymlinks(symlinks map[string]SymlinkTarget) error { + var ( + errs []error + numPairs int + ) + + for oldpath, cfg := range symlinks { + var err error + if oldpath == "" { + err = fmt.Errorf("symlink source is empty") + errs = append(errs, err) + } + + if (cfg.Path != "" && len(cfg.Paths) > 0) || (cfg.Path == "" && len(cfg.Paths) == 0 ) { + err = fmt.Errorf("'path' and 'paths' fields are mutually exclusive, and at least one is required: "+ + "symlink to %s", oldpath) + + errs = append(errs, err) + } + + if err != nil { + continue + } + + if cfg.Path != "" { // this means .Paths is empty + numPairs++ + continue + } + + for _, newpath := range cfg.Paths { // this means .Path is empty + numPairs++ + if newpath == "" { + errs = append(errs, fmt.Errorf("symlink newpath should not be empty")) + continue + } + } + } + + // The remainder of this function checks for duplicate `newpath`s in the + // symlink pairs. This is not allowed: neither the ordering of the + // `oldpath` map keys, nor that of the `.Paths` values can be trusted. We + // also sort both to avoid cache misses, so we would end up with + // inconsistent behavior -- regardless of whether the inputs are the same. + if numPairs < 2 { + return goerrors.Join(errs...) + } + + var ( + oldpath string + cfg SymlinkTarget + ) + + seen := make(map[string]string, numPairs) + checkDuplicateNewpath := func(newpath string) { + if newpath == "" { + return + } + + if seenPath, found := seen[newpath]; found { + errs = append(errs, fmt.Errorf("symlink 'newpaths' must be unique: %q points to both %q and %q", + newpath, oldpath, seenPath)) + } + + seen[newpath] = oldpath + } + + for oldpath, cfg = range symlinks { + checkDuplicateNewpath(cfg.Path) + + for _, newpath := range cfg.Paths { + checkDuplicateNewpath(newpath) + } + } + + return goerrors.Join(errs...) +} diff --git a/load_test.go b/load_test.go index 4b8104586..c6ea41c07 100644 --- a/load_test.go +++ b/load_test.go @@ -8,6 +8,7 @@ import ( "maps" "os" "reflect" + "slices" "testing" "github.com/moby/buildkit/frontend/dockerui" @@ -1099,6 +1100,8 @@ targets: assert.Check(t, spec.Targets["foo"].Image.Bases[0].Rootfs.DockerImage != nil) assert.Check(t, cmp.Equal(spec.Targets["foo"].Image.Bases[0].Rootfs.DockerImage.Ref, "busybox:latest")) }) + + t.Run("postinstall", testPostInstallFillDefaults) } func TestImage_validate(t *testing.T) { @@ -1147,6 +1150,125 @@ func TestImage_validate(t *testing.T) { }, }, }, + { + Name: "valid SymlinkTarget should pass validation (path)", + Image: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "newpath", + }, + }, + }, + }, + }, + { + Name: "valid SymlinkTarget should pass validation (paths, single)", + Image: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Paths: []string{"newpath"}, + }, + }, + }, + }, + }, + { + Name: "valid SymlinkTarget should pass validation (paths, multiple)", + Image: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Paths: []string{"newpath1", "newpath2"}, + }, + }, + }, + }, + }, + { + Name: "invalid SymlinkTarget should fail validation: empty target", + Image: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": {}, + }, + }, + }, + expectErr: "'path' and 'paths' fields are mutually exclusive, and at least one is required", + }, + { + Name: "invalid SymlinkTarget should fail validation: empty key, valid target(paths)", + Image: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "": { + Paths: []string{"/newpath_z", "/newpath_a"}, + }, + }, + }, + }, + expectErr: "symlink source is empty", + }, + { + Name: "invalid SymlinkTarget should fail validation: empty key: valid target(path)", + Image: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "": { + Path: "/newpath_z", + }, + }, + }, + }, + expectErr: "symlink source is empty", + }, + { + Name: "invalid SymlinkTarget should fail validation: all symlink 'newpaths' should be unique(paths)", + Image: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "perfectly_valid": { + Path: "/also_valid", + }, + "also_perfectly_valid": { + Paths: []string{"/also_valid"}, + }, + }, + }, + }, + expectErr: "symlink 'newpaths' must be unique:", + }, + { + Name: "invalid SymlinkTarget should fail validation: all symlink 'newpaths' should be unique(path)", + Image: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "perfectly_valid": { + Path: "/also_valid", + }, + "also_perfectly_valid": { + Path: "/also_valid", + }, + }, + }, + }, + expectErr: "symlink 'newpaths' must be unique:", + }, + { + Name: "invalid SymlinkTarget should fail validation: path and paths are mutually exclusive", + Image: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "perfectly_valid": { + Path: "/also_valid", + Paths: []string{"/also_valid_too", "also_valid_too,_also"}, + }, + }, + }, + }, + expectErr: "'path' and 'paths' fields are mutually exclusive, and at least one is required", + }, } for _, tc := range cases { @@ -1160,3 +1282,127 @@ func TestImage_validate(t *testing.T) { }) } } + +func testPostInstallFillDefaults(t *testing.T) { + t.Run("symlinks", testSymlinkFillDefaults) +} + +func testSymlinkFillDefaults(t *testing.T) { + type tableEntry struct { + desc string + input ImageConfig + output ImageConfig + } + + // note: fillDefaults is run after validation, so input is assumed to be + // valid + + table := []tableEntry{ + { + desc: "empty Path and single Paths should remain untouched", + input: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "", + Paths: []string{"/newpath"}, + }, + }, + }, + }, + output: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "", + Paths: []string{"/newpath"}, + }, + }, + }, + }, + }, + { + desc: "path should be moved to Paths", + input: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "/newpath1", + }, + }, + }, + }, + output: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "", + Paths: []string{"/newpath1"}, + }, + }, + }, + }, + }, + { + desc: "should work if Paths is nil", + input: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "/newpath", + Paths: nil, + }, + }, + }, + }, + output: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "", + Paths: []string{"/newpath"}, + }, + }, + }, + }, + }, + } + + for _, test := range table { + t.Run(test.desc, func(t *testing.T) { + cmp := func(v1 SymlinkTarget, v2 SymlinkTarget) bool { + if v1.Path != v2.Path { + return false + } + + if !slices.Equal(v1.Paths, v2.Paths) { + return false + } + + return true + } + + test.input.fillDefaults() + + in := test.input.Post.Symlinks + out := test.output.Post.Symlinks + if err := validateSymlinks(in); err != nil { + t.Errorf("you wrote a bad test. the input must be valid for the defaults to be filled.") + return + } + + if err := validateSymlinks(out); err != nil { + t.Errorf("you wrote a bad test. the output specified fails validation") + return + } + + if !maps.EqualFunc(in, out, cmp) { + in, _ := json.MarshalIndent(in, "", "\t") + out, _ := json.MarshalIndent(out, "", "\t") + + t.Errorf("input and output are not matched:\nexpected: %s\n=======\nactual:%s\n", string(out), string(in)) + } + }) + } + +} diff --git a/spec.go b/spec.go index da4251fca..ec35c6e0e 100644 --- a/spec.go +++ b/spec.go @@ -130,7 +130,11 @@ type PostInstall struct { // SymlinkTarget specifies the properties of a symlink type SymlinkTarget struct { // Path is the path where the symlink should be placed - Path string `yaml:"path" json:"path" jsonschema:"required"` + // + // Deprecated: This is here for backward compatibility. Use `Paths` instead. + Path string `yaml:"path" json:"path" jsonschema:"oneof_required=path"` + // Path is a list of `newpath`s that will all point to the same `oldpath`. + Paths []string `yaml:"paths" json:"paths" jsonschema:"oneof_required=paths"` } type SourceDockerImage struct { diff --git a/target.go b/target.go index bc4b7df30..654bbc309 100644 --- a/target.go +++ b/target.go @@ -46,6 +46,10 @@ func (t *Target) validate() error { } } + if err := t.Image.validate(); err != nil { + errs = append(errs, errors.Wrap(err, "postinstall")) + } + return goerrors.Join(errs...) } diff --git a/test/azlinux_test.go b/test/azlinux_test.go index 8d7b55faf..a878b5c94 100644 --- a/test/azlinux_test.go +++ b/test/azlinux_test.go @@ -466,7 +466,7 @@ echo "$BAR" > bar.txt Post: &dalec.PostInstall{ Symlinks: map[string]dalec.SymlinkTarget{ "/usr/bin/src1": {Path: "/src1"}, - "/usr/bin/src3": {Path: "/non/existing/dir/src3"}, + "/usr/bin/src3": {Paths: []string{"/non/existing/dir/src3", "/non/existing/dir2/src3"}}, }, }, }, @@ -588,8 +588,13 @@ echo "$BAR" > bar.txt Steps: []dalec.TestStep{ {Command: "/bin/bash -c 'test -L /src1'"}, {Command: "/bin/bash -c 'test \"$(readlink /src1)\" = \"/usr/bin/src1\"'"}, + {Command: "/bin/bash -c 'test -L /non/existing/dir/src3'"}, + {Command: "/bin/bash -c 'test \"$(readlink /non/existing/dir/src3)\" = \"/usr/bin/src3\"'"}, + {Command: "/bin/bash -c 'test -L /non/existing/dir2/src3'"}, + {Command: "/bin/bash -c 'test \"$(readlink /non/existing/dir2/src3)\" = \"/usr/bin/src3\"'"}, {Command: "/src1", Stdout: dalec.CheckOutput{Equals: "hello world\n"}, Stderr: dalec.CheckOutput{Empty: true}}, {Command: "/non/existing/dir/src3", Stdout: dalec.CheckOutput{Equals: "goodbye\n"}, Stderr: dalec.CheckOutput{Empty: true}}, + {Command: "/non/existing/dir2/src3", Stdout: dalec.CheckOutput{Equals: "goodbye\n"}, Stderr: dalec.CheckOutput{Empty: true}}, }, }, { diff --git a/test/windows_test.go b/test/windows_test.go index 33fd89a62..470d4302a 100644 --- a/test/windows_test.go +++ b/test/windows_test.go @@ -306,7 +306,7 @@ echo "$BAR" > bar.txt Post: &dalec.PostInstall{ Symlinks: map[string]dalec.SymlinkTarget{ "/Windows/System32/src1": {Path: "/src1"}, - "/Windows/System32/src3": {Path: "/non/existing/dir/src3"}, + "/Windows/System32/src3": {Paths: []string{"/non/existing/dir/src3", "/non/existing/dir2/src3"}}, }, }, }, @@ -328,30 +328,33 @@ echo "$BAR" > bar.txt validateSymlinks := func(ctx context.Context, t *testing.T, ref gwclient.Reference, spec dalec.Spec) { post := spec.GetImagePost("windowscross") - for srcPath, l := range post.Symlinks { + for oldpath, newpaths := range post.Symlinks { b1, err := ref.ReadFile(ctx, gwclient.ReadRequest{ - Filename: srcPath, + Filename: oldpath, }) if err != nil { - t.Fatalf("couldn't find Windows \"symlink\" target %q: %v", srcPath, err) + t.Fatalf("couldn't find Windows \"symlink\" target %q: %v", oldpath, err) } - b2, err := ref.ReadFile(ctx, gwclient.ReadRequest{ - Filename: l.Path, - }) - if err != nil { - t.Fatalf("couldn't find Windows \"symlink\" at destination %q: %v", l.Path, err) - } - - if len(b1) != len(b2) { - t.Fatalf("Windows \"symlink\" not identical to target file") - } + for _, newpath := range newpaths.Paths { + b2, err := ref.ReadFile(ctx, gwclient.ReadRequest{ + Filename: newpath, + }) + if err != nil { + t.Fatalf("couldn't find Windows \"symlink\" at destination %q: %v", newpath, err) + } - for i := range b1 { - if b1[i] != b2[i] { + if len(b1) != len(b2) { t.Fatalf("Windows \"symlink\" not identical to target file") } + + for i := range b1 { + if b1[i] != b2[i] { + t.Fatalf("Windows \"symlink\" not identical to target file") + } + } } + } }