diff --git a/pkg/buildpacks/builder.go b/pkg/buildpacks/builder.go index 2125cfd59f..76fda93799 100644 --- a/pkg/buildpacks/builder.go +++ b/pkg/buildpacks/builder.go @@ -189,7 +189,7 @@ func (b *Builder) Build(ctx context.Context, f fn.Function, platforms []fn.Platf // set scaffolding path to buildpacks builder if f.Runtime == "go" { - opts.Env["BP_GO_WORKDIR"] = ".func/build" + opts.Env["BP_GO_WORKDIR"] = filepath.Join(fn.RunDataDir, fn.BuildDir) } // Get middleware version and set as image label via BP_IMAGE_LABELS diff --git a/pkg/buildpacks/scaffolder.go b/pkg/buildpacks/scaffolder.go index d151e3afa5..f13dd28f42 100644 --- a/pkg/buildpacks/scaffolder.go +++ b/pkg/buildpacks/scaffolder.go @@ -10,7 +10,7 @@ import ( "knative.dev/func/pkg/scaffolding" ) -const defaultPath = ".func/build" +const defaultPath = fn.RunDataDir + "/" + fn.BuildDir // Scaffolder for buildpacks builder type Scaffolder struct { diff --git a/pkg/functions/function.go b/pkg/functions/function.go index 9e001c4948..c37bdc272e 100644 --- a/pkg/functions/function.go +++ b/pkg/functions/function.go @@ -28,6 +28,10 @@ const ( RunDataDir = ".func" RunDataLocalFile = "local.yaml" + // BuildDir is the subdirectory within RunDataDir for build scaffolding + // and artifacts (e.g. ".func/build"). + BuildDir = "build" + // BuiltHash is a name of a file that holds hash of built Function in runtime // metadata dir (RunDataDir) BuiltHash = "built-hash" @@ -583,6 +587,12 @@ func (f Function) Initialized() bool { return !f.Created.IsZero() } +// HasScaffolding returns true if the function runtime supports scaffolding. +// Scaffoldable runtimes (go, python) require a generated main wrapper to build. +func (f Function) HasScaffolding() bool { + return f.Runtime == "go" || f.Runtime == "python" +} + // LabelsMap combines default labels with the labels slice provided. // It will the resulting slice with ValidateLabels and return a key/value map. // - key: EXAMPLE1 # Label directly from a value diff --git a/pkg/oci/builder.go b/pkg/oci/builder.go index 1e62a2bc99..5096235640 100644 --- a/pkg/oci/builder.go +++ b/pkg/oci/builder.go @@ -857,13 +857,13 @@ func newBuildJob(ctx context.Context, f fn.Function, pp []fn.Platform, verbose b // some convenience accessors func (j buildJob) buildDir() string { - return filepath.Join(j.function.Root, fn.RunDataDir, "build") + return filepath.Join(j.function.Root, fn.RunDataDir, fn.BuildDir) } func (j buildJob) ociDir() string { - return filepath.Join(j.function.Root, fn.RunDataDir, "build", "oci") + return filepath.Join(j.function.Root, fn.RunDataDir, fn.BuildDir, "oci") } func (j buildJob) blobsDir() string { - return filepath.Join(j.function.Root, fn.RunDataDir, "build", "oci", "blobs", "sha256") + return filepath.Join(j.function.Root, fn.RunDataDir, fn.BuildDir, "oci", "blobs", "sha256") } func (j buildJob) cacheDir() string { return filepath.Join(j.function.Root, fn.RunDataDir, "blob-cache") diff --git a/pkg/oci/builder_test.go b/pkg/oci/builder_test.go index db089d6e38..902be70684 100644 --- a/pkg/oci/builder_test.go +++ b/pkg/oci/builder_test.go @@ -83,7 +83,7 @@ func TestBuilder_BuildGo(t *testing.T) { t.Fatal(err) } - oci := filepath.Join(f.Root, fn.RunDataDir, "build", "oci") + oci := filepath.Join(f.Root, fn.RunDataDir, fn.BuildDir, "oci") validateOCIStructure(oci, t) // validate OCI compliant } @@ -117,7 +117,7 @@ func TestBuilder_BuildPython(t *testing.T) { t.Fatal(err) } - oci := filepath.Join(f.Root, fn.RunDataDir, "build", "oci") + oci := filepath.Join(f.Root, fn.RunDataDir, fn.BuildDir, "oci") validateOCIStructure(oci, t) // validate OCI compliant } @@ -184,7 +184,7 @@ func TestBuilder_Files(t *testing.T) { {Path: "/func/go.mod"}, } - oci := filepath.Join(f.Root, fn.RunDataDir, "build", "oci") + oci := filepath.Join(f.Root, fn.RunDataDir, fn.BuildDir, "oci") validateOCIFiles(oci, expected, t) } @@ -357,7 +357,7 @@ func TestBuilder_StaticEnvs(t *testing.T) { // variables on each of the constituent containers. // --- // Get the images list (manifest descripors) from the index - ociPath := filepath.Join(f.Root, fn.RunDataDir, "build", "oci") + ociPath := filepath.Join(f.Root, fn.RunDataDir, fn.BuildDir, "oci") data, err := os.ReadFile(filepath.Join(ociPath, "index.json")) if err != nil { t.Fatal(err) diff --git a/pkg/oci/pusher.go b/pkg/oci/pusher.go index 5858946090..1d24ca33e8 100644 --- a/pkg/oci/pusher.go +++ b/pkg/oci/pusher.go @@ -170,7 +170,7 @@ func (p *Pusher) handleUpdates(ctx context.Context) { // getBuildDir returns the build directory func getBuildDir(f fn.Function) (string, error) { - dir := filepath.Join(f.Root, fn.RunDataDir, "build") + dir := filepath.Join(f.Root, fn.RunDataDir, fn.BuildDir) if _, err := os.Stat(dir); os.IsNotExist(err) { return dir, fmt.Errorf("build directory not found '%v'. Has it been built?", dir) } diff --git a/pkg/oci/scaffolder.go b/pkg/oci/scaffolder.go index 7825f8c2cc..3e019b49a5 100644 --- a/pkg/oci/scaffolder.go +++ b/pkg/oci/scaffolder.go @@ -10,9 +10,7 @@ import ( "knative.dev/func/pkg/scaffolding" ) -const ( - defaultPath = ".func/build" -) +const defaultPath = fn.RunDataDir + "/" + fn.BuildDir // Scaffolder for host (OCI) builder type Scaffolder struct { @@ -26,9 +24,9 @@ func NewScaffolder(verbose bool) *Scaffolder { // Scaffold the function so that it can be built via oci builder. // 'path' is an optional override. Assign "" (empty string) most of the time func (s Scaffolder) Scaffold(ctx context.Context, f fn.Function, path string) error { - if f.Runtime != "go" && f.Runtime != "python" { + if !f.HasScaffolding() { if s.verbose { - fmt.Println("Scaffolding skipped. Currently available for runtimes go & python") + fmt.Println("Scaffolding skipped. Runtime does not support scaffolding.") } return nil } diff --git a/pkg/pipelines/tekton/templates.go b/pkg/pipelines/tekton/templates.go index 149d9bb313..657514de8c 100644 --- a/pkg/pipelines/tekton/templates.go +++ b/pkg/pipelines/tekton/templates.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path" + "path/filepath" "regexp" "strings" "text/template" @@ -28,7 +29,9 @@ const ( // S2I related properties defaultS2iImageScriptsUrl = "image:///usr/libexec/s2i" quarkusS2iImageScriptsUrl = "image:///usr/local/s2i" - + // Note: function calls (like path.Join()) can't be used in const declaration + // therefore this URL is built via concatenation. + scaffoldedS2iImageScriptsUrl = "file://" + fn.RunDataDir + "/" + fn.BuildDir + "/bin" // The branch or tag we are targeting with Pipelines (ie: main, refs/tags/*) defaultPipelinesTargetBranch = "main" ) @@ -153,6 +156,8 @@ func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string) error s2iImageScriptsUrl := defaultS2iImageScriptsUrl if f.Runtime == "quarkus" { s2iImageScriptsUrl = quarkusS2iImageScriptsUrl + } else if f.HasScaffolding() { + s2iImageScriptsUrl = scaffoldedS2iImageScriptsUrl } image := f.Deploy.Image @@ -349,12 +354,14 @@ func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels m // add BP_GO_WORKDIR for go-build buildpack if f.Runtime == "go" { - buildEnvs = append(buildEnvs, "BP_GO_WORKDIR=.func/build") + buildEnvs = append(buildEnvs, "BP_GO_WORKDIR="+filepath.Join(fn.RunDataDir, fn.BuildDir)) } s2iImageScriptsUrl := defaultS2iImageScriptsUrl if f.Runtime == "quarkus" { s2iImageScriptsUrl = quarkusS2iImageScriptsUrl + } else if f.HasScaffolding() { + s2iImageScriptsUrl = scaffoldedS2iImageScriptsUrl } // Determine if TLS verification should be skipped diff --git a/pkg/s2i/assemblers.go b/pkg/s2i/assemblers.go index 5438f3b1a8..cbabd10da3 100644 --- a/pkg/s2i/assemblers.go +++ b/pkg/s2i/assemblers.go @@ -41,7 +41,7 @@ func assembler(f fn.Function) (string, error) { // GoAssembler // // Adapted from /usr/libexec/s2i/assemble within the UBI-8 go-toolchain -// such that the "go build" command builds subdirectory .s2i/build +// such that the "go build" command builds subdirectory .func/build // (where main resides) rather than the root. const GoAssembler = ` #!/bin/bash @@ -76,7 +76,7 @@ if [[ $(go list -f {{.Incomplete}}) == "true" ]]; then /$STI_SCRIPTS_PATH/usage exit 1 else - pushd .s2i/build + pushd .func/build go mod tidy go build -o /opt/app-root/gobinary popd @@ -87,9 +87,9 @@ fi // PythonAssembler // // Adapted from /usr/libexec/s2i/assemble within the UBI-8 python-toolchain -// such that the script executes from subdirectory .s2i/builds/last +// such that the script executes from subdirectory .func/build // (where main resides) rather than the root, and indicates the main is -// likewise in .s2i/build/service/main.py via Procfile. See the comment +// likewise in .func/build/service/main.py via Procfile. See the comment // inline on line 50 of the script for where the directory change instruction // was added. const PythonAssembler = ` @@ -151,12 +151,12 @@ echo "---> (Functions) Writing app.sh ..." cat << 'EOF' > app.sh #!/bin/bash set -e -exec python .s2i/build/service/main.py +exec python .func/build/service/main.py EOF chmod +x app.sh -echo "---> (Functions) Changing directory to .s2i/build ..." -cd .s2i/build +echo "---> (Functions) Changing directory to .func/build ..." +cd .func/build # END MODIFICATION FOR FUNCTIONS # ------------------------------ diff --git a/pkg/s2i/builder.go b/pkg/s2i/builder.go index 4cf74edf4e..5b7793dbbb 100644 --- a/pkg/s2i/builder.go +++ b/pkg/s2i/builder.go @@ -167,11 +167,21 @@ func (b *Builder) Build(ctx context.Context, f fn.Function, platforms []fn.Platf // https://github.com/openshift/source-to-image/issues/1141 ForceCopy: true, // Excludes - // Do not include .git, .env, .func or any language-specific cache directories + // Do not include .git, .env or any language-specific cache directories // (node_modules, etc) in the tar file sent to the builder, as this both - // bloats the build process and can cause unexpected errors in the resultant - // function. - ExcludeRegExp: "(^|/)\\.git|\\.env|\\.func|node_modules(/|$)", + // bloats the build process and can cause unexpected errors in the + // resultant function. + // Note: .func/build is allowed through (scaffolding for go/python), but + // other .func subdirs (blob-cache, built-hash, etc.) are excluded to + // avoid bloating the tar and leaking local metadata. + ExcludeRegExp: "(^|/)\\.git|\\.env|\\.func/(blob-cache|built-hash|built-image|built\\.log|local\\.yaml)|node_modules(/|$)", + } + + // Tell S2I where to find the assemble script written by Scaffolder. + // Use forward slashes (not filepath.Join) because this is a file:// URL, + // and filepath.Join would use OS-specific separators. + if f.HasScaffolding() { + cfg.ScriptsURL = "file://" + f.Root + "/" + fn.RunDataDir + "/" + fn.BuildDir + "/bin" } // Set middleware version label diff --git a/pkg/s2i/builder_test.go b/pkg/s2i/builder_test.go index 2486c0eeee..a8150dd0bb 100644 --- a/pkg/s2i/builder_test.go +++ b/pkg/s2i/builder_test.go @@ -4,6 +4,8 @@ import ( "context" "errors" "io" + "os" + "path/filepath" "testing" "github.com/docker/docker/api/types" @@ -356,3 +358,53 @@ func (n notFoundErr) NotFound() { // Just a type assert in case docker decides to change NotFoundError interface again var _ errdefs.ErrNotFound = notFoundErr{} + +// Test_ScaffoldWritesToFuncBuild ensures that scaffolding for Go/Python +// runtimes is written to .func/build/ instead of .s2i/build/ +func Test_ScaffoldWritesToFuncBuild(t *testing.T) { + runtimes := []string{"go", "python"} + for _, rt := range runtimes { + t.Run(rt, func(t *testing.T) { + var ( + root, done = Mktemp(t) + f = fn.Function{ + Name: "test", + Root: root, + Runtime: rt, + Registry: "example.com/alice", + } + scaffolder = s2i.NewScaffolder(false) + err error + ) + defer done() + + // Initialize the test function + if f, err = fn.New().Init(f); err != nil { + t.Fatal(err) + } + + // Call Scaffold + if err := scaffolder.Scaffold(context.Background(), f, ""); err != nil { + t.Fatal(err) + } + + // Assert: scaffolding should be in .func/build/ + expectedPath := filepath.Join(root, fn.RunDataDir, fn.BuildDir) + if _, err := os.Stat(expectedPath); os.IsNotExist(err) { + t.Errorf("expected scaffolding at %s, but directory does not exist", expectedPath) + } + + // Assert: S2I scripts should be at .func/build/bin/ + scriptsPath := filepath.Join(root, fn.RunDataDir, fn.BuildDir, "bin", "assemble") + if _, err := os.Stat(scriptsPath); os.IsNotExist(err) { + t.Errorf("expected assemble script at %s, but file does not exist", scriptsPath) + } + + // Assert: .s2i directory should NOT exist at root level + s2iPath := filepath.Join(root, ".s2i") + if _, err := os.Stat(s2iPath); err == nil { + t.Errorf(".s2i directory should not exist at root level, but found at %s", s2iPath) + } + }) + } +} diff --git a/pkg/s2i/scaffolder.go b/pkg/s2i/scaffolder.go index d24d3df054..e1a313c58e 100644 --- a/pkg/s2i/scaffolder.go +++ b/pkg/s2i/scaffolder.go @@ -10,9 +10,7 @@ import ( "knative.dev/func/pkg/scaffolding" ) -const ( - defaultPath = ".s2i/build" -) +const defaultPath = fn.RunDataDir + "/" + fn.BuildDir // Scaffolder for S2I builder type Scaffolder struct { @@ -26,9 +24,9 @@ func NewScaffolder(verbose bool) *Scaffolder { // Scaffold the function so that it can be built via s2i builder. // 'path' is an optional override. Assign "" (empty string) most of the time func (s Scaffolder) Scaffold(ctx context.Context, f fn.Function, path string) error { - if f.Runtime != "go" && f.Runtime != "python" { + if !f.HasScaffolding() { if s.verbose { - fmt.Println("Scaffolding skipped. Currently available for runtimes go & python") + fmt.Println("Scaffolding skipped. Runtime does not support scaffolding.") } return nil } @@ -55,10 +53,11 @@ func (s Scaffolder) Scaffold(ctx context.Context, f fn.Function, path string) er return err } if assemble != "" { - if err := os.MkdirAll(filepath.Join(f.Root, ".s2i", "bin"), 0755); err != nil { - return fmt.Errorf("unable to create .s2i bin dir. %w", err) + binDir := filepath.Join(appRoot, "bin") + if err := os.MkdirAll(binDir, 0755); err != nil { + return fmt.Errorf("unable to create scaffolding bin dir. %w", err) } - if err := os.WriteFile(filepath.Join(f.Root, ".s2i", "bin", "assemble"), []byte(assemble), 0700); err != nil { + if err := os.WriteFile(filepath.Join(binDir, "assemble"), []byte(assemble), 0700); err != nil { return fmt.Errorf("unable to write assembler script. %w", err) } } diff --git a/pkg/scaffolding/scaffold.go b/pkg/scaffolding/scaffold.go index 843e098721..2341f58212 100644 --- a/pkg/scaffolding/scaffold.go +++ b/pkg/scaffolding/scaffold.go @@ -2,6 +2,7 @@ package scaffolding import ( "bytes" + "errors" "fmt" "os" "path/filepath" @@ -107,7 +108,7 @@ func detectSignature(src, runtime, invoke string) (s Signature, err error) { } } -// patch scaffolding main.go and go.mod based on the users function +// patch scaffolding main.go, go.mod and go.sum based on the users function func patchScaffolding(src, out string) error { goModPath := filepath.Join(src, "go.mod") data, err := os.ReadFile(goModPath) @@ -124,13 +125,16 @@ func patchScaffolding(src, out string) error { moduleName := goMod.Module.Mod.Path - if err := patchGoMod(out, moduleName, goMod.Go); err != nil { + if err := patchGoMod(out, moduleName, goMod.Go, goMod.Require); err != nil { + return err + } + if err := mergeGoSum(src, out); err != nil { return err } return patchMain(out, moduleName) } -func patchGoMod(dir, moduleName string, goDirective *modfile.Go) error { +func patchGoMod(dir, moduleName string, goDirective *modfile.Go, funcRequires []*modfile.Require) error { path := filepath.Join(dir, "go.mod") data, err := os.ReadFile(path) if err != nil { @@ -159,6 +163,22 @@ func patchGoMod(dir, moduleName string, goDirective *modfile.Go) error { return fmt.Errorf("cannot add require: %w", err) } + // Merge function's dependencies that aren't already in the scaffolding + // go.mod. This ensures transitive requires (e.g. private dependencies) + // are present without downgrading versions the scaffolding already has. + existing := make(map[string]bool, len(f.Require)) + for _, req := range f.Require { + existing[req.Mod.Path] = true + } + for _, req := range funcRequires { + if existing[req.Mod.Path] { + continue + } + if err := f.AddRequire(req.Mod.Path, req.Mod.Version); err != nil { + return fmt.Errorf("cannot add require %s: %w", req.Mod.Path, err) + } + } + formatted, err := f.Format() if err != nil { return fmt.Errorf("cannot format scaffolding go.mod: %w", err) @@ -166,6 +186,32 @@ func patchGoMod(dir, moduleName string, goDirective *modfile.Go) error { return os.WriteFile(path, formatted, 0644) } +// mergeGoSum appends the function's go.sum entries into the scaffolding's +// go.sum so that all transitive dependency checksums are available at build +// time. Duplicate entries are harmless; Go tooling ignores them. +func mergeGoSum(src, out string) error { + funcGoSum := filepath.Join(src, "go.sum") + data, err := os.ReadFile(funcGoSum) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil + } + return fmt.Errorf("cannot read function go.sum: %w", err) + } + + scaffoldGoSum := filepath.Join(out, "go.sum") + f, err := os.OpenFile(scaffoldGoSum, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + return fmt.Errorf("cannot open scaffolding go.sum: %w", err) + } + defer f.Close() + + if _, err := f.Write(data); err != nil { + return fmt.Errorf("cannot append to scaffolding go.sum: %w", err) + } + return nil +} + func patchMain(out, moduleName string) error { targetMainPath := filepath.Join(out, "main.go") targetMainData, err := os.ReadFile(targetMainPath) diff --git a/pkg/scaffolding/scaffold_test.go b/pkg/scaffolding/scaffold_test.go index 206e03cc60..5a8f0c3e84 100644 --- a/pkg/scaffolding/scaffold_test.go +++ b/pkg/scaffolding/scaffold_test.go @@ -4,6 +4,7 @@ import ( "errors" "os" "path/filepath" + "strings" "testing" "knative.dev/func/pkg/filesystem" @@ -169,3 +170,67 @@ func TestNewScaffoldingError(t *testing.T) { t.Logf("ok: %v", err) } + +// TestMergeGoSum ensures that the function's go.sum entries are appended +// to the scaffolding's go.sum during patching. +func TestMergeGoSum(t *testing.T) { + src, done := Mktemp(t) + defer done() + out, doneOut := Mktemp(t) + defer doneOut() + + funcGoSum := "git-private.localtest.me/foo.git v0.0.1 h1:abc123=\n" + + "git-private.localtest.me/foo.git v0.0.1/go.mod h1:def456=\n" + scaffoldGoSum := "github.com/rs/zerolog v1.32.0 h1:existing=\n" + + if err := os.WriteFile(filepath.Join(src, "go.sum"), []byte(funcGoSum), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(out, "go.sum"), []byte(scaffoldGoSum), 0644); err != nil { + t.Fatal(err) + } + + if err := mergeGoSum(src, out); err != nil { + t.Fatal(err) + } + + merged, err := os.ReadFile(filepath.Join(out, "go.sum")) + if err != nil { + t.Fatal(err) + } + + result := string(merged) + if !strings.Contains(result, "git-private.localtest.me/foo.git v0.0.1 h1:abc123=") { + t.Fatal("merged go.sum should contain function's dependency checksum") + } + if !strings.Contains(result, "github.com/rs/zerolog v1.32.0 h1:existing=") { + t.Fatal("merged go.sum should still contain scaffolding's original entries") + } +} + +// TestMergeGoSum_NoFunctionGoSum ensures that when the function has no +// go.sum file, mergeGoSum returns nil without error. +func TestMergeGoSum_NoFunctionGoSum(t *testing.T) { + src, done := Mktemp(t) + defer done() + out, doneOut := Mktemp(t) + defer doneOut() + + scaffoldGoSum := "github.com/rs/zerolog v1.32.0 h1:existing=\n" + if err := os.WriteFile(filepath.Join(out, "go.sum"), []byte(scaffoldGoSum), 0644); err != nil { + t.Fatal(err) + } + + if err := mergeGoSum(src, out); err != nil { + t.Fatalf("mergeGoSum should not error when function has no go.sum: %v", err) + } + + // Scaffolding go.sum should remain unchanged + data, err := os.ReadFile(filepath.Join(out, "go.sum")) + if err != nil { + t.Fatal(err) + } + if string(data) != scaffoldGoSum { + t.Fatalf("scaffolding go.sum should be unchanged, got: %q", string(data)) + } +}