WIP

pulp_ansible/app/migrations/0057_collectionversion_sha256_migrate.py

@@ -30,15 +30,14 @@ def find_and_update_sha256():
                 collections_on_demand.append(found_collection)
         collection_bulk.clear()
 
-    for collection_version in CollectionVersion.objects.only("pk", "sha256").iterator():
-        if not collection_version.sha256:
-            collection_bulk[collection_version.pk] = collection_version
-            if len(collection_bulk) >= 1024:
-                find_and_update_sha256()
-            if len(collections_to_update) >= 1024:
-                with transaction.atomic():
-                    CollectionVersion.objects.bulk_update(collections_to_update, ["sha256",])
-                collections_to_update.clear()
+    for collection_version in CollectionVersion.objects.filter(sha256__isnull=True).only("pk", "sha256").iterator():
+        collection_bulk[collection_version.pk] = collection_version
+        if len(collection_bulk) >= 1024:
+            find_and_update_sha256()
+        if len(collections_to_update) >= 1024:
+            with transaction.atomic():
+                CollectionVersion.objects.bulk_update(collections_to_update, ["sha256",])
+            collections_to_update.clear()
     # Update remaining collections
     if len(collection_bulk) > 0:
         find_and_update_sha256()

pulp_ansible/app/models.py

@@ -179,7 +179,7 @@ class CollectionVersion(Content):
     namespace = models.CharField(max_length=64, editable=False)
     repository = models.CharField(default="", blank=True, max_length=2000, editable=False)
     requires_ansible = models.CharField(null=True, max_length=255)
-    sha256 = models.CharField(max_length=64, db_index=True, null=False)
+    sha256 = models.CharField(max_length=64, db_index=True, null=False, blank=False)
 
     version = models.CharField(max_length=128, db_collation="pulp_ansible_semver")
     version_major = models.IntegerField()

pulp_ansible/app/serializers.py

@@ -490,8 +490,10 @@ def deferred_validate(self, data):
         # Call super to ensure that data contains artifact
         data = super().deferred_validate(data)
         artifact = data.get("artifact")
-        if (sha256 := data.pop("sha256", None)) and sha256 != artifact.sha256:
+        if (sha256 := data.get("sha256")) and sha256 != artifact.sha256:
             raise ValidationError(_("Expected sha256 did not match uploaded artifact's sha256"))
+        else:
+            data["sha256"] = artifact.sha256
 
         collection_info = process_collection_artifact(
             artifact=artifact,
@@ -509,23 +511,7 @@ def deferred_validate(self, data):
 
     def retrieve(self, validated_data):
         """Reuse existing CollectionVersion if provided artifact matches."""
-        namespace = validated_data["namespace"]
-        name = validated_data["name"]
-        version = validated_data["version"]
-        artifact = self.context["artifact"]
-        # TODO switch this check to use digest when ColVersion uniqueness constraint is changed
-        col = CollectionVersion.objects.filter(
-            namespace=namespace, name=name, version=version
-        ).first()
-        if col:
-            if col._artifacts.get() != artifact:
-                raise ValidationError(
-                    _("Collection {}.{}-{} already exists with a different artifact").format(
-                        namespace, name, version
-                    )
-                )
-
-        return col
+        return CollectionVersion.objects.filter(sha256=validated_data["sha256"]).first()
 
     def create(self, validated_data):
         """Final step in creating the CollectionVersion."""
@@ -549,6 +535,8 @@ class Meta:
             "expected_version",
         )
         model = CollectionVersion
+        # There was an autogenerated validator rendering sha256 required.
+        validators = []
 
 
 class CollectionVersionSerializer(ContentChecksumSerializer, CollectionVersionUploadSerializer):

pulp_ansible/app/tasks/collections.py

