search

pulp / pulp_rpm

RPM support for Pulp Platform
https://pulpproject.org/pulp_rpm/
GNU General Public License v2.0
48 stars 124 forks source link

changelogs, files, and other rpm fields are represented incorrectly in the api spec #3694

Open jlsherrill opened 3 months ago

jlsherrill commented 3 months ago

Version

      "versions": {
        "gem": "0.5.1",
        "rpm": "3.25.3",
        "core": "3.54.1",
        "file": "3.54.1",
        "ostree": "2.4.1",
        "python": "3.12.0",
        "certguard": "3.54.1"
      }

Describe the bug When we produce Go bindings for the pulp api spec, and try to fetch rpms from the ContentRpmPackagesList api, we get an error:

ERROR[json] found bytes "{"count":1,"next":null,"previous":null,"results":[{"pulp_href":"/api/pulp/cdc33308/api/v3/content/rpm/packages/0190dc41-9e00-795f-87d5-2be8edb2af7f/","pulp_created":"2024-07-22T21:03:26.210140Z","pulp_last_updated":"2024-07-22T21:03:26.210160Z","md5":null,"sha1":null,"sha224":"a59f871a90f3dd827c7ce356c199f7c02dddea2e859e6e4373609b36","sha256":"7a831f9f90bf4d21027572cb503d20b702de8e8785b02c0397445c2e481d81b3","sha384":"0262833d7750a698834869af93e82582e39a09c3f32cc1100dfe3af010852f35de8a4360a8483d6ab80d40f15e3644bf","sha512":"b0a97309e0a29257e87fba4c6c65b54fb4c6ae39549f9279b8a0c4fd3027753c87f82f977ccbb87f97c5e26e8cb788d0d73659799e4a01778995b5cae0a2d432","artifact":"/api/pulp/cdc33308/api/v3/artifacts/0190dc35-d78e-7d6f-b824-c2dc3e38a34f/","name":"bear","epoch":"0","version":"4.1","release":"1","arch":"noarch","pkgId":"7a831f9f90bf4d21027572cb503d20b702de8e8785b02c0397445c2e481d81b3","checksum_type":"sha256","summary":"A dummy package of bear","description":"A dummy package of bear","url":"http://tstrachota.fedorapeople.org","changelogs":[], "location_base":"","location_href":"Packages","rpm_buildhost":"smqe-ws15","rpm_group":"Internet/Applications","rpm_license":"GPLv2","rpm_packager":"","rpm_sourcerpm":"bear-4.1-1.src.rpm","rpm_vendor":"","rpm_header_start":872,"rpm_header_end":2289,"is_modular":false,"size_archive":296,"size_installed":42,"size_package":2438,"time_build":1331831374,"time_file":1721682206}]}", but failed to unmarshal: json: cannot unmarshal array into Go struct field _PaginatedrpmPackageResponseList.results of type map[string]interface

Playing around with this with Go, it appears that its related to the fact that changelogs, files etc.. are declared as an 'Object', but in fact are an array of an array of strings

To Reproduce Steps to reproduce the behavior:

Expected behavior I would expect the api spec to reflect the data returned.

Additional context This may be related to https://github.com/pulp/pulp_rpm/issues/3657 ?

