Project

Profile

Help

Issue #4676

closed

Story #4687: As a user, I can use either Python or Ruby bindings to interact with Pulp

Documentation for PATCH /pulp/api/v3/remotes/file/file/:ref doesn't match behavior

Added by jdjeffers over 5 years ago. Updated over 4 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Category:
-
Sprint/Milestone:
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
Platform Release:
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
API Bindings, Documentation
Sprint:
Sprint 53
Quarter:

Description

When issuing a PATCH missing fields are triggering an Bad Request when the docs indicate they are not required.

Here's an example request:

    request:
        method: patch
        uri: http://192.168.121.125/pulp/api/v3/remotes/file/file/9fbe786a-8a90-4a65-9127-8c7ca0cf536c/
        body:
          encoding: UTF-8
          base64_string: |
            eyJ2YWxpZGF0ZSI6dHJ1ZSwic3NsX3ZhbGlkYXRpb24iOnRydWUsIm5hbWUi
            OiJvcmdfZGVmYXVsdF9sYWJlbC1NeV9GaWxlcyIsInVybCI6Imh0dHA6Ly90
            ZXN0L3Rlc3QvL1BVTFBfTUFOSUZFU1QiLCJzc2xfY2xpZW50X2NlcnRpZmlj
            YXRlIjpudWxsLCJzc2xfY2xpZW50X2tleSI6bnVsbCwic3NsX2NhX2NlcnRp
            ZmljYXRlIjpudWxsfQ==
            "{"validate":true,"ssl_validation":true,"name":"org_default_label-My_Files","url":"http://test/test//PULP_MANIFEST","ssl_client_certificate":null,"ssl_client_key":null,"ssl_ca_certificate":null}"
        headers:
          User-Agent:
          - Swagger-Codegen/0.0.4/ruby
          Content-Type:
          - application/json
          Accept:
          - application/json
          Authorization:
          - Basic YWRtaW46cGFzc3dvcmQ=
          Expect:
          - ''
      response:
        status:
          code: 400
          message: Bad Request
        headers:
          Server:
          - nginx/1.12.2
          Date:
          - Thu, 11 Apr 2019 14:01:03 GMT
          Content-Type:
          - application/json
          Content-Length:
          - '160'
          Connection:
          - keep-alive
          Vary:
          - Accept, Cookie
          Allow:
          - GET, PUT, PATCH, DELETE, HEAD, OPTIONS
          X-Frame-Options:
          - SAMEORIGIN
        body:
          encoding: UTF-8
          base64_string: |
            eyJzc2xfY2FfY2VydGlmaWNhdGUiOlsiVGhpcyBmaWVsZCBtYXkgbm90IGJl
            IG51bGwuIl0sInNzbF9jbGllbnRfY2VydGlmaWNhdGUiOlsiVGhpcyBmaWVs
            ZCBtYXkgbm90IGJlIG51bGwuIl0sInNzbF9jbGllbnRfa2V5IjpbIlRoaXMg
            ZmllbGQgbWF5IG5vdCBiZSBudWxsLiJdfQ==   
            "{"ssl_ca_certificate":["This field may not be null."],"ssl_client_certificate":["This field may not be null."],"ssl_client_key":["This field may not be null."]}"
        http_version:
      recorded_at: Thu, 11 Apr 2019 14:58:53 GMT
Actions #1

Updated by jdjeffers over 5 years ago

