Project

Profile

Help

Issue #4676

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 6 months ago. Updated 4 months ago.

Status:
MODIFIED
Priority:
Normal
Category:
Documentation
Sprint/Milestone:
Start date:
Due date:
Severity:
2. Medium
Version:
Platform Release:
Blocks Release:
OS:
Backwards Incompatible:
No
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
API Bindings
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:
Sprint 53

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

Associated revisions

Revision 0fea20fd View on GitHub
Added by dkliban@redhat.com 6 months ago

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

Revision aef490e2 View on GitHub
Added by dkliban@redhat.com 5 months ago

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

Revision 084412c4 View on GitHub
Added by dkliban@redhat.com 5 months ago

Problem: initial migration is out of date

Solution: generate a new migration

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

History

#1 Updated by jdjeffers 6 months ago

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

#2 Updated by daviddavis 6 months 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.

#3 Updated by amacdona@redhat.com 6 months ago

  • Sprint/Milestone set to 3.0
  • Triaged changed from No to Yes
  • Sprint set to Sprint 51
  • Tags API Bindings added

#4 Updated by dkliban@redhat.com 6 months 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

#5 Updated by dkliban@redhat.com 6 months ago

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

#6 Updated by dkliban@redhat.com 6 months ago

  • Parent task set to #4687

#7 Updated by rchan 6 months ago

  • Sprint changed from Sprint 51 to Sprint 52

#8 Updated by dkliban@redhat.com 6 months ago

  • Status changed from ASSIGNED to POST

#9 Updated by dkliban@redhat.com 6 months ago

  • Status changed from POST to MODIFIED

#10 Updated by bmbouter 6 months ago

  • Tags deleted (Pulp 3)

#11 Updated by dkliban@redhat.com 5 months ago

  • Status changed from MODIFIED to ASSIGNED

#12 Updated by jdjeffers 5 months 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/

#13 Updated by dkliban@redhat.com 5 months ago

  • Sprint changed from Sprint 52 to Sprint 53

#14 Updated by jsherril@redhat.com 5 months 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." 
    ]
}

#15 Updated by dkliban@redhat.com 5 months ago

  • Status changed from ASSIGNED to MODIFIED

Please register to edit this issue

Also available in: Atom PDF