these fields in the schema:

                 "changelogs": {
                        "type": "object",
                        "readOnly": true,
                        "default": "[]",
                        "description": "Changelogs that package contains"
                    },
                    "files": {
                        "type": "object",
                        "readOnly": true,
                        "default": "[]",
                        "description": "Files that package contains"
                    },
                    "requires": {
                        "type": "object",
                        "readOnly": true,
                        "default": "[]",
                        "description": "Capabilities the package requires"
                    },
                    "provides": {
                        "type": "object",
                        "readOnly": true,
                        "default": "[]",
                        "description": "Capabilities the package provides"
                    },
                    "conflicts": {
                        "type": "object",
                        "readOnly": true,
                        "default": "[]",
                        "description": "Capabilities the package conflicts"

example json fetched from the api:

{
   "count":1,
   "next":null,
   "previous":null,
   "results":[
      {
         "pulp_href":"/api/pulp/cdc33308/api/v3/content/rpm/packages/0190dc41-9e00-795f-87d5-2be8edb2af7f/",
         "pulp_created":"2024-07-22T21:03:26.210140Z",
         "pulp_last_updated":"2024-07-22T21:03:26.210160Z",
         "md5":null,
         "sha1":null,
         "sha224":"a59f871a90f3dd827c7ce356c199f7c02dddea2e859e6e4373609b36",
         "sha256":"7a831f9f90bf4d21027572cb503d20b702de8e8785b02c0397445c2e481d81b3",
         "sha384":"0262833d7750a698834869af93e82582e39a09c3f32cc1100dfe3af010852f35de8a4360a8483d6ab80d40f15e3644bf",
         "sha512":"b0a97309e0a29257e87fba4c6c65b54fb4c6ae39549f9279b8a0c4fd3027753c87f82f977ccbb87f97c5e26e8cb788d0d73659799e4a01778995b5cae0a2d432",
         "artifact":"/api/pulp/cdc33308/api/v3/artifacts/0190dc35-d78e-7d6f-b824-c2dc3e38a34f/",
         "name":"bear",
         "epoch":"0",
         "version":"4.1",
         "release":"1",
         "arch":"noarch",
         "pkgId":"7a831f9f90bf4d21027572cb503d20b702de8e8785b02c0397445c2e481d81b3",
         "checksum_type":"sha256",
         "summary":"A dummy package of bear",
         "description":"A dummy package of bear",
         "url":"http://tstrachota.fedorapeople.org",
         "changelogs":[

         ],
         "files":[
            [
               "",
               "/tmp/",
               "bear.txt",
               "5938462bfd4a5d750e0851f5b82f3ade"
            ]
         ],
         "requires":[

         ],
         "provides":[
            [
               "bear",
               "EQ",
               "0",
               "4.1",
               "1",
               false
            ]
         ],
         "conflicts":[

         ],
         "obsoletes":[

         ],
         "suggests":[

         ],
         "enhances":[

         ],
         "recommends":[

         ],
         "supplements":[

         ],
         "location_base":"",
         "location_href":"Packages",
         "rpm_buildhost":"smqe-ws15",
         "rpm_group":"Internet/Applications",
         "rpm_license":"GPLv2",
         "rpm_packager":"",
         "rpm_sourcerpm":"bear-4.1-1.src.rpm",
         "rpm_vendor":"",
         "rpm_header_start":872,
         "rpm_header_end":2289,
         "is_modular":false,
         "size_archive":296,
         "size_installed":42,
         "size_package":2438,
         "time_build":1331831374,
         "time_file":1721682206
      }
   ]
}
jlsherrill commented 3 months ago

the following patch to the pulp api spec seems to help, but it may not be a full fix:

--- pulp_api.json   2024-07-23 09:25:12.179493116 -0400
+++ patched_pulp.json   2024-07-23 09:59:30.669329005 -0400
@@ -58586,61 +58586,130 @@
                         "description": "URL with more information about the packaged software"
                     },
                     "changelogs": {
-                        "type": "object",
+                        "type": "array",
+           "items": {
+                           "type": "array",
+                           "items": {
+               "type": "string"
+                  }
+           },
                         "readOnly": true,
                         "default": "[]",
                         "description": "Changelogs that package contains"
                     },
                     "files": {
-                        "type": "object",
-                        "readOnly": true,
+                        "type": "array",
+                        "items": {
+                           "type": "array",
+                           "items": {
+                                "type": "string"
+                           }       
+                        },
+                   "readOnly": true,
                         "default": "[]",
                         "description": "Files that package contains"
                     },
                     "requires": {
-                        "type": "object",
-                        "readOnly": true,
+                        "type": "array",
+                        "items": {
+                           "type": "array",
+                           "items": {
+                                "type": "string"
+                           }       
+                        },
+               "readOnly": true,
                         "default": "[]",
                         "description": "Capabilities the package requires"
                     },
                     "provides": {
-                        "type": "object",
-                        "readOnly": true,
+                        "type": "array",
+                        "items": {
+                           "type": "array",
+                           "items": {
+               "oneOf": [
+                                  {"type": "string"},
+                 {"type": "bool"}
+               ]
+                           }       
+                        },
+           "readOnly": true,
                         "default": "[]",
                         "description": "Capabilities the package provides"
                     },
                     "conflicts": {
-                        "type": "object",
+                        "type": "array",
+                        "items": {
+                           "type": "array",
+                           "items": {
+                                "oneOf": [
+                                  {"type": "string"},
+                                  {"type": "bool"}
+                                ]
+              }       
+                        },
                         "readOnly": true,
                         "default": "[]",
                         "description": "Capabilities the package conflicts"
                     },
                     "obsoletes": {
-                        "type": "object",
-                        "readOnly": true,
+                        "type": "array",
+                        "items": {
+                           "type": "array",
+                           "items": {
+                                "oneOf": [
+                                  {"type": "string"},
+                                  {"type": "bool"}
+                                ]
+              }       
+                        },
+                   "readOnly": true,
                         "default": "[]",
                         "description": "Capabilities the package obsoletes"
                     },
                     "suggests": {
-                        "type": "object",
-                        "readOnly": true,
+                        "type": "array",
+                        "items": {
+                           "type": "array",
+                           "items": {
+                                "type": "string"
+                           }       
+                        },
+                       "readOnly": true,
                         "default": "[]",
                         "description": "Capabilities the package suggests"
                     },
                     "enhances": {
-                        "type": "object",
+                        "type": "array",
+                        "items": {
+                           "type": "array",
+                           "items": {
+                                "type": "string"
+                           }       
+                        },
                         "readOnly": true,
                         "default": "[]",
                         "description": "Capabilities the package enhances"
                     },
                     "recommends": {
-                        "type": "object",
-                        "readOnly": true,
+                        "type": "array",
+                        "items": {
+                           "type": "array",
+                           "items": {
+                                "type": "string"
+                           }       
+                        },
+               "readOnly": true,
                         "default": "[]",
                         "description": "Capabilities the package recommends"
                     },
                     "supplements": {
-                        "type": "object",
+                        "type": "array",
+                        "items": {
+                           "type": "array",
+                           "items": {
+                                "type": "string"
+                           }       
+                        },
                         "readOnly": true,
                         "default": "[]",
                         "description": "Capabilities the package supplements"
@@ -60266,4 +60335,4 @@
             "url": "http://localhost:8080/"
         }
     ]
