Complete policy.json inclusion (Also add to win installer) #21985
Conversation
n1hility
commented
Mar 7, 2024
n1hility
commented
- Sets default search location to always be the peer directory
- make podman-remote now creates binaries that work the same as release zips
- Updates release zip to match expected search location
- Updates win installer to include the file if present in the repo cross-build archive
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: n1hility The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Fixes #21964 |
| @@ -135,6 +135,13 @@ try { | |||
| Copy-Artifact("gvproxy.exe") | |||
| } | |||
|
|
|||
| $loc = Get-ChildItem -Recurse -Path . -Name policy.json | |||
There was a problem hiding this comment.
i wondered about how this would work ... neat
|
LGTM |
| @@ -103,6 +103,8 @@ FISHINSTALLDIR=${PREFIX}/share/fish/vendor_completions.d | |||
|
|
|||
| SELINUXOPT ?= $(shell test -x /usr/sbin/selinuxenabled && selinuxenabled && echo -Z) | |||
|
|
|||
| MACHINE_POLICY_JSON_DIR ?= . | |||
There was a problem hiding this comment.
I don't think we should set the default for everything to ., that just doesn't make sense for local development, i.e. see #21855
Using $(pwd)/pkg/machine/ocipull and then overwriting this to . for the zip file target should be better IMO.
There was a problem hiding this comment.
Anyhow it doesn't make the current situation any worse though so consider this non blocking.
There was a problem hiding this comment.
IMO a binary you build with podman-remote should work just as well (and ideally the same) as the one installed by a package (and behave similarly). It's a pain to have to build an installer just to try out a change, or to have always pass custom flags to podman-remote (that are always the same). For example, a common workflow a lot of folks use is to just copy up a new binary in the same location on the path. Another example is we had CI tasks that used the binary built from podman-remote (and not the full zip), and so the different behavior was an issue there and required installing a local override (not a huge deal, but it just illustrates the issue). IMO the issue you link is just another form of it
The path you link above is also not quite right because it only works on a system that was building the binary, you can't copy it to another system or hand to someone else to test and have it work.
I think the only thing we can do to address this reliably is to search multiple locations, which is at least already the case on Linux (where /etc/containers is likely to exist), or that other idea you had to possibly fetch a policy from somewhere if it doesnt exist.
There was a problem hiding this comment.
Except a binary compiled on a dev machine running with bin/podman won't work because the policy.json is not in the same directory as in the binary.
|
Tested Mac pkginstaller and it seemed to work, so LGTM |
|
CI issue is legit fix coming in one sec |
- Sets default search location to always be the peer directory + make podman-remote now creates binaries that work the same as release zips - Updates release zip to match expected search location - Updates win installer to include the file if present in the repo cross-build archive [NO NEW TESTS NEEDED] Signed-off-by: Jason T. Greene <[email protected]>
n1hility
force-pushed
the
fin-policy-json
branch
from
003bbd7 to
d54a373
Compare
|
/lgtm |
openshift-merge-bot
bot
merged commit 4506812
into
containers:main
stale-locking-app
bot
added
the
locked - please file new issue/PR
