diff --git a/.github/workflows/catalogd-crd-diff.yaml b/.github/workflows/catalogd-crd-diff.yaml deleted file mode 100644 index d3c6ca099..000000000 --- a/.github/workflows/catalogd-crd-diff.yaml +++ /dev/null @@ -1,19 +0,0 @@ -name: catalogd-crd-diff -on: - pull_request: -jobs: - crd-diff: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - uses: actions/setup-go@v5 - with: - go-version-file: go.mod - - - name: Run make verify-crd-compatibility - working-directory: catalogd - run: make verify-crd-compatibility CRD_DIFF_ORIGINAL_REF=${{ github.event.pull_request.base.sha }} CRD_DIFF_UPDATED_SOURCE="git://${{ github.event.pull_request.head.sha }}?path=config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml" - diff --git a/.github/workflows/crd-diff.yaml b/.github/workflows/crd-diff.yaml index 49a1b32da..8f34947a9 100644 --- a/.github/workflows/crd-diff.yaml +++ b/.github/workflows/crd-diff.yaml @@ -14,5 +14,8 @@ jobs: go-version-file: go.mod - name: Run make verify-crd-compatibility - run: make verify-crd-compatibility CRD_DIFF_ORIGINAL_REF=${{ github.event.pull_request.base.sha }} CRD_DIFF_UPDATED_SOURCE="git://${{ github.event.pull_request.head.sha }}?path=config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml" - + run: | + make verify-crd-compatibility \ + CRD_DIFF_ORIGINAL_REF=${{ github.event.pull_request.base.sha }} \ + CRD_DIFF_UPDATED_SOURCE="git://${{ github.event.pull_request.head.sha }}?path=config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml" \ + CATALOGD_CRD_DIFF_UPDATED_SOURCE="git://${{ github.event.pull_request.head.sha }}?path=catalogd/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml" \ No newline at end of file diff --git a/.goreleaser.yml b/.goreleaser.yml index d8c5d1a64..d828849dd 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -124,6 +124,7 @@ release: disable: '{{ ne .Env.ENABLE_RELEASE_PIPELINE "true" }}' extra_files: - glob: 'operator-controller.yaml' + - glob: './catalogd/config/base/default/clustercatalogs/default-catalogs.yaml' - glob: 'install.sh' header: | ## Installation diff --git a/Makefile b/Makefile index c188e901e..5625d8696 100644 --- a/Makefile +++ b/Makefile @@ -142,9 +142,13 @@ bingo-upgrade: $(BINGO) #EXHELP Upgrade tools .PHONY: verify-crd-compatibility CRD_DIFF_ORIGINAL_REF := main CRD_DIFF_UPDATED_SOURCE := file://config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml +CATALOGD_CRD_DIFF_UPDATED_SOURCE := file://catalogd/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml CRD_DIFF_CONFIG := crd-diff-config.yaml + verify-crd-compatibility: $(CRD_DIFF) manifests $(CRD_DIFF) --config="${CRD_DIFF_CONFIG}" "git://${CRD_DIFF_ORIGINAL_REF}?path=config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml" ${CRD_DIFF_UPDATED_SOURCE} + $(CRD_DIFF) --config="${CRD_DIFF_CONFIG}" "git://${CRD_DIFF_ORIGINAL_REF}?path=catalogd/config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml" ${CATALOGD_CRD_DIFF_UPDATED_SOURCE} + .PHONY: test test: manifests generate fmt vet test-unit test-e2e #HELP Run all tests. @@ -244,9 +248,10 @@ kind-load: $(KIND) #EXHELP Loads the currently constructed images into the KIND .PHONY: kind-deploy kind-deploy: export MANIFEST := ./operator-controller.yaml +kind-deploy: export DEFAULT_CATALOG := ./catalogd/config/base/default/clustercatalogs/default-catalogs.yaml kind-deploy: manifests $(KUSTOMIZE) ($(KUSTOMIZE) build $(KUSTOMIZE_BUILD_DIR) && echo "---" && $(KUSTOMIZE) build catalogd/config/overlays/cert-manager | sed "s/cert-git-version/cert-$(VERSION)/g") > $(MANIFEST) - envsubst '$$CERT_MGR_VERSION,$$INSTALL_DEFAULT_CATALOGS,$$MANIFEST' < scripts/install.tpl.sh | bash -s + envsubst '$$DEFAULT_CATALOG,$$CERT_MGR_VERSION,$$INSTALL_DEFAULT_CATALOGS,$$MANIFEST' < scripts/install.tpl.sh | bash -s .PHONY: kind-cluster @@ -325,9 +330,10 @@ release: $(GORELEASER) #EXHELP Runs goreleaser for the operator-controller. By d .PHONY: quickstart quickstart: export MANIFEST := https://github.com/operator-framework/operator-controller/releases/download/$(VERSION)/operator-controller.yaml +quickstart: export DEFAULT_CATALOG := "https://github.com/operator-framework/operator-controller/releases/download/$(VERSION)/default-catalogs.yaml" quickstart: $(KUSTOMIZE) manifests #EXHELP Generate the unified installation release manifests and scripts. ($(KUSTOMIZE) build $(KUSTOMIZE_BUILD_DIR) && echo "---" && $(KUSTOMIZE) build catalogd/config/overlays/cert-manager) | sed "s/cert-git-version/cert-$(VERSION)/g" | sed "s/:devel/:$(VERSION)/g" > operator-controller.yaml - envsubst '$$CERT_MGR_VERSION,$$INSTALL_DEFAULT_CATALOGS,$$MANIFEST' < scripts/install.tpl.sh > install.sh + envsubst '$$DEFAULT_CATALOG,$$CERT_MGR_VERSION,$$INSTALL_DEFAULT_CATALOGS,$$MANIFEST' < scripts/install.tpl.sh > install.sh ##@ Docs diff --git a/RELEASE.md b/RELEASE.md index 9b221cbed..a7f01c00d 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -17,16 +17,25 @@ Note that throughout this guide, the `upstream` remote refers to the `operator-f The release process differs slightly based on whether a patch or major/minor release is being made. ### Patch Release -#### Step 1 -In this example we will be creating a new patch release from version `v1.2.3`, on the branch `release-v1.2`. -First ensure that the release branch has been updated on remote with the changes from the patch, then perform the following: + +In this example, we will be creating a new patch release from version `v1.2.3` on the branch `release-v1.2`. + +#### Step 1 +First, make sure the `release-v1.2` branch is updated with the latest changes from upstream: ```bash git fetch upstream release-v1.2 -git pull release-v1.2 git checkout release-v1.2 +git reset --hard upstream/release-v1.2 ``` #### Step 2 +Run the following command to confirm that your local branch has the latest expected commit: +```bash +git log --oneline -n 5 +``` +Check that the most recent commit matches the latest commit in the upstream `release-v1.2` branch. + +#### Step 3 Create a new tag, incrementing the patch number from the previous version. In this case, we'll be incrementing from `v1.2.3` to `v1.2.4`: ```bash ## Previous version was v1.2.3, so we bump the patch number up by one diff --git a/catalogd/Makefile b/catalogd/Makefile index 570eedfd0..d58367f42 100644 --- a/catalogd/Makefile +++ b/catalogd/Makefile @@ -88,19 +88,6 @@ e2e: run image-registry test-e2e kind-cluster-cleanup ## Run e2e test suite on l image-registry: ## Setup in-cluster image registry ./test/tools/imageregistry/registry.sh $(ISSUER_KIND) $(ISSUER_NAME) -.PHONY: verify-crd-compatibility -CRD_DIFF_ORIGINAL_REF := main -CRD_DIFF_UPDATED_SOURCE := file://config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml -CRD_DIFF_CONFIG := crd-diff-config.yaml -verify-crd-compatibility: $(CRD_DIFF) - @if git show ${CRD_DIFF_ORIGINAL_REF}:config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml > /dev/null 2>&1; then \ - echo "Running CRD diff..."; \ - $(CRD_DIFF) --config="${CRD_DIFF_CONFIG}" "git://${CRD_DIFF_ORIGINAL_REF}?path=config/base/crd/bases/olm.operatorframework.io_clustercatalogs.yaml" ${CRD_DIFF_UPDATED_SOURCE}; \ - else \ - echo "Skipping CRD diff: CRD does not exist in ${CRD_DIFF_ORIGINAL_REF}"; \ - fi - - ## image-registry target has to come after run-latest-release, ## because the image-registry depends on the olm-ca issuer. .PHONY: test-upgrade-e2e @@ -115,7 +102,7 @@ pre-upgrade-setup: .PHONY: run-latest-release run-latest-release: - curl -L -s https://github.com/operator-framework/operator-controller/releases/latest/download/install.sh | bash -s + 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) diff --git a/catalogd/cmd/catalogd/main.go b/catalogd/cmd/catalogd/main.go index 77698444c..91d82bedd 100644 --- a/catalogd/cmd/catalogd/main.go +++ b/catalogd/cmd/catalogd/main.go @@ -42,6 +42,7 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" "k8s.io/klog/v2" "k8s.io/klog/v2/textlogger" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" crcache "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/certwatcher" @@ -62,6 +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" ) var ( @@ -96,7 +98,7 @@ func main() { certFile string keyFile string webhookPort int - caCertDir string + pullCasDir string globalPullSecret string ) flag.StringVar(&metricsAddr, "metrics-bind-address", "", "The address for the metrics endpoint. Requires tls-cert and tls-key. (Default: ':7443')") @@ -114,7 +116,7 @@ func main() { flag.StringVar(&certFile, "tls-cert", "", "The certificate file used for serving catalog and metrics. Required to enable the metrics server. Requires tls-key.") flag.StringVar(&keyFile, "tls-key", "", "The key file used for serving catalog contents and metrics. Required to enable the metrics server. Requires tls-cert.") flag.IntVar(&webhookPort, "webhook-server-port", 9443, "The port that the mutating webhook server serves at.") - flag.StringVar(&caCertDir, "ca-certs-dir", "", "The directory of CA certificate to use for verifying HTTPS connections to image registries.") + flag.StringVar(&pullCasDir, "pull-cas-dir", "", "The directory of TLS certificate authoritiess to use for verifying HTTPS connections to image registries.") flag.StringVar(&globalPullSecret, "global-pull-secret", "", "The / of the global pull secret that is going to be used to pull bundle images.") klog.InitFlags(flag.CommandLine) @@ -231,8 +233,14 @@ func main() { HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "catalogd-operator-lock", - WebhookServer: webhookServer, - Cache: cacheOptions, + // Recommended Leader Election values + // https://github.com/openshift/enhancements/blob/61581dcd985130357d6e4b0e72b87ee35394bf6e/CONVENTIONS.md#handling-kube-apiserver-disruption + LeaseDuration: ptr.To(137 * time.Second), + RenewDeadline: ptr.To(107 * time.Second), + RetryPeriod: ptr.To(26 * time.Second), + + WebhookServer: webhookServer, + Cache: cacheOptions, }) if err != nil { setupLog.Error(err, "unable to create manager") @@ -250,8 +258,8 @@ func main() { systemNamespace = podNamespace() } - if err := os.MkdirAll(cacheDir, 0700); err != nil { - setupLog.Error(err, "unable to create cache directory") + if err := util.EnsureEmptyDirectory(cacheDir, 0700); err != nil { + setupLog.Error(err, "unable to ensure empty cache directory") os.Exit(1) } @@ -264,8 +272,8 @@ func main() { BaseCachePath: unpackCacheBasePath, SourceContextFunc: func(logger logr.Logger) (*types.SystemContext, error) { srcContext := &types.SystemContext{ - DockerCertPath: caCertDir, - OCICertPath: caCertDir, + DockerCertPath: pullCasDir, + OCICertPath: pullCasDir, } if _, err := os.Stat(authFilePath); err == nil && globalPullSecretKey != nil { logger.Info("using available authentication information for pulling image") diff --git a/catalogd/config/components/ca/patches/manager_deployment_cacerts.yaml b/catalogd/config/components/ca/patches/manager_deployment_cacerts.yaml index b5b03633e..6b0816706 100644 --- a/catalogd/config/components/ca/patches/manager_deployment_cacerts.yaml +++ b/catalogd/config/components/ca/patches/manager_deployment_cacerts.yaml @@ -6,4 +6,4 @@ value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/ca-certs/"} - op: add path: /spec/template/spec/containers/0/args/- - value: "--ca-certs-dir=/var/ca-certs" + value: "--pull-cas-dir=/var/ca-certs" diff --git a/catalogd/crd-diff-config.yaml b/catalogd/crd-diff-config.yaml deleted file mode 100644 index 8cce39378..000000000 --- a/catalogd/crd-diff-config.yaml +++ /dev/null @@ -1,109 +0,0 @@ -checks: - crd: - scope: - enabled: true - existingFieldRemoval: - enabled: true - storedVersionRemoval: - enabled: true - version: - sameVersion: - enabled: true - unhandledFailureMode: "Closed" - enum: - enabled: true - removalEnforcement: "Strict" - additionEnforcement: "Strict" - default: - enabled: true - changeEnforcement: "Strict" - removalEnforcement: "Strict" - additionEnforcement: "Strict" - required: - enabled: true - newEnforcement: "Strict" - type: - enabled: true - changeEnforcement: "Strict" - maximum: - enabled: true - additionEnforcement: "Strict" - decreaseEnforcement: "Strict" - maxItems: - enabled: true - additionEnforcement: "Strict" - decreaseEnforcement: "Strict" - maxProperties: - enabled: true - additionEnforcement: "Strict" - decreaseEnforcement: "Strict" - maxLength: - enabled: true - additionEnforcement: "Strict" - decreaseEnforcement: "Strict" - minimum: - enabled: true - additionEnforcement: "Strict" - increaseEnforcement: "Strict" - minItems: - enabled: true - additionEnforcement: "Strict" - increaseEnforcement: "Strict" - minProperties: - enabled: true - additionEnforcement: "Strict" - increaseEnforcement: "Strict" - minLength: - enabled: true - additionEnforcement: "Strict" - increaseEnforcement: "Strict" - servedVersion: - enabled: true - unhandledFailureMode: "Closed" - enum: - enabled: true - removalEnforcement: "Strict" - additionEnforcement: "Strict" - default: - enabled: true - changeEnforcement: "Strict" - removalEnforcement: "Strict" - additionEnforcement: "Strict" - required: - enabled: true - newEnforcement: "Strict" - type: - enabled: true - changeEnforcement: "Strict" - maximum: - enabled: true - additionEnforcement: "Strict" - decreaseEnforcement: "Strict" - maxItems: - enabled: true - additionEnforcement: "Strict" - decreaseEnforcement: "Strict" - maxProperties: - enabled: true - additionEnforcement: "Strict" - decreaseEnforcement: "Strict" - maxLength: - enabled: true - additionEnforcement: "Strict" - decreaseEnforcement: "Strict" - minimum: - enabled: true - additionEnforcement: "Strict" - increaseEnforcement: "Strict" - minItems: - enabled: true - additionEnforcement: "Strict" - increaseEnforcement: "Strict" - minProperties: - enabled: true - additionEnforcement: "Strict" - increaseEnforcement: "Strict" - minLength: - enabled: true - additionEnforcement: "Strict" - increaseEnforcement: "Strict" diff --git a/catalogd/internal/source/containers_image.go b/catalogd/internal/source/containers_image.go index c36536ae2..03df10f2f 100644 --- a/catalogd/internal/source/containers_image.go +++ b/catalogd/internal/source/containers_image.go @@ -30,6 +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/rukpak/source" + "github.com/operator-framework/operator-controller/internal/util" ) const ConfigDirLabel = "operators.operatorframework.io.index.configs.v1" @@ -76,12 +78,11 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, catalog *catalogdv // ////////////////////////////////////////////////////// unpackPath := i.unpackPath(catalog.Name, canonicalRef.Digest()) - if unpackStat, err := os.Stat(unpackPath); err == nil { - if !unpackStat.IsDir() { - panic(fmt.Sprintf("unexpected file at unpack path %q: expected a directory", unpackPath)) - } + if isUnpacked, unpackTime, err := source.IsImageUnpacked(unpackPath); isUnpacked && err == nil { l.Info("image already unpacked", "ref", imgRef.String(), "digest", canonicalRef.Digest().String()) - return successResult(unpackPath, canonicalRef, unpackStat.ModTime()), nil + return successResult(unpackPath, canonicalRef, unpackTime), nil + } else if err != nil { + return nil, fmt.Errorf("error checking image already unpacked: %w", err) } ////////////////////////////////////////////////////// @@ -154,7 +155,7 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, catalog *catalogdv // ////////////////////////////////////////////////////// if err := i.unpackImage(ctx, unpackPath, layoutRef, specIsCanonical, srcCtx); err != nil { - if cleanupErr := deleteRecursive(unpackPath); cleanupErr != nil { + if cleanupErr := source.DeleteReadOnlyRecursive(unpackPath); cleanupErr != nil { err = errors.Join(err, cleanupErr) } return nil, fmt.Errorf("error unpacking image: %w", err) @@ -195,7 +196,7 @@ func successResult(unpackPath string, canonicalRef reference.Canonical, lastUnpa } func (i *ContainersImageRegistry) Cleanup(_ context.Context, catalog *catalogdv1.ClusterCatalog) error { - if err := deleteRecursive(i.catalogPath(catalog.Name)); err != nil { + if err := source.DeleteReadOnlyRecursive(i.catalogPath(catalog.Name)); err != nil { return fmt.Errorf("error deleting catalog cache: %w", err) } return nil @@ -296,8 +297,8 @@ 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 := os.MkdirAll(unpackPath, 0700); err != nil { - return fmt.Errorf("error creating unpack directory: %w", err) + if err := util.EnsureEmptyDirectory(unpackPath, 0700); err != nil { + return fmt.Errorf("error ensuring empty unpack directory: %w", err) } l := log.FromContext(ctx) l.Info("unpacking image", "path", unpackPath) @@ -315,10 +316,10 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st l.Info("applied layer", "layer", i) return nil }(); err != nil { - return errors.Join(err, deleteRecursive(unpackPath)) + return errors.Join(err, source.DeleteReadOnlyRecursive(unpackPath)) } } - if err := setReadOnlyRecursive(unpackPath); err != nil { + if err := source.SetReadOnlyRecursive(unpackPath); err != nil { return fmt.Errorf("error making unpack directory read-only: %w", err) } return nil @@ -362,69 +363,13 @@ func (i *ContainersImageRegistry) deleteOtherImages(catalogName string, digestTo continue } imgDirPath := filepath.Join(catalogPath, imgDir.Name()) - if err := deleteRecursive(imgDirPath); err != nil { + if err := source.DeleteReadOnlyRecursive(imgDirPath); err != nil { return fmt.Errorf("error removing image directory: %w", err) } } return nil } -func setReadOnlyRecursive(root string) error { - if err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { - if err != nil { - return err - } - - fi, err := d.Info() - if err != nil { - return err - } - - if err := func() error { - 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, 0500) - case 0: // regular file - return os.Chmod(path, 0400) - default: - return fmt.Errorf("refusing to change ownership of file %q with type %v", path, typ.String()) - } - }(); err != nil { - return err - } - return nil - }); err != nil { - return fmt.Errorf("error making catalog cache read-only: %w", err) - } - return nil -} - -func deleteRecursive(root string) error { - if err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { - if os.IsNotExist(err) { - return nil - } - if err != nil { - return err - } - if !d.IsDir() { - return nil - } - if err := os.Chmod(path, 0700); err != nil { - return err - } - return nil - }); err != nil { - return fmt.Errorf("error making catalog cache writable for deletion: %w", err) - } - return os.RemoveAll(root) -} - func wrapTerminal(err error, isTerminal bool) error { if !isTerminal { return err diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index e45a130f7..76c0e4af4 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -40,6 +40,7 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth" "k8s.io/klog/v2" "k8s.io/klog/v2/textlogger" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" crcache "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/certwatcher" @@ -67,6 +68,7 @@ import ( "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" ) @@ -100,12 +102,14 @@ func main() { cachePath string operatorControllerVersion bool systemNamespace string - caCertDir string + catalogdCasDir string + pullCasDir string globalPullSecret string ) flag.StringVar(&metricsAddr, "metrics-bind-address", "", "The address for the metrics endpoint. Requires tls-cert and tls-key. (Default: ':8443')") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") - flag.StringVar(&caCertDir, "ca-certs-dir", "", "The directory of TLS certificate to use for verifying HTTPS connections to the Catalogd and docker-registry web servers.") + flag.StringVar(&catalogdCasDir, "catalogd-cas-dir", "", "The directory of TLS certificate authorities to use for verifying HTTPS connections to the Catalogd web service.") + flag.StringVar(&pullCasDir, "pull-cas-dir", "", "The directory of TLS certificate authorities to use for verifying HTTPS connections to image registries.") flag.StringVar(&certFile, "tls-cert", "", "The certificate file used for the metrics server. Required to enable the metrics server. Requires tls-key.") flag.StringVar(&keyFile, "tls-key", "", "The key file used for the metrics server. Required to enable the metrics server. Requires tls-cert") flag.BoolVar(&enableLeaderElection, "leader-elect", false, @@ -229,7 +233,13 @@ func main() { HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "9c4404e7.operatorframework.io", - Cache: cacheOptions, + // Recommended Leader Election values + // https://github.com/openshift/enhancements/blob/61581dcd985130357d6e4b0e72b87ee35394bf6e/CONVENTIONS.md#handling-kube-apiserver-disruption + LeaseDuration: ptr.To(137 * time.Second), + RenewDeadline: ptr.To(107 * time.Second), + RetryPeriod: ptr.To(26 * time.Second), + + Cache: cacheOptions, // LeaderElectionReleaseOnCancel defines if the leader should step down voluntarily // when the Manager ends. This requires the binary to immediately end when the // Manager is stopped, otherwise, this setting is unsafe. Setting this significantly @@ -276,7 +286,7 @@ func main() { os.Exit(1) } - certPoolWatcher, err := httputil.NewCertPoolWatcher(caCertDir, ctrl.Log.WithName("cert-pool")) + certPoolWatcher, err := httputil.NewCertPoolWatcher(catalogdCasDir, ctrl.Log.WithName("cert-pool")) if err != nil { setupLog.Error(err, "unable to create CA certificate pool") os.Exit(1) @@ -290,12 +300,17 @@ func main() { } } + if err := util.EnsureEmptyDirectory(cachePath, 0700); err != nil { + setupLog.Error(err, "unable to ensure empty cache directory") + os.Exit(1) + } + unpacker := &source.ContainersImageRegistry{ BaseCachePath: filepath.Join(cachePath, "unpack"), SourceContextFunc: func(logger logr.Logger) (*types.SystemContext, error) { srcContext := &types.SystemContext{ - DockerCertPath: caCertDir, - OCICertPath: caCertDir, + DockerCertPath: pullCasDir, + OCICertPath: pullCasDir, } if _, err := os.Stat(authFilePath); err == nil && globalPullSecretKey != nil { logger.Info("using available authentication information for pulling image") @@ -355,7 +370,7 @@ func main() { crdupgradesafety.NewPreflight(aeClient.CustomResourceDefinitions()), } - applier := &applier.Helm{ + helmApplier := &applier.Helm{ ActionClientGetter: acg, Preflights: preflights, } @@ -375,7 +390,7 @@ func main() { Client: cl, Resolver: resolver, Unpacker: unpacker, - Applier: applier, + Applier: helmApplier, InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg}, Finalizers: clusterExtensionFinalizers, Manager: cm, diff --git a/config/components/tls/patches/manager_deployment_cert.yaml b/config/components/tls/patches/manager_deployment_cert.yaml index 18afac59d..8fbdb5592 100644 --- a/config/components/tls/patches/manager_deployment_cert.yaml +++ b/config/components/tls/patches/manager_deployment_cert.yaml @@ -6,7 +6,10 @@ value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/certs/"} - op: add path: /spec/template/spec/containers/0/args/- - value: "--ca-certs-dir=/var/certs" + value: "--catalogd-cas-dir=/var/certs" +- op: add + path: /spec/template/spec/containers/0/args/- + value: "--pull-cas-dir=/var/certs" - op: add path: /spec/template/spec/containers/0/args/- value: "--tls-cert=/var/certs/tls.cert" diff --git a/go.mod b/go.mod index 0fab3290f..3a0581966 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/operator-framework/helm-operator-plugins v0.8.0 github.com/operator-framework/operator-registry v1.50.0 github.com/prometheus/client_golang v1.20.5 - github.com/spf13/pflag v1.0.5 + github.com/spf13/pflag v1.0.6 github.com/stretchr/testify v1.10.0 golang.org/x/exp v0.0.0-20241009180824-f66d83c29e7c gopkg.in/yaml.v2 v2.4.0 diff --git a/go.sum b/go.sum index 1f5d51bc8..c73ce9b99 100644 --- a/go.sum +++ b/go.sum @@ -638,8 +638,9 @@ github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= -github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= +github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.7.0/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg= github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6 h1:pnnLyeX7o/5aX8qUQ69P/mLojDqwda8hFOCBTmP/6hw= github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6/go.mod h1:39R/xuhNgVhi+K0/zst4TLrJrVmbm6LVgl4A0+ZFS5M= diff --git a/internal/httputil/certpoolwatcher.go b/internal/httputil/certpoolwatcher.go index 2a250d069..0cce70312 100644 --- a/internal/httputil/certpoolwatcher.go +++ b/internal/httputil/certpoolwatcher.go @@ -4,6 +4,8 @@ import ( "crypto/x509" "fmt" "os" + "slices" + "strings" "sync" "time" @@ -44,8 +46,26 @@ func NewCertPoolWatcher(caDir string, log logr.Logger) (*CertPoolWatcher, error) if err != nil { return nil, err } - if err = watcher.Add(caDir); err != nil { - return nil, err + + // If the SSL_CERT_DIR or SSL_CERT_FILE environment variables are + // specified, this means that we have some control over the system root + // location, thus they may change, thus we should watch those locations. + watchPaths := strings.Split(os.Getenv("SSL_CERT_DIR"), ":") + watchPaths = append(watchPaths, caDir, os.Getenv("SSL_CERT_FILE")) + watchPaths = slices.DeleteFunc(watchPaths, func(p string) bool { + if p == "" { + return true + } + if _, err := os.Stat(p); err != nil { + return true + } + return false + }) + + for _, p := range watchPaths { + if err := watcher.Add(p); err != nil { + return nil, err + } } cpw := &CertPoolWatcher{ diff --git a/internal/httputil/certpoolwatcher_test.go b/internal/httputil/certpoolwatcher_test.go index bfebebd28..2ea3f862a 100644 --- a/internal/httputil/certpoolwatcher_test.go +++ b/internal/httputil/certpoolwatcher_test.go @@ -72,6 +72,10 @@ func TestCertPoolWatcher(t *testing.T) { t.Logf("Create cert file at %q\n", certName) createCert(t, certName) + // Update environment variables for the watcher - some of these should not exist + os.Setenv("SSL_CERT_DIR", tmpDir+":/tmp/does-not-exist.dir") + os.Setenv("SSL_CERT_FILE", "/tmp/does-not-exist.file") + // Create the cert pool watcher cpw, err := httputil.NewCertPoolWatcher(tmpDir, log.FromContext(context.Background())) require.NoError(t, err) diff --git a/internal/rukpak/source/containers_image.go b/internal/rukpak/source/containers_image.go index 1981ca06a..67f0f0625 100644 --- a/internal/rukpak/source/containers_image.go +++ b/internal/rukpak/source/containers_image.go @@ -24,6 +24,8 @@ import ( "github.com/opencontainers/go-digest" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/operator-framework/operator-controller/internal/util" ) var insecurePolicy = []byte(`{"default":[{"type":"insecureAcceptAnything"}]}`) @@ -69,12 +71,11 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, bundle *BundleSour // ////////////////////////////////////////////////////// unpackPath := i.unpackPath(bundle.Name, canonicalRef.Digest()) - if unpackStat, err := os.Stat(unpackPath); err == nil { - if !unpackStat.IsDir() { - panic(fmt.Sprintf("unexpected file at unpack path %q: expected a directory", unpackPath)) - } + if isUnpacked, _, err := IsImageUnpacked(unpackPath); isUnpacked && err == nil { l.Info("image already unpacked", "ref", imgRef.String(), "digest", canonicalRef.Digest().String()) return successResult(bundle.Name, unpackPath, canonicalRef), nil + } else if err != nil { + return nil, fmt.Errorf("error checking bundle already unpacked: %w", err) } ////////////////////////////////////////////////////// @@ -147,7 +148,7 @@ func (i *ContainersImageRegistry) Unpack(ctx context.Context, bundle *BundleSour // ////////////////////////////////////////////////////// if err := i.unpackImage(ctx, unpackPath, layoutRef, srcCtx); err != nil { - if cleanupErr := deleteRecursive(unpackPath); cleanupErr != nil { + if cleanupErr := DeleteReadOnlyRecursive(unpackPath); cleanupErr != nil { err = errors.Join(err, cleanupErr) } return nil, fmt.Errorf("error unpacking image: %w", err) @@ -175,7 +176,7 @@ func successResult(bundleName, unpackPath string, canonicalRef reference.Canonic } func (i *ContainersImageRegistry) Cleanup(_ context.Context, bundle *BundleSource) error { - return deleteRecursive(i.bundlePath(bundle.Name)) + return DeleteReadOnlyRecursive(i.bundlePath(bundle.Name)) } func (i *ContainersImageRegistry) bundlePath(bundleName string) string { @@ -265,8 +266,8 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st } }() - if err := os.MkdirAll(unpackPath, 0700); err != nil { - return fmt.Errorf("error creating unpack directory: %w", err) + if err := util.EnsureEmptyDirectory(unpackPath, 0700); err != nil { + return fmt.Errorf("error ensuring empty unpack directory: %w", err) } l := log.FromContext(ctx) l.Info("unpacking image", "path", unpackPath) @@ -284,10 +285,10 @@ func (i *ContainersImageRegistry) unpackImage(ctx context.Context, unpackPath st l.Info("applied layer", "layer", i) return nil }(); err != nil { - return errors.Join(err, deleteRecursive(unpackPath)) + return errors.Join(err, DeleteReadOnlyRecursive(unpackPath)) } } - if err := setReadOnlyRecursive(unpackPath); err != nil { + if err := SetReadOnlyRecursive(unpackPath); err != nil { return fmt.Errorf("error making unpack directory read-only: %w", err) } return nil @@ -324,65 +325,9 @@ func (i *ContainersImageRegistry) deleteOtherImages(bundleName string, digestToK continue } imgDirPath := filepath.Join(bundlePath, imgDir.Name()) - if err := deleteRecursive(imgDirPath); err != nil { + if err := DeleteReadOnlyRecursive(imgDirPath); err != nil { return fmt.Errorf("error removing image directory: %w", err) } } return nil } - -func setReadOnlyRecursive(root string) error { - if err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { - if err != nil { - return err - } - - fi, err := d.Info() - if err != nil { - return err - } - - if err := func() error { - 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, 0500) - case 0: // regular file - return os.Chmod(path, 0400) - default: - return fmt.Errorf("refusing to change ownership of file %q with type %v", path, typ.String()) - } - }(); err != nil { - return err - } - return nil - }); err != nil { - return fmt.Errorf("error making bundle cache read-only: %w", err) - } - return nil -} - -func deleteRecursive(root string) error { - if err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { - if os.IsNotExist(err) { - return nil - } - if err != nil { - return err - } - if !d.IsDir() { - return nil - } - if err := os.Chmod(path, 0700); err != nil { - return err - } - return nil - }); err != nil { - return fmt.Errorf("error making bundle cache writable for deletion: %w", err) - } - return os.RemoveAll(root) -} diff --git a/internal/rukpak/source/containers_image_test.go b/internal/rukpak/source/containers_image_test.go index ea7a69832..29f2788c6 100644 --- a/internal/rukpak/source/containers_image_test.go +++ b/internal/rukpak/source/containers_image_test.go @@ -277,7 +277,16 @@ func TestUnpackUnexpectedFile(t *testing.T) { require.NoError(t, os.WriteFile(unpackPath, []byte{}, 0600)) // Attempt to pull and unpack the image - assert.Panics(t, func() { _, _ = unpacker.Unpack(context.Background(), bundleSource) }) + _, err := unpacker.Unpack(context.Background(), bundleSource) + require.NoError(t, err) + + // Ensure unpack path is now a directory + stat, err := os.Stat(unpackPath) + require.NoError(t, err) + require.True(t, stat.IsDir()) + + // Unset read-only to allow cleanup + require.NoError(t, source.UnsetReadOnlyRecursive(unpackPath)) } func TestUnpackCopySucceedsMountFails(t *testing.T) { diff --git a/internal/rukpak/source/util.go b/internal/rukpak/source/util.go new file mode 100644 index 000000000..ca9aa9c2b --- /dev/null +++ b/internal/rukpak/source/util.go @@ -0,0 +1,86 @@ +package source + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "time" +) + +// SetReadOnlyRecursive sets directory with path given by `root` as read-only +func SetReadOnlyRecursive(root string) error { + return filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + + fi, err := d.Info() + if err != nil { + return err + } + + if err := func() error { + 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, 0500) + case 0: // regular file + return os.Chmod(path, 0400) + default: + return fmt.Errorf("refusing to change ownership of file %q with type %v", path, typ.String()) + } + }(); err != nil { + return err + } + return nil + }) +} + +// UnsetReadOnlyRecursive unsets directory with path given by `root` as read-only +func UnsetReadOnlyRecursive(root string) error { + return filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + if !d.IsDir() { + return nil + } + if err := os.Chmod(path, 0700); err != nil { + return err + } + return nil + }) +} + +// DeleteReadOnlyRecursive deletes read-only directory with path given by `root` +func DeleteReadOnlyRecursive(root string) error { + if err := UnsetReadOnlyRecursive(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 err != nil { + if errors.Is(err, os.ErrNotExist) { + return false, time.Time{}, nil + } + return false, time.Time{}, err + } + if !unpackStat.IsDir() { + return false, time.Time{}, os.Remove(unpackPath) + } + return true, unpackStat.ModTime(), nil +} diff --git a/internal/util/fs.go b/internal/util/fs.go new file mode 100644 index 000000000..137b0735d --- /dev/null +++ b/internal/util/fs.go @@ -0,0 +1,23 @@ +package util + +import ( + "io/fs" + "os" + "path/filepath" +) + +// 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`. +func EnsureEmptyDirectory(path string, perm fs.FileMode) error { + entries, err := os.ReadDir(path) + if err != nil && !os.IsNotExist(err) { + return err + } + for _, entry := range entries { + if err := os.RemoveAll(filepath.Join(path, entry.Name())); err != nil { + return err + } + } + return os.MkdirAll(path, perm) +} diff --git a/requirements.txt b/requirements.txt index cb6de6a88..814a741bb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -21,7 +21,7 @@ paginate==0.5.7 pathspec==0.12.1 platformdirs==4.3.6 Pygments==2.19.1 -pymdown-extensions==10.14.1 +pymdown-extensions==10.14.2 pyquery==2.0.1 python-dateutil==2.9.0.post0 PyYAML==6.0.2 diff --git a/scripts/install.tpl.sh b/scripts/install.tpl.sh index 705a9d88b..0138baade 100644 --- a/scripts/install.tpl.sh +++ b/scripts/install.tpl.sh @@ -9,7 +9,7 @@ if [[ -z "$olmv1_manifest" ]]; then exit 1 fi -default_catalogs_manifest="./catalogd/config/base/default/clustercatalogs/default-catalogs.yaml" +default_catalogs_manifest=$DEFAULT_CATALOG cert_mgr_version=$CERT_MGR_VERSION install_default_catalogs=$INSTALL_DEFAULT_CATALOGS