Add validation for uploaded and synced manifests #866

lubosmj · 2022-06-23T07:43:47Z

In this commit, a couple of validation schemas were
introduced to the sync and push workflows. Newly added
manifests or manifest lists are now being validated by
the JSON validator.

closes #854
closes #853
closes #672

lubosmj · 2022-06-24T11:48:28Z

The reported problems with invalid manifest types are now gone (#854, #853):

(pulp) [vagrant@pulp3-source-fedora35 ~]$ podman login localhost:24817 --tls-verify=false
Username: admin
Password: 
Login Succeeded!
(pulp) [vagrant@pulp3-source-fedora35 ~]$ podman manifest push --all   report-bug localhost:24817/foo:report-bug --format oci --tls-verify=false
Getting image list signatures
Copying 0 of 0 images in list
Writing manifest list to image destination
Error: Uploading manifest list failed, attempted the following formats: application/vnd.oci.image.index.v1+json(uploading manifest report-bug to localhost:24817/foo: manifest invalid: manifest invalid)
(pulp) [vagrant@pulp3-source-fedora35 ~]$ podman -v
podman version 3.4.7
(pulp) [vagrant@pulp3-source-fedora35 ~]$ podman manifest push --all   report-bug localhost:24817/foo:report-bug --format v2s2 --tls-verify=false
Getting image list signatures
Copying 0 of 0 images in list
Writing manifest list to image destination
Storing list signatures
(pulp) [vagrant@pulp3-source-fedora35 ~]$ podman manifest push --all   report-bug localhost:24817/foo:report-bug --format oci --tls-verify=false
Getting image list signatures
Copying 0 of 0 images in list
Writing manifest list to image destination
Error: Uploading manifest list failed, attempted the following formats: application/vnd.oci.image.index.v1+json(uploading manifest report-bug to localhost:24817/foo: manifest invalid: manifest invalid)

pulp_container/app/json_schemas.py

ipanova

This is looking great overall, thank you for adding the validation!
I don't see validation for schema1 will you add it too?

ipanova · 2022-07-01T22:10:52Z

It would be great to add this validation to the sync worklow too. Will you do it as part of this PR or in a separate one?

lubosmj · 2022-07-04T16:12:14Z

@ipanova, I have added a schema for v1 manifests (I think they share the schema with signed v1 manifests) and added the validation to the sync pipeline. How do you like it now? Do I need to attach a fourth issue for adding the validation during the sync procedure?

Unrelated:
I also wonder what are we going to do with #648. Because it will not be easily doable anymore after adding the validation (e.g., initiating a skip task for a parent manifest list that was already parsed and validated). The question is: do we really need to allow users to perform "partial" sync?

CHANGES/882.bugfix

pulp_container/app/json_schemas.py

pulp_container/app/registry_api.py

pulp_container/app/tasks/sync_stages.py

pulp_container/app/registry_api.py

pulp_container/app/json_schemas.py

ipanova · 2022-07-19T15:56:50Z

your PR might fix this issue too #883

pulp_container/app/tasks/sync_stages.py

ipanova · 2022-08-01T10:04:44Z

@ipanova, I have added a schema for v1 manifests (I think they share the schema with signed v1 manifests) and added the validation to the sync pipeline. How do you like it now? Do I need to attach a fourth issue for adding the validation during the sync procedure?

Re-using this isseure should be fine to include validation for synced and uploaded content.

Unrelated: I also wonder what are we going to do with #648. Because it will not be easily doable anymore after adding the validation (e.g., initiating a skip task for a parent manifest list that was already parsed and validated). The question is: do we really need to allow users to perform "partial" sync?

That's a good question, we will need to evaluate the complexity of the #648 but from the user perspective, this would be a handy sync behavior.

pulp_container/app/json_schemas.py

pulp_container/app/utils.py

pulp_container/constants.py

pulp_container/app/tasks/sync_stages.py

pulp_container/app/registry_api.py

ipanova

Thank you! This work will greatly improve the quality of the sync and upload pipelines! 🎆

ipanova · 2022-08-01T14:31:11Z

pulp_container/app/registry_api.py

+        # when a user uploads a manifest list with zero listed manifests (no blobs were uploaded
+        # before) and the specified repository has not been created yet, create the repository
+        # without raising an error
+        create_new_repo = request.content_type in (


s/request.content_type/media_type

ipanova · 2022-08-01T14:31:43Z

pulp_container/app/registry_api.py

+            models.MEDIA_TYPE.INDEX_OCI,
+        )
+        _, repository = self.get_dr_push(request, path, create=create_new_repo)
+
        if request.content_type in (


s/request.content_type/media_type

https://github.com/pulp/pulp_container/pull/866/files#diff-1b944e15118f82233aa03a435e9e05c188ad0d371b71be43260506b4a5eeddffL914

https://github.com/pulp/pulp_container/pull/866/files#diff-1b944e15118f82233aa03a435e9e05c188ad0d371b71be43260506b4a5eeddffR982

ipanova · 2022-08-01T15:01:40Z

To make it a meaningful number of comments to 45 ;) ( only by @lubosmj request) I will leave here some more 🐒

ipanova · 2022-08-01T15:02:15Z

and the last one! 🦭

In this commit, a couple of validation schemas were introduced to the sync and push workflows. Newly added manifests or manifest lists are now being validated by the JSON validator. closes pulp#854 closes pulp#853 closes pulp#672

lubosmj force-pushed the add-validation-for-manifest-list-853 branch 8 times, most recently from 3fba8c0 to 43bbd44 Compare June 24, 2022 11:38

lubosmj marked this pull request as ready for review June 24, 2022 11:48

ipanova self-requested a review June 24, 2022 12:01

lubosmj commented Jun 24, 2022

View reviewed changes

pulp_container/app/json_schemas.py Outdated Show resolved Hide resolved

ipanova reviewed Jul 1, 2022

View reviewed changes

lubosmj marked this pull request as draft July 4, 2022 15:30

lubosmj force-pushed the add-validation-for-manifest-list-853 branch from 43bbd44 to d24891b Compare July 4, 2022 16:01

lubosmj marked this pull request as ready for review July 4, 2022 16:16

lubosmj marked this pull request as draft July 7, 2022 07:47

lubosmj force-pushed the add-validation-for-manifest-list-853 branch from d24891b to dbed3c2 Compare July 8, 2022 08:09

lubosmj marked this pull request as ready for review July 8, 2022 08:21

lubosmj changed the title ~~Add validation for uploaded manifests~~ Add validation for uploaded and synced manifests Jul 8, 2022

lubosmj force-pushed the add-validation-for-manifest-list-853 branch from dbed3c2 to 694c83d Compare July 8, 2022 09:43

ipanova self-requested a review July 11, 2022 09:43

lubosmj marked this pull request as draft July 13, 2022 09:00

lubosmj force-pushed the add-validation-for-manifest-list-853 branch from 694c83d to 16bd36f Compare July 13, 2022 09:42

lubosmj marked this pull request as ready for review July 13, 2022 09:45

ipanova reviewed Jul 19, 2022

View reviewed changes

CHANGES/882.bugfix Outdated Show resolved Hide resolved

ipanova requested changes Jul 19, 2022

View reviewed changes

lubosmj marked this pull request as draft July 19, 2022 15:51

ipanova reviewed Jul 19, 2022

View reviewed changes

pulp_container/app/tasks/sync_stages.py Outdated Show resolved Hide resolved

lubosmj force-pushed the add-validation-for-manifest-list-853 branch 6 times, most recently from f4773a4 to 020c434 Compare July 31, 2022 18:14

lubosmj marked this pull request as ready for review July 31, 2022 18:14

lubosmj requested a review from ipanova July 31, 2022 18:15

ipanova reviewed Aug 1, 2022

View reviewed changes

lubosmj force-pushed the add-validation-for-manifest-list-853 branch from 020c434 to b6e389d Compare August 1, 2022 13:27

ipanova approved these changes Aug 1, 2022

View reviewed changes

lubosmj force-pushed the add-validation-for-manifest-list-853 branch from b6e389d to 68ac260 Compare August 1, 2022 14:57

Add validation for uploaded and synced manifests

06538ff

In this commit, a couple of validation schemas were introduced to the sync and push workflows. Newly added manifests or manifest lists are now being validated by the JSON validator. closes pulp#854 closes pulp#853 closes pulp#672

lubosmj force-pushed the add-validation-for-manifest-list-853 branch from 68ac260 to 06538ff Compare August 1, 2022 15:43

lubosmj merged commit 343859e into pulp:main Aug 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add validation for uploaded and synced manifests #866

Add validation for uploaded and synced manifests #866

lubosmj commented Jun 23, 2022 •

edited

Loading

lubosmj commented Jun 24, 2022

ipanova left a comment

ipanova commented Jul 1, 2022

lubosmj commented Jul 4, 2022 •

edited

Loading

ipanova commented Jul 19, 2022

ipanova commented Aug 1, 2022 •

edited

Loading

ipanova left a comment

ipanova Aug 1, 2022

ipanova Aug 1, 2022

ipanova Aug 1, 2022

ipanova Aug 1, 2022

ipanova commented Aug 1, 2022

ipanova commented Aug 1, 2022

Add validation for uploaded and synced manifests #866

Add validation for uploaded and synced manifests #866

Conversation

lubosmj commented Jun 23, 2022 • edited Loading

lubosmj commented Jun 24, 2022

ipanova left a comment

Choose a reason for hiding this comment

ipanova commented Jul 1, 2022

lubosmj commented Jul 4, 2022 • edited Loading

ipanova commented Jul 19, 2022

ipanova commented Aug 1, 2022 • edited Loading

ipanova left a comment

Choose a reason for hiding this comment

ipanova Aug 1, 2022

Choose a reason for hiding this comment

ipanova Aug 1, 2022

Choose a reason for hiding this comment

ipanova Aug 1, 2022

Choose a reason for hiding this comment

ipanova Aug 1, 2022

Choose a reason for hiding this comment

ipanova commented Aug 1, 2022

ipanova commented Aug 1, 2022

lubosmj commented Jun 23, 2022 •

edited

Loading

lubosmj commented Jul 4, 2022 •

edited

Loading

ipanova commented Aug 1, 2022 •

edited

Loading