Skip to content
Closed
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
6 changes: 0 additions & 6 deletions .bingo/Variables.mk
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,6 @@ $(CRD_REF_DOCS): $(BINGO_DIR)/crd-ref-docs.mod
@echo "(re)installing $(GOBIN)/crd-ref-docs-v0.1.0"
@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=crd-ref-docs.mod -o=$(GOBIN)/crd-ref-docs-v0.1.0 "github.com/elastic/crd-ref-docs"

GINKGO := $(GOBIN)/ginkgo-v2.22.2
$(GINKGO): $(BINGO_DIR)/ginkgo.mod
@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
@echo "(re)installing $(GOBIN)/ginkgo-v2.22.2"
@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=ginkgo.mod -o=$(GOBIN)/ginkgo-v2.22.2 "github.com/onsi/ginkgo/v2/ginkgo"

GOLANGCI_LINT := $(GOBIN)/golangci-lint-v1.63.4
$(GOLANGCI_LINT): $(BINGO_DIR)/golangci-lint.mod
@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
Expand Down
7 changes: 0 additions & 7 deletions .bingo/ginkgo.mod

This file was deleted.

8 changes: 0 additions & 8 deletions .bingo/ginkgo.sum

This file was deleted.

2 changes: 0 additions & 2 deletions .bingo/variables.env
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ CRD_DIFF="${GOBIN}/crd-diff-v0.1.0"

CRD_REF_DOCS="${GOBIN}/crd-ref-docs-v0.1.0"

GINKGO="${GOBIN}/ginkgo-v2.22.2"

GOLANGCI_LINT="${GOBIN}/golangci-lint-v1.63.4"

GORELEASER="${GOBIN}/goreleaser-v1.26.2"
Expand Down
6 changes: 4 additions & 2 deletions catalogd/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ TESTDATA_DIR := testdata
CATALOGD_NAMESPACE := olmv1-system
KIND_CLUSTER_IMAGE := kindest/node:v1.30.0@sha256:047357ac0cfea04663786a612ba1eaba9702bef25227a794b52890dd8bcd692e

GINKGO := go run github.com/onsi/ginkgo/v2/ginkgo

##@ General

# The help target prints out all targets with their descriptions organized
Expand Down Expand Up @@ -76,7 +78,7 @@ FOCUS := $(if $(TEST),-v -focus "$(TEST)")
ifeq ($(origin E2E_FLAGS), undefined)
E2E_FLAGS :=
endif
test-e2e: $(GINKGO) ## Run the e2e tests on existing cluster
test-e2e: ## Run the e2e tests on existing cluster
$(GINKGO) $(E2E_FLAGS) -trace -vv $(FOCUS) test/e2e

e2e: KIND_CLUSTER_NAME := catalogd-e2e
Expand Down Expand Up @@ -105,7 +107,7 @@ run-latest-release:
cd ..; curl -L -s https://github.com/operator-framework/operator-controller/releases/latest/download/install.sh | bash -s

.PHONY: post-upgrade-checks
post-upgrade-checks: $(GINKGO)
post-upgrade-checks:
$(GINKGO) $(E2E_FLAGS) -trace -vv $(FOCUS) test/upgrade

##@ Build
Expand Down
4 changes: 2 additions & 2 deletions catalogd/cmd/catalogd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ import (
"github.com/operator-framework/operator-controller/catalogd/internal/storage"
"github.com/operator-framework/operator-controller/catalogd/internal/version"
"github.com/operator-framework/operator-controller/catalogd/internal/webhook"
"github.com/operator-framework/operator-controller/internal/util"
"github.com/operator-framework/operator-controller/internal/fsutil"
)

