Merge pull request openshift#2689 from umohnani8/wildcards

Add parsing for registries.conf wildcard entries

Author: openshift-merge-robot

Makefile

@@ -109,7 +109,7 @@ Dockerfile.rhel7: Dockerfile Makefile
 
 # This was copied from https://github.com/openshift/cluster-image-registry-operator
 test-e2e:
-	go test -failfast -timeout 90m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e/
+	go test -tags=$(GOTAGS) -failfast -timeout 90m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e/
 
 test-e2e-single-node:
-	go test -failfast -timeout 90m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e-single-node/
+	go test -tags=$(GOTAGS) -failfast -timeout 90m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e-single-node/

go.mod

@@ -12,9 +12,8 @@ require (
 	github.com/apparentlymart/go-cidr v1.0.0
 	github.com/ashcrow/osrelease v0.0.0-20180626175927-9b292693c55c
 	github.com/clarketm/json v1.14.1
-	github.com/containers/image v3.0.2+incompatible
-	github.com/containers/image/v5 v5.5.1
-	github.com/containers/storage v1.20.2
+	github.com/containers/image/v5 v5.14.0
+	github.com/containers/storage v1.32.6
 	github.com/coreos/fcct v0.5.0
 	github.com/coreos/go-semver v0.3.0
 	github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f // indirect
@@ -28,28 +27,28 @@ require (
 	github.com/go-bindata/go-bindata v3.1.2+incompatible
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
 	github.com/golangci/golangci-lint v1.18.0
-	github.com/google/go-cmp v0.5.2
+	github.com/google/go-cmp v0.5.5
 	github.com/google/renameio v0.1.0
 	github.com/gostaticanalysis/analysisutil v0.0.3 // indirect
 	github.com/hashicorp/golang-lru v0.5.3 // indirect
 	github.com/huandu/xstrings v1.2.0 // indirect
-	github.com/imdario/mergo v0.3.9
+	github.com/imdario/mergo v0.3.12
+	github.com/mattn/go-isatty v0.0.12 // indirect
 	github.com/opencontainers/go-digest v1.0.0
 	github.com/openshift/api v0.0.0-20210629145910-15a1cae1fca8
 	github.com/openshift/client-go v0.0.0-20210521082421-73d9475a9142
 	github.com/openshift/library-go v0.0.0-20210702104503-39570b4a2ae8
-	github.com/openshift/runtime-utils v0.0.0-20200415173359-c45d4ff3f912
+	github.com/openshift/runtime-utils v0.0.0-20210722191527-8b8348d80d1d
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.7.1
 	github.com/securego/gosec v0.0.0-20191002120514-e680875ea14d
 	github.com/spf13/cobra v1.1.1
-	github.com/spf13/jwalterweatherman v1.1.0 // indirect
 	github.com/spf13/pflag v1.0.5
-	github.com/stretchr/testify v1.6.1
+	github.com/stretchr/testify v1.7.0
 	github.com/ultraware/funlen v0.0.2 // indirect
 	github.com/vincent-petithory/dataurl v0.0.0-20160330182126-9a301d65acbb
 	github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect
-	golang.org/x/net v0.0.0-20210224082022-3d97a244fca7
+	golang.org/x/net v0.0.0-20210226172049-e18ecbb05110
 	golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba
 	k8s.io/api v0.21.1
 	k8s.io/apiextensions-apiserver v0.21.1
@@ -58,7 +57,7 @@ require (
 	k8s.io/code-generator v0.21.1
 	k8s.io/kubectl v0.21.0-rc.0
 	k8s.io/kubelet v0.21.0-rc.0
-	k8s.io/utils v0.0.0-20201110183641-67b214c5f920
+	k8s.io/utils v0.0.0-20210707171843-4b05e18ac7d9
 )
 
 replace (
@@ -80,6 +79,8 @@ replace (
 	k8s.io/cluster-bootstrap => k8s.io/cluster-bootstrap v0.21.0-rc.0
 	k8s.io/code-generator => k8s.io/code-generator v0.21.0-rc.0
 	k8s.io/component-base => k8s.io/component-base v0.21.0-rc.0
+	k8s.io/component-helpers => k8s.io/component-helpers v0.20.0-alpha.2.0.20210708095128-55a3896515e9
+	k8s.io/controller-manager => k8s.io/controller-manager v0.20.0-alpha.1.0.20210712075914-83508d18fce2
 	k8s.io/cri-api => k8s.io/cri-api v0.21.0-rc.0
 	k8s.io/csi-translation-lib => k8s.io/csi-translation-lib v0.21.0-rc.0
 	k8s.io/kube-aggregator => k8s.io/kube-aggregator v0.21.0-rc.0
@@ -91,5 +92,6 @@ replace (
 	k8s.io/kubernetes => k8s.io/kubernetes v1.21.0-rc.0
 	k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.21.0-rc.0
 	k8s.io/metrics => k8s.io/metrics v0.21.0-rc.0
+	k8s.io/mount-utils => k8s.io/mount-utils v0.23.0-alpha.0
 	k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.21.0-rc.0
 )

go.sum

@@ -8,11 +8,12 @@ import (
 	"fmt"
 	"reflect"
 	"strconv"
+	"strings"
 
 	"github.com/BurntSushi/toml"
-	"github.com/containers/image/docker/reference"
-	"github.com/containers/image/pkg/sysregistriesv2"
-	signature "github.com/containers/image/signature"
+	"github.com/containers/image/v5/docker/reference"
+	"github.com/containers/image/v5/pkg/sysregistriesv2"
+	signature "github.com/containers/image/v5/signature"
 	storageconfig "github.com/containers/storage/pkg/config"
 	ign3types "github.com/coreos/ignition/v2/config/v3_2/types"
 	"github.com/golang/glog"
@@ -361,6 +362,10 @@ func updateRegistriesConfig(data []byte, internalInsecure, internalBlocked []str
 		return nil, fmt.Errorf("error unmarshalling registries config: %v", err)
 	}
 
+	if err := validateRegistriesConfScopes(internalInsecure, internalBlocked, []string{}, icspRules); err != nil {
+		return nil, err
+	}
+
 	if err := registries.EditRegistriesConfig(&tomlConf, internalInsecure, internalBlocked, icspRules); err != nil {
 		return nil, err
 	}
@@ -383,12 +388,16 @@ func updatePolicyJSON(data []byte, internalBlocked, internalAllowed []string) ([
 		return nil, fmt.Errorf("invalid images config: only one of AllowedRegistries or BlockedRegistries may be specified")
 	}
 	// Return original data if neither allowed or blocked registries are configured
-	// Note: this is just for testing, the controller does not call this functio till
+	// Note: this is just for testing, the controller does not call this function till
 	// either allowed or blocked registries are configured
 	if internalAllowed == nil && internalBlocked == nil {
 		return data, nil
 	}
 
+	if err := validateRegistriesConfScopes([]string{}, internalBlocked, internalAllowed, nil); err != nil {
+		return nil, err
+	}
+
 	policyObj := &signature.Policy{}
 	decoder := json.NewDecoder(bytes.NewBuffer(data))
 	err := decoder.Decode(policyObj)
@@ -495,3 +504,38 @@ func getValidBlockedRegistries(releaseImage string, imgSpec *apicfgv1.ImageSpec)
 	}
 	return blockedRegs, nil
 }
+
+func validateRegistriesConfScopes(insecure, blocked, allowed []string, icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy) error {
+	for _, scope := range insecure {
+		if !registries.IsValidRegistriesConfScope(scope) {
+			return fmt.Errorf("invalid entry for insecure registries %q", scope)
+		}
+	}
+
+	for _, scope := range blocked {
+		if !registries.IsValidRegistriesConfScope(scope) {
+			return fmt.Errorf("invalid entry for blocked registries %q", scope)
+		}
+	}
+
+	for _, scope := range allowed {
+		if !registries.IsValidRegistriesConfScope(scope) {
+			return fmt.Errorf("invalid entry for allowed registries %q", scope)
+		}
+	}
+
+	for _, icsp := range icspRules {
+		for _, mirrorSet := range icsp.Spec.RepositoryDigestMirrors {
+			if strings.Contains(mirrorSet.Source, "*") {
+				return fmt.Errorf("wildcard entries are not supported with mirror configuration %q", mirrorSet.Source)
+			}
+			for _, mirror := range mirrorSet.Mirrors {
+				if strings.Contains(mirror, "*") {
+					return fmt.Errorf("wildcard entries are not supported with mirror configuration %q", mirror)
+				}
+			}
+		}
+
+	}
+	return nil
+}

pkg/controller/container-runtime-config/helpers.go

@@ -9,9 +9,9 @@ import (
 	"testing"
 
 	"github.com/BurntSushi/toml"
-	"github.com/containers/image/pkg/sysregistriesv2"
-	signature "github.com/containers/image/signature"
-	"github.com/containers/image/types"
+	"github.com/containers/image/v5/pkg/sysregistriesv2"
+	signature "github.com/containers/image/v5/signature"
+	"github.com/containers/image/v5/types"
 	apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -46,49 +46,49 @@ func TestUpdateRegistriesConfig(t *testing.T) {
 				Registries: []sysregistriesv2.Registry{
 					{
 						Endpoint: sysregistriesv2.Endpoint{
-							Location: "registry.access.redhat.com",
-							Insecure: true,
+							Location: "blocked.com",
 						},
+						Blocked: true,
 					},
 					{
 						Endpoint: sysregistriesv2.Endpoint{
-							Location: "insecure.com",
+							Location: "common.com",
 							Insecure: true,
 						},
+						Blocked: true,
 					},
 					{
 						Endpoint: sysregistriesv2.Endpoint{
-							Location: "common.com",
-							Insecure: true,
+							Location: "docker.io",
 						},
 						Blocked: true,
 					},
 					{
 						Endpoint: sysregistriesv2.Endpoint{
-							Location: "blocked.com",
+							Location: "registry.access.redhat.com",
+							Insecure: true,
 						},
-						Blocked: true,
 					},
 					{
 						Endpoint: sysregistriesv2.Endpoint{
-							Location: "docker.io",
+							Location: "insecure.com",
+							Insecure: true,
 						},
-						Blocked: true,
 					},
 				},
 			},
 		},
 		{
-			name:     "insecure+blocked prefixes",
-			insecure: []string{"insecure.com"},
-			blocked:  []string{"blocked.com"},
+			name:     "insecure+blocked prefixes with wildcard entries",
+			insecure: []string{"insecure.com", "*.insecure-example.com", "*.insecure.blocked-example.com"},
+			blocked:  []string{"blocked.com", "*.blocked.insecure-example.com", "*.blocked-example.com"},
 			icspRules: []*apioperatorsv1alpha1.ImageContentSourcePolicy{
 				{
 					Spec: apioperatorsv1alpha1.ImageContentSourcePolicySpec{
 						RepositoryDigestMirrors: []apioperatorsv1alpha1.RepositoryDigestMirrors{ // other.com is neither insecure nor blocked
 							{Source: "insecure.com/ns-i1", Mirrors: []string{"blocked.com/ns-b1", "other.com/ns-o1"}},
 							{Source: "blocked.com/ns-b/ns2-b", Mirrors: []string{"other.com/ns-o2", "insecure.com/ns-i2"}},
-							{Source: "other.com/ns-o3", Mirrors: []string{"insecure.com/ns-i2", "blocked.com/ns-b/ns3-b"}},
+							{Source: "other.com/ns-o3", Mirrors: []string{"insecure.com/ns-i2", "blocked.com/ns-b/ns3-b", "foo.insecure-example.com/bar"}},
 						},
 					},
 				},
@@ -128,20 +128,50 @@ func TestUpdateRegistriesConfig(t *testing.T) {
 						Mirrors: []sysregistriesv2.Endpoint{
 							{Location: "insecure.com/ns-i2", Insecure: true},
 							{Location: "blocked.com/ns-b/ns3-b"},
+							{Location: "foo.insecure-example.com/bar", Insecure: true},
 						},
 					},
-
+					{
+						Endpoint: sysregistriesv2.Endpoint{
+							Location: "blocked.com",
+						},
+						Blocked: true,
+					},
+					{
+						Prefix:  "*.blocked.insecure-example.com",
+						Blocked: true,
+						Endpoint: sysregistriesv2.Endpoint{
+							Location: "",
+							Insecure: true,
+						},
+					},
+					{
+						Prefix: "*.blocked-example.com",
+						Endpoint: sysregistriesv2.Endpoint{
+							Location: "",
+						},
+						Blocked: true,
+					},
 					{
 						Endpoint: sysregistriesv2.Endpoint{
 							Location: "insecure.com",
 							Insecure: true,
 						},
 					},
 					{
+						Prefix: "*.insecure-example.com",
 						Endpoint: sysregistriesv2.Endpoint{
-							Location: "blocked.com",
+							Location: "",
+							Insecure: true,
 						},
+					},
+					{
+						Prefix:  "*.insecure.blocked-example.com",
 						Blocked: true,
+						Endpoint: sysregistriesv2.Endpoint{
+							Location: "",
+							Insecure: true,
+						},
 					},
 				},
 			},
@@ -203,15 +233,17 @@ func TestUpdatePolicyJSON(t *testing.T) {
 		},
 		{
 			name:    "allowed",
-			allowed: []string{"allow.io"},
+			allowed: []string{"allow.io", "*.allowed-example.com"},
 			want: signature.Policy{
 				Default: signature.PolicyRequirements{signature.NewPRReject()},
 				Transports: map[string]signature.PolicyTransportScopes{
 					"atomic": map[string]signature.PolicyRequirements{
-						"allow.io": signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()},
+						"allow.io":              signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()},
+						"*.allowed-example.com": signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()},
 					},
 					"docker": map[string]signature.PolicyRequirements{
-						"allow.io": signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()},
+						"allow.io":              signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()},
+						"*.allowed-example.com": signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()},
 					},
 					"docker-daemon": map[string]signature.PolicyRequirements{
 						"": signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()},
@@ -221,15 +253,17 @@ func TestUpdatePolicyJSON(t *testing.T) {
 		},
 		{
 			name:    "blocked",
-			blocked: []string{"block.com"},
+			blocked: []string{"block.com", "*.blocked-example.com"},
 			want: signature.Policy{
 				Default: signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()},
 				Transports: map[string]signature.PolicyTransportScopes{
 					"atomic": map[string]signature.PolicyRequirements{
-						"block.com": signature.PolicyRequirements{signature.NewPRReject()},
+						"block.com":             signature.PolicyRequirements{signature.NewPRReject()},
+						"*.blocked-example.com": signature.PolicyRequirements{signature.NewPRReject()},
 					},
 					"docker": map[string]signature.PolicyRequirements{
-						"block.com": signature.PolicyRequirements{signature.NewPRReject()},
+						"block.com":             signature.PolicyRequirements{signature.NewPRReject()},
+						"*.blocked-example.com": signature.PolicyRequirements{signature.NewPRReject()},
 					},
 					"docker-daemon": map[string]signature.PolicyRequirements{
 						"": signature.PolicyRequirements{signature.NewPRInsecureAcceptAnything()},

pkg/controller/container-runtime-config/helpers_test.go

@@ -6,7 +6,7 @@ import (
 	"time"
 
 	"github.com/BurntSushi/toml"
-	"github.com/containers/image/pkg/sysregistriesv2"
+	"github.com/containers/image/v5/pkg/sysregistriesv2"
 	ign3types "github.com/coreos/ignition/v2/config/v3_2/types"
 	"github.com/golang/glog"
 	mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
@@ -223,12 +223,20 @@ func isSafeContainerRegistryConfChanges(oldConfig, newConfig *mcfgv1.MachineConf
 
 	oldRegHashMap := make(map[string]sysregistriesv2.Registry)
 	for _, reg := range tomlConfOldReg.Registries {
-		oldRegHashMap[reg.Location] = reg
+		scope := reg.Location
+		if reg.Prefix != "" {
+			scope = reg.Prefix
+		}
+		oldRegHashMap[scope] = reg
 	}
 
 	newRegHashMap := make(map[string]sysregistriesv2.Registry)
 	for _, reg := range tomlConfNewReg.Registries {
-		newRegHashMap[reg.Location] = reg
+		scope := reg.Location
+		if reg.Prefix != "" {
+			scope = reg.Prefix
+		}
+		newRegHashMap[scope] = reg
 	}
 
 	// Check for removed registry
@@ -251,6 +259,11 @@ func isSafeContainerRegistryConfChanges(oldConfig, newConfig *mcfgv1.MachineConf
 					containerRegistryConfPath, regLoc, oldReg.Prefix, newReg.Prefix)
 				return false, nil
 			}
+			if oldReg.Location != newReg.Location {
+				glog.Infof("%s: location value for registry %s has changed from %s to %s",
+					containerRegistryConfPath, regLoc, oldReg.Location, newReg.Location)
+				return false, nil
+			}
 			if oldReg.Blocked != newReg.Blocked {
 				glog.Infof("%s: blocked value for registry %s has changed from %t to %t",
 					containerRegistryConfPath, regLoc, oldReg.Blocked, newReg.Blocked)