Skip to content

Commit

Permalink
Merge pull request #385 from QiWang19/release-4.14
Browse files Browse the repository at this point in the history
[release-4.14] OCPBUGS-22826: Allow ICSP IDMS coexisting
  • Loading branch information
openshift-merge-bot[bot] committed Nov 8, 2023
2 parents 5e7788a + 1ea0213 commit 690b5a2
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 40 deletions.
15 changes: 2 additions & 13 deletions pkg/dockerregistry/server/simplelookupicsp.go
Expand Up @@ -2,7 +2,6 @@ package server

import (
"context"
"fmt"
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -26,7 +25,7 @@ type simpleLookupImageMirrorSets struct {
}

// NewSimpleLookupImageMirrorSetsStrategy returns a new entity of simpleLookupImageMirrorSets using provided client
// to obtain cluster wide ICSP or IDMS and ITMS configuration.
// to obtain cluster wide ICSP, IDMS and ITMS configuration.
func NewSimpleLookupImageMirrorSetsStrategy(
icspcli operatorv1alpha1client.ImageContentSourcePolicyInterface,
idmscli cfgv1client.ImageDigestMirrorSetInterface,
Expand Down Expand Up @@ -63,16 +62,6 @@ func (s *simpleLookupImageMirrorSets) FirstRequest(
return []reference.DockerImageReference{ref.AsRepository()}, nil
}

if len(icspList.Items) > 0 && len(idmsList.Items) > 0 {
err := fmt.Errorf("found both ICSP and IDMS resources, but only one or the other is supported")
return []reference.DockerImageReference{ref.AsRepository()}, err
}

if len(icspList.Items) > 0 && len(itmsList.Items) > 0 {
err := fmt.Errorf("found both ICSP and ITMS resources, but only one or the other is supported")
return []reference.DockerImageReference{ref.AsRepository()}, err
}

imageRefList, err := s.alternativeImageSources(ref, icspList.Items, idmsList.Items, itmsList.Items)
if err != nil {
klog.Errorf("error looking for alternate repositories: %s", err)
Expand Down Expand Up @@ -105,7 +94,7 @@ type mirrorSource struct {
}

// alternativeImageSources returns unique list of DockerImageReference objects from list of
// ImageContentSourcePolicy or ImageDigestMirrorSet, ImageTagMirrorSet objects
// ImageContentSourcePolicy, ImageDigestMirrorSet, and ImageTagMirrorSet objects
func (s *simpleLookupImageMirrorSets) alternativeImageSources(
ref reference.DockerImageReference, icspList []operatorv1alpha1.ImageContentSourcePolicy,
idmsList []cfgv1.ImageDigestMirrorSet, itmsList []cfgv1.ImageTagMirrorSet,
Expand Down
119 changes: 119 additions & 0 deletions pkg/dockerregistry/server/simplelookupicsp_test.go
Expand Up @@ -442,3 +442,122 @@ func newITMSRule(r rule) runtime.Object {
},
}
}

func TestFirstRequestICSPandIDMS(t *testing.T) {
for _, tt := range []struct {
name string
icsprules []rule
idmsrules []rule
itmsrules []rule
ref string
res []reference.DockerImageReference
}{
{
name: "multiple mirrors",
ref: "i.do.not.exist/repo/image:latest",
res: []reference.DockerImageReference{
{
Registry: "i.exist",
Namespace: "ns0",
Name: "img0",
},
{
Registry: "i.also.exist",
Namespace: "ns1",
Name: "img1",
},
{
Registry: "i.also.exist",
Namespace: "ns2",
Name: "img2",
},
{
Registry: "me.too",
Namespace: "ns2",
Name: "img2",
},
{
Registry: "i.do.not.exist",
Namespace: "repo",
Name: "image",
},
},
idmsrules: []rule{
{
name: "rule",
ruleElement: []element{
{source: "i.do.not.exist/repo/image",
mirrors: []string{
"i.also.exist/ns1/img1",
}},
},
},
},
icsprules: []rule{
{
name: "rule",
ruleElement: []element{
{source: "i.do.not.exist/repo/image",
mirrors: []string{
"i.exist/ns0/img0",
}},
},
},
},
itmsrules: []rule{
{
name: "rule",
ruleElement: []element{
{source: "i.do.not.exist/repo/image",
mirrors: []string{
"i.also.exist/ns2/img2",
"me.too/ns2/img2",
}},
},
},
},
},
} {
t.Run(tt.name, func(t *testing.T) {
icspRules := []runtime.Object{}
for _, rule := range tt.icsprules {
icspRules = append(icspRules, newICSPRule(rule))
}

cli := fake.NewSimpleClientset(icspRules...)

idmsRules := []runtime.Object{}
for _, rule := range tt.idmsrules {
idmsRules = append(idmsRules, newIDMSRule(rule))
}

itmsRules := []runtime.Object{}
for _, rule := range tt.itmsrules {
itmsRules = append(itmsRules, newITMSRule(rule))
}

cfgcli := cfgfake.NewSimpleClientset(append(idmsRules, itmsRules...)...)

lookup := NewSimpleLookupImageMirrorSetsStrategy(
cli.OperatorV1alpha1().ImageContentSourcePolicies(),
cfgcli.ConfigV1().ImageDigestMirrorSets(),
cfgcli.ConfigV1().ImageTagMirrorSets(),
)

ref, err := reference.Parse(tt.ref)
if err != nil {
t.Fatalf("unexpected error parsing reference: %s", err)
}

alternates, err := lookup.FirstRequest(context.Background(), ref)
if err != nil {
t.Fatalf("FirstRequest does not return error, received: %s", err)
}

if !reflect.DeepEqual(alternates, tt.res) {
t.Errorf("expected %+v, received %+v", tt.res, alternates)
}
})

}
}
27 changes: 0 additions & 27 deletions test/integration/pullthrough/pullthrough_test.go
Expand Up @@ -298,15 +298,6 @@ func TestPullThroughICSP(t *testing.T) {

opclient := operatorclientv1alpha1.NewForConfigOrDie(master.AdminKubeConfig())
imgclient := imageclientv1.NewForConfigOrDie(testuser.KubeConfig())
cfgclient := cfgclientv1.NewForConfigOrDie(master.AdminKubeConfig())

idmsList, err := cfgclient.ImageDigestMirrorSets().List(ctx, metav1.ListOptions{})
if err != nil {
t.Fatal(err)
}
if len(idmsList.Items) > 0 {
t.Skip("default IDMS mirror configuration exists, can't create ImageContentSourcePolicy when ImageDigestMirrorSet resources exist. Consider update the test")
}

imageData, err := testframework.NewSchema2ImageData()
if err != nil {
Expand Down Expand Up @@ -480,18 +471,9 @@ func TestPullThroughIDMS(t *testing.T) {
testuser := master.CreateUser("testuser", "testp@ssw0rd")
master.CreateProject(namespace, testuser.Name)

opclient := operatorclientv1alpha1.NewForConfigOrDie(master.AdminKubeConfig())
cfgclient := cfgclientv1.NewForConfigOrDie(master.AdminKubeConfig())
imgclient := imageclientv1.NewForConfigOrDie(testuser.KubeConfig())

icspList, err := opclient.ImageContentSourcePolicies().List(ctx, metav1.ListOptions{})
if err != nil {
t.Fatal(err)
}
if len(icspList.Items) > 0 {
t.Skip("default ICSP mirror configuration exists, can't create ImageDigestMirrorSet when ImageContentSourcePolicy resources exist. Consider update the test")
}

imageData, err := testframework.NewSchema2ImageData()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -664,18 +646,9 @@ func TestPullThroughITMS(t *testing.T) {
testuser := master.CreateUser("testuser", "testp@ssw0rd")
master.CreateProject(namespace, testuser.Name)

opclient := operatorclientv1alpha1.NewForConfigOrDie(master.AdminKubeConfig())
cfgclient := cfgclientv1.NewForConfigOrDie(master.AdminKubeConfig())
imgclient := imageclientv1.NewForConfigOrDie(testuser.KubeConfig())

icspList, err := opclient.ImageContentSourcePolicies().List(ctx, metav1.ListOptions{})
if err != nil {
t.Fatal(err)
}
if len(icspList.Items) > 0 {
t.Skip("default ICSP mirror configuration exists, can't create ImageTagMirrorSet when ImageContentSourcePolicy resources exist. Consider update the test")
}

imageData, err := testframework.NewSchema2ImageData()
if err != nil {
t.Fatal(err)
Expand Down

0 comments on commit 690b5a2

Please sign in to comment.