fix: preserve user-specified registry on OpenShift redeploy

Author: gauron99

cmd/deploy.go

@@ -291,9 +291,7 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
 	// also update the registry because there is a registry per namespace,
 	// and their name includes the namespace.
 	// This saves needing a manual flag ``--registry={destination namespace registry}``
-	if changingNamespace(f) && k8s.IsOpenShift() {
-		// TODO(lkingland): this appears to force use of the openshift
-		// internal registry.
+	if changingNamespace(f) && k8s.IsOpenShift() && k8s.IsOpenShiftInternalRegistry(f.Registry) {
 		f.Registry = "image-registry.openshift-image-registry.svc:5000/" + f.Namespace
 		if cfg.Verbose {
 			fmt.Fprintf(cmd.OutOrStdout(), "Info: Overriding openshift registry to %s\n", f.Registry)

cmd/deploy_test.go

@@ -1337,6 +1337,78 @@ func TestDeploy_BasicRedeployPipelinesCorrectNamespace(t *testing.T) {
 	}
 }
 
+// TestDeploy_NamespaceChangePreservesExternalRegistry ensures that changing
+// namespace on OpenShift does not overwrite an external registry (e.g.
+// docker.io/user) with the internal OpenShift registry.
+// Regression test for https://github.com/knative/func/issues/2172
+func TestDeploy_NamespaceChangePreservesExternalRegistry(t *testing.T) {
+	root := FromTempDirectory(t)
+
+	cleanup := k8s.SetOpenShiftForTest(true)
+	defer cleanup()
+
+	// Create a function deployed to "ns1" with an external registry
+	f := fn.Function{
+		Runtime:  "go",
+		Root:     root,
+		Registry: "docker.io/user",
+		Deploy:   fn.DeploySpec{Namespace: "ns1"},
+	}
+	f, err := fn.New().Init(f)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Deploy to a different namespace
+	cmd := NewDeployCmd(NewTestClient(fn.WithDeployer(mock.NewDeployer())))
+	cmd.SetArgs([]string{"--namespace=ns2"})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Reload and verify the external registry was preserved
+	f, _ = fn.NewFunction(root)
+	if f.Registry != "docker.io/user" {
+		t.Errorf("expected registry 'docker.io/user' to be preserved, got %q", f.Registry)
+	}
+}
+
+// TestDeploy_NamespaceChangeUpdatesInternalRegistry ensures that changing
+// namespace on OpenShift DOES update the registry when the function uses
+// the internal OpenShift registry (the namespace is part of the registry path).
+func TestDeploy_NamespaceChangeUpdatesInternalRegistry(t *testing.T) {
+	root := FromTempDirectory(t)
+
+	cleanup := k8s.SetOpenShiftForTest(true)
+	defer cleanup()
+
+	// Create a function deployed to "ns1" using the internal registry
+	f := fn.Function{
+		Runtime:  "go",
+		Root:     root,
+		Registry: "image-registry.openshift-image-registry.svc:5000/ns1",
+		Deploy:   fn.DeploySpec{Namespace: "ns1"},
+	}
+	f, err := fn.New().Init(f)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Deploy to a different namespace
+	cmd := NewDeployCmd(NewTestClient(fn.WithDeployer(mock.NewDeployer())))
+	cmd.SetArgs([]string{"--namespace=ns2"})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Reload and verify the internal registry was updated to the new namespace
+	f, _ = fn.NewFunction(root)
+	expected := "image-registry.openshift-image-registry.svc:5000/ns2"
+	if f.Registry != expected {
+		t.Errorf("expected registry to update to %q, got %q", expected, f.Registry)
+	}
+}
+
 // TestDeploy_Registry ensures that a function's registry member is kept in
 // sync with the image tag.
 // During normal operation (using the client API) a function's state on disk

pkg/config/config_test.go

@@ -226,7 +226,7 @@ func TestApply(t *testing.T) {
 	}
 }
 
-// TestConfigyre ensures that configuring a function results in every member
+// TestConfigure ensures that configuring a function results in every member
 // of the function in the intersection of the two sets, global config and function
 // members, to be set to the values of the config.
 // (See the associated cfg.Apply)
@@ -253,7 +253,7 @@ func TestConfigure(t *testing.T) {
 		t.Error("configure missing map for f.Registry")
 	}
 
-	// empty values in the global config shoul not zero out function values
+	// empty values in the global config should not zero out function values
 	// when configuring.
 	f = config.Global{}.Configure(f)
 	if f.Build.Builder == "" {
@@ -268,7 +268,6 @@ func TestConfigure(t *testing.T) {
 	if f.Registry == "" {
 		t.Error("empty cfg.Registry should not mutate f")
 	}
-
 }
 
 // TestGet_Invalid ensures that attempting to get the value of a nonexistent

pkg/k8s/openshift.go

@@ -5,6 +5,7 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"errors"
+	"strings"
 	"sync"
 	"time"
 
@@ -93,6 +94,12 @@ func GetDefaultOpenShiftRegistry() string {
 	return openShiftRegistryHostPort + "/" + ns
 }
 
+// IsOpenShiftInternalRegistry returns true if the given registry string
+// refers to the OpenShift internal image registry.
+func IsOpenShiftInternalRegistry(registry string) bool {
+	return strings.HasPrefix(registry, openShiftRegistryHost)
+}
+
 func GetOpenShiftDockerCredentialLoaders() []creds.CredentialsCallback {
 	conf := GetClientConfig()
 
@@ -126,6 +133,15 @@ func GetOpenShiftDockerCredentialLoaders() []creds.CredentialsCallback {
 var isOpenShift bool
 var checkOpenShiftOnce sync.Once
 
+// SetOpenShiftForTest overrides OpenShift detection for testing.
+// Returns a cleanup function that restores the previous state.
+func SetOpenShiftForTest(val bool) func() {
+	checkOpenShiftOnce.Do(func() {}) // ensure real detection won't run
+	prev := isOpenShift
+	isOpenShift = val
+	return func() { isOpenShift = prev }
+}
+
 func IsOpenShift() bool {
 	checkOpenShiftOnce.Do(func() {
 		isOpenShift = false

pkg/k8s/openshift_unit_test.go

@@ -0,0 +1,26 @@
+package k8s
+
+import "testing"
+
+func TestIsOpenShiftInternalRegistry(t *testing.T) {
+	tests := []struct {
+		name     string
+		registry string
+		want     bool
+	}{
+		{"internal with port and namespace", "image-registry.openshift-image-registry.svc:5000/mynamespace", true},
+		{"internal without port", "image-registry.openshift-image-registry.svc/mynamespace", true},
+		{"internal host only", "image-registry.openshift-image-registry.svc", true},
+		{"docker.io", "docker.io/user", false},
+		{"ghcr.io", "ghcr.io/user", false},
+		{"quay.io", "quay.io/user", false},
+		{"empty string", "", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := IsOpenShiftInternalRegistry(tt.registry); got != tt.want {
+				t.Errorf("expected IsOpenShiftInternalRegistry(%q) = %v, got %v", tt.registry, got, tt.want)
+			}
+		})
+	}
+}