Skip to content

🌱 Add registry+v1 bundle config unmarshal and validation layer #2278

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
Oct 21, 2025
Merged

🌱 Add registry+v1 bundle config unmarshal and validation layer #2278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d164dee
Add registry+v1 bundle config unmarshal function
Oct 20, 2025
3a28e97
Update manifest provider to use config unmarshal and remove getWatchN…
Oct 20, 2025
a5a903c
Address reviewer comments and add required case
Oct 21, 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
74 changes: 9 additions & 65 deletions internal/operator-controller/applier/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,17 @@ package applier
import (
"crypto/sha256"
"encoding/json"
"errors"
"fmt"
"io/fs"
"strings"

"helm.sh/helm/v3/pkg/chart"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"

"github.com/operator-framework/api/pkg/operators/v1alpha1"

ocv1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle"
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
)
Expand All @@ -35,9 +32,6 @@ type RegistryV1ManifestProvider struct {
IsWebhookSupportEnabled bool
IsSingleOwnNamespaceEnabled bool
}
type registryV1Config struct {
WatchNamespace string `json:"watchNamespace"`
}

func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) {
rv1, err := source.FromFS(bundleFS).GetBundle()
Expand Down Expand Up @@ -74,44 +68,18 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens
render.WithCertificateProvider(r.CertificateProvider),
}

watchNamespace, err := r.getWatchNamespace(ext)
if err != nil {
return nil, fmt.Errorf("invalid bundle configuration: %w", err)
}

if watchNamespace != "" {
opts = append(opts, render.WithTargetNamespaces(watchNamespace))
}

return r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...)
}

// getWatchNamespace determines the watch namespace the ClusterExtension should use based on the
// configuration in .spec.config.Inline. Only active if SingleOwnNamespace support is enabled.
func (r *RegistryV1ManifestProvider) getWatchNamespace(ext *ocv1.ClusterExtension) (string, error) {
if !r.IsSingleOwnNamespaceEnabled {
return "", nil
}

var watchNamespace string
if ext.Spec.Config != nil && ext.Spec.Config.Inline != nil {
cfg := &registryV1Config{}
// Using k8s.io/yaml package as that is able to handle both json and yaml
// In most cases, at this point we should have a valid JSON/YAML object in the byte slice and failures will
// be related to object structure (e.g. additional fields).
if err := yaml.UnmarshalStrict(ext.Spec.Config.Inline.Raw, cfg); err != nil {
return "", fmt.Errorf("error unmarshalling registry+v1 configuration: %w", formatUnmarshallError(err))
if r.IsSingleOwnNamespaceEnabled && ext.Spec.Config != nil && ext.Spec.Config.ConfigType == ocv1.ClusterExtensionConfigTypeInline {
bundleConfig, err := bundle.UnmarshallConfig(ext.Spec.Config.Inline.Raw, rv1, ext.Spec.Namespace)
if err != nil {
return nil, fmt.Errorf("invalid bundle configuration: %w", err)
}
watchNamespace = cfg.WatchNamespace
} else {
return "", nil
}

if errs := validation.IsDNS1123Subdomain(watchNamespace); len(errs) > 0 {
return "", fmt.Errorf("invalid watch namespace '%s': namespace must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", watchNamespace)
if bundleConfig != nil && bundleConfig.WatchNamespace != nil {
opts = append(opts, render.WithTargetNamespaces(*bundleConfig.WatchNamespace))
}
}

return watchNamespace, nil
return r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...)
}

// RegistryV1HelmChartProvider creates a Helm-Chart from a registry+v1 bundle and its associated ClusterExtension
Expand Down Expand Up @@ -160,27 +128,3 @@ func (r *RegistryV1HelmChartProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExten

return chrt, nil
}

func formatUnmarshallError(err error) error {
var unmarshalErr *json.UnmarshalTypeError
if errors.As(err, &unmarshalErr) {
if unmarshalErr.Field == "" {
return errors.New("input is not a valid JSON object")
} else {
return fmt.Errorf("invalid value type for field %q: expected %q but got %q", unmarshalErr.Field, unmarshalErr.Type.String(), unmarshalErr.Value)
}
}

// unwrap error until the core and process it
for {
unwrapped := errors.Unwrap(err)
if unwrapped == nil {
// usually the errors present in the form json: <message> or yaml: <message>
// we want to extract <message> if we can
errMessageComponents := strings.Split(err.Error(), ":")
coreErrMessage := strings.TrimSpace(errMessageComponents[len(errMessageComponents)-1])
return errors.New(coreErrMessage)
}
err = unwrapped
}
}
155 changes: 84 additions & 71 deletions internal/operator-controller/applier/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,42 @@ func Test_RegistryV1ManifestProvider_Integration(t *testing.T) {
require.Contains(t, err.Error(), "some error")
})

t.Run("surfaces bundle config unmarshall errors", func(t *testing.T) {
provider := applier.RegistryV1ManifestProvider{
BundleRenderer: render.BundleRenderer{
ResourceGenerators: []render.ResourceGenerator{
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
return nil, nil
},
},
},
// must be true for now as we only unmarshal configuration when this feature is on
// once we go GA and remove IsSingleOwnNamespaceEnabled it's ok to just delete this
IsSingleOwnNamespaceEnabled: true,
}

// The contents of the bundle are not important for this tesy, only that it be a valid bundle
// to avoid errors in the deserialization process
bundleFS := bundlefs.Builder().WithPackageName("test").
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build()).Build()

