diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index e331ec63e..6de62b0e1 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -98,10 +98,13 @@ type ClusterExtensionSpec struct { // +optional Install *ClusterExtensionInstallConfig `json:"install,omitempty"` - // config contains optional configuration values applied during rendering of the - // ClusterExtension's manifests. Values can be specified inline. + // config is an optional field used to specify bundle specific configuration + // used to configure the bundle. Configuration is bundle specific and a bundle may provide + // a configuration schema. When not specified, the default configuration of the resolved bundle will be used. // - // config is optional. When not specified, the default configuration of the resolved bundle will be used. + // config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide + // a configuration schema the final manifests will be derived on a best-effort basis. More information on how + // to configure the bundle should be found in its end-user documentation. // // // +optional @@ -174,6 +177,8 @@ type ClusterExtensionConfig struct { // ClusterExtension. // // inline must be set if configType is 'Inline'. + // inline accepts arbitrary JSON/YAML objects. + // inline is validation at runtime against the schema provided by the bundle if a schema is provided. // // +kubebuilder:validation:Type=object // +optional diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 1b1ad6656..b21e40452 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -254,7 +254,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `configType` _[ClusterExtensionConfigType](#clusterextensionconfigtype)_ | configType is a required reference to the type of configuration source.

Allowed values are "Inline"

When this field is set to "Inline", the cluster extension configuration is defined inline within the
ClusterExtension resource. | | Enum: [Inline]
Required: \{\}
| -| `inline` _[JSON](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#json-v1-apiextensions-k8s-io)_ | inline contains JSON or YAML values specified directly in the
ClusterExtension.

inline must be set if configType is 'Inline'. | | Type: object
| +| `inline` _[JSON](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#json-v1-apiextensions-k8s-io)_ | inline contains JSON or YAML values specified directly in the
ClusterExtension.

inline must be set if configType is 'Inline'.
inline accepts arbitrary JSON/YAML objects.
inline is validation at runtime against the schema provided by the bundle if a schema is provided. | | Type: object
| #### ClusterExtensionConfigType @@ -343,7 +343,7 @@ _Appears in:_ | `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions
with the cluster that are required to manage the extension.
The ServiceAccount must be configured with the necessary permissions to perform these interactions.
The ServiceAccount must exist in the namespace referenced in the spec.
serviceAccount is required. | | Required: \{\}
| | `source` _[SourceConfig](#sourceconfig)_ | source is a required field which selects the installation source of content
for this ClusterExtension. Selection is performed by setting the sourceType.

Catalog is currently the only implemented sourceType, and setting the
sourcetype to "Catalog" requires the catalog field to also be defined.

Below is a minimal example of a source definition (in yaml):

source:
sourceType: Catalog
catalog:
packageName: example-package | | Required: \{\}
| | `install` _[ClusterExtensionInstallConfig](#clusterextensioninstallconfig)_ | install is an optional field used to configure the installation options
for the ClusterExtension such as the pre-flight check configuration. | | | -| `config` _[ClusterExtensionConfig](#clusterextensionconfig)_ | config contains optional configuration values applied during rendering of the
ClusterExtension's manifests. Values can be specified inline.

config is optional. When not specified, the default configuration of the resolved bundle will be used.

| | | +| `config` _[ClusterExtensionConfig](#clusterextensionconfig)_ | config is an optional field used to specify bundle specific configuration
used to configure the bundle. Configuration is bundle specific and a bundle may provide
a configuration schema. When not specified, the default configuration of the resolved bundle will be used.

config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide
a configuration schema the final manifests will be derived on a best-effort basis. More information on how
to configure the bundle should be found in its end-user documentation.

| | | #### ClusterExtensionStatus diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index 4cae796a6..1038b7fdf 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -59,10 +59,13 @@ spec: properties: config: description: |- - config contains optional configuration values applied during rendering of the - ClusterExtension's manifests. Values can be specified inline. + config is an optional field used to specify bundle specific configuration + used to configure the bundle. Configuration is bundle specific and a bundle may provide + a configuration schema. When not specified, the default configuration of the resolved bundle will be used. - config is optional. When not specified, the default configuration of the resolved bundle will be used. + config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide + a configuration schema the final manifests will be derived on a best-effort basis. More information on how + to configure the bundle should be found in its end-user documentation. properties: configType: description: |- @@ -81,6 +84,8 @@ spec: ClusterExtension. inline must be set if configType is 'Inline'. + inline accepts arbitrary JSON/YAML objects. + inline is validation at runtime against the schema provided by the bundle if a schema is provided. type: object x-kubernetes-preserve-unknown-fields: true required: diff --git a/internal/operator-controller/applier/provider.go b/internal/operator-controller/applier/provider.go index ed1a1c5ec..4e957aadc 100644 --- a/internal/operator-controller/applier/provider.go +++ b/internal/operator-controller/applier/provider.go @@ -3,13 +3,16 @@ 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" @@ -32,6 +35,9 @@ 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() @@ -70,7 +76,7 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens watchNamespace, err := r.getWatchNamespace(ext) if err != nil { - return nil, err + return nil, fmt.Errorf("invalid bundle configuration: %w", err) } if watchNamespace != "" { @@ -89,11 +95,12 @@ func (r *RegistryV1ManifestProvider) getWatchNamespace(ext *ocv1.ClusterExtensio var watchNamespace string if ext.Spec.Config != nil && ext.Spec.Config.Inline != nil { - cfg := struct { - WatchNamespace string `json:"watchNamespace"` - }{} - if err := json.Unmarshal(ext.Spec.Config.Inline.Raw, &cfg); err != nil { - return "", fmt.Errorf("invalid bundle configuration: %w", err) + cfg := ®istryV1Config{} + // 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)) } watchNamespace = cfg.WatchNamespace } else { @@ -153,3 +160,27 @@ 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: or yaml: + // we want to extract if we can + errMessageComponents := strings.Split(err.Error(), ":") + coreErrMessage := strings.TrimSpace(errMessageComponents[len(errMessageComponents)-1]) + return errors.New(coreErrMessage) + } + err = unwrapped + } +} diff --git a/internal/operator-controller/applier/provider_test.go b/internal/operator-controller/applier/provider_test.go index 1816bdd33..a34b0abb2 100644 --- a/internal/operator-controller/applier/provider_test.go +++ b/internal/operator-controller/applier/provider_test.go @@ -188,6 +188,77 @@ 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{ diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 39ff01d61..e1fc7b6f5 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -864,10 +864,13 @@ spec: properties: config: description: |- - config contains optional configuration values applied during rendering of the - ClusterExtension's manifests. Values can be specified inline. + config is an optional field used to specify bundle specific configuration + used to configure the bundle. Configuration is bundle specific and a bundle may provide + a configuration schema. When not specified, the default configuration of the resolved bundle will be used. - config is optional. When not specified, the default configuration of the resolved bundle will be used. + config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide + a configuration schema the final manifests will be derived on a best-effort basis. More information on how + to configure the bundle should be found in its end-user documentation. properties: configType: description: |- @@ -886,6 +889,8 @@ spec: ClusterExtension. inline must be set if configType is 'Inline'. + inline accepts arbitrary JSON/YAML objects. + inline is validation at runtime against the schema provided by the bundle if a schema is provided. type: object x-kubernetes-preserve-unknown-fields: true required: diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 86bba145d..0675dc8df 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -829,10 +829,13 @@ spec: properties: config: description: |- - config contains optional configuration values applied during rendering of the - ClusterExtension's manifests. Values can be specified inline. + config is an optional field used to specify bundle specific configuration + used to configure the bundle. Configuration is bundle specific and a bundle may provide + a configuration schema. When not specified, the default configuration of the resolved bundle will be used. - config is optional. When not specified, the default configuration of the resolved bundle will be used. + config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide + a configuration schema the final manifests will be derived on a best-effort basis. More information on how + to configure the bundle should be found in its end-user documentation. properties: configType: description: |- @@ -851,6 +854,8 @@ spec: ClusterExtension. inline must be set if configType is 'Inline'. + inline accepts arbitrary JSON/YAML objects. + inline is validation at runtime against the schema provided by the bundle if a schema is provided. type: object x-kubernetes-preserve-unknown-fields: true required: