Warn when legacy .s2i/bin/assemble scaffolding is detected (#3584)

* Warn when legacy .s2i/bin/assemble scaffolding is detected

Prior to #3436, func wrote S2I scaffolding for go/python
to <func-root>/.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).

Add WarnIfLegacyS2IScaffolding helper in pkg/s2i that checks only
scaffolded runtimes (go/python) and only the exact file func generated,
so user-managed .s2i directories are not flagged. Call it from both the
local Build() path and the remote PipelinesProvider.Run() path so the
warning is emitted regardless of how the function is built.

Fixes #3452

* Move legacy .s2i warning into Client.Build

Per review feedback on #3584, the warning helper now lives in
pkg/functions as a standalone WarnIfLegacyS2IScaffolding and is invoked
from Client.Build, so it fires for every build (and therefore every
local deploy) regardless of which builder the user switched to.

The Tekton pipelines provider keeps its own call for remote deploys
(which don't go through Client.Build) and no longer needs to import
pkg/s2i.  The s2i builder's redundant call is removed.  The test moves
to pkg/functions and exercises the helper directly.

* Update pkg/functions/client.go

Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com>

* Update pkg/functions/function.go

Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com>

* Update pkg/functions/function_unit_test.go

Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com>

* Fix govet/staticcheck: drop unused Fprintf args in legacy s2i warning

* Update pkg/functions/function.go

Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com>

* Update pkg/functions/function.go

Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com>

* Update pkg/functions/function.go

Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com>

* Update pkg/functions/function.go

---------

Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com>

Author: Ankitsinghsisodya

pkg/functions/client.go

@@ -692,6 +692,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()
 

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

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())
+			}
+		})
+	}
+}

pkg/pipelines/tekton/pipelines_provider.go

@@ -114,6 +114,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)