Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
jlsherrill opened this issue Jul 22, 2024 · 5 comments
Open

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

jlsherrill opened this issue Jul 22, 2024 · 5 comments
Labels
Issue

Comments

@jlsherrill
Copy link
Contributor

jlsherrill commented Jul 22, 2024 •
edited
Loading

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 #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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@pedro-psb
Copy link
Member

pedro-psb commented Jul 24, 2024 •
edited
Loading

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 #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
Copy link
Contributor Author

@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
Copy link
Contributor

dralley commented Aug 8, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jlsherrill @dralley @pedro-psb