diff --git a/pulp_ansible/app/migrations/0057_collectionversion_sha256_migrate.py b/pulp_ansible/app/migrations/0057_collectionversion_sha256_migrate.py index 165235502..fea054dcc 100644 --- a/pulp_ansible/app/migrations/0057_collectionversion_sha256_migrate.py +++ b/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() diff --git a/pulp_ansible/app/models.py b/pulp_ansible/app/models.py index b40e0dd00..3d2813882 100644 --- a/pulp_ansible/app/models.py +++ b/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() diff --git a/pulp_ansible/app/serializers.py b/pulp_ansible/app/serializers.py index a523189d4..c8028de6d 100644 --- a/pulp_ansible/app/serializers.py +++ b/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 autocreated validator rendering sha256 required. + validators = [] class CollectionVersionSerializer(ContentChecksumSerializer, CollectionVersionUploadSerializer): diff --git a/pulp_ansible/app/tasks/collections.py b/pulp_ansible/app/tasks/collections.py index 62a38baff..582394e3e 100644 --- a/pulp_ansible/app/tasks/collections.py +++ b/pulp_ansible/app/tasks/collections.py @@ -100,6 +100,7 @@ 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 @@ -112,15 +113,17 @@ async def declarative_content_from_git_repo(remote, url, git_ref=None, metadata_ 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 + # What is metadata_only actually? + 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, @@ -319,6 +322,10 @@ 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) @@ -1134,7 +1141,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 ) diff --git a/pulp_ansible/app/tasks/upload.py b/pulp_ansible/app/tasks/upload.py index d10278905..fdb237034 100644 --- a/pulp_ansible/app/tasks/upload.py +++ b/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( diff --git a/pulp_ansible/tests/functional/api/collection/v3/test_collection.py b/pulp_ansible/tests/functional/api/collection/v3/test_collection.py index 69cb730c2..b760f2143 100644 --- a/pulp_ansible/tests/functional/api/collection/v3/test_collection.py +++ b/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. diff --git a/pulp_ansible/tests/functional/cli/test_collection_upload.py b/pulp_ansible/tests/functional/cli/test_collection_upload.py index 57773ecf7..91283f59b 100644 --- a/pulp_ansible/tests/functional/cli/test_collection_upload.py +++ b/pulp_ansible/tests/functional/cli/test_collection_upload.py @@ -36,7 +36,8 @@ def test_upload_collection( ) collection_meta = ansible_dir / "pulp" / collection_name / "meta" - collection_meta.mkdir() + if not collection_meta.exists(): + collection_meta.mkdir() runtime_yml = collection_meta / "runtime.yml" runtime_yml.write_text('requires_ansible: ">=2.9"\n') diff --git a/pulp_ansible/tests/unit/test_search.py b/pulp_ansible/tests/unit/test_search.py index 159504334..7b816298c 100644 --- a/pulp_ansible/tests/unit/test_search.py +++ b/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 ... diff --git a/pulp_ansible/tests/unit/test_serializers.py b/pulp_ansible/tests/unit/test_serializers.py index 9cefd7c75..b1d656195 100644 --- a/pulp_ansible/tests/unit/test_serializers.py +++ b/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), diff --git a/pulp_ansible/tests/unit/utils.py b/pulp_ansible/tests/unit/utils.py index 0a354eb5b..bc7d1c152 100644 --- a/pulp_ansible/tests/unit/utils.py +++ b/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