Skip to content

Commit

Permalink
feat: host builder nonregular file support (#2156)
Browse files Browse the repository at this point in the history
* host builder nonregular files

* disallow links outside root

* add back in correct path support for windows

* code review updates

* host builder file test

* windows-specific absolute link check

* windows-specific test

* refactor

Resolve the link target one level higher such that it can be used
when creating the tar header without repetition.

* validate link header targets

* expand link test

* merge os-specific test cases

* remove os filter and unused arg
  • Loading branch information
lkingland committed Feb 20, 2024
1 parent b97d841 commit 9ca1b6d
Show file tree
Hide file tree
Showing 15 changed files with 206 additions and 32 deletions.
113 changes: 90 additions & 23 deletions pkg/oci/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,69 @@ func TestBuilder_Build(t *testing.T) {

last := path(f.Root, fn.RunDataDir, "builds", "last", "oci")

validateOCI(last, t)
validateOCIStructure(last, t) // validate it adheres to the basics of the OCI spec
}

// TestBuilder_Files ensures that static files are added to the container
// image as expected. This includes template files, regular files and links.
func TestBuilder_Files(t *testing.T) {
root, done := Mktemp(t)
defer done()

// Create a function with the default template
f, err := fn.New().Init(fn.Function{Root: root, Runtime: "go"})
if err != nil {
t.Fatal(err)
}

// Add a regular file
if err := os.WriteFile("a.txt", []byte("file a"), 0644); err != nil {
t.Fatal(err)
}

// Links
var link struct {
Target string
Mode fs.FileMode
Executable bool
}
if runtime.GOOS != "windows" {
// Default case: use symlinks
link.Target = "a.txt"
link.Mode = fs.ModeSymlink
link.Executable = true

if err := os.Symlink("a.txt", "a.lnk"); err != nil {
t.Fatal(err)
}
} else {
// Windows: create a copy
if err := os.WriteFile("a.lnk", []byte("file a"), 0644); err != nil {
t.Fatal(err)
}
}

if err := NewBuilder("", true).Build(context.Background(), f, TestPlatforms); err != nil {
t.Fatal(err)
}

expected := []fileInfo{
{Path: "/etc/pki/tls/certs/ca-certificates.crt"},
{Path: "/etc/ssl/certs/ca-certificates.crt"},
{Path: "/func", Type: fs.ModeDir},
{Path: "/func/README.md"},
{Path: "/func/a.lnk", Linkname: link.Target, Type: link.Mode, Executable: link.Executable},
{Path: "/func/a.txt"},
{Path: "/func/f", Executable: true},
{Path: "/func/func.yaml"},
{Path: "/func/go.mod"},
{Path: "/func/handle.go"},
{Path: "/func/handle_test.go"},
}

last := path(f.Root, fn.RunDataDir, "builds", "last", "oci")

validateOCIFiles(last, expected, t)
}

// TestBuilder_Concurrency
Expand Down Expand Up @@ -145,9 +207,9 @@ type ImageIndex struct {
} `json:"manifests"`
}

