Merge pull request #5345 from pablintino/ocpbugs-62714

OCPBUGS-62714: Temporary policy.json for PIS rpm-ostree rebasing

Author: openshift-merge-bot[bot]

pkg/daemon/command_runner.go

@@ -0,0 +1,64 @@
+package daemon
+
+import (
+	"fmt"
+	"os/exec"
+	"strings"
+
+	"k8s.io/klog/v2"
+)
+
+// CommandRunner abstracts command execution for testing and flexibility.
+type CommandRunner interface {
+	// RunGetOut executes a command and returns its stdout output.
+	// On error, stderr is included in the error message.
+	RunGetOut(command string, args ...string) ([]byte, error)
+}
+
+// CommandRunnerOS is the production implementation that executes real OS commands.
+type CommandRunnerOS struct{}
+
+// RunGetOut executes a command, logs it, and returns stdout.
+// On error, stderr is included in the error message (truncated to 256 chars).
+func (r *CommandRunnerOS) RunGetOut(command string, args ...string) ([]byte, error) {
+	klog.Infof("Running captured: %s %s", command, strings.Join(args, " "))
+	cmd := exec.Command(command, args...)
+	rawOut, err := cmd.Output()
+	if err != nil {
+		errtext := ""
+		if e, ok := err.(*exec.ExitError); ok {
+			// Trim to max of 256 characters
+			errtext = fmt.Sprintf("\n%s", truncate(string(e.Stderr), 256))
+		}
+		return nil, fmt.Errorf("error running %s %s: %s%s", command, strings.Join(args, " "), err, errtext)
+	}
+	return rawOut, nil
+}
+
+// TODO: Delete this function to always consume the interface instance
+// Tracking story: https://issues.redhat.com/browse/MCO-1924
+//
+//	Conserved the old signature to avoid a big footprint bugfix with this change
+func runGetOut(command string, args ...string) ([]byte, error) {
+	return (&CommandRunnerOS{}).RunGetOut(command, args...)
+}
+
+// MockCommandRunner is a test implementation that returns pre-configured outputs.
+type MockCommandRunner struct {
+	outputs map[string][]byte
+	errors  map[string]error
+}
+
+// RunGetOut returns pre-configured output or error based on the command string.
+// It matches commands using "command arg1 arg2..." as the key.
+// Returns an error if no matching output or error is found.
+func (m *MockCommandRunner) RunGetOut(command string, args ...string) ([]byte, error) {
+	key := command + " " + strings.Join(args, " ")
+	if out, ok := m.outputs[key]; ok {
+		return out, nil
+	}
+	if err, ok := m.errors[key]; ok {
+		return nil, err
+	}
+	return nil, fmt.Errorf("no output for command %s found", command)
+}

pkg/daemon/command_runner_test.go

@@ -0,0 +1,93 @@
+package daemon
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCommandRunnerOS_RunGetOut(t *testing.T) {
+	runner := &CommandRunnerOS{}
+
+	// Test successful command with no output
+	o, err := runner.RunGetOut("true")
+	assert.Nil(t, err)
+	assert.Equal(t, len(o), 0)
+
+	// Test failed command
+	o, err = runner.RunGetOut("false")
+	assert.NotNil(t, err)
+
+	// Test successful command with output
+	o, err = runner.RunGetOut("echo", "hello")
+	assert.Nil(t, err)
+	assert.Equal(t, string(o), "hello\n")
+
+	// Test command that outputs to stderr and exits with error
+	// base64 encode "oops" so we can't match on the command arguments
+	o, err = runner.RunGetOut("/bin/sh", "-c", "echo hello; echo b29vcwo== | base64 -d 1>&2; exit 1")
+	assert.Error(t, err)
+	errtext := err.Error()
+	assert.Contains(t, errtext, "exit status 1\nooos\n")
+
+	// Test command that doesn't exist
+	o, err = runner.RunGetOut("/usr/bin/test-failure-to-exec-this-should-not-exist", "arg")
+	assert.Error(t, err)
+}
+
+func TestMockCommandRunner_RunGetOut(t *testing.T) {
+	// Test mock with pre-configured output
+	mock := &MockCommandRunner{
+		outputs: map[string][]byte{
+			"echo hello":        []byte("hello\n"),
+			"rpm-ostree status": []byte("State: idle\n"),
+		},
+		errors: map[string]error{},
+	}
+
+	o, err := mock.RunGetOut("echo", "hello")
+	assert.Nil(t, err)
+	assert.Equal(t, []byte("hello\n"), o)
+
+	o, err = mock.RunGetOut("rpm-ostree", "status")
+	assert.Nil(t, err)
+	assert.Equal(t, []byte("State: idle\n"), o)
+
+	// Test mock with pre-configured error
+	mock = &MockCommandRunner{
+		outputs: map[string][]byte{},
+		errors: map[string]error{
+			"cmd --fail": assert.AnError,
+		},
+	}
+
+	o, err = mock.RunGetOut("cmd", "--fail")
+	assert.Error(t, err)
+	assert.Equal(t, assert.AnError, err)
+
+	// Test mock with no matching command (should return error)
+	mock = &MockCommandRunner{
+		outputs: map[string][]byte{},
+		errors:  map[string]error{},
+	}
+
+	o, err = mock.RunGetOut("unknown", "command")
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "no output for command")
+}
+
+func TestMockCommandRunner_Priority(t *testing.T) {
+	// Test that outputs take priority over errors
+	mock := &MockCommandRunner{
+		outputs: map[string][]byte{
+			"test cmd": []byte("output\n"),
+		},
+		errors: map[string]error{
+			"test cmd": assert.AnError,
+		},
+	}
+
+	o, err := mock.RunGetOut("test", "cmd")
+	assert.Nil(t, err)
+	assert.Equal(t, []byte("output\n"), o)
+}

