diff --git a/docs/spec.schema.json b/docs/spec.schema.json index 66e12a151..6485069e2 100644 --- a/docs/spec.schema.json +++ b/docs/spec.schema.json @@ -1077,16 +1077,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 969577708..1b7082936 100644 --- a/frontend/azlinux/handle_container.go +++ b/frontend/azlinux/handle_container.go @@ -75,7 +75,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 8d58207f2..3e423c97a 100644 --- a/frontend/windows/handle_container.go +++ b/frontend/windows/handle_container.go @@ -116,21 +116,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)) + + for oldpath, newpaths := range post.Symlinks { + for _, newpath := range newpaths.Paths { + s = s.File(llb.Mkdir(path.Dir(newpath), 0755, llb.WithParents(true))) + s = s.File(llb.Copy(s, oldpath, newpath)) + } } return s } - } func getTargetPlatform(bc *dockerui.Client) (ocispecs.Platform, error) { diff --git a/helpers.go b/helpers.go index 1891beb15..bf5582ce2 100644 --- a/helpers.go +++ b/helpers.go @@ -383,9 +383,11 @@ 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)) + for oldpath, newpaths := range post.Symlinks { + for _, newpath := range newpaths.Paths { + 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/load.go b/load.go index f6dcdd8c1..5472b0c4a 100644 --- a/load.go +++ b/load.go @@ -323,6 +323,8 @@ func (s *Spec) FillDefaults() { t.fillDefaults() s.Targets[k] = t } + + s.Image.fillDefaults() } func (s Spec) Validate() error { @@ -381,8 +383,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 @@ -447,3 +454,107 @@ func (b *ArtifactBuild) processBuildArgs(lex *shell.Lex, args map[string]string, return goerrors.Join(errs...) } + +func (i *ImageConfig) validate() error { + if i == nil { + return nil + } + + var errs []error + if err := i.Post.validate(); err != nil { + errs = append(errs, errors.Wrap(err, "post")) + } + + return goerrors.Join(errs...) +} + +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 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 0a4027b13..e370a5964 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" @@ -996,7 +997,7 @@ build: }) } -func Test_validatePatch(t *testing.T) { +func TestPatchValidation(t *testing.T) { type testCase struct { name string patchSrc Source @@ -1073,3 +1074,359 @@ func Test_validatePatch(t *testing.T) { }) } } + +func TestImageConfigValidation(t *testing.T) { + t.Run("test postinstall", testPostInstall) +} + +func testPostInstall(t *testing.T) { + t.Run("test symlinks", testSymlinks) +} + +func testSymlinks(t *testing.T) { + type symlinkTableEntry struct { + ImageConfig + shouldPassVaildation bool + desc string + } + + table := []symlinkTableEntry{ + { + desc: "valid SymlinkTarget should pass validation (path)", + ImageConfig: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "newpath", + }, + }, + }, + }, + shouldPassVaildation: true, + }, + { + desc: "valid SymlinkTarget should pass validation (paths, single)", + ImageConfig: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Paths: []string{"newpath"}, + }, + }, + }, + }, + shouldPassVaildation: true, + }, + { + desc: "valid SymlinkTarget should pass validation (paths, multiple)", + ImageConfig: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Paths: []string{"newpath1", "newpath2"}, + }, + }, + }, + }, + shouldPassVaildation: true, + }, + { + desc: "invalid SymlinkTarget should fail validation: empty target", + ImageConfig: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": {}, + }, + }, + }, + }, + { + desc: "invalid SymlinkTarget should fail validation: empty key, valid target(paths)", + ImageConfig: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "": { + Paths: []string{"/newpath_z", "/newpath_a"}, + }, + }, + }, + }, + }, + { + desc: "invalid SymlinkTarget should fail validation: empty key: valid target(path)", + ImageConfig: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "": { + Path: "/newpath_z", + }, + }, + }, + }, + }, + { + desc: "invalid SymlinkTarget should fail validation: all symlink 'newpaths' should be unique(paths)", + ImageConfig: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "perfectly_valid": { + Path: "/also_valid", + }, + "also_perfectly_valid": { + Paths: []string{"/also_valid"}, + }, + }, + }, + }, + }, + { + desc: "invalid SymlinkTarget should fail validation: all symlink 'newpaths' should be unique(path)", + ImageConfig: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "perfectly_valid": { + Path: "/also_valid", + }, + "also_perfectly_valid": { + Path: "/also_valid", + }, + }, + }, + }, + }, + { + desc: "invalid SymlinkTarget should fail validation: path and paths are mutually exclusive", + ImageConfig: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "perfectly_valid": { + Path: "/also_valid", + Paths: []string{"/also_valid_too", "also_valid_too,_also"}, + }, + }, + }, + }, + }, + } + + for _, test := range table { + t.Run(test.desc, func(t *testing.T) { + symlinks := test.ImageConfig.Post.Symlinks + + err := validateSymlinks(symlinks) + if err == nil { + if test.shouldPassVaildation { + return + } + + t.Error("should not have passed validation, but did anyway") + return + } + + // err is non-nil + + if test.shouldPassVaildation { + t.Errorf("should have passed validation, but failed with error: %s\n%#v\n", err, symlinks) + return + } + }) + } +} + +func TestImageFillDefaults(t *testing.T) { + t.Run("postinstall", testPostInstallFillDefaults) +} + +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: "/newpath2", + Paths: []string{"/newpath1"}, + }, + }, + }, + }, + output: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "", + Paths: []string{"/newpath1", "/newpath2"}, + }, + }, + }, + }, + }, + { + desc: "Path should be appended to Paths", + input: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "/newpath3", + Paths: []string{"/newpath1", "/newpath2"}, + }, + }, + }, + }, + output: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "", + Paths: []string{"/newpath1", "/newpath2", "/newpath3"}, + }, + }, + }, + }, + }, + { + 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"}, + }, + }, + }, + }, + }, + { + desc: "should work if Paths is empty", + 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"}, + }, + }, + }, + }, + }, + { + desc: "empty Path and multimple Paths should have Paths sorted", + input: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "", + Paths: []string{"/newpath2", "/newpath1"}, + }, + }, + }, + }, + output: ImageConfig{ + Post: &PostInstall{ + Symlinks: map[string]SymlinkTarget{ + "oldpath": { + Path: "", + Paths: []string{"/newpath1", "/newpath2"}, + }, + }, + }, + }, + }, + } + + 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 5f332e7d9..9510370ef 100644 --- a/target.go +++ b/target.go @@ -2,6 +2,7 @@ package dalec import ( goerrors "errors" + "sort" "github.com/moby/buildkit/frontend/dockerfile/shell" "github.com/pkg/errors" @@ -42,6 +43,10 @@ func (t *Target) validate() error { } } + if err := t.Image.validate(); err != nil { + errs = append(errs, errors.Wrap(err, "postinstall")) + } + return goerrors.Join(errs...) } @@ -68,4 +73,30 @@ func (t *Target) processBuildArgs(lex *shell.Lex, args map[string]string, allowA func (t *Target) fillDefaults() { t.Dependencies.fillDefaults() + t.Image.fillDefaults() +} + +func (i *ImageConfig) fillDefaults() { + if i == nil { + return + } + + i.Post.fillDefaults() +} + +func (p *PostInstall) fillDefaults() { + if p == nil { + return + } + + for oldpath := range p.Symlinks { + cfg := p.Symlinks[oldpath] + if cfg.Path != "" { + cfg.Paths = append(cfg.Paths, cfg.Path) + cfg.Path = "" + } + + sort.Strings(cfg.Paths) + p.Symlinks[oldpath] = cfg + } } 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 afe50148b..628ff338b 100644 --- a/test/windows_test.go +++ b/test/windows_test.go @@ -299,7 +299,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"}}, }, }, }, @@ -329,30 +329,33 @@ echo "$BAR" > bar.txt } 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") + } + } } + } }) })