[WIP] [release-1.20] fix: Use tlsVerify param in PipelineRun template for pack too

cmd/build.go

@@ -3,6 +3,8 @@ package cmd
 import (
 	"errors"
 	"fmt"
+	"io"
+	"os"
 	"strings"
 
 	"github.com/AlecAivazis/survey/v2"
@@ -100,7 +102,7 @@ EXAMPLES
 		fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s. ($FUNC_BUILDER)", KnownBuilders()))
 	cmd.Flags().StringP("registry", "r", cfg.Registry,
 		"Container registry + registry namespace. (ex 'ghcr.io/myuser').  The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)")
-	cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)")
+	cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)")
 
 	// Function-Context Flags:
 	// Options whose value is available on the function with context only
@@ -209,6 +211,10 @@ For more options, run 'func build --help'`, err)
 	if !f.Initialized() {
 		return fn.NewErrNotInitialized(f.Root)
 	}
+
+	// Warn if registry changed but registryInsecure is still true
+	warnRegistryInsecureChange(os.Stderr, cfg.Registry, f)
+
 	f = cfg.Configure(f) // Returns an f updated with values from the config (flags, envs, etc)
 
 	// Client
@@ -240,6 +246,15 @@ For more options, run 'func build --help'`, err)
 	return f.Stamp()
 }
 