@@ -100,27 +100,30 @@ async def declarative_content_from_git_repo(remote, url, git_ref=None, metadata_
         gitrepo = Repo.clone_from(url, uuid4(), depth=1, multi_options=["--recurse-submodules"])
     commit_sha = gitrepo.head.commit.hexsha
     metadata, artifact_path = sync_collection(gitrepo.working_dir, ".")
+    artifact = Artifact.init_and_validate(artifact_path)
     if metadata_only:
         metadata["artifact"] = None
         metadata["artifact_url"] = None
     else:
-        artifact = Artifact.init_and_validate(artifact_path)
-        try:
-            await sync_to_async(artifact.save)()
-        except IntegrityError:
-            artifact = Artifact.objects.get(sha256=artifact.sha256)
+        if existing_artifact := Artifact.objects.filter(sha256=artifact.sha256).first():
+            existing_artifact.touch()
+            artifact = existing_artifact
+        else:
+            try:
+                await sync_to_async(artifact.save)()
+            except IntegrityError:
+                artifact = Artifact.objects.get(sha256=artifact.sha256)
         metadata["artifact_url"] = reverse("artifacts-detail", args=[artifact.pk])
         metadata["artifact"] = artifact
     metadata["remote_artifact_url"] = "{}/commit/{}".format(url.rstrip("/"), commit_sha)
-    # Where do we put this?
-    # metadata["sha256"] = artifact.sha256
+    metadata["sha256"] = artifact.sha256
 
     collection_version = await sync_to_async(create_collection_from_importer)(
         metadata, metadata_only=True
     )
     d_artifact = DeclarativeArtifact(
         artifact=artifact,
-        url=remote_artifact_url,
+        url=metadata["remote_artifact_url"],
         relative_path=collection_version.relative_path,
         remote=remote,
         deferred_download=metadata_only,
@@ -307,8 +310,12 @@ def create_collection_from_importer(importer_result, metadata_only=False):
         if collection_info[key] is None:
             collection_info.pop(key)
 
+    collection, created = Collection.objects.get_or_create(
+        namespace=collection_info["namespace"], name=collection_info["name"]
+    )
     collection_version = CollectionVersion(
         **collection_info,
+        collection=collection,
         requires_ansible=importer_result.get("requires_ansible"),
         contents=importer_result["contents"],
         docs_blob=importer_result["docs_blob"],
@@ -319,18 +326,16 @@ def create_collection_from_importer(importer_result, metadata_only=False):
     data = {k: v for k, v in collection_version.__dict__.items() if k in serializer_fields}
     data["id"] = collection_version.pulp_id
 
+    # I don't even know why I am doing this.
+    data.pop("sha256", None)
+    data.pop("pulp_last_updated", None)
+    # ----------------
     serializer = CollectionVersionSerializer(data=data)
 
     serializer.is_valid(raise_exception=True)
 
     if not metadata_only:
         with transaction.atomic():
-            collection, created = Collection.objects.get_or_create(
-                namespace=collection_info["namespace"], name=collection_info["name"]
-            )
-            collection_version.collection = collection
-            collection_version.save()
-
             for name in tags:
                 tag, created = Tag.objects.get_or_create(name=name)
                 collection_version.tags.add(tag)
@@ -1134,7 +1139,6 @@ def _pre_save(self, batch):
             if d_content is None:
                 continue
             if isinstance(d_content.content, CollectionVersion):
-                info = d_content.content.natural_key_dict()
                 collection, created = Collection.objects.get_or_create(
                     namespace=d_content.content.namespace, name=d_content.content.name
                 )

pulp_ansible/app/tasks/upload.py

@@ -46,6 +46,7 @@ def process_collection_artifact(artifact, namespace, name, version):
 
     # Set CollectionVersion metadata
     collection_info = importer_result["metadata"]
+    collection_info["sha256"] = artifact.sha256
 
     with transaction.atomic():
         collection, created = Collection.objects.get_or_create(

pulp_ansible/tests/functional/api/collection/v3/test_collection.py

@@ -333,7 +333,6 @@ def test_collection_version(collection_artifact, pulp_client, collection_detail)
     #     #             'tags': ['collectiontest']},
 
 
-@pytest.mark.skip("Blocked by open ticket: https://github.com/pulp/pulp_ansible/issues/698")
 def test_collection_download(collection_artifact, pulp_client, collection_detail):
     """Test collection download URL.
 

pulp_ansible/tests/functional/api/role/test_crud_distribution.py

@@ -76,4 +76,7 @@ def test_crud_distribution(
     monitor_task(ansible_distro_api_client.delete(distribution.pulp_href).task)
     with pytest.raises(ApiException) as exc_info:
         ansible_distro_api_client.delete(distribution.pulp_href)
-    assert "Not found." in exc_info.value.body
+    assert (
+        "Not found." in exc_info.value.body
+        or "No AnsibleDistribution matches the given query." in exc_info.value.body
+    )

pulp_ansible/tests/functional/cli/test_collection_upload.py

@@ -36,7 +36,7 @@ def test_upload_collection(
     )
 
     collection_meta = ansible_dir / "pulp" / collection_name / "meta"
-    collection_meta.mkdir()
+    collection_meta.mkdir(exist_ok=True)
     runtime_yml = collection_meta / "runtime.yml"
     runtime_yml.write_text('requires_ansible: ">=2.9"\n')
 

pulp_ansible/tests/unit/test_search.py

@@ -43,8 +43,9 @@ def setUp(self):
             self.collections[spec]["col"] = col
 
             # make the collection version
-            cv = CollectionVersion(collection=col, namespace=spec[0], name=spec[1], version=spec[2])
-            cv.save()
+            cv = CollectionVersion.objects.create(
+                collection=col, sha256=str(ids), namespace=spec[0], name=spec[1], version=spec[2]
+            )
             self.collections[spec]["cv"] = cv
 
             # add tags ...

pulp_ansible/tests/unit/test_serializers.py

@@ -1,3 +1,4 @@
+import hashlib
 from unittest import mock
 from django.conf import settings
 from django.core.files.uploadedfile import SimpleUploadedFile
@@ -13,13 +14,11 @@ class TestRoleSerializer(TestCase):
 
     def setUp(self):
         """Set up the RoleSerializer tests."""
+        rawbin = b"test content"
         self.artifact = Artifact.objects.create(
-            sha224="9a6297eb28d91fad5277c0833856031d0e940432ad807658bd2b60f4",
-            sha256="c8ddb3dcf8da48278d57b0b94486832c66a8835316ccf7ca39e143cbfeb9184f",
-            sha384="53a8a0cebcb7780ed7624790c9d9a4d09ba74b47270d397f5ed7bc1c46777a0fbe362aaf2bbe7f0966a350a12d76e28d",  # noqa
-            sha512="a94a65f19b864d184a2a5e07fa29766f08c6d49b6f624b3dd3a36a98267b9137d9c35040b3e105448a869c23c2aec04c9e064e3555295c1b8de6515eed4da27d",  # noqa
             size=1024,
-            file=SimpleUploadedFile("test_filename", b"test content"),
+            file=SimpleUploadedFile("test_filename", rawbin),
+            **{k: getattr(hashlib, k)(rawbin).hexdigest() for k in Artifact.DIGEST_FIELDS},
         )
         self.data = {
             "artifact": "{}artifacts/{}/".format(settings.V3_API_ROOT, self.artifact.pk),

pulp_ansible/tests/unit/utils.py

@@ -19,7 +19,7 @@ def make_cv_tarball(namespace, name, version):
     """Create a collection version from scratch."""
     tdir = tempfile.mkdtemp()
     subprocess.run(f"ansible-galaxy collection init {namespace}.{name}", shell=True, cwd=tdir)
-    os.makedirs(os.path.join(tdir, namespace, name, "meta"))
+    os.makedirs(os.path.join(tdir, namespace, name, "meta"), exist_ok=True)
     with open(os.path.join(tdir, namespace, name, "meta", "runtime.yml"), "w") as f:
         f.write('requires_ansible: ">=2.13"\n')
     with open(os.path.join(tdir, namespace, name, "README.md"), "w") as f:
@@ -45,28 +45,24 @@ def build_cvs_from_specs(specs, build_artifacts=True):
     """Make CVs from a list of [namespace, name, version] specs."""
     collection_versions = []
     for spec in specs:
+        with make_cv_tarball(spec[0], spec[1], spec[2]) as tarfn:
+            with open(tarfn, "rb") as fp:
+                rawbin = fp.read()
+            artifact = Artifact.objects.create(
+                size=os.path.getsize(tarfn),
+                file=SimpleUploadedFile(tarfn, rawbin),
+                **{k: getattr(hashlib, k)(rawbin).hexdigest() for k in Artifact.DIGEST_FIELDS},
+            )
+
         col, _ = Collection.objects.get_or_create(name=spec[0])
         col.save()
-        cv = CollectionVersion(collection=col, namespace=spec[0], name=spec[1], version=spec[2])
-        cv.save()
+        cv = CollectionVersion.objects.create(
+            collection=col, sha256=artifact.sha256, namespace=spec[0], name=spec[1], version=spec[2]
+        )
         collection_versions.append(cv)
 
-        if build_artifacts:
-            with make_cv_tarball(spec[0], spec[1], spec[2]) as tarfn:
-                rawbin = open(tarfn, "rb").read()
-                artifact = Artifact.objects.create(
-                    sha224=hashlib.sha224(rawbin).hexdigest(),
-                    sha256=hashlib.sha256(rawbin).hexdigest(),
-                    sha384=hashlib.sha384(rawbin).hexdigest(),
-                    sha512=hashlib.sha512(rawbin).hexdigest(),
-                    size=os.path.getsize(tarfn),
-                    file=SimpleUploadedFile(tarfn, rawbin),
-                )
-                artifact.save()
-
-            ca = ContentArtifact.objects.create(
-                artifact=artifact, content=cv, relative_path=cv.relative_path
-            )
-            ca.save()
+        ContentArtifact.objects.create(
+            artifact=artifact, content=cv, relative_path=cv.relative_path
+        )
 
     return collection_versions