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.

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

Complete policy.json inclusion (Also add to win installer) #21985

Merged
merged 1 commit into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?= .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyhow it doesn't make the current situation any worse though so consider this non blocking.

Copy link
Member Author

@n1hility n1hility Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


COMMIT_NO ?= $(shell git rev-parse HEAD 2> /dev/null || true)
GIT_COMMIT ?= $(if $(shell git status --porcelain --untracked-files=no),$(call err_if_empty,COMMIT_NO)-dirty,$(COMMIT_NO))
DATE_FMT = %s
Expand Down Expand Up @@ -766,10 +768,10 @@ podman-remote-release-%.zip: test/version/version ## Build podman-remote for %=$
$(MAKE) GOOS=$(GOOS) GOARCH=$(GOARCH) \
clean-binaries podman-remote-$(GOOS)-docs
if [[ "$(GOARCH)" != "$(NATIVE_GOARCH)" ]]; then \
$(MAKE) CGO_ENABLED=0 $(GOPLAT) BUILDTAGS="$(BUILDTAGS_CROSS)" MACHINE_POLICY_JSON_DIR="." \
$(MAKE) CGO_ENABLED=0 $(GOPLAT) BUILDTAGS="$(BUILDTAGS_CROSS)" \
clean-binaries podman-remote; \
else \
$(MAKE) $(GOPLAT) MACHINE_POLICY_JSON_DIR="." podman-remote; \
$(MAKE) $(GOPLAT) podman-remote; \
fi
if [[ "$(GOOS)" == "windows" ]]; then \
$(MAKE) $(GOPLAT) TMPDIR="" win-gvproxy; \
Expand All @@ -779,8 +781,11 @@ podman-remote-release-%.zip: test/version/version ## Build podman-remote for %=$
fi
cp -r ./docs/build/remote/$(GOOS) "$(tmpsubdir)/$(releasedir)/docs/"
cp ./contrib/remote/containers.conf "$(tmpsubdir)/$(releasedir)/"
cp ./pkg/machine/ocipull/policy.json "$(tmpsubdir)/$(releasedir)/"
$(MAKE) $(GOPLAT) $(_dstargs) SELINUXOPT="" install.remote
# Placing the policy file in the bin directory is intentional This
# could be changed in the future to mirror LSB on Linux/Unix but would
# require path resolution logic changes to sustain the Win flat model
cp ./pkg/machine/ocipull/policy.json "$(tmpsubdir)/$(releasedir)/$(RELEASE_PREFIX)/bin"
cd "$(tmpsubdir)" && \
zip --recurse-paths "$(CURDIR)/$@" "./$(releasedir)"
if [[ "$(GOARCH)" != "$(NATIVE_GOARCH)" ]]; then $(MAKE) clean-binaries; fi
Expand Down
8 changes: 8 additions & 0 deletions contrib/win-installer/build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,18 @@ SignItem @("artifacts/win-sshproxy.exe",
$gvExists = Test-Path "artifacts/gvproxy.exe"
if ($gvExists) {
SignItem @("artifacts/gvproxy.exe")
Remove-Item Env:\UseGVProxy -ErrorAction SilentlyContinue
} else {
$env:UseGVProxy = "Skip"
}

$pExists = Test-Path "artifacts/policy.json"
if ($pExists) {
Remove-Item Env:\IncludePolicyJSON -ErrorAction SilentlyContinue
} else {
$env:IncludePolicyJSON = "Skip"
}

.\build-msi.bat $ENV:INSTVER; ExitOnError
SignItem @("podman.msi")

Expand Down
14 changes: 14 additions & 0 deletions contrib/win-installer/podman.wxs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
<?define UseGVProxy = ""?>
<?endif?>

<?ifdef env.IncludePolicyJSON?>
<?define IncludePolicyJSON = "$(env.IncludePolicyJSON)"?>
<?else?>
<?define IncludePolicyJSON = ""?>
<?endif?>

<Product Name="Podman $(var.VERSION)" Id="*" UpgradeCode="696BAB5D-CA1F-4B05-B123-320F245B8D6D" Version="$(var.VERSION)" Language="1033" Manufacturer="Red Hat Inc.">

<Package Id="*" Platform="x64" Keywords="Installer" Description="Red Hat's Podman $(var.VERSION) Installer" Comments="Apache 2.0 License" Manufacturer="Red Hat Inc." InstallScope="perMachine" InstallerVersion="200" Compressed="yes"/>
Expand Down Expand Up @@ -41,6 +47,11 @@
<File Id="GvProxyExecutableFile" Name="gvproxy.exe" Source="artifacts/gvproxy.exe" KeyPath="yes"/>
</Component>
<?endif?>
<?if $(var.IncludePolicyJSON) != Skip?>
<Component Id="PolicyJSON" Guid="C6135EDA-7C17-4A0E-BC52-5AB38BD54A61" Win64="yes">
<File Id="PolicyJSONFile" Name="policy.json" Source="artifacts/policy.json" KeyPath="yes"/>
</Component>
<?endif?>
<Component Id="GuideHTMLComponent" Guid="8B23C76B-F7D4-4030-8C46-1B5729E616B5" Win64="yes">
<File Id="GuideHTMLFile" Name="welcome-podman.html" Source="docs/podman-for-windows.html" KeyPath="yes"/>
</Component>
Expand Down Expand Up @@ -74,6 +85,9 @@
<?if $(var.UseGVProxy) != Skip?>
<ComponentRef Id="GvProxyExecutable"/>
<?endif?>
<?if $(var.IncludePolicyJSON) != Skip?>
<ComponentRef Id="PolicyJSON"/>
<?endif?>
<ComponentRef Id="GuideHTMLComponent"/>
<ComponentGroupRef Id="ManFiles"/>
<ComponentGroupRef Id="WSLFeature"/>
Expand Down
7 changes: 7 additions & 0 deletions contrib/win-installer/process-release.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ try {
Copy-Artifact("gvproxy.exe")
}

$loc = Get-ChildItem -Recurse -Path . -Name policy.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wondered about how this would work ... neat

if (!$loc) {
Write-Host "Skipping policy.json artifact"
} else {
Copy-Artifact("policy.json")
}

$docsloc = Get-ChildItem -Path . -Name docs -Recurse
$loc = Get-ChildItem -Recurse -Path . -Name podman-for-windows.html
if (!$loc) {
Expand Down