From 1ea0213d4ec338d1214f1b4049a132dfc0213f69 Mon Sep 17 00:00:00 2001 From: Qi Wang Date: Sun, 3 Sep 2023 22:01:35 -0400 Subject: [PATCH] [release-4.14] Allow ICSP IDMS coexisting Backport: https://github.com/openshift/image-registry/pull/377 Allow ICSP IDMS coexisting Signed-off-by: Qi Wang --- pkg/dockerregistry/server/simplelookupicsp.go | 15 +-- .../server/simplelookupicsp_test.go | 119 ++++++++++++++++++ .../pullthrough/pullthrough_test.go | 27 ---- 3 files changed, 121 insertions(+), 40 deletions(-) diff --git a/pkg/dockerregistry/server/simplelookupicsp.go b/pkg/dockerregistry/server/simplelookupicsp.go index 11d1e68345..9e21d9d250 100644 --- a/pkg/dockerregistry/server/simplelookupicsp.go +++ b/pkg/dockerregistry/server/simplelookupicsp.go @@ -2,7 +2,6 @@ package server import ( "context" - "fmt" "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -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, @@ -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) @@ -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, diff --git a/pkg/dockerregistry/server/simplelookupicsp_test.go b/pkg/dockerregistry/server/simplelookupicsp_test.go index 5f29f70c17..5763c7f3de 100644 --- a/pkg/dockerregistry/server/simplelookupicsp_test.go +++ b/pkg/dockerregistry/server/simplelookupicsp_test.go @@ -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) + } + }) + + } +} diff --git a/test/integration/pullthrough/pullthrough_test.go b/test/integration/pullthrough/pullthrough_test.go index 55b17fea47..ce5283e827 100644 --- a/test/integration/pullthrough/pullthrough_test.go +++ b/test/integration/pullthrough/pullthrough_test.go @@ -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 { @@ -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) @@ -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)