Skip to content

✨ Add support for deploying OCI helm charts in OLM v1 #1971

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 10, 2025
Merged

✨ Add support for deploying OCI helm charts in OLM v1 #1971

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe56052
Add support for deploying OCI helm charts in OLM v1
OchiengEd May 2, 2025
76b7f7c
Helm chart support documentation
OchiengEd Jul 1, 2025
02f39cc
add overlays for Helm Chart experimental features
OchiengEd Jul 2, 2025
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
415 changes: 415 additions & 0 deletions docs/draft/howto/enable-helm-chart-support.md
View file
Open in desktop

Large diffs are not rendered by default.

13 changes: 13 additions & 0 deletions internal/operator-controller/applier/helm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ import (

ocv1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
Copy link
Contributor

@camilamacedo86 camilamacedo86 Jun 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OchiengEd

Here: https://github.com/operator-framework/operator-controller/pull/1724/files

@perdasilva added a demo and the doc under the docs/draft for another alpha feature
It would be very nice if we could do your demo within and add the doc for that. Such as it was done in this PR. However, I am okay with it being a follow-up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be for adding documentation and a demo as a follow up.

)

const (
Expand Down Expand Up @@ -209,6 +211,17 @@ func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*char
if err != nil {
return nil, err
}
if features.OperatorControllerFeatureGate.Enabled(features.HelmChartSupport) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been trying to put the feature checks in main. Wdyt about refactoring to have another implementation of the BundleToHelmChart converter that accepts chart bundles, or maybe that can route between different bundle types? Then if the feature gate is enabled, use that one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not have any concerns with having feature checks in main. I think that would be the ideal scenario as it would allow for feature-gated functionality to be tested at build time.

Trying to look at the issue from afar, our challenge in this case is we are trying to run tests on a feature gated function, pullChart() while we are not allowing a portion of the code in the cache's Store() method not to run since the feature gate is not enabled. The big question therefore is how do we navigate this quandary?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not have any concerns with having feature checks in main. I think that would be the ideal scenario as it would allow for feature-gated functionality to be tested at build time.

We are working towards to it
See; #1980

We will use the experimental CRD and have all feature flags enabled for our e2e tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OchiengEd I think in order to do it properly, we'll need some architectural changes to the applier. As it is now, it assumes it will be receiving a registry+v1 bundle FS. As the code is now, we are expanding the scope of the applier to also identify the bundle type and produce a chart accordingly. I think we should pivot and instead of the applier having a BundleToHelmChartConverter it should rather have something like a HelmChartGetter (or something mode sophisticated) that takes an FS + config and returns a helm chart object.

The HelmChartGetter could use a pattern similar to what Joe suggested for the cache layer, e.g. have some kind of interface that can identify the bundle type from the FS and apply and build the chart.

In the main function, we can then configure the HelmChartGetter to either include (or not) the the interface implementation that deals with Helm bundles.

Alternatively, we compose have two HelmChartGetters, one with the standard implementation for registry+v1 bundles and another that uses the standard implementation and extends it by basically the code you have here (e.g. first check whether its a helm chart fs, if not, then just use the standard implementation).

The same pattern could be used for the Cache. If the flag is set, configure it one way, if not, configure it another.

Breaking it down like this makes it easy to test independently and you don't even need to rely on flags being set or not for (unit) testing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep momentum, I'll approve this for now and maybe we could work together on a refactor as a fast follow.

meta := new(chart.Metadata)
if ok, _ := imageutil.IsBundleSourceChart(bundleFS, meta); ok {
return imageutil.LoadChartFSWithOptions(
bundleFS,
fmt.Sprintf("%s-%s.tgz", meta.Name, meta.Version),
imageutil.WithInstallNamespace(ext.Spec.Namespace),
)
}
}

return h.BundleToHelmChartConverter.ToHelmChart(source.FromFS(bundleFS), ext.Spec.Namespace, watchNamespace)
}

Expand Down
9 changes: 9 additions & 0 deletions internal/operator-controller/features/features.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const (
SyntheticPermissions featuregate.Feature = "SyntheticPermissions"
WebhookProviderCertManager featuregate.Feature = "WebhookProviderCertManager"
WebhookProviderOpenshiftServiceCA featuregate.Feature = "WebhookProviderOpenshiftServiceCA"
HelmChartSupport featuregate.Feature = "HelmChartSupport"
)

var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
Expand Down Expand Up @@ -63,6 +64,14 @@ var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.Feature
PreRelease: featuregate.Alpha,
LockToDefault: false,
},

// HelmChartSupport enables support for installing,
// updating and uninstalling Helm Charts via Cluster Extensions.
HelmChartSupport: {
Default: false,
PreRelease: featuregate.Alpha,
LockToDefault: false,
},
}

var OperatorControllerFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate()
Expand Down
86 changes: 81 additions & 5 deletions internal/shared/util/image/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package image

