From 0f26d6891e8995ebc4ea34f4975d5c407a0d9721 Mon Sep 17 00:00:00 2001 From: David Fridrich Date: Fri, 17 Apr 2026 10:05:38 +0200 Subject: [PATCH] feat: apply .funcignore rules in Fingerprint and OCI builder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Export ParseFuncIgnore and IsIgnored from pkg/functions so both the Fingerprint and OCI builder share the same matching logic. This ensures that files excluded by .funcignore do not affect the fingerprint hash and are consistently excluded from the OCI image. Supported pattern forms: - basename (e.g. "foo") — matches at any depth - glob (e.g. "*.tmp") — filepath.Match semantics - root-relative (e.g. "/docs") — matches only at root - trailing slash (e.g. "build/") — stripped before matching Also fixes the .funcignore boilerplate to document glob syntax instead of the incorrect "regex pattern" reference. Closes #3732 Follow-up to #3624 review feedback from @gauron99. --- pkg/buildpacks/builder.go | 21 +-- pkg/buildpacks/builder_test.go | 46 +++--- pkg/functions/client.go | 102 +++++++++++-- pkg/functions/client_test.go | 261 +++++++++++++++++++++++++++++++++ pkg/oci/builder.go | 54 +------ pkg/oci/builder_test.go | 164 ++++++++++++++++----- pkg/s2i/builder.go | 14 +- pkg/s2i/builder_test.go | 111 ++++++++++++++ 8 files changed, 636 insertions(+), 137 deletions(-) diff --git a/pkg/buildpacks/builder.go b/pkg/buildpacks/builder.go index 40a9492068..832a15aaee 100644 --- a/pkg/buildpacks/builder.go +++ b/pkg/buildpacks/builder.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "io" - "os" "path/filepath" "regexp" "runtime" @@ -142,23 +141,9 @@ func (b *Builder) Build(ctx context.Context, f fn.Function, platforms []fn.Platf buildpacks = defaultBuildpacks[f.Runtime] } - // Reading .funcignore file - var excludes []string - filePath := filepath.Join(f.Root, ".funcignore") - file, err := os.Open(filePath) - if err != nil { - if !os.IsNotExist(err) { - return fmt.Errorf("\nfailed to open file: %s", err) - } - } else { - defer file.Close() - buf := new(bytes.Buffer) - _, err := io.Copy(buf, file) - if err != nil { - return fmt.Errorf("\nfailed to read file: %s", err) - } - excludes = strings.Split(buf.String(), "\n") - } + // Reading .funcignore file (user patterns only — pack handles its own + // build context and needs access to directories like .func/build) + excludes := fn.ParseFuncIgnore(f.Root) // Pack build options opts := pack.BuildOptions{ GroupID: -1, diff --git a/pkg/buildpacks/builder_test.go b/pkg/buildpacks/builder_test.go index 99138a4b50..26c81c0715 100644 --- a/pkg/buildpacks/builder_test.go +++ b/pkg/buildpacks/builder_test.go @@ -145,45 +145,45 @@ func TestBuild_BuilderImageConfigurable(t *testing.T) { } } -// TestBuild_BuilderImageExclude ensures that ignored files are not added to the func -// image -func TestBuild_BuilderImageExclude(t *testing.T) { +// TestBuild_BuilderImageExcludePatterns verifies that all supported +// .funcignore pattern forms are correctly passed to pack's Exclude option. +func TestBuild_BuilderImageExcludePatterns(t *testing.T) { root, done := Mktemp(t) defer done() var ( - i = &mockImpl{} // mock underlying implementation - b = NewBuilder( // Func Builder logic - WithName(builders.Pack), WithImpl(i)) - f = fn.Function{ - Runtime: "go", - Root: root, - Registry: "example.com/alice", - } + i = &mockImpl{} + b = NewBuilder(WithName(builders.Pack), WithImpl(i)) + f = fn.Function{Runtime: "go", Root: root, Registry: "example.com/alice"} err error ) - // Initialize the function to create proper source files if f, err = fn.New().Init(f); err != nil { t.Fatal(err) } - funcIgnoreContent := []byte(`#testing comments -hello.txt`) - expected := []string{"hello.txt"} - - //create a .funcignore file containing the details of the files to be ignored - err = os.WriteFile(filepath.Join(f.Root, ".funcignore"), funcIgnoreContent, 0644) - if err != nil { + content := []byte("# comment stripped\nnotes.txt\n*.tmp\n/docs\ndist/\n") + if err = os.WriteFile(filepath.Join(f.Root, ".funcignore"), content, 0644); err != nil { t.Fatal(err) } i.BuildFn = func(ctx context.Context, opts pack.BuildOptions) error { - if len(opts.ProjectDescriptor.Build.Exclude) != 2 { - t.Fatalf("expected 2 lines of exclusions , got %v", len(opts.ProjectDescriptor.Build.Exclude)) + excludes := opts.ProjectDescriptor.Build.Exclude + // 4 user patterns: notes.txt, *.tmp, /docs, dist/ (comment stripped) + if len(excludes) != 4 { + t.Fatalf("expected 4 exclusions, got %v: %v", len(excludes), excludes) + } + want := map[string]bool{"notes.txt": true, "*.tmp": true, "/docs": true, "dist/": true} + for _, e := range excludes { + if !want[e] { + t.Errorf("unexpected exclusion: %q", e) + } } - if opts.ProjectDescriptor.Build.Exclude[1] != expected[0] { - t.Fatalf("expected excluded file to be '%v', got '%v'", expected[1], opts.ProjectDescriptor.Build.Exclude[1]) + // Verify comment was stripped + for _, e := range excludes { + if len(e) > 0 && e[0] == '#' { + t.Errorf("comment line in excludes: %q", e) + } } return nil } diff --git a/pkg/functions/client.go b/pkg/functions/client.go index 54b9c1d115..7546a3461c 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -1334,11 +1334,16 @@ func ensureFuncIgnore(root string) error { defer file.Close() // Write the desired string to the file - _, err = file.WriteString(` -# Use the .funcignore file to exclude files which should not be -# tracked in the image build. To instruct the system not to track -# files in the image build, add the regex pattern or file information -# to this file. + _, err = file.WriteString(`# Use the .funcignore file to exclude files from the image build. +# Excluded files do not affect the rebuild fingerprint. +# +# Supported patterns: +# foo - exclude any file or directory named "foo" at any depth +# *.tmp - glob; exclude matching files at any depth (except S2I: root only) +# /docs - root-relative; exclude only the top-level "docs" entry +# build/ - trailing slash stripped, then matched as a basename +# +# Lines starting with # are comments and are ignored. `) if err != nil { return err @@ -1346,16 +1351,85 @@ func ensureFuncIgnore(root string) error { return nil } +// DefaultIgnored contains entries that are always ignored regardless of +// .funcignore file contents. +var DefaultIgnored = []string{ + ".git", + ".func", + ".funcignore", + ".gitignore", +} + +// ParseFuncIgnore reads .funcignore and returns patterns with comments +// and blank lines stripped. +func ParseFuncIgnore(root string) []string { + f, err := os.Open(filepath.Join(root, ".funcignore")) + if err != nil { + return nil + } + defer f.Close() + + var patterns []string + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + patterns = append(patterns, line) + } + return patterns +} + +// IsIgnored reports whether a path should be skipped based on the +// default ignored entries and user-defined patterns from .funcignore. +// Four pattern forms are supported: +// +// "/foo" — root-relative; matches that exact path and everything beneath it. +// "foo/" — trailing slash stripped, then matched as basename. +// "*.tmp" — glob; matched against basename and full relative path (filepath.Match semantics). +// "foo" — basename; matches any entry named "foo" at any depth. +// +// Used by Fingerprint and the OCI builder. Known differences from other builders: +// - Pack (go-gitignore): supports "**" recursive globs and "!" negation — this does not. +// - S2I (filepath.Glob): non-recursive — "*.tmp" only matches at root level, this matches at any depth. +func IsIgnored(relPath string, userPatterns []string) bool { + base := filepath.Base(relPath) + for _, d := range DefaultIgnored { + if base == d { + return true + } + } + slashRel := filepath.ToSlash(relPath) + for _, pattern := range userPatterns { + if strings.HasPrefix(pattern, "/") { + p := pattern[1:] + if slashRel == p || strings.HasPrefix(slashRel, p+"/") { + return true + } + continue + } + pattern = strings.TrimSuffix(pattern, "/") + if matched, _ := filepath.Match(pattern, base); matched { + return true + } + if matched, _ := filepath.Match(pattern, relPath); matched { + return true + } + } + return false +} + // Fingerprint the files at a given path. Returns a hash calculated from the // filenames and modification timestamps of the files within the given root. // Also returns a logfile consisting of the filenames and modification times // which contributed to the hash. // Intended to determine if there were appreciable changes to a function's // source code, certain directories and files are ignored, such as -// .git and .func. -// Future updates will include files explicitly marked as ignored by a -// .funcignore. +// .git and .func. User-defined patterns from .funcignore are also respected. func Fingerprint(root string) (hash, log string, err error) { + userPatterns := ParseFuncIgnore(root) + h := sha256.New() // Hash builder l := bytes.Buffer{} // Log buffer @@ -1366,9 +1440,15 @@ func Fingerprint(root string) (hash, log string, err error) { if path == root { return nil } - // Always ignore .func, .git (TODO: .funcignore) - if info.IsDir() && (info.Name() == RunDataDir || info.Name() == ".git") { - return filepath.SkipDir + relPath, err := filepath.Rel(root, path) + if err != nil { + return err + } + if IsIgnored(relPath, userPatterns) { + if info.IsDir() { + return filepath.SkipDir + } + return nil } fmt.Fprintf(h, "%v:%v:", path, info.ModTime().UnixNano()) // Write to the Hasher fmt.Fprintf(&l, "%v:%v\n", path, info.ModTime().UnixNano()) // Write to the Log diff --git a/pkg/functions/client_test.go b/pkg/functions/client_test.go index c950ec4f86..c82087c614 100644 --- a/pkg/functions/client_test.go +++ b/pkg/functions/client_test.go @@ -2144,6 +2144,267 @@ func TestClient_BuildCleanFingerprint(t *testing.T) { } } +// TestFingerprint_FuncIgnore ensures that adding a pattern to .funcignore +// changes the fingerprint (because tracked files change), and that +// subsequent fingerprints remain stable. +func TestFingerprint_FuncIgnore(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + client := fn.New() + f := fn.Function{Root: root, Runtime: TestRuntime, Registry: TestRegistry} + + if _, err := client.Init(f); err != nil { + t.Fatal(err) + } + + // Create a file that will later be ignored + if err := os.WriteFile(filepath.Join(root, "notes.txt"), []byte("design notes"), 0644); err != nil { + t.Fatal(err) + } + + // State 1: fingerprint includes notes.txt + hash1, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + + // Add notes.txt to .funcignore + if err := os.WriteFile(filepath.Join(root, ".funcignore"), []byte("notes.txt\n"), 0644); err != nil { + t.Fatal(err) + } + + // State 2: fingerprint should change because notes.txt is now excluded + hash2, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + if hash1 == hash2 { + t.Fatal("adding pattern to .funcignore did not change the fingerprint") + } + + // State 3: fingerprinting again should be stable + hash3, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + if hash2 != hash3 { + t.Fatal("fingerprint not stable after .funcignore change") + } +} + +// TestFingerprint_FuncIgnoreRootRelative ensures that root-relative +// patterns (e.g. /docs) exclude only at the root, not nested matches. +func TestFingerprint_FuncIgnoreRootRelative(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + client := fn.New() + f := fn.Function{Root: root, Runtime: TestRuntime, Registry: TestRegistry} + + if _, err := client.Init(f); err != nil { + t.Fatal(err) + } + + // Create a root-level dir and a nested dir with the same name + if err := os.MkdirAll(filepath.Join(root, "docs"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "docs", "design.md"), []byte("root docs"), 0644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(root, "src", "docs"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "src", "docs", "api.md"), []byte("nested docs"), 0644); err != nil { + t.Fatal(err) + } + + // Fingerprint with both docs dirs + hashBoth, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + + // Add /docs (root-relative) to .funcignore — should only exclude root docs/ + if err := os.WriteFile(filepath.Join(root, ".funcignore"), []byte("/docs\n"), 0644); err != nil { + t.Fatal(err) + } + + hashRootIgnored, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + if hashBoth == hashRootIgnored { + t.Fatal("root-relative /docs pattern did not change the fingerprint") + } + + // src/docs/api.md should still be tracked — verify by removing it + if err := os.Remove(filepath.Join(root, "src", "docs", "api.md")); err != nil { + t.Fatal(err) + } + + hashNestedRemoved, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + if hashRootIgnored == hashNestedRemoved { + t.Fatal("nested src/docs was incorrectly ignored by root-relative /docs pattern") + } +} + +// TestFingerprint_FuncIgnoreGlob ensures glob patterns exclude matching +// files at any depth. +func TestFingerprint_FuncIgnoreGlob(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + if _, err := fn.New().Init(fn.Function{Root: root, Runtime: TestRuntime, Registry: TestRegistry}); err != nil { + t.Fatal(err) + } + + // Create files matching *.tmp at root and in a subdir + if err := os.WriteFile(filepath.Join(root, "cache.tmp"), []byte("cache"), 0644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(root, "subdir"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "subdir", "build.tmp"), []byte("build"), 0644); err != nil { + t.Fatal(err) + } + + hash1, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + + // Ignore *.tmp — both root and subdir files should be excluded + if err := os.WriteFile(filepath.Join(root, ".funcignore"), []byte("*.tmp\n"), 0644); err != nil { + t.Fatal(err) + } + + hash2, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + if hash1 == hash2 { + t.Fatal("*.tmp pattern did not change the fingerprint") + } + + // Modifying ignored files should not change the fingerprint + if err := os.WriteFile(filepath.Join(root, "cache.tmp"), []byte("changed"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "subdir", "build.tmp"), []byte("changed"), 0644); err != nil { + t.Fatal(err) + } + + hash3, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + if hash2 != hash3 { + t.Fatal("modifying *.tmp files changed the fingerprint — they should be ignored") + } +} + +// TestFingerprint_FuncIgnoreTrailingSlash ensures trailing slash patterns +// work identically to basename patterns. +func TestFingerprint_FuncIgnoreTrailingSlash(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + if _, err := fn.New().Init(fn.Function{Root: root, Runtime: TestRuntime, Registry: TestRegistry}); err != nil { + t.Fatal(err) + } + + if err := os.MkdirAll(filepath.Join(root, "dist"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "dist", "output.bin"), []byte("binary"), 0644); err != nil { + t.Fatal(err) + } + + hash1, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(root, ".funcignore"), []byte("dist/\n"), 0644); err != nil { + t.Fatal(err) + } + + hash2, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + if hash1 == hash2 { + t.Fatal("dist/ pattern did not change the fingerprint") + } + + // Changes inside dist/ should not affect fingerprint + if err := os.WriteFile(filepath.Join(root, "dist", "output.bin"), []byte("changed"), 0644); err != nil { + t.Fatal(err) + } + + hash3, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + if hash2 != hash3 { + t.Fatal("modifying dist/ files changed the fingerprint — they should be ignored") + } +} + +// TestFingerprint_FuncIgnoreComments ensures comment lines are not treated +// as patterns — a commented-out pattern still tracks the file. +func TestFingerprint_FuncIgnoreComments(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + if _, err := fn.New().Init(fn.Function{Root: root, Runtime: TestRuntime, Registry: TestRegistry}); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(root, "notes.txt"), []byte("notes"), 0644); err != nil { + t.Fatal(err) + } + + // Ignore notes.txt — fingerprint should change + if err := os.WriteFile(filepath.Join(root, ".funcignore"), []byte("notes.txt\n"), 0644); err != nil { + t.Fatal(err) + } + hashIgnored, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + + // Now comment it out — notes.txt is tracked again + if err := os.WriteFile(filepath.Join(root, ".funcignore"), []byte("# notes.txt\n"), 0644); err != nil { + t.Fatal(err) + } + hashCommented, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + if hashIgnored == hashCommented { + t.Fatal("commenting out a pattern did not restore the file to the fingerprint") + } + + // Modifying notes.txt should now change the fingerprint (it is tracked again) + if err := os.WriteFile(filepath.Join(root, "notes.txt"), []byte("changed"), 0644); err != nil { + t.Fatal(err) + } + hashModified, _, err := fn.Fingerprint(root) + if err != nil { + t.Fatal(err) + } + if hashCommented == hashModified { + t.Fatal("comment line was treated as pattern — notes.txt should be tracked") + } +} + // TestClient_DeployRemoves ensures that the Remover is invoked when a // function is moved to a new namespace. // specifically: deploy to 'nsone' -> simulate change of namespace with change to diff --git a/pkg/oci/builder.go b/pkg/oci/builder.go index 1e16dc9634..0050820e2d 100644 --- a/pkg/oci/builder.go +++ b/pkg/oci/builder.go @@ -34,32 +34,6 @@ const ( DefaultGid = 1000 ) -var defaultIgnored = []string{ - ".git", - ".func", - ".funcignore", - ".gitignore", -} - -// readFuncIgnore reads the .funcignore file from the given root directory -// and returns its patterns with blank lines and '#' comments removed. A -// missing or unreadable file returns nil; .funcignore is optional. -func readFuncIgnore(root string) []string { - data, err := os.ReadFile(filepath.Join(root, ".funcignore")) - if err != nil { - return nil - } - var patterns []string - for _, line := range strings.Split(string(data), "\n") { - line = strings.TrimSpace(line) - if line == "" || strings.HasPrefix(line, "#") { - continue - } - patterns = append(patterns, line) - } - return patterns -} - var builders = map[string]languageBuilder{ "go": goBuilder{}, "python": pythonBuilder{}, @@ -337,10 +311,9 @@ func writeDataLayer(job buildJob) (layer imageLayer, err error) { source := job.function.Root // The source is the function's entire filesystem target := filepath.Join(job.buildDir(), "datalayer.tar.gz") - ignored := append([]string{}, defaultIgnored...) - ignored = append(ignored, readFuncIgnore(source)...) + userPatterns := fn.ParseFuncIgnore(source) - if err = newDataTarball(source, target, ignored, job.verbose); err != nil { + if err = newDataTarball(source, target, userPatterns, job.verbose); err != nil { return } @@ -386,26 +359,11 @@ func newDataTarball(root, target string, ignored []string, verbose bool) error { return err } - // Skip files explicitly ignored. Two pattern forms are supported: - // "/foo/bar" — root-relative; matches that exact path and - // everything beneath it. - // "foo" — base-name; matches any entry named "foo" at any - // depth (the historical default). - for _, v := range ignored { - match := false - if strings.HasPrefix(v, "/") { - p := v[1:] - r := filepath.ToSlash(relPath) - match = r == p || strings.HasPrefix(r, p+"/") - } else { - match = info.Name() == v - } - if match { - if info.IsDir() { - return filepath.SkipDir - } - return nil + if relPath != "." && fn.IsIgnored(relPath, ignored) { + if info.IsDir() { + return filepath.SkipDir } + return nil } lnk := "" // if link, this will be used as the target diff --git a/pkg/oci/builder_test.go b/pkg/oci/builder_test.go index a8ca6c2b89..c08d973e92 100644 --- a/pkg/oci/builder_test.go +++ b/pkg/oci/builder_test.go @@ -469,39 +469,6 @@ func (l *TestLanguageBuilder) Configure(job buildJob, p v1.Platform, c v1.Config return l.ConfigureFn(job, p, c) } -// Test_readFuncIgnore ensures that the .funcignore file is parsed correctly, -// skipping comments and blank lines. -func Test_readFuncIgnore(t *testing.T) { - root := t.TempDir() - - content := ` -# Comment -/design -/archive -.jj -` - if err := os.WriteFile(filepath.Join(root, ".funcignore"), []byte(content), 0644); err != nil { - t.Fatal(err) - } - - patterns := readFuncIgnore(root) - expected := []string{"/design", "/archive", ".jj"} - - if diff := cmp.Diff(expected, patterns); diff != "" { - t.Error("patterns differ (-want, +got):", diff) - } -} - -// Test_readFuncIgnore_missing ensures that a missing .funcignore returns nil. -func Test_readFuncIgnore_missing(t *testing.T) { - root := t.TempDir() - - patterns := readFuncIgnore(root) - if patterns != nil { - t.Fatalf("expected nil, got %v", patterns) - } -} - // Test_newDataTarball_funcIgnore ensures that directories listed in // .funcignore are excluded from the data tarball, even if they contain // absolute symlinks that would otherwise cause an error. @@ -528,14 +495,13 @@ func Test_newDataTarball_funcIgnore(t *testing.T) { // Without funcignore, the build should fail due to absolute symlink target := filepath.Join(root, "test-fail.tar.gz") - err := newDataTarball(root, target, defaultIgnored, false) + err := newDataTarball(root, target, nil, false) if err == nil { t.Fatal("expected error for absolute symlink without funcignore") } // With /.jj in the ignored list (as read from .funcignore), it should succeed - ignored := append([]string{}, defaultIgnored...) - ignored = append(ignored, "/.jj") + ignored := []string{"/.jj"} target = filepath.Join(root, "test-pass.tar.gz") if err := newDataTarball(root, target, ignored, false); err != nil { @@ -577,6 +543,132 @@ func Test_newDataTarball_funcIgnore(t *testing.T) { } } +// tarbállContains returns the set of paths found in the gzipped tarball. +func tarballContents(t *testing.T, path string) map[string]bool { + t.Helper() + f, err := os.Open(path) + if err != nil { + t.Fatal(err) + } + defer f.Close() + gr, err := gzip.NewReader(f) + if err != nil { + t.Fatal(err) + } + defer gr.Close() + entries := map[string]bool{} + tr := tar.NewReader(gr) + for { + hdr, err := tr.Next() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + t.Fatal(err) + } + entries[hdr.Name] = true + } + return entries +} + +// Test_newDataTarball_funcIgnorePatterns verifies that all supported +// .funcignore pattern forms correctly exclude files from the OCI tarball. +func Test_newDataTarball_funcIgnorePatterns(t *testing.T) { + root := t.TempDir() + + // basename pattern + if err := os.WriteFile(filepath.Join(root, "notes.txt"), []byte("notes"), 0644); err != nil { + t.Fatal(err) + } + // glob pattern — at root and in subdir + if err := os.WriteFile(filepath.Join(root, "cache.tmp"), []byte("tmp"), 0644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(root, "subdir"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "subdir", "build.tmp"), []byte("tmp"), 0644); err != nil { + t.Fatal(err) + } + // trailing slash pattern + if err := os.MkdirAll(filepath.Join(root, "dist"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "dist", "output.bin"), []byte("bin"), 0644); err != nil { + t.Fatal(err) + } + // root-relative pattern + if err := os.MkdirAll(filepath.Join(root, "docs"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "docs", "design.md"), []byte("docs"), 0644); err != nil { + t.Fatal(err) + } + // tracked file that must remain + if err := os.WriteFile(filepath.Join(root, "main.go"), []byte("package main"), 0644); err != nil { + t.Fatal(err) + } + + patterns := []string{ + "notes.txt", // basename + "*.tmp", // glob + "dist/", // trailing slash + "/docs", // root-relative + } + + target := filepath.Join(t.TempDir(), "test.tar.gz") + if err := newDataTarball(root, target, patterns, false); err != nil { + t.Fatal(err) + } + + entries := tarballContents(t, target) + + // Should be present + if !entries["/func/main.go"] { + t.Error("main.go should be in tarball") + } + if !entries["/func/subdir"] { + t.Error("subdir/ should be in tarball") + } + + // Should be excluded + for _, excluded := range []string{ + "/func/notes.txt", + "/func/cache.tmp", + "/func/subdir/build.tmp", + "/func/dist/output.bin", + "/func/docs/design.md", + } { + if entries[excluded] { + t.Errorf("%v should be excluded from tarball", excluded) + } + } +} + +// Test_newDataTarball_funcIgnoreComments verifies comment lines in +// .funcignore are not treated as patterns by the OCI builder. +func Test_newDataTarball_funcIgnoreComments(t *testing.T) { + root := t.TempDir() + + if err := os.WriteFile(filepath.Join(root, "notes.txt"), []byte("notes"), 0644); err != nil { + t.Fatal(err) + } + + // Comment line — notes.txt should remain in tarball + _ = os.WriteFile(filepath.Join(root, ".funcignore"), []byte("# notes.txt\n"), 0644) + patterns := fn.ParseFuncIgnore(root) + + target := filepath.Join(t.TempDir(), "test.tar.gz") + if err := newDataTarball(root, target, patterns, false); err != nil { + t.Fatal(err) + } + + entries := tarballContents(t, target) + if !entries["/func/notes.txt"] { + t.Error("notes.txt should be in tarball — comment line should not be treated as pattern") + } +} + // Test_validatedLinkTarget 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. diff --git a/pkg/s2i/builder.go b/pkg/s2i/builder.go index 9b621e77bb..3e98ef476c 100644 --- a/pkg/s2i/builder.go +++ b/pkg/s2i/builder.go @@ -131,7 +131,19 @@ func (b *Builder) Build(ctx context.Context, f fn.Function, platforms []fn.Platf client = c } - // Link .s2iignore -> .funcignore + // Symlink .s2iignore -> .funcignore so S2I reads .funcignore patterns + // natively - strips comments and handles root-relative and trailing-slash + // patterns via filepath.Join(workingDir, pattern) before globbing. + // + // gauron99 - known diff: S2I glob matching is non-recursive. A pattern like + // "*.tmp" only excludes files at the root of the source tree, whereas the + // host&pack builder, Fingerprint() exclude matching files at any depth. + // See: https://github.com/openshift/source-to-image/blob/master/pkg/ignore/ignore.go + // + // This appears to be the only limitation compared to other builders and + // Fingerprint(). If we wanted full sync we would have to parse .funcignore + // and expand this incompatible pattern to match s2i pattern to write into + // .s2iignore file. funcignorePath := filepath.Join(f.Root, ".funcignore") s2iignorePath := filepath.Join(f.Root, ".s2iignore") if _, err := os.Stat(funcignorePath); err == nil { diff --git a/pkg/s2i/builder_test.go b/pkg/s2i/builder_test.go index 9727350465..fc9fcb2238 100644 --- a/pkg/s2i/builder_test.go +++ b/pkg/s2i/builder_test.go @@ -339,6 +339,117 @@ func (m mockDocker) ServerVersion(ctx context.Context, options client.ServerVers panic("implement me") } +// Test_FuncIgnoreSymlinked verifies that .s2iignore is symlinked to +// .funcignore during the build and removed after. +func Test_FuncIgnoreSymlinked(t *testing.T) { + root, done := Mktemp(t) + defer done() + + f := fn.Function{ + Name: "test", + Root: root, + Runtime: "go", + Registry: "example.com/alice", + } + var err error + if f, err = fn.New().Init(f); err != nil { + t.Fatal(err) + } + + // Write patterns including all supported forms + content := "# comment\nnotes.txt\n*.tmp\n/docs\ndist/\n" + if err = os.WriteFile(filepath.Join(root, ".funcignore"), []byte(content), 0644); err != nil { + t.Fatal(err) + } + + i := &mockImpl{} + c := mockDocker{} + b := s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c)) + + s2iignorePath := filepath.Join(root, ".s2iignore") + + i.BuildFn = func(cfg *api.Config) (*api.Result, error) { + // During build: .s2iignore should exist and be a symlink to .funcignore + info, err := os.Lstat(s2iignorePath) + if err != nil { + t.Fatalf(".s2iignore not found during build: %v", err) + } + if info.Mode()&os.ModeSymlink == 0 { + t.Fatal(".s2iignore should be a symlink") + } + target, err := os.Readlink(s2iignorePath) + if err != nil { + t.Fatal(err) + } + if filepath.Clean(target) != filepath.Clean("./.funcignore") { + t.Fatalf(".s2iignore should point to ./.funcignore, got %q", target) + } + + // Verify .s2iignore content matches .funcignore (comments included — S2I strips them) + data, err := os.ReadFile(s2iignorePath) + if err != nil { + t.Fatal(err) + } + funcignoreData, err := os.ReadFile(filepath.Join(root, ".funcignore")) + if err != nil { + t.Fatal(err) + } + if string(data) != string(funcignoreData) { + t.Fatal(".s2iignore content does not match .funcignore") + } + return nil, nil + } + + if err := b.Build(t.Context(), f, nil); err != nil { + t.Fatal(err) + } + + // After build: .s2iignore should be removed + if _, err := os.Lstat(s2iignorePath); !os.IsNotExist(err) { + t.Fatal(".s2iignore should be removed after build") + } +} + +// Test_FuncIgnorePreexistingS2iIgnore verifies that a pre-existing +// .s2iignore is not overwritten and takes precedence. +func Test_FuncIgnorePreexistingS2iIgnore(t *testing.T) { + root, done := Mktemp(t) + defer done() + + f := fn.Function{Name: "test", Root: root, Runtime: "go", Registry: "example.com/alice"} + var err error + if f, err = fn.New().Init(f); err != nil { + t.Fatal(err) + } + + if err = os.WriteFile(filepath.Join(root, ".funcignore"), []byte("notes.txt\n"), 0644); err != nil { + t.Fatal(err) + } + if err = os.WriteFile(filepath.Join(root, ".s2iignore"), []byte("other.txt\n"), 0644); err != nil { + t.Fatal(err) + } + + i := &mockImpl{} + c := mockDocker{} + b := s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c)) + + i.BuildFn = func(cfg *api.Config) (*api.Result, error) { + // .s2iignore should still contain the original content (not overwritten) + data, err := os.ReadFile(filepath.Join(root, ".s2iignore")) + if err != nil { + t.Fatal(err) + } + if string(data) != "other.txt\n" { + t.Fatalf("pre-existing .s2iignore was overwritten, got: %q", string(data)) + } + return nil, nil + } + + if err := b.Build(t.Context(), f, nil); err != nil { + t.Fatal(err) + } +} + // Test_ScaffoldWritesToFuncBuild ensures that scaffolding for Go/Python // runtimes is written to .func/build/ instead of .s2i/build/ func Test_ScaffoldWritesToFuncBuild(t *testing.T) {