Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 9 additions & 1 deletion pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,15 @@ func (c Global) Configure(f fn.Function) fn.Function {
f.Deploy.Namespace = c.Namespace
}
if c.Registry != "" {
f.Registry = c.Registry
// Apply the config registry unless it's an auto-detected OpenShift
// internal registry that would overwrite a user-configured external
// registry. This prevents OpenShift auto-detection (via RegistryDefault)
// from silently overriding a previously stored registry on redeploy.
// Explicit sources (--registry flag, FUNC_REGISTRY env, global config)
// set non-OpenShift values and are always applied.
if f.Registry == "" || !k8s.IsOpenShiftInternalRegistry(c.Registry) {
f.Registry = c.Registry
}
}
return f
}
Expand Down
25 changes: 25 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,31 @@ func TestConfigure(t *testing.T) {
t.Error("empty cfg.Registry should not mutate f")
}

// An auto-detected OpenShift internal registry should not overwrite
// a function's stored external registry. This is the key guard for
// https://github.com/knative/func/issues/2172
f = fn.Function{Registry: "docker.io/user"}
cfg = config.Global{Registry: "image-registry.openshift-image-registry.svc:5000/ns"}
f = cfg.Configure(f)
if f.Registry != "docker.io/user" {
t.Errorf("auto-detected OpenShift registry should not overwrite stored external registry, got %q", f.Registry)
}

// But when the function has no registry yet, the OpenShift default should apply.
f = fn.Function{}
f = cfg.Configure(f)
if f.Registry != "image-registry.openshift-image-registry.svc:5000/ns" {
t.Errorf("OpenShift registry should be applied when function has no registry, got %q", f.Registry)
}

// An explicit (non-OpenShift) registry in config should always apply,
// even when the function already has a registry.
f = fn.Function{Registry: "docker.io/user"}
cfg = config.Global{Registry: "quay.io/org"}
f = cfg.Configure(f)
if f.Registry != "quay.io/org" {
t.Errorf("explicit config registry should override function registry, got %q", f.Registry)
}
}

// TestGet_Invalid ensures that attempting to get the value of a nonexistent
Expand Down
7 changes: 7 additions & 0 deletions pkg/k8s/openshift.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/x509"
"encoding/pem"
"errors"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -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()

Expand Down
26 changes: 26 additions & 0 deletions pkg/k8s/openshift_unit_test.go
Original file line number Diff line number Diff line change
@@ -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("IsOpenShiftInternalRegistry(%q) = %v, want %v", tt.registry, got, tt.want)
}
})
}
}
Loading