[release-1.21] Remember --registry-insecure setting on recurring runs

cmd/build.go

@@ -3,6 +3,8 @@ package cmd
 import (
 	"errors"
 	"fmt"
+	"io"
+	"os"
 	"strings"
 
 	"github.com/AlecAivazis/survey/v2"
@@ -101,7 +103,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)")
 	cmd.Flags().String("registry-authfile", "", "Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE)")
 
 	// Function-Context Flags:
@@ -170,6 +172,10 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 	if !f.Initialized() {
 		return NewErrNotInitializedFromPath(f.Root, "build")
 	}
+
+	// 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
@@ -206,6 +212,15 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
 	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
@@ -435,7 +450,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 container, 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),
+	}
 
 	t := newTransport(c.RegistryInsecure)
 	creds := newCredentialsProvider(config.Dir(), t, c.RegistryAuthfile)

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

@@ -160,7 +160,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)")
 	cmd.Flags().String("registry-authfile", "", "Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE)")
 
 	// Function-Context Flags:
@@ -282,6 +282,10 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
 	if err = cfg.Validate(cmd); err != nil {
 		return wrapValidateError(err, "deploy")
 	}
+
+	// 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

@@ -2346,3 +2346,7 @@ func TestDeploy_ValidDomain(t *testing.T) {
 		})
 	}
 }
+
+func TestDeploy_RegistryInsecurePersists(t *testing.T) {
+	testRegistryInsecurePersists(NewDeployCmd, t)
+}

docs/reference/func_build.md

@@ -1,3 +1,3 @@
 ## func build
 
 Build a function container
@@ -64,12 +64,15 @@
   -c, --confirm                    Prompt to confirm options interactively ($FUNC_CONFIRM)
   -h, --help                       help for build
   -i, --image string               Full image name in the form [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry ($FUNC_IMAGE)
+      --password string            Password to use when pushing to the registry. ($FUNC_PASSWORD)
   -p, --path string                Path to the function.  Default is current directory ($FUNC_PATH)
       --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-authfile string   Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE)
-      --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)
+      --token string               Token to use when pushing to the registry. ($FUNC_TOKEN)
+      --username string            Username to use when pushing to the registry. ($FUNC_USERNAME)
   -v, --verbose                    Print verbose logs ($FUNC_VERBOSE)
 ```
 

docs/reference/func_deploy.md

@@ -134,7 +134,7 @@ func deploy
       --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-authfile string      Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE)
-      --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)