+// warnRegistryInsecureChange checks if the registry has changed but
+// registryInsecure is still set to true, and prints a warning if so.
+// This helps users avoid accidentally skipping TLS verification on a new registry.
+func warnRegistryInsecureChange(w io.Writer, newRegistry string, f fn.Function) {
+	if f.Registry != "" && newRegistry != "" && f.Registry != newRegistry && f.RegistryInsecure {
+		fmt.Fprintf(w, "Warning: Registry changed from '%s' to '%s', but registryInsecure is still true. Consider setting --registry-insecure=false if the new registry requires TLS verification.\n", f.Registry, newRegistry)
+	}
+}
+
 type buildConfig struct {
 	// Globals (builder, confirm, registry, verbose)
 	config.Global
@@ -432,7 +447,10 @@ func (c buildConfig) Validate(cmd *cobra.Command) (err error) {
 // image necessary for the target cluster, since the end product of  a function
 // deployment is not the contiainer, but rather the running service.
 func (c buildConfig) clientOptions() ([]fn.Option, error) {
-	o := []fn.Option{fn.WithRegistry(c.Registry)}
+	o := []fn.Option{
+		fn.WithRegistry(c.Registry),
+		fn.WithRegistryInsecure(c.RegistryInsecure),
+	}
 	switch c.Builder {
 	case builders.Host:
 		t := newTransport(c.RegistryInsecure) // may provide a custom impl which proxies

cmd/build_registry_insecure_test.go

@@ -0,0 +1,268 @@
+package cmd
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/spf13/cobra"
+	fn "knative.dev/func/pkg/functions"
+	"knative.dev/func/pkg/mock"
+	. "knative.dev/func/pkg/testing"
+)
+
+func TestBuild_RegistryInsecurePersists(t *testing.T) {
+	testRegistryInsecurePersists(NewBuildCmd, t)
+}
+
+// testRegistryInsecurePersists ensures that the registryInsecure flag
+// value is persisted to func.yaml and remembered across consecutive runs.
+// See issue https://github.com/knative/func/issues/3489
+func testRegistryInsecurePersists(cmdFn func(factory ClientFactory) *cobra.Command, t *testing.T) {
+	root := FromTempDirectory(t)
+
+	// Initialize a new function without registryInsecure set
+	f := fn.Function{
+		Root:     root,
+		Name:     "myfunc",
+		Runtime:  "go",
+		Registry: "example.com/alice",
+	}
+	if _, err := fn.New().Init(f); err != nil {
+		t.Fatal(err)
+	}
+
+	var (
+		builder = mock.NewBuilder()
+		pusher  = mock.NewPusher()
+	)
+
+	// Test 1: Initial state - registryInsecure should be false
+	t.Run("initial state is false", func(t *testing.T) {
+		f, err := fn.NewFunction(root)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if f.RegistryInsecure {
+			t.Fatal("initial registryInsecure should be false")
+		}
+	})
+
+	// Test 2: Set registryInsecure to true with flag
+	t.Run("sets to true when flag passed", func(t *testing.T) {
+		cmd := cmdFn(NewTestClient(
+			fn.WithRegistry(TestRegistry),
+			fn.WithBuilder(builder),
+			fn.WithPusher(pusher),
+		))
+		cmd.SetArgs([]string{"--registry-insecure=true"})
+
+		if err := cmd.Execute(); err != nil {
+			t.Fatal(err)
+		}
+
+		// Load the function and verify registryInsecure is true
+		f, err := fn.NewFunction(root)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if !f.RegistryInsecure {
+			t.Fatal("registryInsecure should be true when flag passed, but was false")
+		}
+	})
+
+	// Test 3: Run build WITHOUT --registry-insecure flag
+	// Expected: registryInsecure should remain true (persisted value)
+	// This is the key test for issue #3489
+	t.Run("persists true when flag not passed", func(t *testing.T) {
+		cmd := cmdFn(NewTestClient(
+			fn.WithRegistry(TestRegistry),
+			fn.WithBuilder(builder),
+			fn.WithPusher(pusher),
+		))
+		cmd.SetArgs([]string{})
+
+		if err := cmd.Execute(); err != nil {
+			t.Fatal(err)
+		}
+
+		// Load the function and verify registryInsecure is still true
+		f, err := fn.NewFunction(root)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if !f.RegistryInsecure {
+			t.Fatal("registryInsecure should be preserved as true, but was false")
+		}
+	})
+
+	// Test 4: Explicitly set --registry-insecure=false
+	// Expected: registryInsecure should be cleared (set to false)
+	t.Run("clears when flag set to false", func(t *testing.T) {
+		cmd := cmdFn(NewTestClient(
+			fn.WithRegistry(TestRegistry),
+			fn.WithBuilder(builder),
+			fn.WithPusher(pusher),
+		))
+		cmd.SetArgs([]string{"--registry-insecure=false"})
+
+		if err := cmd.Execute(); err != nil {
+			t.Fatal(err)
+		}
+
+		// Load the function and verify registryInsecure is false
+		f, err := fn.NewFunction(root)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if f.RegistryInsecure {
+			t.Fatal("registryInsecure should be false when flag set to false, but was true")
+		}
+	})
+
+	// Test 5: Run build again WITHOUT flag after clearing
+	// Expected: registryInsecure should stay false
+	t.Run("stays false when not set", func(t *testing.T) {
+		cmd := cmdFn(NewTestClient(
+			fn.WithRegistry(TestRegistry),
+			fn.WithBuilder(builder),
+			fn.WithPusher(pusher),
+		))
+		cmd.SetArgs([]string{})
+
+		if err := cmd.Execute(); err != nil {
+			t.Fatal(err)
+		}
+
+		// Load the function and verify registryInsecure is still false
+		f, err := fn.NewFunction(root)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if f.RegistryInsecure {
+			t.Fatal("registryInsecure should remain false, but was true")
+		}
+	})
+
+	// Test 6: Set back to true and verify multiple consecutive runs
+	t.Run("persists across multiple consecutive runs", func(t *testing.T) {
+		// First set it to true
+		cmd := cmdFn(NewTestClient(
+			fn.WithRegistry(TestRegistry),
+			fn.WithBuilder(builder),
+			fn.WithPusher(pusher),
+		))
+		cmd.SetArgs([]string{"--registry-insecure=true"})
+
+		if err := cmd.Execute(); err != nil {
+			t.Fatal(err)
+		}
+
+		// Run 3 times without the flag
+		for i := 0; i < 3; i++ {
+			cmd := cmdFn(NewTestClient(
+				fn.WithRegistry(TestRegistry),
+				fn.WithBuilder(builder),
+				fn.WithPusher(pusher),
+			))
+			cmd.SetArgs([]string{})
+
+			if err := cmd.Execute(); err != nil {
+				t.Fatalf("build %d failed: %v", i+1, err)
+			}
+
+			// Load and verify after each build
+			f, err := fn.NewFunction(root)
+			if err != nil {
+				t.Fatalf("loading function after build %d failed: %v", i+1, err)
+			}
+
+			if !f.RegistryInsecure {
+				t.Fatalf("build %d: registryInsecure should be true, but was false", i+1)
+			}
+		}
+	})
+}
+
+// TestWarnRegistryInsecureChange ensures that the warning is printed when
+// the registry changes but registryInsecure is still true.
+func TestWarnRegistryInsecureChange(t *testing.T) {
+	tests := []struct {
+		name            string
+		cfgRegistry     string
+		funcRegistry    string
+		funcInsecure    bool
+		expectWarning   bool
+		expectedMessage string
+	}{
+		{
+			name:          "no warning - registry not changed",
+			cfgRegistry:   "example.com/alice",
+			funcRegistry:  "example.com/alice",
+			funcInsecure:  true,
+			expectWarning: false,
+		},
+		{
+			name:          "no warning - registryInsecure is false",
+			cfgRegistry:   "example.com/bob",
+			funcRegistry:  "example.com/alice",
+			funcInsecure:  false,
+			expectWarning: false,
+		},
+		{
+			name:          "no warning - func registry is empty",
+			cfgRegistry:   "example.com/bob",
+			funcRegistry:  "",
+			funcInsecure:  true,
+			expectWarning: false,
+		},
+		{
+			name:          "no warning - cfg registry is empty",
+			cfgRegistry:   "",
+			funcRegistry:  "example.com/alice",
+			funcInsecure:  true,
+			expectWarning: false,
+		},
+		{
+			name:            "warning - registry changed and insecure is true",
+			cfgRegistry:     "example.com/bob",
+			funcRegistry:    "example.com/alice",
+			funcInsecure:    true,
+			expectWarning:   true,
+			expectedMessage: "Warning: Registry changed from 'example.com/alice' to 'example.com/bob', but registryInsecure is still true.",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			f := fn.Function{
+				Registry:         tt.funcRegistry,
+				RegistryInsecure: tt.funcInsecure,
+			}
+
+			// Capture output
+			var buf bytes.Buffer
+			warnRegistryInsecureChange(&buf, tt.cfgRegistry, f)
+
+			output := buf.String()
+
+			if tt.expectWarning {
+				if output == "" {
+					t.Fatal("expected warning but got none")
+				}
+				if tt.expectedMessage != "" && !strings.Contains(output, tt.expectedMessage) {
+					t.Fatalf("expected message to contain '%s', got '%s'", tt.expectedMessage, output)
+				}
+			} else {
+				if output != "" {
+					t.Fatalf("expected no warning but got: %s", output)
+				}
+			}
+		})
+	}
+}

cmd/deploy.go

@@ -159,7 +159,7 @@ EXAMPLES
 		fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s.", KnownBuilders()))
 	cmd.Flags().StringP("registry", "r", cfg.Registry,
 		"Container registry + registry namespace. (ex 'ghcr.io/myuser').  The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)")
-	cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)")
+	cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)")
 
 	// Function-Context Flags:
 	// Options whose value is available on the function with context only