import (
"bytes"
"context"
"errors"
"fmt"
Expand All @@ -10,22 +11,28 @@ import (
"os"
"path/filepath"
"slices"
"testing"
"time"

"github.com/containerd/containerd/archive"
"github.com/containers/image/v5/docker/reference"
"github.com/google/renameio/v2"
"github.com/opencontainers/go-digest"
ocispecv1 "github.com/opencontainers/image-spec/specs-go/v1"
"helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/registry"
"sigs.k8s.io/controller-runtime/pkg/log"

"github.com/operator-framework/operator-controller/internal/operator-controller/features"
errorutil "github.com/operator-framework/operator-controller/internal/shared/util/error"
fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs"
)

type LayerData struct {
Reader io.Reader
Index int
Err error
MediaType string
Reader io.Reader
Index int
Err error
}

type Cache interface {
Expand Down Expand Up @@ -106,6 +113,40 @@ func (a *diskCache) unpackPath(ownerID string, digest digest.Digest) string {
return filepath.Join(a.ownerIDPath(ownerID), digest.String())
}

type LayerUnpacker interface {
Unpack(_ context.Context, path string, layer LayerData, opts ...archive.ApplyOpt) error
}

type defaultLayerUnpacker struct{}

type chartLayerUnpacker struct{}

var _ LayerUnpacker = &defaultLayerUnpacker{}
var _ LayerUnpacker = &chartLayerUnpacker{}

func imageLayerUnpacker(layer LayerData) LayerUnpacker {
if features.OperatorControllerFeatureGate.Enabled(features.HelmChartSupport) || testing.Testing() {
if layer.MediaType == registry.ChartLayerMediaType {
return &chartLayerUnpacker{}
}
}
return &defaultLayerUnpacker{}
}

func (u *chartLayerUnpacker) Unpack(_ context.Context, path string, layer LayerData, _ ...archive.ApplyOpt) error {
if err := storeChartLayer(path, layer); err != nil {
return fmt.Errorf("error applying chart layer[%d]: %w", layer.Index, err)
}
return nil
}

func (u *defaultLayerUnpacker) Unpack(ctx context.Context, path string, layer LayerData, opts ...archive.ApplyOpt) error {
if _, err := archive.Apply(ctx, path, layer.Reader, opts...); err != nil {
return fmt.Errorf("error applying layer[%d]: %w", layer.Index, err)
}
return nil
}

func (a *diskCache) Store(ctx context.Context, ownerID string, srcRef reference.Named, canonicalRef reference.Canonical, imgCfg ocispecv1.Image, layers iter.Seq[LayerData]) (fs.FS, time.Time, error) {
var applyOpts []archive.ApplyOpt
if a.filterFunc != nil {
Expand All @@ -128,8 +169,9 @@ func (a *diskCache) Store(ctx context.Context, ownerID string, srcRef reference.
if layer.Err != nil {
return fmt.Errorf("error reading layer[%d]: %w", layer.Index, layer.Err)
}
if _, err := archive.Apply(ctx, dest, layer.Reader, applyOpts...); err != nil {
return fmt.Errorf("error applying layer[%d]: %w", layer.Index, err)
layerUnpacker := imageLayerUnpacker(layer)
if err := layerUnpacker.Unpack(ctx, dest, layer, applyOpts...); err != nil {
return fmt.Errorf("unpacking layer: %w", err)
}
l.Info("applied layer", "layer", layer.Index)
}
Expand All @@ -147,6 +189,40 @@ func (a *diskCache) Store(ctx context.Context, ownerID string, srcRef reference.
return os.DirFS(dest), modTime, nil
}

func storeChartLayer(path string, layer LayerData) error {
if layer.Err != nil {
return fmt.Errorf("error found in layer data: %w", layer.Err)
}
data, err := io.ReadAll(layer.Reader)
if err != nil {
return fmt.Errorf("error reading layer[%d]: %w", layer.Index, err)
}
meta := new(chart.Metadata)
_, err = inspectChart(data, meta)
if err != nil {
return fmt.Errorf("inspecting chart layer: %w", err)
}
chart, err := renameio.TempFile("",
filepath.Join(path,
fmt.Sprintf("%s-%s.tgz", meta.Name, meta.Version),
),
)
if err != nil {
return fmt.Errorf("create temp file: %w", err)
}
defer func() {
_ = chart.Cleanup()
}()
if _, err := io.Copy(chart, bytes.NewReader(data)); err != nil {
return fmt.Errorf("copying chart archive: %w", err)
}
_, err = chart.Seek(0, io.SeekStart)
if err != nil {
return fmt.Errorf("seek chart archive start: %w", err)
}
return chart.CloseAtomicallyReplace()
}

func (a *diskCache) Delete(_ context.Context, ownerID string) error {
return fsutil.DeleteReadOnlyRecursive(a.ownerIDPath(ownerID))
}
Expand Down
Loading