Skip to content

Commit

Permalink
Merge pull request #406 from dperaza4dustbit/bug_2027745
Browse files Browse the repository at this point in the history 
Bug 2027745: Allowing ImageStream creation when config registry empty
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Jan 27, 2022
2 parents 674c290 + 6e9cbab commit d41950d
Show file tree
Hide file tree
Showing 2 changed files with 324 additions and 1 deletion.
55 changes: 54 additions & 1 deletion pkg/stub/handler.go
View file
Expand Up @@ -339,6 +339,38 @@ func (h *Handler) updateCfgArch(cfg *v1.Config) *v1.Config {
return cfg
}

func redHatRegistriesFound(allowedRegistries map[string]bool) bool {
// Empty Sample Registry will be allowed as long as allowed registries contanis:
// - registry.redhat.io
// - registry.access.redhat.com
// - quay.io
return allowedRegistries["registry.redhat.io"] &&
allowedRegistries["registry.access.redhat.io"] &&
allowedRegistries["quay.io"]
}

func redHatRegistriesDomainFound(allowedDomains map[string]bool) bool {
// Empty Sample Registry will be allowed as long as allowed domains contanis:
// - registry.redhat.io
// - registry.access.redhat.com
// - quay.io
// or a domain combination that covers above registries
return (allowedDomains["registry.redhat.io"] &&
allowedDomains["registry.access.redhat.io"] &&
allowedDomains["quay.io"]) ||
(allowedDomains["redhat.io"] &&
allowedDomains["quay.io"]) ||
(allowedDomains["registry.redhat.io"] &&
allowedDomains["access.redhat.io"] &&
allowedDomains["quay.io"])

}

var getImageConfig = func(h *Handler) (*configv1.Image, error) {
// Extracting call to ConfigV1Client as a functor to simplify unit testing
return h.configclient.Images().Get(context.TODO(), "cluster", metav1.GetOptions{})
}