// validateOCI performs a cursory check that the given path exists and
// validateOCIStructue performs a cursory check that the given path exists and
// has the basics of an OCI compliant structure.
func validateOCI(path string, t *testing.T) {
func validateOCIStructure(path string, t *testing.T) {
if _, err := os.Stat(path); err != nil {
t.Fatalf("unable to stat output path. %v", path)
return
Expand Down Expand Up @@ -185,9 +247,24 @@ func validateOCI(path string, t *testing.T) {
}

if len(imageIndex.Manifests) < 1 {
t.Fatal("fewer manifests")
t.Fatal("no manifests")
}
}

// validateOCIFiles ensures that the OCI image at path contains files with
// the given attributes.
func validateOCIFiles(path string, expected []fileInfo, t *testing.T) {
// Load the Image Index
bb, err := os.ReadFile(filepath.Join(path, "index.json"))
if err != nil {
t.Fatalf("failed to read index.json: %v", err)
}
var imageIndex ImageIndex
if err = json.Unmarshal(bb, &imageIndex); err != nil {
t.Fatalf("failed to parse index.json: %v", err)
}

// Load the first manifest
digest := strings.TrimPrefix(imageIndex.Manifests[0].Digest, "sha256:")
manifestFile := filepath.Join(path, "blobs", "sha256", digest)
manifestFileData, err := os.ReadFile(manifestFile)
Expand All @@ -203,12 +280,6 @@ func validateOCI(path string, t *testing.T) {
if err != nil {
t.Fatal(err)
}

type fileInfo struct {
Path string
Type fs.FileMode
Executable bool
}
var files []fileInfo

for _, layer := range mf.Layers {
Expand Down Expand Up @@ -239,6 +310,7 @@ func validateOCI(path string, t *testing.T) {
Path: hdr.Name,
Type: hdr.FileInfo().Mode() & fs.ModeType,
Executable: (hdr.FileInfo().Mode()&0111 == 0111) && !hdr.FileInfo().IsDir(),
Linkname: hdr.Linkname,
})
}
}()
Expand All @@ -247,19 +319,14 @@ func validateOCI(path string, t *testing.T) {
return files[i].Path < files[j].Path
})

expectedFiles := []fileInfo{
{Path: "/etc/pki/tls/certs/ca-certificates.crt"},
{Path: "/etc/ssl/certs/ca-certificates.crt"},
{Path: "/func", Type: fs.ModeDir},
{Path: "/func/README.md"},
{Path: "/func/f", Executable: true},
{Path: "/func/func.yaml"},
{Path: "/func/go.mod"},
{Path: "/func/handle.go"},
{Path: "/func/handle_test.go"},
}

if diff := cmp.Diff(expectedFiles, files); diff != "" {
if diff := cmp.Diff(expected, files); diff != "" {
t.Error("files in oci differ from expectation (-want, +got):", diff)
}
}

type fileInfo struct {
Path string
Type fs.FileMode
Executable bool
Linkname string
}
58 changes: 49 additions & 9 deletions pkg/oci/containerize.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"fmt"
"io"
"io/fs"
"os"
slashpath "path"
"path/filepath"
Expand All @@ -14,6 +15,7 @@ import (
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/tarball"
"github.com/google/go-containerregistry/pkg/v1/types"
"github.com/pkg/errors"
)

// languageLayerBuilder builds the layer for the given language whuch may
Expand Down Expand Up @@ -123,7 +125,7 @@ func newDataLayer(cfg *buildConfig) (desc v1.Descriptor, layer v1.Layer, err err
return
}

func newDataTarball(source, target string, ignored []string, verbose bool) error {
func newDataTarball(root, target string, ignored []string, verbose bool) error {
targetFile, err := os.Create(target)
if err != nil {
return err
Expand All @@ -136,11 +138,12 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error
tw := tar.NewWriter(gw)
defer tw.Close()

return filepath.Walk(source, func(path string, info os.FileInfo, err error) error {
return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}

// Skip files explicitly ignored
for _, v := range ignored {
if info.Name() == v {
if info.IsDir() {
Expand All @@ -150,27 +153,30 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error
}
}

header, err := tar.FileInfoHeader(info, info.Name())
lnk := "" // if link, this will be used as the target
if info.Mode()&fs.ModeSymlink != 0 {
if lnk, err = validatedLinkTarget(root, path); err != nil {
return err
}
}

header, err := tar.FileInfoHeader(info, lnk)
if err != nil {
return err
}

relPath, err := filepath.Rel(source, path)
relPath, err := filepath.Rel(root, path)
if err != nil {
return err
}

header.Name = slashpath.Join("/func", filepath.ToSlash(relPath))
// TODO: should we set file timestamps to the build start time of cfg.t?
// header.ModTime = timestampArgument

if err := tw.WriteHeader(header); err != nil {
return err
}
if verbose {
fmt.Printf("→ %v \n", header.Name)
}
if info.IsDir() {
if !info.Mode().IsRegular() { //nothing more to do for non-regular
return nil
}

Expand All @@ -185,6 +191,40 @@ func newDataTarball(source, target string, ignored []string, verbose bool) error
})
}

// validatedLinkTarget returns the target of a given link or an error if
// that target is either absolute or outside the given project root.
func validatedLinkTarget(root, path string) (tgt string, err error) {
// tgt is the raw target of the link.
// This path is either absolute or relative to the link's location.
tgt, err = os.Readlink(path)
if err != nil {
return tgt, fmt.Errorf("cannot read link: %w", err)
}

// Absolute links will not be correct when copied into the runtime
// container, because they are placed into path into '/func',
if filepath.IsAbs(tgt) {
return tgt, errors.New("project may not contain absolute links")
}

// Calculate the actual target of the link
// (relative to the parent of the symlink)
lnkTgt := filepath.Join(filepath.Dir(path), tgt)

// Calculate the relative path from the function's root to
// this actual target location
relLnkTgt, err := filepath.Rel(root, lnkTgt)
if err != nil {
return
}

// Fail if this path is outside the function's root.
if strings.HasPrefix(relLnkTgt, ".."+string(filepath.Separator)) || relLnkTgt == ".." {
return tgt, errors.New("links must stay within project root")
}
return
}

// newCertLayer creates the shared data layer in the container file hierarchy and
// returns both its descriptor and layer metadata.
func newCertsLayer(cfg *buildConfig) (desc v1.Descriptor, layer v1.Layer, err error) {
Expand Down
55 changes: 55 additions & 0 deletions pkg/oci/containerize_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package oci

import (
"path/filepath"
"runtime"
"testing"
)

// Test_validatedLinkTaarget ensures that the function disallows
// links which are absolute or refer to targets outside the given root, in
// addition to the basic job of returning the value of reading the link.
func Test_validatedLinkTarget(t *testing.T) {
root := "testdata/test-links"

// Windows-specific absolute link and link target values:
absoluteLink := "absoluteLink"
linkTarget := "./a.txt"
if runtime.GOOS == "windows" {
absoluteLink = "absoluteLinkWindows"
linkTarget = ".\\a.txt"
}

tests := []struct {
path string // path of the file within test project root
valid bool // If it should be considered valid
target string // optional test of the returned value (target)
name string // descriptive name of the test
}{
{absoluteLink, false, "", "disallow absolute-path links on linux"},
{"a.lnk", true, linkTarget, "spot-check link target"},
{"a.lnk", true, "", "links to files within the root are allowed"},
{"...validName.lnk", true, "", "allow links with target of dot prefixed names"},
{"linkToRoot", true, "", "allow links to the project root"},
{"b/linkToRoot", true, "", "allow links to the project root from within subdir"},
{"b/linkToCurrentDir", true, "", "allow links to a subdirectory within the project"},
{"b/linkToRootsParent", false, "", "disallow links to the project's immediate parent"},
{"b/linkOutsideRootsParent", false, "", "disallow links outside project root and its parent"},
{"b/c/linkToParent", true, "", " allow links up, but within project"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
path := filepath.Join(root, tt.path)
target, err := validatedLinkTarget(root, path)

if err == nil != tt.valid {
t.Fatalf("expected validity '%v', got '%v'", tt.valid, err)
}
if tt.target != "" && target != tt.target {
t.Fatalf("expected target %q, got %q", tt.target, target)
}
})
}

}
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/...validName.lnk
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/...validName.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
validName.txt
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/a.lnk
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/a.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
file a
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/absoluteLink
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/absoluteLinkWindows
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/b/c/linkToParent
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/b/linkOutsideRootsParent
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/b/linkToCurrentDir
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/b/linkToRoot
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/b/linkToRootsParent
1 change: 1 addition & 0 deletions pkg/oci/testdata/test-links/linkToRoot

0 comments on commit 9ca1b6d

Please sign in to comment.