From 67e562d9880d4da5f11f7e588660b75f63d7a813 Mon Sep 17 00:00:00 2001
From: Paul Holzinger <pholzing@redhat.com>
Date: Wed, 26 Apr 2023 12:11:14 +0200
Subject: [PATCH] pkg/subscription: fix regression in saveTo()

The name is actually a relative path, so rename the field to make it
clear add add comments + test to prevent future people from making the
same mistake.

Regression was caught in podman CI:
https://github.com/containers/podman/pull/18350

Fixes d4a3de3a ("Don't call `umask` in subscriptions")

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 pkg/subscriptions/subscriptions.go      | 14 +++++++----
 pkg/subscriptions/subscriptions_test.go | 32 +++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 5 deletions(-)
 create mode 100644 pkg/subscriptions/subscriptions_test.go

diff --git a/pkg/subscriptions/subscriptions.go b/pkg/subscriptions/subscriptions.go
index c21f5ecc1..b751f4877 100644
--- a/pkg/subscriptions/subscriptions.go
+++ b/pkg/subscriptions/subscriptions.go
@@ -27,9 +27,10 @@ var (
 	UserOverrideMountsFile = filepath.Join(os.Getenv("HOME"), ".config/containers/mounts.conf")
 )
 
-// subscriptionData stores the name of the file and the content read from it
+// subscriptionData stores the relative name of the file and the content read from it
 type subscriptionData struct {
-	name    string
+	// relPath is the relative path to the file
+	relPath string
 	data    []byte
 	mode    os.FileMode
 	dirMode os.FileMode
@@ -37,10 +38,13 @@ type subscriptionData struct {
 
 // saveTo saves subscription data to given directory
 func (s subscriptionData) saveTo(dir string) error {
-	if err := umask.MkdirAllIgnoreUmask(dir, s.dirMode); err != nil {
+	// We need to join the path here and create all parent directories, only
+	// creating dir is not good enough as relPath could also contain directories.
+	path := filepath.Join(dir, s.relPath)
+	if err := umask.MkdirAllIgnoreUmask(filepath.Dir(path), s.dirMode); err != nil {
 		return fmt.Errorf("create subscription directory: %w", err)
 	}
-	if err := umask.WriteFileIgnoreUmask(filepath.Join(dir, s.name), s.data, s.mode); err != nil {
+	if err := umask.WriteFileIgnoreUmask(path, s.data, s.mode); err != nil {
 		return fmt.Errorf("write subscription data: %w", err)
 	}
 	return nil
@@ -96,7 +100,7 @@ func readFileOrDir(root, name string, parentMode os.FileMode) ([]subscriptionDat
 		return nil, err
 	}
 	return []subscriptionData{{
-		name:    name,
+		relPath: name,
 		data:    bytes,
 		mode:    s.Mode(),
 		dirMode: parentMode,
diff --git a/pkg/subscriptions/subscriptions_test.go b/pkg/subscriptions/subscriptions_test.go
new file mode 100644
index 000000000..5b3aee1af
--- /dev/null
+++ b/pkg/subscriptions/subscriptions_test.go
@@ -0,0 +1,32 @@
+package subscriptions
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestReadAllAndSaveTo(t *testing.T) {
+	const testMode = os.FileMode(0o700)
+
+	rootDir := t.TempDir()
+	childDir := filepath.Join(rootDir, "child")
+	err := os.Mkdir(childDir, testMode)
+	assert.NoError(t, err, "mkdir child")
+
+	filePath := "child/file"
+	err = os.WriteFile(filepath.Join(rootDir, filePath), []byte("test"), testMode)
+	assert.NoError(t, err, "write file")
+
+	data, err := readAll(rootDir, "", testMode)
+	assert.NoError(t, err, "readAll")
+	assert.Len(t, data, 1, "readAll should return one result")
+
+	tmpDir := t.TempDir()
+	err = data[0].saveTo(tmpDir)
+	assert.NoError(t, err, "saveTo()")
+
+	assert.FileExists(t, filepath.Join(tmpDir, filePath), "file exists at correct location")
+}