{"versions":[{"component":"pulpcore","version":"3.0.0b23"},{"component":"pulpcore-plugin","version":"0.1.0b21"},{"component":"pulp_file","version":"0.0.1b9"}]
Actions #2

Updated by daviddavis over 5 years ago

I tested from the command line and this works:

$ http PATCH :8000/pulp/api/v3/remotes/file/file/d7c7a5f0-fb6f-4df3-9691-76d75ad4b3b4/ name='foo'
{
    "task": "/pulp/api/v3/tasks/11e704c2-7639-434e-a723-5fb8824db885/"
}
$ http :8000/pulp/api/v3/tasks/11e704c2-7639-434e-a723-5fb8824db885/
{
    "_created": "2019-04-12T14:26:51.162008Z",
    "_href": "/pulp/api/v3/tasks/11e704c2-7639-434e-a723-5fb8824db885/",
    "created_resources": [],
    "error": null,
    "finished_at": "2019-04-12T14:26:51.395693Z",
    "name": "pulpcore.app.tasks.base.general_update",
    "non_fatal_errors": [],
    "parent": null,
    "progress_reports": [],
    "spawned_tasks": [],
    "started_at": "2019-04-12T14:26:51.332536Z",
    "state": "completed",
    "worker": "/pulp/api/v3/workers/6fa4f409-ab53-4fcd-84d5-c8ec2921fce3/"
}

I think this might be a problem with the bindings. It looks like null values are being set in the request in for ssl_client_certificate, ssl_client_key, etc.

Actions #3

Updated by amacdona@redhat.com over 5 years ago

  • Sprint/Milestone set to 3.0.0
  • Triaged changed from No to Yes
  • Sprint set to Sprint 51
  • Tags API Bindings added
Actions #4

Updated by dkliban@redhat.com over 5 years ago

We are currently using swagger-codegen, but we should switch to openapi-generator (it's a fork). I found this bug in their issue tracker. https://github.com/OpenAPITools/openapi-generator/issues/2555

Actions #5

Updated by dkliban@redhat.com over 5 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to dkliban@redhat.com
Actions #6

Updated by dkliban@redhat.com over 5 years ago

  • Parent issue set to #4687
Actions #7

Updated by rchan over 5 years ago

  • Sprint changed from Sprint 51 to Sprint 52
Actions #8

Updated by dkliban@redhat.com over 5 years ago

  • Status changed from ASSIGNED to POST

Added by dkliban@redhat.com over 5 years ago

Revision 0fea20fd | View on GitHub

Problem: ssl fields on remote are hard to use

Solution: switch all ssl fields to TextField

This patch switches the storage of SSL certs, keys, and CAs from filesystem to database. This patch also introduces a new serializer field that returns a SHA256 digest for GET operations.

Required PR: https://github.com/pulp/pulpcore-plugin/pull/91

fixes: #4506 https://pulp.plan.io/issues/4506

re: #4676 https://pulp.plan.io/issues/4676

Actions #9

Updated by dkliban@redhat.com over 5 years ago

  • Status changed from POST to MODIFIED
Actions #10

Updated by bmbouter over 5 years ago

  • Tags deleted (Pulp 3)
Actions #11

Updated by dkliban@redhat.com over 5 years ago

  • Status changed from MODIFIED to ASSIGNED
Actions #12

Updated by jdjeffers over 5 years ago

Follow up with more recent instance of this issue.
From our application log (katello):

2019-05-16T14:44:05 [D|app|] ETHON: performed EASY effective_url=http://centos7-pulp3-github.jjeffers.example.com/pulp/api/v3/remotes/file/file/76835055-9770-40b0-baf4-ffa706f075e4/ response_code=400 return_code=ok total_time=1.557061
...
2019-05-16T14:44:05 [E|bac|] Error message: the server returns an error
 | HTTP status code: 400
 | Response headers: {"Server"=>"nginx/1.12.2", "Date"=>"Thu, 16 May 2019 14:44:04 GMT", "Content-Type"=>"application/json", "Content-Length"=>"160", "Connection"=>"keep-alive", "Vary"=>"Accept, Cookie", "Allow"=>"GET, PUT, PATCH, DELETE, HEAD, OPTIONS", "X-Frame-Options"=>"SAMEORIGIN"}
 | Response body: {"ssl_ca_certificate":["This field may not be null."],"ssl_client_certificate":["This field may not be null."],"ssl_client_key":["This field may not be null."]} (PulpFileClient::ApiError)
 | /home/vagrant/foreman/.vendor/ruby/2.5.0/gems/pulp_file_client-0.0.1b10.dev.1557779852/lib/pulp_file_client/api_client.rb:65:in `call_api'
 | /home/vagrant/foreman/.vendor/ruby/2.5.0/gems/pulp_file_client-0.0.1b10.dev.1557779852/lib/pulp_file_client/api/remotes_api.rb:297:in `remotes_file_file_partial_update_with_http_info'
 | /home/vagrant/foreman/.vendor/ruby/2.5.0/gems/pulp_file_client-0.0.1b10.dev.1557779852/lib/pulp_file_client/api/remotes_api.rb:241:in `remotes_file_file_partial_update'

And the gem source at the indicated method:

# Call an API with given options.
    #
    # @return [Array<(Object, Integer, Hash)>] an array of 3 elements:
    #   the data deserialized from response body (could be nil), response status code and response headers.
    def call_api(http_method, path, opts = {})
      request = build_request(http_method, path, opts)
      response = request.run

      if @config.debugging
        @config.logger.debug "HTTP response body ~BEGIN~\n#{response.body}\n~END~\n"
      end

      unless response.success?
        if response.timed_out?
          fail ApiError.new('Connection timed out')
        elsif response.code == 0
          # Errors from libcurl will be made visible here
          fail ApiError.new(:code => 0,
                            :message => response.return_message)
        else
          fail ApiError.new(:code => response.code,
                            :response_headers => response.headers,
                            :response_body => response.body),
               response.status_message
        end
      end

      if opts[:return_type]
        data = deserialize(response, opts[:return_type])
      else
        data = nil
      end
      return data, response.code, response.headers
    end

And the associated journalctl entry in the pulp server:

May 16 14:44:04 centos7-pulp3-github.jjeffers.example.com gunicorn[2934]: pulp: django.request:WARNING: Bad Request: /pulp/api/v3/remotes/file/file/76835055-9770-40b0-baf4-ffa706f075e4/
Actions #13

Updated by dkliban@redhat.com over 5 years ago

  • Sprint changed from Sprint 52 to Sprint 53
Actions #14

Updated by jsherril@redhat.com over 5 years ago

To reproduce simply try to update a remote with:


http -v PATCH                                                                     
localhost:/pulp/api/v3/remotes/file/file/6b30daf7-be48-4fc5-8bdc-2d9614e0ed36/    
ssl_ca_certificate:=null ssl_client_certificate:=null ssl_client_key:=null        

and you should get:

PATCH /pulp/api/v3/remotes/file/file/6b30daf7-be48-4fc5-8bdc-2d9614e0ed36/ HTTP/1.1
Accept: application/json
Accept-Encoding: gzip, deflate
Authorization: Basic YWRtaW46cGFzc3dvcmQ=
Connection: keep-alive
Content-Length: 84
Content-Type: application/json
Host: localhost
User-Agent: HTTPie/0.9.4

{
    "ssl_ca_certificate": null,
    "ssl_client_certificate": null,
    "ssl_client_key": null
}

HTTP/1.1 400 Bad Request
Allow: GET, PUT, PATCH, DELETE, HEAD, OPTIONS
Connection: keep-alive
Content-Length: 160
Content-Type: application/json
Date: Thu, 16 May 2019 17:49:27 GMT
Server: nginx/1.14.2
Vary: Accept, Cookie
X-Frame-Options: SAMEORIGIN

{
    "ssl_ca_certificate": [
        "This field may not be null."
    ],
    "ssl_client_certificate": [
        "This field may not be null."
    ],
    "ssl_client_key": [
        "This field may not be null."
    ]
}

Added by dkliban@redhat.com over 5 years ago

Revision aef490e2 | View on GitHub

Problem: REST API does not accept null for string values

Solution: update serializers to accept null

This patch also removes the ability to specify an empty string for string fields.

re: #4676 https://pulp.plan.io/issues/4676

Added by dkliban@redhat.com over 5 years ago

Revision 084412c4 | View on GitHub

Problem: initial migration is out of date

Solution: generate a new migration

re: #4676 https://pulp.plan.io/issues/4676

Actions #15

Updated by dkliban@redhat.com over 5 years ago

  • Status changed from ASSIGNED to MODIFIED
Actions #16

Updated by bmbouter almost 5 years ago

  • Status changed from MODIFIED to CLOSED - CURRENTRELEASE
Actions #17

Updated by bmbouter over 4 years ago

  • Tags Documentation added
Actions #18

Updated by bmbouter over 4 years ago

  • Category deleted (23)

Also available in: Atom PDF