Skip to content
Draft
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
25 changes: 16 additions & 9 deletions api/v1/clusterextension_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,21 @@ type ClusterExtensionSpec struct {
// +kubebuilder:validation:Required
Namespace string `json:"namespace"`

// serviceAccount is a reference to a ServiceAccount used to perform all interactions
// with the cluster that are required to manage the extension.
// <opcon:standard:description>
// serviceAccount is a required field that references a ServiceAccount used to
// perform all interactions with the cluster that are required to manage the extension.
// </opcon:standard:description>
// <opcon:standard:validation:Required>
//
// <opcon:experimental:description>
// serviceAccount is an optional field that references a ServiceAccount used to
// perform all interactions with the cluster that are required to manage the extension.
// If not set, operator-controller will use its own ServiceAccount for extension management.
// 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.
//
// +kubebuilder:validation:Required
ServiceAccount ServiceAccountReference `json:"serviceAccount"`
// </opcon:experimental:description>
// <opcon:experimental:validation:Optional>
ServiceAccount ServiceAccountReference `json:"serviceAccount,omitzero"`

// source is a required field which selects the installation source of content
// for this ClusterExtension. Selection is performed by setting the sourceType.
Expand Down Expand Up @@ -374,7 +381,7 @@ type CatalogFilter struct {
UpgradeConstraintPolicy UpgradeConstraintPolicy `json:"upgradeConstraintPolicy,omitempty"`
}

// ServiceAccountReference identifies the serviceAccount used fo install a ClusterExtension.
// ServiceAccountReference identifies the serviceAccount used to install a ClusterExtension.
type ServiceAccountReference struct {
// name is a required, immutable reference to the name of the ServiceAccount
// to be used for installation and management of the content for the package
Expand Down Expand Up @@ -403,8 +410,8 @@ type ServiceAccountReference struct {
// +kubebuilder:validation:MaxLength:=253
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="name is immutable"
// +kubebuilder:validation:XValidation:rule="self.matches(\"^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$\")",message="name must be a valid DNS1123 subdomain. It must contain only lowercase alphanumeric characters, hyphens (-) or periods (.), start and end with an alphanumeric character, and be no longer than 253 characters"
// +kubebuilder:validation:Required
Name string `json:"name"`
// +optional
Name string `json:"name,omitempty"`
}

// PreflightConfig holds the configuration for the preflight checks. If used, at least one preflight check must be non-nil.
Expand Down
61 changes: 61 additions & 0 deletions api/v1/clusterextension_types_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package v1_test

import (
"encoding/json"
"fmt"
"go/ast"
"go/parser"
Expand All @@ -11,6 +12,7 @@ import (

"golang.org/x/exp/slices" // replace with "slices" in go 1.21

v1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets"
)

Expand Down Expand Up @@ -100,3 +102,62 @@ func parseConstants(prefix string) ([]string, error) {
}
return constValues, nil
}

func TestServiceAccountMarshaling(t *testing.T) {
tests := []struct {
name string
spec v1.ClusterExtensionSpec
expectField string
unexpectField string
}{
{
name: "ServiceAccount with name is marshaled",
spec: v1.ClusterExtensionSpec{
Namespace: "test-ns",
ServiceAccount: v1.ServiceAccountReference{
Name: "test-sa",
},
Source: v1.SourceConfig{
SourceType: "Catalog",
Catalog: &v1.CatalogFilter{
PackageName: "test-package",
},
},
},
expectField: "serviceAccount",
},
{
name: "ServiceAccount with empty name is omitted",
spec: v1.ClusterExtensionSpec{
Namespace: "test-ns",
ServiceAccount: v1.ServiceAccountReference{},
Source: v1.SourceConfig{
SourceType: "Catalog",
Catalog: &v1.CatalogFilter{
PackageName: "test-package",
},
},
},
unexpectField: "serviceAccount",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
data, err := json.Marshal(tt.spec)
if err != nil {
t.Fatalf("failed to marshal spec: %v", err)
}

jsonStr := string(data)

if tt.expectField != "" && !strings.Contains(jsonStr, tt.expectField) {
t.Errorf("expected field %q to be present in JSON output, got: %s", tt.expectField, jsonStr)
}

if tt.unexpectField != "" && strings.Contains(jsonStr, tt.unexpectField) {
t.Errorf("expected field %q to be omitted from JSON output, got: %s", tt.unexpectField, jsonStr)
}
})
}
}
6 changes: 3 additions & 3 deletions docs/api-reference/olmv1-api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ _Appears in:_
| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `namespace` _string_ | namespace is a reference to a Kubernetes namespace.<br />This is the namespace in which the provided ServiceAccount must exist.<br />It also designates the default namespace where namespace-scoped resources<br />for the extension are applied to the cluster.<br />Some extensions may contain namespace-scoped resources to be applied in other namespaces.<br />This namespace must exist.<br />namespace is required, immutable, and follows the DNS label standard<br />as defined in [RFC 1123]. It must contain only lowercase alphanumeric characters or hyphens (-),<br />start and end with an alphanumeric character, and be no longer than 63 characters<br />[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 63 <br />Required: \{\} <br /> |
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />The ServiceAccount must exist in the namespace referenced in the spec.<br />serviceAccount is required. | | Required: \{\} <br /> |
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | <opcon:standard:description><br />serviceAccount is a required field that references a ServiceAccount used to<br />perform all interactions with the cluster that are required to manage the extension.<br /></opcon:standard:description><br /><opcon:standard:validation:Required><br /><opcon:experimental:description><br />serviceAccount is an optional field that references a ServiceAccount used to<br />perform all interactions with the cluster that are required to manage the extension.<br />If not set, operator-controller will use its own ServiceAccount for extension management.<br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />The ServiceAccount must exist in the namespace referenced in the spec.<br /></opcon:experimental:description><br /><opcon:experimental:validation:Optional> | | |
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The documentation contains raw <opcon:*> tags that should have been processed by the CRD generator. These internal markup tags should not appear in end-user documentation. The formatDescription function should have removed or processed these tags to show only the appropriate channel-specific content.

Suggested change
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | <opcon:standard:description><br />serviceAccount is a required field that references a ServiceAccount used to<br />perform all interactions with the cluster that are required to manage the extension.<br /></opcon:standard:description><br /><opcon:standard:validation:Required><br /><opcon:experimental:description><br />serviceAccount is an optional field that references a ServiceAccount used to<br />perform all interactions with the cluster that are required to manage the extension.<br />If not set, operator-controller will use its own ServiceAccount for extension management.<br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />The ServiceAccount must exist in the namespace referenced in the spec.<br /></opcon:experimental:description><br /><opcon:experimental:validation:Optional> | | |
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a required field that references a ServiceAccount used to perform all interactions with the cluster that are required to manage the extension.<br />**Experimental:** serviceAccount is an optional field that references a ServiceAccount used to perform all interactions with the cluster that are required to manage the extension. If not set, operator-controller will use its own ServiceAccount for extension management. The ServiceAccount must be configured with the necessary permissions to perform these interactions. The ServiceAccount must exist in the namespace referenced in the spec. | | Required (standard), Optional (experimental) |

Copilot uses AI. Check for mistakes.
| `source` _[SourceConfig](#sourceconfig)_ | source is a required field which selects the installation source of content<br />for this ClusterExtension. Selection is performed by setting the sourceType.<br />Catalog is currently the only implemented sourceType, and setting the<br />sourcetype to "Catalog" requires the catalog field to also be defined.<br />Below is a minimal example of a source definition (in yaml):<br />source:<br /> sourceType: Catalog<br /> catalog:<br /> packageName: example-package | | Required: \{\} <br /> |
| `install` _[ClusterExtensionInstallConfig](#clusterextensioninstallconfig)_ | install is an optional field used to configure the installation options<br />for the ClusterExtension such as the pre-flight check configuration. | | |
| `config` _[ClusterExtensionConfig](#clusterextensionconfig)_ | config is an optional field used to specify bundle specific configuration<br />used to configure the bundle. Configuration is bundle specific and a bundle may provide<br />a configuration schema. When not specified, the default configuration of the resolved bundle will be used.<br />config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide<br />a configuration schema the final manifests will be derived on a best-effort basis. More information on how<br />to configure the bundle should be found in its end-user documentation.<br /><opcon:experimental> | | |
Expand Down Expand Up @@ -439,7 +439,7 @@ _Appears in:_



ServiceAccountReference identifies the serviceAccount used fo install a ClusterExtension.
ServiceAccountReference identifies the serviceAccount used to install a ClusterExtension.



Expand All @@ -448,7 +448,7 @@ _Appears in:_

| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `name` _string_ | name is a required, immutable reference to the name of the ServiceAccount<br />to be used for installation and management of the content for the package<br />specified in the packageName field.<br />This ServiceAccount must exist in the installNamespace.<br />name follows the DNS subdomain standard as defined in [RFC 1123].<br />It must contain only lowercase alphanumeric characters,<br />hyphens (-) or periods (.), start and end with an alphanumeric character,<br />and be no longer than 253 characters.<br />Some examples of valid values are:<br /> - some-serviceaccount<br /> - 123-serviceaccount<br /> - 1-serviceaccount-2<br /> - someserviceaccount<br /> - some.serviceaccount<br />Some examples of invalid values are:<br /> - -some-serviceaccount<br /> - some-serviceaccount-<br />[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 253 <br />Required: \{\} <br /> |
| `name` _string_ | name is a required, immutable reference to the name of the ServiceAccount<br />to be used for installation and management of the content for the package<br />specified in the packageName field.<br />This ServiceAccount must exist in the installNamespace.<br />name follows the DNS subdomain standard as defined in [RFC 1123].<br />It must contain only lowercase alphanumeric characters,<br />hyphens (-) or periods (.), start and end with an alphanumeric character,<br />and be no longer than 253 characters.<br />Some examples of valid values are:<br /> - some-serviceaccount<br /> - 123-serviceaccount<br /> - 1-serviceaccount-2<br /> - someserviceaccount<br /> - some.serviceaccount<br />Some examples of invalid values are:<br /> - -some-serviceaccount<br /> - some-serviceaccount-<br />[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 253 <br /> |


#### SourceConfig
Expand Down
89 changes: 66 additions & 23 deletions hack/tools/crd-generator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"log"
"os"
"regexp"
"slices"
"strings"

apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
Expand Down Expand Up @@ -136,7 +137,7 @@ func runGenerator(args ...string) {
if channel == StandardChannel && strings.Contains(version.Name, "alpha") {
channelCrd.Spec.Versions[i].Served = false
}
version.Schema.OpenAPIV3Schema.Properties = opconTweaksMap(channel, version.Schema.OpenAPIV3Schema.Properties)
channelCrd.Spec.Versions[i].Schema.OpenAPIV3Schema.Properties, channelCrd.Spec.Versions[i].Schema.OpenAPIV3Schema.Required = opconTweaksMap(channel, version.Schema.OpenAPIV3Schema.Properties, version.Schema.OpenAPIV3Schema.Required)
}

conv, err := crd.AsVersion(*channelCrd, apiextensionsv1.SchemeGroupVersion)
Expand Down Expand Up @@ -179,25 +180,43 @@ func runGenerator(args ...string) {
}
}

func opconTweaksMap(channel string, props map[string]apiextensionsv1.JSONSchemaProps) map[string]apiextensionsv1.JSONSchemaProps {
func opconTweaksMap(channel string, props map[string]apiextensionsv1.JSONSchemaProps, existingRequired []string) (map[string]apiextensionsv1.JSONSchemaProps, []string) {
// Start with existing required fields (from kubebuilder markers)
requiredFields := slices.Clone(existingRequired)

for name := range props {
jsonProps := props[name]
p := opconTweaks(channel, name, jsonProps)
p, reqStatus := opconTweaks(channel, name, jsonProps)
if p == nil {
delete(props, name)
// Remove from required list if present
requiredFields = slices.DeleteFunc(requiredFields, func(s string) bool { return s == name })
} else {
props[name] = *p
// Update required list based on tag
switch reqStatus {
case "required":
if !slices.Contains(requiredFields, name) {
requiredFields = append(requiredFields, name)
}
case "optional":
requiredFields = slices.DeleteFunc(requiredFields, func(s string) bool { return s == name })
// "" (unspecified) means keep existing status
}
}
}
return props
return props, requiredFields
}

// Custom Opcon API Tweaks for tags prefixed with `<opcon:` that get past
// the limitations of Kubebuilder annotations.
func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSchemaProps) *apiextensionsv1.JSONSchemaProps {
// Returns the modified schema and a string indicating required status:
// "required", "optional", or "" (unspecified - preserve existing)
func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSchemaProps) (*apiextensionsv1.JSONSchemaProps, string) {
requiredStatus := "" // "required", "optional", or "" (unspecified)
if channel == StandardChannel {
if strings.Contains(jsonProps.Description, "<opcon:experimental>") {
return nil
return nil, ""
}
}

Expand Down Expand Up @@ -237,6 +256,22 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche
Rule: celMatch[2],
})
}

optReqRe := regexp.MustCompile(validationPrefix + "(Optional|Required)>")
optReqMatches := optReqRe.FindAllStringSubmatch(jsonProps.Description, 64)
for _, optReqMatch := range optReqMatches {
if len(optReqMatch) != 2 {
log.Fatalf("Invalid %s Optional/Required tag for %s", validationPrefix, name)
}

numValid++
switch optReqMatch[1] {
case "Optional":
requiredStatus = "optional"
case "Required":
requiredStatus = "required"
}
}
}

if numValid < numExpressions {
Expand All @@ -246,34 +281,42 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche
jsonProps.Description = formatDescription(jsonProps.Description, channel, name)

if len(jsonProps.Properties) > 0 {
jsonProps.Properties = opconTweaksMap(channel, jsonProps.Properties)
jsonProps.Properties, jsonProps.Required = opconTweaksMap(channel, jsonProps.Properties, jsonProps.Required)
} else if jsonProps.Items != nil && jsonProps.Items.Schema != nil {
jsonProps.Items.Schema = opconTweaks(channel, name, *jsonProps.Items.Schema)
jsonProps.Items.Schema, _ = opconTweaks(channel, name, *jsonProps.Items.Schema)
}

return &jsonProps
return &jsonProps, requiredStatus
}

func formatDescription(description string, channel string, name string) string {
startTag := "<opcon:experimental:description>"
endTag := "</opcon:experimental:description>"
if channel == StandardChannel && strings.Contains(description, startTag) {
regexPattern := `\n*` + regexp.QuoteMeta(startTag) + `(?s:(.*?))` + regexp.QuoteMeta(endTag) + `\n*`
re := regexp.MustCompile(regexPattern)
match := re.FindStringSubmatch(description)
if len(match) != 2 {
log.Fatalf("Invalid <opcon:experimental:description> tag for %s", name)
tagset := []struct {
channel string
start string
end string
}{
{channel: ExperimentalChannel, start: "<opcon:standard:description>", end: "</opcon:standard:description>"},
{channel: StandardChannel, start: "<opcon:experimental:description>", end: "</opcon:experimental:description>"},
}
for _, ts := range tagset {
if channel == ts.channel && strings.Contains(description, ts.start) {
regexPattern := `\n*` + regexp.QuoteMeta(ts.start) + `(?s:(.*?))` + regexp.QuoteMeta(ts.end) + `\n*`
re := regexp.MustCompile(regexPattern)
match := re.FindStringSubmatch(description)
if len(match) != 2 {
log.Fatalf("Invalid <opcon:experimental:description> tag for %s", name)
}
description = re.ReplaceAllString(description, "\n\n")
} else {
description = strings.ReplaceAll(description, ts.start, "")
description = strings.ReplaceAll(description, ts.end, "")
}
description = re.ReplaceAllString(description, "\n\n")
} else {
description = strings.ReplaceAll(description, startTag, "")
description = strings.ReplaceAll(description, endTag, "")
}

// Comments within "opcon:util:excludeFromCRD" tag are not included in the generated CRD and all trailing \n operators before
// and after the tags are removed and replaced with three \n operators.
startTag = "<opcon:util:excludeFromCRD>"
endTag = "</opcon:util:excludeFromCRD>"
startTag := "<opcon:util:excludeFromCRD>"
endTag := "</opcon:util:excludeFromCRD>"
if strings.Contains(description, startTag) {
regexPattern := `\n*` + regexp.QuoteMeta(startTag) + `(?s:(.*?))` + regexp.QuoteMeta(endTag) + `\n*`
re := regexp.MustCompile(regexPattern)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,11 @@ spec:
rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")
serviceAccount:
description: |-
serviceAccount is a reference to a ServiceAccount used to perform all interactions
with the cluster that are required to manage the extension.
serviceAccount is an optional field that references a ServiceAccount used to
perform all interactions with the cluster that are required to manage the extension.
If not set, operator-controller will use its own ServiceAccount for extension management.
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.
properties:
name:
description: |-
Expand Down Expand Up @@ -212,8 +212,6 @@ spec:
(.), start and end with an alphanumeric character, and be
no longer than 253 characters
rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$")
required:
- name
type: object
source:
description: |-
Expand Down Expand Up @@ -498,7 +496,6 @@ spec:
has(self.catalog) : !has(self.catalog)'
required:
- namespace
- serviceAccount
- source
type: object
status:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,8 @@ spec:
rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")
serviceAccount:
description: |-
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.
serviceAccount is a required field that references a ServiceAccount used to
perform all interactions with the cluster that are required to manage the extension.
Comment on lines +135 to +136
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The description states that serviceAccount is 'a required field' but the required list at line 173 has been removed, making this field actually optional in the CRD schema. This creates an inconsistency between the description and the actual schema enforcement. For the standard channel, either the description should be corrected to say it's optional, or the field should remain in the required list.

Copilot uses AI. Check for mistakes.
properties:
name:
description: |-
Expand Down Expand Up @@ -173,8 +170,6 @@ spec:
(.), start and end with an alphanumeric character, and be
no longer than 253 characters
rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$")
required:
- name
type: object
source:
description: |-
Expand Down
Loading
Loading