From 8a653a534741a5af3e967cf099db0dd39550d416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Min=C3=A1=C5=99?= Date: Tue, 20 Mar 2018 17:26:09 +0000 Subject: [PATCH 1/3] signature-controller: stop adding managed annotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michal Minář --- .../controller/signature/container_image_downloader.go | 3 --- .../controller/signature/signature_import_controller.go | 6 ------ 2 files changed, 9 deletions(-) diff --git a/pkg/image/controller/signature/container_image_downloader.go b/pkg/image/controller/signature/container_image_downloader.go index 1e561f260efe..cffb3913a5d1 100644 --- a/pkg/image/controller/signature/container_image_downloader.go +++ b/pkg/image/controller/signature/container_image_downloader.go @@ -62,9 +62,6 @@ func (s *containerImageSignatureDownloader) DownloadImageSignatures(image *image // signature. sig.Name = imageapi.JoinImageStreamImage(image.Name, fmt.Sprintf("%x", sha256.Sum256(blob))) sig.Content = blob - sig.Annotations = map[string]string{ - SignatureManagedAnnotation: "true", - } sig.CreationTimestamp = metav1.Now() ret = append(ret, sig) } diff --git a/pkg/image/controller/signature/signature_import_controller.go b/pkg/image/controller/signature/signature_import_controller.go index 52e01c4dd538..85da8b20e491 100644 --- a/pkg/image/controller/signature/signature_import_controller.go +++ b/pkg/image/controller/signature/signature_import_controller.go @@ -19,12 +19,6 @@ import ( imagelister "github.com/openshift/origin/pkg/image/generated/listers/image/internalversion" ) -const ( - // SignatureManagedAnnotation marks signatures that were imported by this - // controller. - SignatureManagedAnnotation = "image.openshift.io/managed-signature" -) - type SignatureDownloader interface { DownloadImageSignatures(*imageapi.Image) ([]imageapi.ImageSignature, error) } From 9742ee01ea215f47d6211bb4381c92207ce1d370 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Min=C3=A1=C5=99?= Date: Tue, 20 Mar 2018 17:32:40 +0000 Subject: [PATCH 2/3] image-strategy: unset image signature annotations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michal Minář --- pkg/image/registry/image/strategy.go | 16 +++++ pkg/image/registry/image/strategy_test.go | 77 +++++++++++++++++++++-- 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/pkg/image/registry/image/strategy.go b/pkg/image/registry/image/strategy.go index 86401e9ba221..420a2cc2678e 100644 --- a/pkg/image/registry/image/strategy.go +++ b/pkg/image/registry/image/strategy.go @@ -16,6 +16,9 @@ import ( "github.com/openshift/origin/pkg/image/util" ) +// managedSignatureAnnotation used to be set by image signature import controller as a signature annotation. +const managedSignatureAnnotation = "image.openshift.io/managed-signature" + // imageStrategy implements behavior for Images. type imageStrategy struct { runtime.ObjectTyper @@ -49,6 +52,8 @@ func (s imageStrategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Obje newImage.DockerImageManifest = "" newImage.DockerImageConfig = "" } + + removeManagedSignatureAnnotation(newImage) } // Validate validates a new image. @@ -127,9 +132,20 @@ func (s imageStrategy) PrepareForUpdate(ctx apirequest.Context, obj, old runtime newImage.DockerImageManifest = "" newImage.DockerImageConfig = "" } + + removeManagedSignatureAnnotation(newImage) } // ValidateUpdate is the default update validation for an end user. func (imageStrategy) ValidateUpdate(ctx apirequest.Context, obj, old runtime.Object) field.ErrorList { return validation.ValidateImageUpdate(old.(*imageapi.Image), obj.(*imageapi.Image)) } + +// removeManagedSignatureAnnotation removes deprecated annotation from image signatures. A bug in image update +// logic allowed to set arbitrary annotations that would otherwise be rejected by validation. +// Resolves rhbz#1557607 +func removeManagedSignatureAnnotation(img *imageapi.Image) { + for i := range img.Signatures { + delete(img.Signatures[i].Annotations, managedSignatureAnnotation) + } +} diff --git a/pkg/image/registry/image/strategy_test.go b/pkg/image/registry/image/strategy_test.go index 564f7b43a681..ef406e1749d0 100644 --- a/pkg/image/registry/image/strategy_test.go +++ b/pkg/image/registry/image/strategy_test.go @@ -76,13 +76,20 @@ func TestStrategyPrepareForCreate(t *testing.T) { Strategy.PrepareForCreate(ctx, image) - if len(image.Signatures) != len(fuzzed.Signatures) { - t.Errorf("unexpected number of signatures: %d != %d", len(image.Signatures), len(fuzzed.Signatures)) + testVerifySignatures(t, fuzzed, image) +} + +func testVerifySignatures(t *testing.T, orig, new *imageapi.Image) { + if len(new.Signatures) != len(orig.Signatures) { + t.Errorf("unexpected number of signatures: %d != %d", len(new.Signatures), len(orig.Signatures)) } - for i, sig := range image.Signatures { + for i, sig := range new.Signatures { + // expect annotations to be cleared + delete(orig.Signatures[i].Annotations, managedSignatureAnnotation) + vi := reflect.ValueOf(&sig).Elem() - vf := reflect.ValueOf(&fuzzed.Signatures[i]).Elem() + vf := reflect.ValueOf(&orig.Signatures[i]).Elem() typeOfT := vf.Type() for j := 0; j < vf.NumField(); j++ { @@ -98,3 +105,65 @@ func TestStrategyPrepareForCreate(t *testing.T) { } } } + +func TestStrategyPrepareForCreateSignature(t *testing.T) { + ctx := apirequest.NewDefaultContext() + + original := imageapi.Image{ + ObjectMeta: metav1.ObjectMeta{ + Name: "image", + }, + } + + seed := int64(2703387474910584091) //rand.Int63() + fuzzed := fuzzImage(t, &original, seed) + + if len(fuzzed.Signatures) == 0 { + t.Fatalf("fuzzifier failed to generate signatures") + } + + for _, tc := range []struct { + name string + annotations map[string]string + expected map[string]string + }{ + { + name: "unset annotations", + annotations: nil, + expected: nil, + }, + { + name: "empty annotations", + annotations: map[string]string{}, + expected: map[string]string{}, + }, + { + name: "managed annotation shall be removed", + annotations: map[string]string{managedSignatureAnnotation: "value"}, + expected: map[string]string{}, + }, + { + name: "other annotations shall stay", + annotations: map[string]string{"key": "value"}, + expected: map[string]string{"key": "value"}, + }, + { + name: "remove and keep", + annotations: map[string]string{"key": "value", managedSignatureAnnotation: "true"}, + expected: map[string]string{"key": "value"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + fuzzed.Signatures[0].Annotations = tc.annotations + image := fuzzed.DeepCopy() + + Strategy.PrepareForCreate(ctx, image) + + testVerifySignatures(t, fuzzed, image) + + if !reflect.DeepEqual(image.Signatures[0].Annotations, tc.expected) { + t.Errorf("unexpected signature annotations: %s", diff.ObjectGoPrintDiff(image.Annotations, tc.expected)) + } + }) + } +} From 6feafafbbfd36b642ab179bf02a4a780fea2eb57 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 20 Mar 2018 09:21:20 -0400 Subject: [PATCH 3/3] switch reversed old/new objects to validation --- pkg/image/registry/image/strategy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/image/registry/image/strategy.go b/pkg/image/registry/image/strategy.go index 420a2cc2678e..9f04aea4ecaf 100644 --- a/pkg/image/registry/image/strategy.go +++ b/pkg/image/registry/image/strategy.go @@ -138,7 +138,7 @@ func (s imageStrategy) PrepareForUpdate(ctx apirequest.Context, obj, old runtime // ValidateUpdate is the default update validation for an end user. func (imageStrategy) ValidateUpdate(ctx apirequest.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateImageUpdate(old.(*imageapi.Image), obj.(*imageapi.Image)) + return validation.ValidateImageUpdate(obj.(*imageapi.Image), old.(*imageapi.Image)) } // removeManagedSignatureAnnotation removes deprecated annotation from image signatures. A bug in image update