var (
Expand Down Expand Up @@ -258,7 +258,7 @@ func main() {
systemNamespace = podNamespace()
}

if err := util.EnsureEmptyDirectory(cacheDir, 0700); err != nil {
if err := fsutil.EnsureEmptyDirectory(cacheDir, 0700); err != nil {
setupLog.Error(err, "unable to ensure empty cache directory")
os.Exit(1)
}
Expand Down
4 changes: 2 additions & 2 deletions catalogd/internal/source/containers_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

catalogdv1 "github.com/operator-framework/operator-controller/catalogd/api/v1"
"github.com/operator-framework/operator-controller/internal/fsutil"
"github.com/operator-framework/operator-controller/internal/rukpak/source"
"github.com/operator-framework/operator-controller/internal/util"
)

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

if err := util.EnsureEmptyDirectory(unpackPath, 0700); err != nil {
if err := fsutil.EnsureEmptyDirectory(unpackPath, 0700); err != nil {
return fmt.Errorf("error ensuring empty unpack directory: %w", err)
}
l := log.FromContext(ctx)
Expand Down
11 changes: 11 additions & 0 deletions catalogd/test/upgrade/unpack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,17 @@ var _ = Describe("ClusterCatalog Unpacking", func() {
managerPod = managerPods.Items[0]
}).Should(Succeed())

By("Waiting for acquired leader election")
// Average case is under 1 minute but in the worst case: (previous leader crashed)
// we could have LeaseDuration (137s) + RetryPeriod (26s) +/- 163s
leaderCtx, leaderCancel := context.WithTimeout(ctx, 3*time.Minute)
defer leaderCancel()

leaderSubstrings := []string{"successfully acquired lease"}
leaderElected, err := watchPodLogsForSubstring(leaderCtx, &managerPod, "manager", leaderSubstrings...)
Expect(err).To(Succeed())
Expect(leaderElected).To(BeTrue())

By("Reading logs to make sure that ClusterCatalog was reconciled by catalogdv1")
logCtx, cancel := context.WithTimeout(ctx, time.Minute)
defer cancel()
Expand Down
4 changes: 2 additions & 2 deletions cmd/operator-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ import (
"github.com/operator-framework/operator-controller/internal/controllers"
"github.com/operator-framework/operator-controller/internal/features"
"github.com/operator-framework/operator-controller/internal/finalizers"
"github.com/operator-framework/operator-controller/internal/fsutil"
"github.com/operator-framework/operator-controller/internal/httputil"
"github.com/operator-framework/operator-controller/internal/resolve"
"github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety"
"github.com/operator-framework/operator-controller/internal/rukpak/source"
"github.com/operator-framework/operator-controller/internal/scheme"
"github.com/operator-framework/operator-controller/internal/util"
"github.com/operator-framework/operator-controller/internal/version"
)

Expand Down Expand Up @@ -300,7 +300,7 @@ func main() {
}
}

if err := util.EnsureEmptyDirectory(cachePath, 0700); err != nil {
if err := fsutil.EnsureEmptyDirectory(cachePath, 0700); err != nil {
setupLog.Error(err, "unable to ensure empty cache directory")
os.Exit(1)
}
Expand Down
6 changes: 4 additions & 2 deletions internal/util/fs.go → internal/fsutil/helpers.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package util
package fsutil

import (
"io/fs"
Expand All @@ -8,7 +8,9 @@ import (

// EnsureEmptyDirectory ensures the directory given by `path` is empty.
// If the directory does not exist, it will be created with permission bits
// given by `perm`.
// given by `perm`. If the directory exists, it will not simply rm -rf && mkdir -p
// as the calling process may not have permissions to delete the directory. E.g.
// in the case of a pod mount. Rather, it will delete the contents of the directory.
func EnsureEmptyDirectory(path string, perm fs.FileMode) error {
entries, err := os.ReadDir(path)
if err != nil && !os.IsNotExist(err) {
Expand Down
47 changes: 47 additions & 0 deletions internal/fsutil/helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package fsutil_test

import (
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"

"github.com/operator-framework/operator-controller/internal/fsutil"
)

func TestEnsureEmptyDirectory(t *testing.T) {
tempDir := t.TempDir()
dirPath := filepath.Join(tempDir, "testdir")
dirPerms := os.FileMode(0755)

t.Log("Ensure directory is created with the correct perms if it does not already exist")
require.NoError(t, fsutil.EnsureEmptyDirectory(dirPath, dirPerms))

stat, err := os.Stat(dirPath)
require.NoError(t, err)
require.True(t, stat.IsDir())
require.Equal(t, dirPerms, stat.Mode().Perm())

t.Log("Create a file inside directory")
file := filepath.Join(dirPath, "file1")
// nolint:gosec
require.NoError(t, os.WriteFile(file, []byte("test"), 0640))

t.Log("Create a sub-directory inside directory")
subDir := filepath.Join(dirPath, "subdir")
require.NoError(t, os.Mkdir(subDir, dirPerms))

t.Log("Call EnsureEmptyDirectory against directory with different permissions")
require.NoError(t, fsutil.EnsureEmptyDirectory(dirPath, 0640))

t.Log("Ensure directory is now empty")
entries, err := os.ReadDir(dirPath)
require.NoError(t, err)
require.Empty(t, entries)

t.Log("Ensure original directory permissions are unchanged")
stat, err = os.Stat(dirPath)
require.NoError(t, err)
require.Equal(t, dirPerms, stat.Mode().Perm())
}
4 changes: 2 additions & 2 deletions internal/rukpak/source/containers_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/operator-framework/operator-controller/internal/util"
"github.com/operator-framework/operator-controller/internal/fsutil"
)

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

if err := util.EnsureEmptyDirectory(unpackPath, 0700); err != nil {
if err := fsutil.EnsureEmptyDirectory(unpackPath, 0700); err != nil {
return fmt.Errorf("error ensuring empty unpack directory: %w", err)
}
l := log.FromContext(ctx)
Expand Down
2 changes: 1 addition & 1 deletion internal/rukpak/source/containers_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func TestUnpackUnexpectedFile(t *testing.T) {
require.True(t, stat.IsDir())

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

func TestUnpackCopySucceedsMountFails(t *testing.T) {
Expand Down
80 changes: 80 additions & 0 deletions internal/rukpak/source/helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package source

import (
"errors"
"fmt"
"os"
"path/filepath"
"time"
)

const (
OwnerWritableFileMode os.FileMode = 0700
OwnerWritableDirMode os.FileMode = 0700
OwnerReadOnlyFileMode os.FileMode = 0400
OwnerReadOnlyDirMode os.FileMode = 0500
)

// SetReadOnlyRecursive recursively sets files and directories under the path given by `root` as read-only
func SetReadOnlyRecursive(root string) error {
return setModeRecursive(root, OwnerReadOnlyFileMode, OwnerReadOnlyDirMode)
}

// SetWritableRecursive recursively sets files and directories under the path given by `root` as writable
func SetWritableRecursive(root string) error {
return setModeRecursive(root, OwnerWritableFileMode, OwnerWritableDirMode)
}

// DeleteReadOnlyRecursive deletes read-only directory with path given by `root`
func DeleteReadOnlyRecursive(root string) error {
if err := SetWritableRecursive(root); err != nil {
return fmt.Errorf("error making directory writable for deletion: %w", err)
}
return os.RemoveAll(root)
}

// IsImageUnpacked checks whether an image has been unpacked in `unpackPath`.
// If true, time of unpack will also be returned. If false unpack time is gibberish (zero/epoch time).
// If `unpackPath` is a file, it will be deleted and false will be returned without an error.
func IsImageUnpacked(unpackPath string) (bool, time.Time, error) {
unpackStat, err := os.Stat(unpackPath)
if errors.Is(err, os.ErrNotExist) {
return false, time.Time{}, nil
}
if err != nil {
return false, time.Time{}, err
}
if !unpackStat.IsDir() {
return false, time.Time{}, os.Remove(unpackPath)
}
return true, unpackStat.ModTime(), nil
}

func setModeRecursive(path string, fileMode os.FileMode, dirMode os.FileMode) error {
return filepath.WalkDir(path, func(path string, d os.DirEntry, err error) error {
if os.IsNotExist(err) {
return nil
}
if err != nil {
return err
}
fi, err := d.Info()
if err != nil {
return err
}

switch typ := fi.Mode().Type(); typ {
case os.ModeSymlink:
// do not follow symlinks
// 1. if they resolve to other locations in the root, we'll find them anyway
// 2. if they resolve to other locations outside the root, we don't want to change their permissions
return nil
case os.ModeDir:
return os.Chmod(path, dirMode)
case 0: // regular file
return os.Chmod(path, fileMode)
default:
return fmt.Errorf("refusing to change ownership of file %q with type %v", path, typ.String())
}
})
}
Loading
Loading