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) {