pkg/daemon/daemon.go

@@ -165,6 +165,12 @@ type Daemon struct {
 
 	// Ensures that only a single syncOSImagePullSecrets call can run at a time.
 	osImageMux *sync.Mutex
+
+	// Abstraction for running commands against the OS
+	cmdRunner CommandRunner
+
+	// Bare minimal podman client
+	podmanInterface PodmanInterface
 }
 
 // CoreOSDaemon protects the methods that should only be called on CoreOS variants
@@ -307,10 +313,11 @@ func New(
 	}
 
 	var nodeUpdaterClient *RpmOstreeClient
-
+	cmdRunner := &CommandRunnerOS{}
+	podmanInterface := NewPodmanExec(cmdRunner)
 	// Only pull the osImageURL from OSTree when we are on RHCOS or FCOS
 	if hostos.IsCoreOSVariant() {
-		nodeUpdaterClientVal := NewNodeUpdaterClient()
+		nodeUpdaterClientVal := NewNodeUpdaterClient(cmdRunner, podmanInterface)
 		nodeUpdaterClient = &nodeUpdaterClientVal
 		err := nodeUpdaterClient.Initialize()
 		if err != nil {
@@ -348,6 +355,8 @@ func New(
 		configDriftMonitor:     NewConfigDriftMonitor(),
 		osImageMux:             &sync.Mutex{},
 		irreconcilableReporter: NewNoOpIrreconcilableReporterImpl(),
+		cmdRunner:              cmdRunner,
+		podmanInterface:        podmanInterface,
 	}, nil
 }
 

pkg/daemon/image_manager_helper.go

@@ -5,13 +5,8 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"os/exec"
 	"path/filepath"
-	"strings"
-	"time"
 
-	"github.com/opencontainers/go-digest"
-	pivotutils "github.com/openshift/machine-config-operator/pkg/daemon/pivot/utils"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2"
 )
