Skip to content

Commit e665d33

Browse files
author
Per Goncalves da Silva
committed
fixes
Signed-off-by: Per Goncalves da Silva <[email protected]>
1 parent 467a02c commit e665d33

File tree

7 files changed

+174
-9
lines changed

7 files changed

+174
-9
lines changed

‎catalogd/cmd/catalogd/main.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ import (
6363
"github.com/operator-framework/operator-controller/catalogd/internal/storage"
6464
"github.com/operator-framework/operator-controller/catalogd/internal/version"
6565
"github.com/operator-framework/operator-controller/catalogd/internal/webhook"
66-
"github.com/operator-framework/operator-controller/internal/util"
66+
"github.com/operator-framework/operator-controller/internal/fsutil"
6767
)
6868

6969
var (
@@ -258,7 +258,7 @@ func main() {
258258
systemNamespace = podNamespace()
259259
}
260260

261-
if err := util.EnsureEmptyDirectory(cacheDir, 0700); err != nil {
261+
if err := fsutil.EnsureEmptyDirectory(cacheDir, 0700); err != nil {
262262
setupLog.Error(err, "unable to ensure empty cache directory")
263263
os.Exit(1)
264264
}

‎catalogd/internal/source/containers_image.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ import (
3030
"sigs.k8s.io/controller-runtime/pkg/reconcile"
3131

3232
catalogdv1 "github.com/operator-framework/operator-controller/catalogd/api/v1"
33+
"github.com/operator-framework/operator-controller/internal/fsutil"
3334
"github.com/operator-framework/operator-controller/internal/rukpak/source"
34-
"github.com/operator-framework/operator-controller/internal/util"
3535
)
3636

3737
const ConfigDirLabel = "operators.operatorframework.io.index.configs.v1"
@@ -297,7 +297,7 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st
297297
return wrapTerminal(fmt.Errorf("catalog image is missing the required label %q", ConfigDirLabel), specIsCanonical)
298298
}
299299

300-
if err := util.EnsureEmptyDirectory(unpackPath, 0700); err != nil {
300+
if err := fsutil.EnsureEmptyDirectory(unpackPath, 0700); err != nil {
301301
return fmt.Errorf("error ensuring empty unpack directory: %w", err)
302302
}
303303
l := log.FromContext(ctx)

‎cmd/operator-controller/main.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,12 @@ import (
6363
"github.com/operator-framework/operator-controller/internal/controllers"
6464
"github.com/operator-framework/operator-controller/internal/features"
6565
"github.com/operator-framework/operator-controller/internal/finalizers"
66+
"github.com/operator-framework/operator-controller/internal/fsutil"
6667
"github.com/operator-framework/operator-controller/internal/httputil"
6768
"github.com/operator-framework/operator-controller/internal/resolve"
6869
"github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety"
6970
"github.com/operator-framework/operator-controller/internal/rukpak/source"
7071
"github.com/operator-framework/operator-controller/internal/scheme"
71-
"github.com/operator-framework/operator-controller/internal/util"
7272
"github.com/operator-framework/operator-controller/internal/version"
7373
)
7474

@@ -300,7 +300,7 @@ func main() {
300300
}
301301
}
302302

303-
if err := util.EnsureEmptyDirectory(cachePath, 0700); err != nil {
303+
if err := fsutil.EnsureEmptyDirectory(cachePath, 0700); err != nil {
304304
setupLog.Error(err, "unable to ensure empty cache directory")
305305
os.Exit(1)
306306
}

‎internal/fsutil/helpers_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package fsutil_test
2+
3+
import (
4+
"github.com/stretchr/testify/require"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
)
9+
10+
func TestEnsureEmptyDirectory(t *testing.T) {
11+
tempDir := t.TempDir()
12+
dirPath := filepath.Join(tempDir, "testdir")
13+
14+
// Ensure directory is created if not exists
15+
require.NoError(t, EnsureEmptyDirectory(dirPath, 0755))
16+
17+
stat, err := os.Stat(dirPath)
18+
require.NoError(t, err)
19+
require.True(t, stat.IsDir())
20+
21+
// Create files inside directory
22+
file := filepath.Join(dirPath, "file1")
23+
require.NoError(t, os.WriteFile(file, []byte("test"), 0644))
24+
25+
subDir := filepath.Join(dirPath, "subdir")
26+
require.NoError(t, os.Mkdir(subDir, 0755))
27+
28+
// Ensure directory is emptied but not deleted
29+
require.NoError(t, EnsureEmptyDirectory(dirPath, 0755))
30+
31+
entries, err := os.ReadDir(dirPath)
32+
require.NoError(t, err)
33+
require.Empty(t, entries)
34+
}

‎internal/rukpak/source/containers_image.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
"sigs.k8s.io/controller-runtime/pkg/log"
2626
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2727

28-
"github.com/operator-framework/operator-controller/internal/util"
28+
"github.com/operator-framework/operator-controller/internal/fsutil"
2929
)
3030

3131
var insecurePolicy = []byte(`{"default":[{"type":"insecureAcceptAnything"}]}`)
@@ -266,7 +266,7 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st
266266
}
267267
}()
268268

