Skip to content

Commit

Permalink
Add parsing for registries.conf wildcard entries
Browse files Browse the repository at this point in the history 
The registries.conf file now supports wildcard entries.
Add validation for possible entries for insecure, blocked,
and allowed and vendor in updated runtime-utils for handling
the wildcard entries correctly.
Add some more test cases for wildcard entries.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
  • Loading branch information
@umohnani8
umohnani8 committed Jul 23, 2021
1 parent 9d1da27 commit 35b4e8e
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 33 deletions.
4 changes: 2 additions & 2 deletions Makefile
View file
Expand Up @@ -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/
52 changes: 48 additions & 4 deletions pkg/controller/container-runtime-config/helpers.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
82 changes: 58 additions & 24 deletions pkg/controller/container-runtime-config/helpers_test.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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"}},
},
},
},
Expand Down Expand Up @@ -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,
},
},
},
},
Expand Down Expand Up @@ -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()},
Expand All @@ -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()},
Expand Down
19 changes: 16 additions & 3 deletions pkg/daemon/drain.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down

0 comments on commit 35b4e8e

Please sign in to comment.