func (h *Handler) imageConfigBlocksImageStreamCreation(name string) bool {
//TODO openshift/client-go/config/clientset/versioned/fake and ConfigV1Interface has compile issues with
// respect to ConfigV1Client (missing method implementations), so for now we cannot use it in our
Expand All @@ -350,7 +382,8 @@ func (h *Handler) imageConfigBlocksImageStreamCreation(name string) bool {
err := wait.PollImmediate(5*time.Second, 20*time.Second, func() (done bool, err error) {
// if the image config allowed registry or blocked registry list will prevent the creation of imagestreams,
// we consider this inaccessible
imgCfg, err = h.configclient.Images().Get(context.TODO(), "cluster", metav1.GetOptions{})
//imgCfg, err = h.configclient.Images().Get(context.TODO(), "cluster", metav1.GetOptions{})
imgCfg, err = getImageConfig(h)
if err != nil {
logrus.Printf("unable to retrieve image configuration as part of testing %s connectivity: %s", name, err.Error())
return false, nil
Expand All @@ -364,14 +397,24 @@ func (h *Handler) imageConfigBlocksImageStreamCreation(name string) bool {
ok := false
// check allowed domain list
if len(imgCfg.Spec.AllowedRegistriesForImport) > 0 {
var visitedDomains = make(map[string]bool)

for _, rl := range imgCfg.Spec.AllowedRegistriesForImport {
visitedDomains[rl.DomainName] = true
logrus.Printf("considering allowed registries domain %s for %s", rl.DomainName, name)
if strings.HasSuffix(name, rl.DomainName) {
logrus.Printf("the allowed registries domain %s allows imagestream creation access for search param %s", rl.DomainName, name)
ok = true
break
}
}

// Checking if Sample Registry is empty (default scenario)
if len(name) == 0 && redHatRegistriesDomainFound(visitedDomains) {
logrus.Printf("imagestream creation will be allowed when sample registry config is empty")
ok = true
}

if !ok {
logrus.Printf("no allowed registries items will permit the use of %s", name)
return true
Expand All @@ -381,14 +424,24 @@ func (h *Handler) imageConfigBlocksImageStreamCreation(name string) bool {
ok = false
// check allowed specific registry list
if len(imgCfg.Spec.RegistrySources.AllowedRegistries) > 0 {
var visitedRegistries = make(map[string]bool)

for _, r := range imgCfg.Spec.RegistrySources.AllowedRegistries {
visitedRegistries[strings.TrimSpace(r)] = true
logrus.Printf("considering allowed registry %s for %s", r, name)
if strings.TrimSpace(r) == strings.TrimSpace(name) {
logrus.Printf("the allowed registry %s allows imagestream creation for search param %s", r, name)
ok = true
break
}
}

// Checking if Sample Registry is empty (default scenario)
if len(name) == 0 && redHatRegistriesFound(visitedRegistries) {
logrus.Printf("imagestream creation will be allowed when sample registry config is empty")
ok = true
}

if !ok {
logrus.Printf("no allowed registries items will permit the use of %s", name)
return true
Expand Down
270 changes: 270 additions & 0 deletions pkg/stub/handler_test.go
View file
Expand Up @@ -21,6 +21,7 @@ import (
operatorsv1api "github.com/openshift/api/operator/v1"
v1 "github.com/openshift/api/samples/v1"
templatev1 "github.com/openshift/api/template/v1"
configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
imagev1client "github.com/openshift/client-go/image/clientset/versioned/typed/image/v1"

operator "github.com/openshift/cluster-samples-operator/pkg/operatorstatus"
Expand Down Expand Up @@ -277,6 +278,275 @@ func TestSkipped(t *testing.T) {
}
}

type BlockTestScenario struct {
Name string
RegistryName string
ImageConfig configv1.Image
ExpectedResult bool
}

func buildBlockTestScenarios() []BlockTestScenario {
testSecenarios := []BlockTestScenario{
{
Name: "Test AllowRegistriesForImport whitelisted",
RegistryName: "registry.redhat.io",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
AllowedRegistriesForImport: []configv1.RegistryLocation{
{
DomainName: "redhat.io",
},
},
},
},
ExpectedResult: false,
},
{
Name: "Test AllowRegistriesForImport whitelisted but empty registry name (1)",
RegistryName: "",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
AllowedRegistriesForImport: []configv1.RegistryLocation{
{
DomainName: "redhat.io",
},
{
DomainName: "quay.io",
},
},
},
},
ExpectedResult: false,
},
{
Name: "Test AllowRegistriesForImport whitelisted but empty registry name (2)",
RegistryName: "",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
AllowedRegistriesForImport: []configv1.RegistryLocation{
{
DomainName: "registry.redhat.io",
},
{
DomainName: "access.redhat.io",
},
{
DomainName: "quay.io",
},
},
},
},
ExpectedResult: false,
},
{
Name: "Test AllowRegistriesForImport whitelisted but empty registry name (2)",
RegistryName: "",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
AllowedRegistriesForImport: []configv1.RegistryLocation{
{
DomainName: "registry.redhat.io",
},
{
DomainName: "registry.access.redhat.io",
},
{
DomainName: "quay.io",
},
},
},
},
ExpectedResult: false,
},
{
Name: "Test AllowRegistriesForImport not whitelisted but empty registry name (1)",
RegistryName: "",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
AllowedRegistriesForImport: []configv1.RegistryLocation{
{
DomainName: "quay.io",
},
},
},
},
ExpectedResult: true,
},
{
Name: "Test AllowRegistriesForImport not whitelisted but empty registry name (2)",
RegistryName: "",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
AllowedRegistriesForImport: []configv1.RegistryLocation{
{
DomainName: "quay.io",
},
{
DomainName: "access.redhat.io",
},
},
},
},
ExpectedResult: true,
},
{
Name: "Test AllowRegistriesForImport not whitelisted but empty registry name (3)",
RegistryName: "",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
AllowedRegistriesForImport: []configv1.RegistryLocation{
{
DomainName: "quay.io",
},
{
DomainName: "registry.redhat.io",
},
},
},
},
ExpectedResult: true,
},
{
Name: "Test AllowRegistriesForImport not whitelisted",
RegistryName: "registry.redhat.io",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
AllowedRegistriesForImport: []configv1.RegistryLocation{
{
DomainName: "quay.io",
},
},
},
},
ExpectedResult: true,
},
{
Name: "Test AllowRegistries whitelisted",
RegistryName: "registry.redhat.io",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
RegistrySources: configv1.RegistrySources{
AllowedRegistries: []string{"registry.redhat.io"},
},
},
},
ExpectedResult: false,
},
{
Name: "Test AllowRegistries whitelisted but empty registry name",
RegistryName: "",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
RegistrySources: configv1.RegistrySources{
AllowedRegistries: []string{
"registry.redhat.io",
"registry.access.redhat.io",
"quay.io",
},
},
},
},
ExpectedResult: false,
},
{
Name: "Test AllowRegistries not whitelisted but empty registry name",
RegistryName: "",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
RegistrySources: configv1.RegistrySources{
AllowedRegistries: []string{
"registry.redhat.io",
"registry.access.redhat.io",
},
},
},
},
ExpectedResult: true,
},
{
Name: "Test AllowRegistries not whitelisted",
RegistryName: "registry.redhat.io",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
RegistrySources: configv1.RegistrySources{
AllowedRegistries: []string{"quay.io"},
},
},
},
ExpectedResult: true,
},
{
Name: "Test BlockedRegistries whitelisted",
RegistryName: "registry.redhat.io",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
RegistrySources: configv1.RegistrySources{
BlockedRegistries: []string{"registry.redhat.io"},
},
},
},
ExpectedResult: true,
},
{
Name: "Test BlockedRegistries not whitelisted but emtpy registry name ",
RegistryName: "",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
RegistrySources: configv1.RegistrySources{
BlockedRegistries: []string{"registry.redhat.io"},
},
},
},
ExpectedResult: false,
},
{
Name: "Test BlockedRegistries not whitelisted",
RegistryName: "registry.redhat.io",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{
RegistrySources: configv1.RegistrySources{
BlockedRegistries: []string{"quay.io"},
},
},
},
ExpectedResult: false,
},
{
Name: "Test None whitelisted",
RegistryName: "registry.redhat.io",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{},
},
ExpectedResult: false,
},
{
Name: "Test None whitelisted and empty registry name",
RegistryName: "",
ImageConfig: configv1.Image{
Spec: configv1.ImageSpec{},
},
ExpectedResult: false,
},
}

return testSecenarios
}

func TestImageConfigBlocksImageStreamCreation(t *testing.T) {
h, _, _ := setup()
h.configclient = new(configv1client.ConfigV1Client)
testSecenarios := buildBlockTestScenarios()
for _, scenario := range testSecenarios {
getImageConfig = func(h *Handler) (*configv1.Image, error) {
return &scenario.ImageConfig, nil
}
blocked := h.imageConfigBlocksImageStreamCreation(scenario.RegistryName)
if blocked != scenario.ExpectedResult {
t.Errorf("Scenario failed: %s", scenario.Name)
}
}
}

func TestTBRInaccessibleBit(t *testing.T) {
h, cfg, event := setup()
event.Object = cfg
Expand Down

0 comments on commit d41950d

Please sign in to comment.