@@ -33,76 +28,13 @@ const (
 
 type imageSystem string
 
-// imageInspection is a public implementation of
-// https://github.com/containers/skopeo/blob/82186b916faa9c8c70cfa922229bafe5ae024dec/cmd/skopeo/inspect.go#L20-L31
-type imageInspection struct {
-	Name          string `json:",omitempty"`
-	Tag           string `json:",omitempty"`
-	Digest        digest.Digest
-	RepoDigests   []string
-	Created       *time.Time
-	DockerVersion string
-	Labels        map[string]string
-	Architecture  string
-	Os            string
-	Layers        []string
-}
-
 // BootedImageInfo stores MCO interested bootec image info
 type BootedImageInfo struct {
 	OSImageURL   string
 	ImageVersion string
 	BaseChecksum string
 }
 
-func podmanInspect(imgURL string) (imgdata *imageInspection, err error) {
-	// Pull the container image if not already available
-	var authArgs []string
-	if _, err := os.Stat(kubeletAuthFile); err == nil {
-		authArgs = append(authArgs, "--authfile", kubeletAuthFile)
-	}
-	args := []string{"pull", "-q"}
-	args = append(args, authArgs...)
-	args = append(args, imgURL)
-	_, err = pivotutils.RunExt(numRetriesNetCommands, "podman", args...)
-	if err != nil {
-		return
-	}
-
-	inspectArgs := []string{"inspect", "--type=image"}
-	inspectArgs = append(inspectArgs, fmt.Sprintf("%s", imgURL))
-	var output []byte
-	output, err = runGetOut("podman", inspectArgs...)
-	if err != nil {
-		return
-	}
-	var imagedataArray []imageInspection
-	err = json.Unmarshal(output, &imagedataArray)
-	if err != nil {
-		err = fmt.Errorf("unmarshaling podman inspect: %w", err)
-		return
-	}
-	imgdata = &imagedataArray[0]
-	return
-
-}
-
-// runGetOut executes a command, logging it, and return the stdout output.
-func runGetOut(command string, args ...string) ([]byte, error) {
-	klog.Infof("Running captured: %s %s", command, strings.Join(args, " "))
-	cmd := exec.Command(command, args...)
-	rawOut, err := cmd.Output()
-	if err != nil {
-		errtext := ""
-		if e, ok := err.(*exec.ExitError); ok {
-			// Trim to max of 256 characters
-			errtext = fmt.Sprintf("\n%s", truncate(string(e.Stderr), 256))
-		}
-		return nil, fmt.Errorf("error running %s %s: %s%s", command, strings.Join(args, " "), err, errtext)
-	}
-	return rawOut, nil
-}
-
 // truncate a string using runes/codepoints as limits.
 // This specifically will avoid breaking a UTF-8 value.
 func truncate(input string, limit int) string {

pkg/daemon/podman.go

@@ -0,0 +1,89 @@
+package daemon
+
+import (
+	"encoding/json"
+	"fmt"
+)
+
+// PodmanStorageConfig contains storage configuration from Podman.
+type PodmanStorageConfig struct {
+	GraphDriverName string `json:"graphDriverName"`
+	GraphRoot       string `json:"graphRoot"`
+}
+
+// PodmanInfo contains system information from Podman.
+type PodmanInfo struct {
+	Store PodmanStorageConfig `json:"store"`
+}
+
+// PodmanImageInfo contains image metadata from Podman.
+type PodmanImageInfo struct {
+	ID          string   `json:"Id"`
+	Digest      string   `json:"Digest"`
+	RepoDigests []string `json:"RepoDigests"`
+	RepoDigest  string   `json:"-"` // Filled with matching digest from RepoDigests
+}
+
+// PodmanInterface abstracts podman operations for testing and flexibility.
+type PodmanInterface interface {
+	// GetPodmanImageInfoByReference retrieves image info for the given reference.
+	// Returns nil if no image is found.
+	GetPodmanImageInfoByReference(reference string) (*PodmanImageInfo, error)
+	// GetPodmanInfo retrieves Podman system information.
+	GetPodmanInfo() (*PodmanInfo, error)
+}
+
+// PodmanExecInterface is the production implementation that executes real podman commands.
+type PodmanExecInterface struct {
+	cmdRunner CommandRunner
+}
+
+// NewPodmanExec creates a new PodmanExecInterface with the given command runner.
+func NewPodmanExec(commandRunner CommandRunner) *PodmanExecInterface {
+	return &PodmanExecInterface{cmdRunner: commandRunner}
+}
+
+// GetPodmanImageInfoByReference retrieves image info for the given reference.
+// It executes 'podman images --format=json --filter reference=<reference>'.
+// Returns nil if no image is found.
+// The RepoDigest field is populated if the reference matches one of the RepoDigests.
+func (p *PodmanExecInterface) GetPodmanImageInfoByReference(reference string) (*PodmanImageInfo, error) {
+	output, err := p.cmdRunner.RunGetOut("podman", "images", "--format=json", "--filter", "reference="+reference)
+	if err != nil {
+		return nil, err
+	}
+	var podmanInfos []PodmanImageInfo
+	if err := json.Unmarshal(output, &podmanInfos); err != nil {
+		return nil, fmt.Errorf("failed to decode podman image ls output: %v", err)
+	}
+	if len(podmanInfos) == 0 {
+
+		return nil, nil
+	}
+
+	info := &podmanInfos[0]
+	// Fill the custom RepoDigest field with the digest that matches the
+	// requested reference as it's convenient to be used by the caller
+	for _, digest := range info.RepoDigests {
+		if digest == reference {
+			info.RepoDigest = digest
+			break
+		}
+	}
+
+	return info, nil
+}
+
+// GetPodmanInfo retrieves Podman system information.
+// It executes 'podman system info --format=json'.
+func (p *PodmanExecInterface) GetPodmanInfo() (*PodmanInfo, error) {
+	output, err := p.cmdRunner.RunGetOut("podman", "system", "info", "--format=json")
+	if err != nil {
+		return nil, err
+	}
+	var podmanInfo PodmanInfo
+	if err := json.Unmarshal(output, &podmanInfo); err != nil {
+		return nil, fmt.Errorf("failed to decode podman system info output: %v", err)
+	}
+	return &podmanInfo, nil
+}