diff --git a/pkg/functions/client.go b/pkg/functions/client.go index d8a7a11de7..75a5e8d3d7 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -691,6 +691,13 @@ func BuildWithPlatforms(pp []Platform) BuildOption { // not contain a populated Image. func (c *Client) Build(ctx context.Context, f Function, options ...BuildOption) (Function, error) { fmt.Fprintf(os.Stderr, "Building function image\n") + + // Warn if a func-generated legacy .s2i/bin/assemble file is left at the + // function root. This runs on every build (and therefore every deploy, + // which uses build under the hood) so users see the warning regardless + // of which builder they switched to. + WarnIfLegacyS2IScaffolding(f, os.Stderr) + ctx, cancel := context.WithCancel(ctx) defer cancel() diff --git a/pkg/functions/function.go b/pkg/functions/function.go index 07504c1103..f1b7253b55 100644 --- a/pkg/functions/function.go +++ b/pkg/functions/function.go @@ -3,6 +3,7 @@ package functions import ( "errors" "fmt" + "io" "os" "path/filepath" "reflect" @@ -601,6 +602,29 @@ func (f Function) HasScaffolding() bool { return f.Runtime == "go" || f.Runtime == "python" } +// WarnIfLegacyS2IScaffolding writes a warning to w when the func-generated +// legacy .s2i/bin/assemble file is present at the function root. +// Prior to https://github.com/knative/func/pull/3436, func wrote S2I +// scaffolding for go/python to /.s2i/bin/assemble; it now lives +// at .func/build/bin/assemble. The stale file can interfere with other +// builders (e.g. pack erroneously detects the assemble script). +// The check is intentionally scoped to scaffolded runtimes (go/python) and +// to the exact file func generated, so user-managed .s2i directories are not +// flagged. +func WarnIfLegacyS2IScaffolding(f Function, w io.Writer) { + if !f.HasScaffolding() { + return + } + legacyAssemble := filepath.Join(f.Root, ".s2i", "bin", "assemble") + if _, err := os.Stat(legacyAssemble); err == nil { + fmt.Fprintf(w, "Warning: a '.s2i/bin/assemble' file was detected at the function root. "+ + "This appears to be leftover func-generated scaffolding from before it was moved to '%s/%s/bin/assemble'. "+ + "It may interfere with other builders. "+ + "If it was generated by func, consider removing '.s2i/' after verifying it is no longer needed.\n", + RunDataDir, BuildDir) + } +} + // 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/functions/function_unit_test.go b/pkg/functions/function_unit_test.go index 7a0715db9e..a5b19b7a03 100644 --- a/pkg/functions/function_unit_test.go +++ b/pkg/functions/function_unit_test.go @@ -1,9 +1,11 @@ package functions_test import ( + "bytes" "os" "path/filepath" "reflect" + "strings" "testing" "gopkg.in/yaml.v2" @@ -296,3 +298,63 @@ func expectedDefaultLabels(f fn.Function) map[string]string { fnlabels.FunctionRuntimeKey: f.Runtime, } } + +// TestWarnIfLegacyS2IScaffolding ensures the warning is emitted only when a +// func-generated legacy .s2i/bin/assemble file is present at the function root +// for a scaffolded runtime (go/python). +func TestWarnIfLegacyS2IScaffolding(t *testing.T) { + tests := []struct { + name string + runtime string + createFile bool + wantWarning bool + }{ + { + name: "scaffolded runtime with legacy assemble warns", + runtime: "go", + createFile: true, + wantWarning: true, + }, + { + name: "non-scaffolded runtime with .s2i/bin/assemble does not warn", + runtime: "node", + createFile: true, + wantWarning: false, + }, + { + name: "scaffolded runtime without legacy assemble does not warn", + runtime: "go", + createFile: false, + wantWarning: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root, cleanup := Mktemp(t) + t.Cleanup(cleanup) + + if tt.createFile { + binDir := filepath.Join(root, ".s2i", "bin") + if err := os.MkdirAll(binDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(binDir, "assemble"), []byte("#!/bin/bash"), 0700); err != nil { + t.Fatal(err) + } + } + + f := fn.Function{Root: root, Runtime: tt.runtime} + var buf bytes.Buffer + fn.WarnIfLegacyS2IScaffolding(f, &buf) + + warned := strings.Contains(buf.String(), ".s2i/bin/assemble") + if tt.wantWarning && !warned { + t.Fatalf("expected a warning about .s2i/bin/assemble, got: %q", buf.String()) + } + if !tt.wantWarning && warned { + t.Fatalf("unexpected warning about .s2i/bin/assemble: %q", buf.String()) + } + }) + } +} diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index 540f995c0d..c51fd198ed 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -113,6 +113,11 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn return "", f, err } + // Warn if the func-generated legacy .s2i/bin/assemble exists; it will be + // uploaded to the PVC and can interfere with the in-cluster build. + // Remote deploy doesn't go through Client.Build, so we re-check here. + fn.WarnIfLegacyS2IScaffolding(f, os.Stderr) + // Namespace is either a new namespace, specified as f.Namespace, or // the currently deployed namespace, recorded on f.Deploy.Namespace. // If neither exist, an error is returned (namespace is required)