Enable FLATPAK_INDEX for pulp registry #363
Conversation
|
cc @sjha4 |
|
CI failures are relevant. |
lfu
force-pushed
the
flatpak_index
branch
3 times, most recently
from
de8a749 to
7fe0bf9
Compare
There was a problem hiding this comment.
Tested functionality and able to hit /pulpcore_registry/index/static?label:org.flatpak.ref:exists=1&tag=latest endpoint based on the updated apache config and verified that pulp is serving the flatpak_index correctly based on the pulp setting change. Test based ACK..Code LGTM.. 👍🏼
Can merge if maintainers are happy with the change.
|
@ekohl Does this look good? |
| @@ -21,6 +21,16 @@ | |||
| ], | |||
| 'request_headers' => $pulpcore::apache::api_default_request_headers + $pulpcore::apache::api_additional_request_headers, | |||
| }, | |||
| { | |||
| 'provider' => 'location', | |||
| 'path' => $location_prefix, | |||
There was a problem hiding this comment.
Should this replace the previous definition instead? I don't see why you would specify both. Then we can also drop the $registry_version_path variable altogether.
The one reason I could see is if you want to make it depend on FLATPAK_INDEX behavior.
There was a problem hiding this comment.
I think both registries are needed here. Right? @sjha4
There was a problem hiding this comment.
My reasoning is that if you have ${location_prefix} mapping to ${pulpcore::apache::api_base_url} then ${location_prefix}${registry_version_path} mapping to ${pulpcore::apache::api_base_url}${registry_version_path} is already implied.
| @@ -31,7 +41,7 @@ | |||
| } | |||
|
|
|||
| pulpcore::plugin { 'container': | |||
| config => 'TOKEN_AUTH_DISABLED=True', | |||
| config => "TOKEN_AUTH_DISABLED=True\nFLATPAK_INDEX=True", | |||
There was a problem hiding this comment.
Is there a reason someone might want to set this to False? Should it be a parameter?
There was a problem hiding this comment.
This setting is required by Pulp to manage Flatpak images.
| class pulpcore::plugin::container ( | ||
| String $location_prefix = '/pulpcore_registry', | ||
| String $registry_version_path = '/v2/', |
There was a problem hiding this comment.
What is the driver for dropping the /v2/ in the path? I think that's the only part that isn't clear.
There was a problem hiding this comment.
My understanding is that flatpak needs a different path, so instead of having two locations to proxy, it was decided to proxy the whole /pulpcore_registry.
There was a problem hiding this comment.
Correct. #363 (review) tells me it needs to retrieve /pulpcore_registry/index/static though the spec says there's also /pulpcore_registry/index/dynamic. Prior to #363 (comment) there were both, but that looks redundant to me.
Add
FLATPAK_INDEX=Trueto settings.py.