@@ -310,6 +310,10 @@ For more options, run 'func deploy --help'`, err)
 		}
 		return
 	}
+
+	// Warn if registry changed but registryInsecure is still true
+	warnRegistryInsecureChange(cmd.OutOrStderr(), cfg.Registry, f)
+
 	if f, err = cfg.Configure(f); err != nil { // Updates f with deploy cfg
 		return
 	}

cmd/deploy_test.go

@@ -2190,3 +2190,7 @@ func testBaseImage(cmdFn commandConstructor, t *testing.T) {
 		})
 	}
 }
+
+func TestDeploy_RegistryInsecurePersists(t *testing.T) {
+	testRegistryInsecurePersists(NewDeployCmd, t)
+}

docs/reference/func_build.md

@@ -68,7 +68,7 @@ func build
       --platform string        Optionally specify a target platform, for example "linux/amd64" when using the s2i build strategy
   -u, --push                   Attempt to push the function image to the configured registry after being successfully built
   -r, --registry string        Container registry + registry namespace. (ex 'ghcr.io/myuser').  The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)
-      --registry-insecure      Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)
+      --registry-insecure      Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)
   -v, --verbose                Print verbose logs ($FUNC_VERBOSE)
 ```
 

docs/reference/func_deploy.md

@@ -132,7 +132,7 @@ func deploy
   -u, --push                          Push the function image to registry before deploying. ($FUNC_PUSH) (default true)
       --pvc-size string               When triggering a remote deployment, set a custom volume size to allocate for the build operation ($FUNC_PVC_SIZE)
   -r, --registry string               Container registry + registry namespace. (ex 'ghcr.io/myuser').  The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)
-      --registry-insecure             Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)
+      --registry-insecure             Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)
   -R, --remote                        Trigger a remote deployment. Default is to deploy and build from the local system ($FUNC_REMOTE)
       --remote-storage-class string   Specify a storage class to use for the volume on-cluster during remote builds
       --service-account string        Service account to be used in the deployed function ($FUNC_SERVICE_ACCOUNT)