ext := &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
Namespace: "install-namespace",
Config: &ocv1.ClusterExtensionConfig{
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
Inline: &apiextensionsv1.JSON{
Raw: []byte(`{"watchNamespace": "install-namespace"}`),
},
},
},
}

_, err := provider.Get(bundleFS, ext)
require.Error(t, err)
require.Contains(t, err.Error(), "invalid bundle configuration")
})

t.Run("returns rendered manifests", func(t *testing.T) {
provider := applier.RegistryV1ManifestProvider{
BundleRenderer: registryv1.Renderer,
Expand Down Expand Up @@ -188,77 +224,6 @@ func Test_RegistryV1ManifestProvider_WebhookSupport(t *testing.T) {
})
}

func Test_RegistryV1ManifestProvider_ConfigUnmarshalling(t *testing.T) {
for _, tc := range []struct {
name string
configBytes []byte
expectedErrMessage string
}{
{
name: "accepts json config",
configBytes: []byte(`{"watchNamespace": "some-namespace"}`),
},
{
name: "accepts yaml config",
configBytes: []byte(`watchNamespace: some-namespace`),
},
{
name: "rejects invalid json",
configBytes: []byte(`{"hello`),
expectedErrMessage: `invalid bundle configuration: error unmarshalling registry+v1 configuration: found unexpected end of stream`,
},
{
name: "rejects valid json that isn't of object type",
configBytes: []byte(`true`),
expectedErrMessage: `invalid bundle configuration: error unmarshalling registry+v1 configuration: input is not a valid JSON object`,
},
{
name: "rejects additional fields",
configBytes: []byte(`somekey: somevalue`),
expectedErrMessage: `invalid bundle configuration: error unmarshalling registry+v1 configuration: unknown field "somekey"`,
},
{
name: "rejects valid json but invalid registry+v1",
configBytes: []byte(`{"watchNamespace": {"hello": "there"}}`),
expectedErrMessage: `invalid bundle configuration: error unmarshalling registry+v1 configuration: invalid value type for field "watchNamespace": expected "string" but got "object"`,
},
} {
t.Run(tc.name, func(t *testing.T) {
provider := applier.RegistryV1ManifestProvider{
BundleRenderer: render.BundleRenderer{
ResourceGenerators: []render.ResourceGenerator{
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
return nil, nil
},
},
},
IsSingleOwnNamespaceEnabled: true,
}

bundleFS := bundlefs.Builder().WithPackageName("test").
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build()).Build()

_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
Namespace: "install-namespace",
Config: &ocv1.ClusterExtensionConfig{
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
Inline: &apiextensionsv1.JSON{
Raw: tc.configBytes,
},
},
},
})
if tc.expectedErrMessage != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tc.expectedErrMessage)
} else {
require.NoError(t, err)
}
})
}
}

func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
t.Run("rejects bundles without AllNamespaces install mode when Single/OwnNamespace install mode support is disabled", func(t *testing.T) {
provider := applier.RegistryV1ManifestProvider{
Expand All @@ -276,6 +241,54 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
require.Equal(t, "unsupported bundle: bundle does not support AllNamespaces install mode", err.Error())
})

t.Run("rejects bundles without AllNamespaces install mode and with SingleNamespace support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) {
expectedWatchNamespace := "some-namespace"
provider := applier.RegistryV1ManifestProvider{
BundleRenderer: render.BundleRenderer{
ResourceGenerators: []render.ResourceGenerator{
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
t.Log("ensure watch namespace is appropriately configured")
require.Equal(t, []string{expectedWatchNamespace}, opts.TargetNamespaces)
return nil, nil
},
},
},
IsSingleOwnNamespaceEnabled: false,
}

bundleFS := bundlefs.Builder().WithPackageName("test").
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build()).Build()

_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
Namespace: "install-namespace",
Config: &ocv1.ClusterExtensionConfig{
ConfigType: ocv1.ClusterExtensionConfigTypeInline,
Inline: &apiextensionsv1.JSON{
Raw: []byte(`{"watchNamespace": "` + expectedWatchNamespace + `"}`),
},
},
},
})
require.Error(t, err)
require.Contains(t, err.Error(), "unsupported bundle")
})

t.Run("rejects bundles without AllNamespaces install mode and with OwnNamespace support when Single/OwnNamespace install mode support is disabled", func(t *testing.T) {
provider := applier.RegistryV1ManifestProvider{
IsSingleOwnNamespaceEnabled: false,
}
bundleFS := bundlefs.Builder().WithPackageName("test").
WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build()
_, err := provider.Get(bundleFS, &ocv1.ClusterExtension{
Spec: ocv1.ClusterExtensionSpec{
Namespace: "install-namespace",
},
})
require.Error(t, err)
require.Contains(t, err.Error(), "unsupported bundle")
})

t.Run("accepts bundles without AllNamespaces install mode and with SingleNamespace support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) {
expectedWatchNamespace := "some-namespace"
provider := applier.RegistryV1ManifestProvider{
Expand Down
Loading
Loading