diff --git a/pkg/functions/client.go b/pkg/functions/client.go index d8a7a11de7..04c427b7b4 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -691,6 +691,11 @@ 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 (pre-1.21.2) + 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..2cf22dc564 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,24 @@ func (f Function) HasScaffolding() bool { return f.Runtime == "go" || f.Runtime == "python" } +// WarnIfLegacyS2IScaffolding writes a warning to 'w' when a legacy +// .s2i/bin/assemble file is present at the function root of a scaffolded +// runtime (go/python). This file was previously generated by func (see +// https://github.com/knative/func/pull/3436) and can interfere with other +// builders (mostly just pack). +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.Fprint(w, "Warning: a '.s2i/bin/assemble' file was detected at the function root. "+ + "This appears to be leftover func-generated scaffolding from older cli builds. "+ + "It may interfere with newer builds. "+ + "Please remove the '.s2i/' directory from your function root.\n") + } +} + // 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..c9bca6e26b 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 Test_WarnIfLegacyS2IScaffolding(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)