Issue #3307: import_upload: improve data validation - RPM Support - Pulp

Actions

Send by e-mail Copy link

Issue #3307

closed

import_upload: improve data validation

Added by mihai.ibanescu@gmail.com over 6 years ago. Updated about 5 years ago.

Status:

CLOSED - CURRENTRELEASE

Priority:

Normal

Assignee:

daviddavis

Sprint/Milestone:

Start date:

Due date:

Estimated time:

Severity:

2. Medium

Version:

Platform Release:

2.15.3

OS:

Triaged:

Yes

Groomed:

Sprint Candidate:

Tags:

Pulp 2

Sprint:

Sprint 33

Quarter:

Description

As a user, uploading a unit using the REST API allows me to specify optional unit_key and unit_metadata.

https://docs.pulpproject.org/dev-guide/integration/rest-api/content/upload.html#import-into-a-repository

In pulp 2.10, unit_key was permitted to be None (looking at /usr/lib/python2.7/site-packages/pulp_rpm/plugins/importers/yum/upload.py _handle_package)

    model_class = plugin_api.get_unit_model_by_id(type_id)
    update_fields_inbound(model_class, unit_key or {})
    update_fields_inbound(model_class, metadata or {})
    ...
    # Update the RPM-extracted data with anything additional the user specified.
    # Allow the user-specified values to override the extracted ones.
    rpm_data.update(metadata or {})
    rpm_data.update(unit_key or {})

    # Validate the user specified data by instantiating the model
    try:
        unit = model_class(**rpm_data)
    except TypeError:
        raise ModelInstantiationError()

It looks like in pulp 2.15 it is required that it is a dict.

    # Update the RPM-extracted data with anything additional the user specified.
    # Allow the user-specified values to override the extracted ones.
    for key, value in metadata.items():
        setattr(unit, key, value)
    for key, value in unit_key.items():
        setattr(unit, key, value)

Change was introduced in https://github.com/pulp/pulp_rpm/commit/62db898a81679b40e3cc3a48af9f3b830f4521cb#diff-12687b797580f9b515df98568ed0efe6R418

Notice how it is now expected that unit_key has an items method, whereas the previous code was working even with None.

Related issues

Actions

Copy link

Updated by mihai.ibanescu@gmail.com over 6 years ago

I am not sure what my expectations are for this.

The REST API documentation should make it clear that unit_key and unit_metadata should be Dict, not the overly generic object.

It would be nice if the input data was validated, because once again the error I was seeing was the very unhelpful: "/usr/lib/python2.7/site-packages/pulp/server/managers/content/upload.py\", line 223, in import_uploaded_unit\n unit_type=unit_type_id, summary=result['summary'], details=result['details']\nPulpCodedException: The importer yum_importer indicated a failed response when uploading rpm unit to repository "

Only slightly more information was present in the task (if you dumped the json with pulp-admin -v -v):

"error": {
    "code": "PLP0047", 
    "data": {
      "unit_type": "rpm", 
      "importer_id": "yum_importer", 
      "repo_id": "_session-x64_redhat_linux_6-yum-20180120012852.593262-yum", 
      "details": {
        "errors": [
          "unexpected error occurred importing uploaded file: 'NoneType' object has no attribute 'items'"
        ]
      }, 
      "summary": ""
    },

Because I currently have client libraries out in the wild that rely on this behavior, I would need it fixed server-side. But I can patch it myself until all my clients catch up.

Making Pulp easier to debug when these problems occur would be the biggest take-away. Having to put breakpoints in a live system is just not right. In this particular case, I believe something was indeed being logged to the journal, but in other cases it's total silence.

Actions

Copy link