-}
\ No newline at end of file
+}
jlsherrill commented 3 months ago

oh and I realized we can use 'fields' to workaround this, so its not high priority for us right now

pedro-psb commented 3 months ago

If I understand correctly this was not a regression, you are just trying this out for the first time, right?

This may be related to https://github.com/pulp/pulp_rpm/issues/3657 ?

Yes, I believe so. The ideal solution would be to fix this in the serializer level, but the problem with that (as pointed here) is that it would affect all the interfaces (API and other client bindings), and that could break some users pipeline if their code rely on this innacurate type representation.

I think that if we can show this doesnt work with other binding clients it would be safer to update.

A second approach is monkey-patching the types in the api-spec, as you've shown. If we go this path maybe we can add some "Known Issues" doc notes on pulp-openapi-generator for people who want to generate Go bindings.

oh and I realized we can use 'fields' to workaround this, so its not high priority for us right now

Can you elaborate? What fields exactly?

jlsherrill commented 3 months ago

@pedro-psb yes, trying for the first time. I'm not sure it ever worked.

Can you elaborate? What fields exactly?

The list packages api: https://pulpproject.org/pulp_rpm/restapi/#tag/Content:-Packages/operation/content_rpm_packages_list

provides a fields and exclude_fields option that excludes some fields from being returned in the api. If I use the 'fields' parameter and only include a few fields i care about (name, version, release, arch), it works perfectly fine, since the problematic fields are excluded.

dralley commented 2 months ago

Discussion in the RPM meeting: it's not necessarily a breaking change for Python and Ruby bindings because of their dynamically typed nature, maybe we can change the schema and fix the Go bindings without breaking usages of Ruby and Python bindings.

We can make the change and compare generated bindings code before and after, maybe it is fine?

Worst case we can release it alongside a breaking release.