diff --git a/pkg/image/apis/image/validation/validation.go b/pkg/image/apis/image/validation/validation.go index 847656530695..9c1e189daa02 100644 --- a/pkg/image/apis/image/validation/validation.go +++ b/pkg/image/apis/image/validation/validation.go @@ -193,7 +193,7 @@ func ValidateImageStreamWithWhitelister( } } if isValid && whitelister != nil { - if err := whitelister.AdmitDockerImageReference(&ref, whitelist.GetWhitelistTransportForFlag(insecureRepository, true)); err != nil { + if err := whitelister.AdmitDockerImageReference(&ref, getWhitelistTransportForFlag(insecureRepository, true)); err != nil { result = append(result, field.Forbidden(dockerImageRepositoryPath, err.Error())) } } @@ -218,7 +218,7 @@ func ValidateImageStreamWithWhitelister( if tr, ok := stream.Spec.Tags[tag]; ok { insecure = tr.ImportPolicy.Insecure } - transport := whitelist.GetWhitelistTransportForFlag(insecure || insecureRepository, true) + transport := getWhitelistTransportForFlag(insecure || insecureRepository, true) if err := whitelister.AdmitDockerImageReference(&ref, transport); err != nil { result = append(result, field.Forbidden(field.NewPath("status", "tags").Key(tag).Child("items").Index(i).Child("dockerImageReference"), err.Error())) } @@ -249,7 +249,7 @@ func ValidateImageStreamTagReference( } else if len(ref.ID) > 0 && tagRef.ImportPolicy.Scheduled { errs = append(errs, field.Invalid(fldPath.Child("from", "name"), tagRef.From.Name, "only tags can be scheduled for import")) } else if whitelister != nil { - transport := whitelist.GetWhitelistTransportForFlag(tagRef.ImportPolicy.Insecure || insecureRepository, true) + transport := getWhitelistTransportForFlag(tagRef.ImportPolicy.Insecure || insecureRepository, true) if err := whitelister.AdmitDockerImageReference(&ref, transport); err != nil { errs = append(errs, field.Forbidden(fldPath.Child("from", "name"), err.Error())) } @@ -341,7 +341,7 @@ func ValidateImageStreamStatusUpdateWithWhitelister( } } } - transport := whitelist.GetWhitelistTransportForFlag(insecure, true) + transport := getWhitelistTransportForFlag(insecure, true) if err := whitelister.AdmitDockerImageReference(&ref, transport); err != nil { // TODO: should we whitelist references imported based on whitelisted/old spec? // report error for each tag/history item having this reference @@ -569,3 +569,13 @@ func isRepositoryInsecure(obj runtime.Object) bool { } return accessor.GetAnnotations()[imageapi.InsecureRepositoryAnnotation] == "true" } + +func getWhitelistTransportForFlag(insecure, allowSecureFallback bool) whitelist.WhitelistTransport { + if insecure { + if allowSecureFallback { + return whitelist.WhitelistTransportAny + } + return whitelist.WhitelistTransportInsecure + } + return whitelist.WhitelistTransportSecure +} diff --git a/pkg/image/apis/image/validation/whitelist/whitelister.go b/pkg/image/apis/image/validation/whitelist/whitelister.go index fbd5b60d7f5d..8ab12650750c 100644 --- a/pkg/image/apis/image/validation/whitelist/whitelister.go +++ b/pkg/image/apis/image/validation/whitelist/whitelister.go @@ -43,7 +43,6 @@ type RegistryWhitelister interface { WhitelistRegistry(hostPortGlob string, transport WhitelistTransport) error // WhitelistPullSpecs allows to whitelist particular pull specs. References must match exactly one of the // given pull specs for it to be whitelisted. - // TODO: accept insecure flag WhitelistPullSpecs(pullSpecs ...string) // Copy returns a deep copy of the whitelister. This is useful for temporarily whitelisting additional // registries/pullSpecs before a specific validation. @@ -78,7 +77,11 @@ func NewRegistryWhitelister( // iterate in reversed order to make the patterns appear in the same order as given (patterns are prepended) for i := len(whitelist) - 1; i >= 0; i-- { registry := whitelist[i] - err := rw.WhitelistRegistry(registry.DomainName, GetWhitelistTransportForFlag(registry.Insecure, false)) + transport := WhitelistTransportSecure + if registry.Insecure { + transport = WhitelistTransportInsecure + } + err := rw.WhitelistRegistry(registry.DomainName, transport) if err != nil { errs = append(errs, err) } @@ -112,9 +115,6 @@ func (rw *registryWhitelister) AdmitPullSpec(pullSpec string, transport Whitelis } func (rw *registryWhitelister) AdmitDockerImageReference(ref *imageapi.DockerImageReference, transport WhitelistTransport) error { - const showMax = 5 - const fullWhitelistLogLevel = 5 - if rw.pullSpecs.Len() > 0 { if rw.pullSpecs.Has(ref.Exact()) || rw.pullSpecs.Has(ref.DockerClientDefaults().Exact()) || rw.pullSpecs.Has(ref.DaemonMinimal().Exact()) { return nil @@ -188,19 +188,12 @@ func (rw *registryWhitelister) AdmitDockerImageReference(ref *imageapi.DockerIma } } whitelist := []string{} - full := []string{} for i := 0; i < len(rw.whitelist); i++ { - if i < showMax-1 || len(rw.whitelist)-showMax == 0 { - whitelist = append(whitelist, fmt.Sprintf("%q", net.JoinHostPort(rw.whitelist[i].host, rw.whitelist[i].port))) - } else if i < showMax { - whitelist = append(whitelist, fmt.Sprintf("and %d more ...", len(rw.whitelist)-showMax+1)) - } else if !glog.V(fullWhitelistLogLevel) { - break - } - full = append(full, fmt.Sprintf("%q", net.JoinHostPort(rw.whitelist[i].host, rw.whitelist[i].port))) + whitelist = append(whitelist, fmt.Sprintf("%q", net.JoinHostPort(rw.whitelist[i].host, rw.whitelist[i].port))) } - glog.V(fullWhitelistLogLevel).Infof("registry %q not allowed by whitelist { %s }", hostname, strings.Join(full, ", ")) - return fmt.Errorf("registry %q not allowed by whitelist { %s }", hostname, strings.Join(whitelist, ", ")) + msg := fmt.Sprintf("registry %q not allowed by whitelist { %s }", hostname, strings.Join(whitelist, ", ")) + glog.V(5).Info(msg) + return fmt.Errorf("%s", msg) } func (rw *registryWhitelister) WhitelistRegistry(hostPortGlob string, transport WhitelistTransport) error { @@ -255,14 +248,3 @@ func (rw *registryWhitelister) Copy() RegistryWhitelister { } return &newRW } - -// GetWhitelistTransportForFlag returns transport for the given insecure flag. -func GetWhitelistTransportForFlag(insecure, allowSecureFallback bool) WhitelistTransport { - if insecure { - if allowSecureFallback { - return WhitelistTransportAny - } - return WhitelistTransportInsecure - } - return WhitelistTransportSecure -} diff --git a/pkg/image/apis/image/validation/whitelist/whitelister_test.go b/pkg/image/apis/image/validation/whitelist/whitelister_test.go index d3ac7e7b51e9..6aeec1fccbf7 100644 --- a/pkg/image/apis/image/validation/whitelist/whitelister_test.go +++ b/pkg/image/apis/image/validation/whitelist/whitelister_test.go @@ -189,10 +189,10 @@ func TestRegistryWhitelister(t *testing.T) { "a.b.c.d.foo.com:80": nil, "domain.ltd": nil, "example.com": nil, - "foo.com": fmt.Errorf(`registry "foo.com" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", and 2 more ... }`), + "foo.com": fmt.Errorf(`registry "foo.com" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", "*.foo.com:80", "*domain.ltd:80" }`), "index.docker.io": nil, "localhost:5000": nil, - "my.domain.ltd:443": fmt.Errorf(`registry "my.domain.ltd:443" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", and 2 more ... }`), + "my.domain.ltd:443": fmt.Errorf(`registry "my.domain.ltd:443" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", "*.foo.com:80", "*domain.ltd:80" }`), "my.domain.ltd:80": nil, "my.domain.ltd": nil, "mydomain.ltd": nil, @@ -201,8 +201,8 @@ func TestRegistryWhitelister(t *testing.T) { }, difs: map[imageapi.DockerImageReference]error{ {Registry: "docker.io", Namespace: "library", Name: "busybox"}: nil, - {Registry: "foo.com", Namespace: "library", Name: "busybox"}: fmt.Errorf(`registry "foo.com" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", and 2 more ... }`), - {Registry: "ffoo.com", Namespace: "library", Name: "busybox"}: fmt.Errorf(`registry "ffoo.com" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", and 2 more ... }`), + {Registry: "foo.com", Namespace: "library", Name: "busybox"}: fmt.Errorf(`registry "foo.com" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", "*.foo.com:80", "*domain.ltd:80" }`), + {Registry: "ffoo.com", Namespace: "library", Name: "busybox"}: fmt.Errorf(`registry "ffoo.com" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", "*.foo.com:80", "*domain.ltd:80" }`), }, }, @@ -211,24 +211,24 @@ func TestRegistryWhitelister(t *testing.T) { transport: WhitelistTransportAny, whitelist: mkAllowed(false, "localhost:5000", "docker.io", "example.com:*", "registry.com:80", "*.foo.com", "*domain.ltd"), hostnames: map[string]error{ - "a.b.c.d.foo.com:80": fmt.Errorf(`registry "a.b.c.d.foo.com:80" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", and 2 more ... }`), + "a.b.c.d.foo.com:80": fmt.Errorf(`registry "a.b.c.d.foo.com:80" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", "*.foo.com:443", "*domain.ltd:443" }`), "domain.ltd": nil, "example.com": nil, - "foo.com": fmt.Errorf(`registry "foo.com" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", and 2 more ... }`), + "foo.com": fmt.Errorf(`registry "foo.com" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", "*.foo.com:443", "*domain.ltd:443" }`), "index.docker.io": nil, "localhost:5000": nil, "my.domain.ltd:443": nil, - "my.domain.ltd:80": fmt.Errorf(`registry "my.domain.ltd:80" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", and 2 more ... }`), + "my.domain.ltd:80": fmt.Errorf(`registry "my.domain.ltd:80" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", "*.foo.com:443", "*domain.ltd:443" }`), "my.domain.ltd": nil, "mydomain.ltd": nil, - "registry.com:443": fmt.Errorf(`registry "registry.com:443" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", and 2 more ... }`), + "registry.com:443": fmt.Errorf(`registry "registry.com:443" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", "*.foo.com:443", "*domain.ltd:443" }`), "registry.com": nil, "registry.foo.com": nil, }, difs: map[imageapi.DockerImageReference]error{ {Registry: "docker.io", Namespace: "library", Name: "busybox"}: nil, - {Registry: "foo.com", Namespace: "library", Name: "busybox"}: fmt.Errorf(`registry "foo.com" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", and 2 more ... }`), - {Registry: "ffoo.com", Namespace: "library", Name: "busybox"}: fmt.Errorf(`registry "ffoo.com" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", and 2 more ... }`), + {Registry: "foo.com", Namespace: "library", Name: "busybox"}: fmt.Errorf(`registry "foo.com" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", "*.foo.com:443", "*domain.ltd:443" }`), + {Registry: "ffoo.com", Namespace: "library", Name: "busybox"}: fmt.Errorf(`registry "ffoo.com" not allowed by whitelist { "localhost:5000", "docker.io:443", "example.com:*", "registry.com:80", "*.foo.com:443", "*domain.ltd:443" }`), }, }, @@ -240,14 +240,14 @@ func TestRegistryWhitelister(t *testing.T) { "a.b.c.d.foo.com:80/repo": nil, "domain.ltd/a/b": nil, "example.com/c/d": nil, - "foo.com/foo": fmt.Errorf(`registry "foo.com" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", and 2 more ... }`), + "foo.com/foo": fmt.Errorf(`registry "foo.com" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", "*.foo.com:80", "*domain.ltd:80" }`), "index.docker.io/bar": nil, "localhost:5000/repo": nil, - "my.domain.ltd:443/a/b": fmt.Errorf(`registry "my.domain.ltd:443" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", and 2 more ... }`), + "my.domain.ltd:443/a/b": fmt.Errorf(`registry "my.domain.ltd:443" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", "*.foo.com:80", "*domain.ltd:80" }`), "my.domain.ltd:80/foo:latest": nil, "my.domain.ltd/bar:1.3.4": nil, "mydomain.ltd/my/repo/sitory": nil, - "registry.com:443/ab:tag": fmt.Errorf(`registry "registry.com:443" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", and 2 more ... }`), + "registry.com:443/ab:tag": fmt.Errorf(`registry "registry.com:443" not allowed by whitelist { "localhost:5000", "docker.io:80", "example.com:*", "registry.com:80", "*.foo.com:80", "*domain.ltd:80" }`), "registry.com/repo": nil, "registry.foo.com/123": nil, "repository:latest": nil,