-
Notifications
You must be signed in to change notification settings - Fork 29
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.
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
Enable FLATPAK_INDEX for pulp registry #363
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,21 +2,17 @@ | |
# @param location_prefix | ||
# In the Apache configuration a location with this prefix is exposed. The | ||
# version (currently v2) will be appended. | ||
# @param registry_version_path | ||
# The path beneath the location prefix to forward. This is also appended to | ||
# the content base url. | ||
class pulpcore::plugin::container ( | ||
String $location_prefix = '/pulpcore_registry', | ||
String $registry_version_path = '/v2/', | ||
) { | ||
$context = { | ||
'directories' => [ | ||
{ | ||
'provider' => 'location', | ||
'path' => "${location_prefix}${registry_version_path}", | ||
'path' => $location_prefix, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this replace the previous definition instead? I don't see why you would specify both. Then we can also drop the The one reason I could see is if you want to make it depend on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think both registries are needed here. Right? @sjha4 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My reasoning is that if you have |
||
'proxy_pass' => [ | ||
{ | ||
'url' => "${pulpcore::apache::api_base_url}${registry_version_path}", | ||
'url' => $pulpcore::apache::api_base_url, | ||
}, | ||
], | ||
'request_headers' => $pulpcore::apache::api_default_request_headers + $pulpcore::apache::api_additional_request_headers, | ||
|
@@ -31,7 +27,7 @@ | |
} | ||
|
||
pulpcore::plugin { 'container': | ||
config => 'TOKEN_AUTH_DISABLED=True', | ||
config => "TOKEN_AUTH_DISABLED=True\nFLATPAK_INDEX=True", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason someone might want to set this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This setting is required by Pulp to manage Flatpak images. |
||
https_content => epp('pulpcore/apache-fragment.epp', $context), | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.