diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 3957146681..f34eb6510c 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -1232,7 +1232,10 @@ func (dn *Daemon) RunFirstbootCompleteMachineconfig(machineConfigFile string) er // This currently will incur a double reboot; see https://github.com/coreos/rpm-ostree/issues/4018 // If skopeo is < 1.22.2 on a multi-arch image, run as a privileged container which has updated skopeo. // See https://redhat.atlassian.net/browse/OCPBUGS-83826 and https://redhat.atlassian.net/browse/OCPBUGS-81187 - if !newEnough || !skopeoSupportsMultiArchSigstore(mc.Spec.OSImageURL) { + // If rpm-ostree is < 2023.5, it has a skopeo-proxy sandboxing bug when rebasing + // from containers-storage or registry sources; run as a privileged container instead. + // See https://issues.redhat.com/browse/OCPBUGS-86768 (temporary until 4.13/4.14 boot images are unsupported). + if !newEnough || !skopeoSupportsMultiArchSigstore(mc.Spec.OSImageURL) || !dn.NodeUpdaterClient.SupportsContainerStorageRebase() { logSystem("rpm-ostree or skopeo is not new enough for new-format image; forcing an update via container and queuing immediate reboot") if err := dn.InplaceUpdateViaNewContainer(mc.Spec.OSImageURL); err != nil { return err diff --git a/pkg/daemon/rpm-ostree.go b/pkg/daemon/rpm-ostree.go index 87674c043c..6150e1632e 100644 --- a/pkg/daemon/rpm-ostree.go +++ b/pkg/daemon/rpm-ostree.go @@ -7,8 +7,10 @@ import ( "path/filepath" "reflect" "strings" + "sync" "github.com/containers/image/v5/signature" + "github.com/coreos/go-semver/semver" rpmostreeclient "github.com/coreos/rpmostree-client-go/pkg/client" "gopkg.in/yaml.v2" "k8s.io/klog/v2" @@ -19,6 +21,24 @@ const imagePolicyFilePath = "/etc/containers/policy.json" const rpmOstreeTemporalDropinFile = "/run/systemd/system/rpm-ostreed.service.d/temporal-policy-binding.conf" const rpmOstreeTemporalPolicyFile = "/run/tmp-rpm-ostree-policy.json" +// minRpmOstreeVersionForContainerStorageRebase is the first upstream rpm-ostree +// release that contains the fix for +// https://github.com/coreos/rpm-ostree/issues/4283 (merged via +// https://github.com/coreos/rpm-ostree/pull/4466), which resolved a skopeo-proxy +// sandboxing bug affecting rebases from both containers-storage and registry +// sources. Hosts running an older rpm-ostree hit the following issue: +// +// https://redhat.atlassian.net/browse/OCPBUGS-86768 +// +// Expressed in dotted-tri form for comparison with go-semver, since rpm-ostree's +// own version strings (e.g. "2023.3") only carry two components. +const minRpmOstreeVersionForContainerStorageRebase = "2023.5.0" + +var ( + rpmOstreeContainerStorageRebaseChecked sync.Once + rpmOstreeContainerStorageRebaseSupported bool +) + // RpmOstreeClient provides all RpmOstree related methods in one structure. // This structure implements DeploymentClient // @@ -192,6 +212,37 @@ func (r *RpmOstreeClient) IsNewEnoughForLayering() (bool, error) { return false, nil } +// normalizeCalVerForSemver adapts rpm-ostree's CalVer-style version strings +// (e.g. "2023.3") to the dotted-tri format go-semver requires (e.g. "2023.3.0") +// by appending a zero patch component when only two are present. Strings that +// already have three (or more) dot-separated components are left untouched, +// and any resulting parse failure is handled by the caller. +func normalizeCalVerForSemver(version string) string { + if strings.Count(version, ".") == 1 { + return version + ".0" + } + return version +} + +// NOTE: This is a temporary workaround for boot images predating rpm-ostree +// v2023.5. Remove once 4.13 and 4.14 boot images are no longer supported. +func (r *RpmOstreeClient) SupportsContainerStorageRebase() bool { + rpmOstreeContainerStorageRebaseChecked.Do(func() { + verdata, err := r.rpmOstreeVersion() + if err != nil { + klog.Errorf("failed to get rpm-ostree version: %v", err) + return + } + hostVersion, err := semver.NewVersion(normalizeCalVerForSemver(verdata.Version)) + if err != nil { + klog.Errorf("failed to parse rpm-ostree version %q: %v", verdata.Version, err) + return + } + rpmOstreeContainerStorageRebaseSupported = hostVersion.Compare(*semver.New(minRpmOstreeVersionForContainerStorageRebase)) >= 0 + }) + return rpmOstreeContainerStorageRebaseSupported +} + // RebaseLayered rebases system or errors if already rebased. func (r *RpmOstreeClient) RebaseLayered(imgURL string) error { // Try to re-link the merged pull secrets if they exist, since it could have been populated without a daemon reboot diff --git a/pkg/daemon/rpm-ostree_test.go b/pkg/daemon/rpm-ostree_test.go index 31a8a10dc9..7d2220de38 100644 --- a/pkg/daemon/rpm-ostree_test.go +++ b/pkg/daemon/rpm-ostree_test.go @@ -1,6 +1,9 @@ package daemon import ( + "errors" + "fmt" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -25,3 +28,89 @@ func TestParseVersion(t *testing.T) { assert.Contains(t, q.Root.Features, "rust") assert.NotContains(t, q.Root.Features, "container") } + +// rpmOstreeVersionYAML builds a fake `rpm-ostree --version` YAML payload for +// the given Version field, mirroring real-world output. +func rpmOstreeVersionYAML(version string) []byte { + return []byte(fmt.Sprintf(`rpm-ostree: + Version: '%s' + Git: 6b302116c969397fd71899e3b9bb3b8c100d1af9 + Features: + - container +`, version)) +} + +func TestSupportsContainerStorageRebase(t *testing.T) { + tests := []struct { + name string + output []byte + cmdErr error + expected bool + }{ + { + // The exact version shipped in both the 4.13 and 4.14 RHCOS boot + // images (confirmed via their commitmeta.json rpmdb.pkglist: + // rpm-ostree-2023.3-1.el9_2 and rpm-ostree-2023.3-2.el9_2 + // respectively), which reproduces OCPBUGS-86768. + name: "4.13/4.14 shipped version is not new enough", + output: rpmOstreeVersionYAML("2023.3"), + expected: false, + }, + { + name: "well below the fixed version", + output: rpmOstreeVersionYAML("2022.10"), + expected: false, + }, + { + name: "exactly the fixed version", + output: rpmOstreeVersionYAML("2023.5"), + expected: true, + }, + { + name: "above the fixed version", + output: rpmOstreeVersionYAML("2023.6"), + expected: true, + }, + { + name: "unparseable version defaults to not supported", + output: rpmOstreeVersionYAML("not-a-version"), + expected: false, + }, + { + name: "command execution failure defaults to not supported", + cmdErr: errors.New("rpm-ostree: command not found"), + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // SupportsContainerStorageRebase caches its result behind a + // package-level sync.Once (matching podmanSupportsSigstore and + // skopeoVersionSupportsMultiArchSigstore); reset both the guard + // and the cached value so each case is independently evaluated. + rpmOstreeContainerStorageRebaseChecked = sync.Once{} + rpmOstreeContainerStorageRebaseSupported = false + + mock := &MockCommandRunner{ + outputs: map[string][]byte{}, + errors: map[string]error{}, + } + if tt.cmdErr != nil { + mock.errors["rpm-ostree --version"] = tt.cmdErr + } else { + mock.outputs["rpm-ostree --version"] = tt.output + } + + r := &RpmOstreeClient{commandRunner: mock} + assert.Equal(t, tt.expected, r.SupportsContainerStorageRebase()) + }) + } +} + +func TestNormalizeCalVerForSemver(t *testing.T) { + assert.Equal(t, "2023.3.0", normalizeCalVerForSemver("2023.3")) + assert.Equal(t, "2023.5.0", normalizeCalVerForSemver("2023.5")) + // Already dotted-tri (or with more components): left untouched. + assert.Equal(t, "2023.5.1", normalizeCalVerForSemver("2023.5.1")) +} diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index d122d341b0..05940e85e8 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -2872,7 +2872,10 @@ func (dn *Daemon) updateLayeredOS(config *mcfgv1.MachineConfig) error { } // If the host isn't new enough to understand the new container model natively, run as a privileged container. // See https://github.com/coreos/rpm-ostree/pull/3961 and https://issues.redhat.com/browse/MCO-356 - if !newEnough { + // If rpm-ostree is < 2023.5, it has a skopeo-proxy sandboxing bug when rebasing + // from containers-storage or registry sources; run as a privileged container instead. + // See https://redhat.atlassian.net/browse/OCPBUGS-86768 (temporary until 4.13/4.14 boot images are unsupported). + if !newEnough || !dn.NodeUpdaterClient.SupportsContainerStorageRebase() { logSystem("rpm-ostree is not new enough for layering; forcing an update via container") return dn.InplaceUpdateViaNewContainer(newURL) }