269-
if err := util.EnsureEmptyDirectory(unpackPath, 0700); err != nil {
269+
if err := fsutil.EnsureEmptyDirectory(unpackPath, 0700); err != nil {
270270
return fmt.Errorf("error ensuring empty unpack directory: %w", err)
271271
}
272272
l := log.FromContext(ctx)

‎internal/rukpak/source/containers_image_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ func TestUnpackUnexpectedFile(t *testing.T) {
286286
require.True(t, stat.IsDir())
287287

288288
// Unset read-only to allow cleanup
289-
require.NoError(t, source.UnsetReadOnlyRecursive(unpackPath))
289+
require.NoError(t, source.SetWritableRecursive(unpackPath))
290290
}
291291

292292
func TestUnpackCopySucceedsMountFails(t *testing.T) {

‎internal/rukpak/source/helpers_test.go‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
package source_test
2+
3+
import (
4+
"errors"
5+
"github.com/operator-framework/operator-controller/internal/rukpak/source"
6+
"github.com/stretchr/testify/require"
7+
"os"
8+
"path/filepath"
9+
"testing"
10+
)
11+
12+
func TestSetReadOnlyRecursive(t *testing.T) {
13+
// Create a nested directory structure that contains a file and sym. link
14+
tempDir := t.TempDir()
15+
nestedDir := filepath.Join(tempDir, "nested")
16+
filePath := filepath.Join(nestedDir, "testfile")
17+
symlinkPath := filepath.Join(nestedDir, "symlink")
18+
19+
require.NoError(t, os.Mkdir(nestedDir, source.OwnerWritableDirMode))
20+
require.NoError(t, os.WriteFile(filePath, []byte("test"), source.OwnerWritableFileMode))
21+
require.NoError(t, os.Symlink(filePath, symlinkPath))
22+
23+
// Set directory structure as read-only
24+
require.NoError(t, source.SetReadOnlyRecursive(tempDir))
25+
26+
// Check file perm
27+
stat, err := os.Stat(filePath)
28+
require.NoError(t, err)
29+
require.Equal(t, source.OwnerReadOnlyFileMode, stat.Mode().Perm())
30+
31+
// Check directory perm
32+
nestedStat, err := os.Stat(nestedDir)
33+
require.NoError(t, err)
34+
require.Equal(t, source.OwnerReadOnlyDirMode, nestedStat.Mode().Perm())
35+
36+
// Check sym. link perm - should not be affected
37+
symlinkStat, err := os.Lstat(symlinkPath)
38+
require.NoError(t, err)
39+
require.True(t, symlinkStat.Mode()&os.ModeSymlink != 0)
40+
41+
// Make directory writable to enable test clean-up
42+
require.NoError(t, source.SetWritableRecursive(tempDir))
43+
}
44+
45+
func TestSetWritableRecursive(t *testing.T) {
46+
// Create a nested directory structure that contains a file and sym. link
47+
tempDir := t.TempDir()
48+
nestedDir := filepath.Join(tempDir, "nested")
49+
filePath := filepath.Join(nestedDir, "testfile")
50+
symlinkPath := filepath.Join(nestedDir, "symlink")
51+
52+
// nested dir mode is writable while we stamp out the other filesystem elements
53+
require.NoError(t, os.Mkdir(nestedDir, source.OwnerWritableDirMode))
54+
require.NoError(t, os.WriteFile(filePath, []byte("test"), source.OwnerReadOnlyFileMode))
55+
require.NoError(t, os.Symlink(filePath, symlinkPath))
56+
57+
// Now, set nested dir as read-only
58+
require.NoError(t, os.Chmod(nestedDir, source.OwnerReadOnlyDirMode))
59+
60+
// Set directory structure as writable
61+
require.NoError(t, source.SetWritableRecursive(tempDir))
62+
63+
// Check file perm
64+
stat, err := os.Stat(filePath)
65+
require.NoError(t, err)
66+
require.Equal(t, source.OwnerWritableFileMode, stat.Mode().Perm())
67+
68+
// Check directory perm
69+
nestedStat, err := os.Stat(nestedDir)
70+
require.NoError(t, err)
71+
require.Equal(t, source.OwnerWritableDirMode, nestedStat.Mode().Perm())
72+
73+
// Check sym link perm - should not be affected
74+
symlinkStat, err := os.Lstat(symlinkPath)
75+
require.NoError(t, err)
76+
require.True(t, symlinkStat.Mode()&os.ModeSymlink != 0)
77+
}
78+
79+
func TestDeleteReadOnlyRecursive(t *testing.T) {
80+
// Create a nested directory structure
81+
tempDir := t.TempDir()
82+
nestedDir := filepath.Join(tempDir, "nested")
83+
filePath := filepath.Join(nestedDir, "testfile")
84+
85+
require.NoError(t, os.Mkdir(nestedDir, source.OwnerWritableDirMode))
86+
require.NoError(t, os.WriteFile(filePath, []byte("test"), source.OwnerReadOnlyFileMode))
87+
require.NoError(t, os.Chmod(nestedDir, source.OwnerReadOnlyDirMode))
88+
89+
// Set directory structure as read-only
90+
require.NoError(t, source.DeleteReadOnlyRecursive(tempDir))
91+
92+
// Ensure directory is gone
93+
_, err := os.Stat(tempDir)
94+
require.True(t, errors.Is(err, os.ErrNotExist))
95+
}
96+
97+
func TestIsImageUnpacked(t *testing.T) {
98+
tempDir := t.TempDir()
99+
unpackPath := filepath.Join(tempDir, "myimage")
100+
101+
// Test case: unpack path does not exist
102+
unpacked, modTime, err := source.IsImageUnpacked(unpackPath)
103+
require.NoError(t, err)
104+
require.False(t, unpacked)
105+
require.True(t, modTime.IsZero())
106+
107+
// Test case: unpack path points to file
108+
require.NoError(t, os.WriteFile(unpackPath, []byte("test"), source.OwnerWritableFileMode))
109+
110+
unpacked, modTime, err = source.IsImageUnpacked(filepath.Join(tempDir, "myimage"))
111+
require.NoError(t, err)
112+
require.False(t, unpacked)
113+
require.True(t, modTime.IsZero())
114+
115+
// Expect file to be deleted
116+
_, err = os.Stat(unpackPath)
117+
require.True(t, errors.Is(err, os.ErrNotExist))
118+
119+
// Test case: unpack path points to directory (happy path)
120+
require.NoError(t, os.Mkdir(unpackPath, source.OwnerWritableDirMode))
121+
122+
unpacked, modTime, err = source.IsImageUnpacked(unpackPath)
123+
require.NoError(t, err)
124+
require.True(t, unpacked)
125+
require.False(t, modTime.IsZero())
126+
127+
// Expect unpack time to match directory mod time
128+
stat, err := os.Stat(unpackPath)
129+
require.NoError(t, err)
130+
require.Equal(t, stat.ModTime(), modTime)
131+
}

0 commit comments

Comments
 (0)