-
Notifications
You must be signed in to change notification settings - Fork 83
⚠ make spec.namespace optional with managed namespace support and PSA support #2825
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
e1183cb
286f7e4
f547bd8
3602f1b
4843c7d
d30a2bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,21 +49,34 @@ const ( | |
|
|
||
| // ClusterExtensionSpec defines the desired state of ClusterExtension | ||
| type ClusterExtensionSpec struct { | ||
| // namespace specifies a Kubernetes namespace. | ||
| // It designates the default namespace where namespace-scoped resources for the extension are applied to the cluster. | ||
| // Some extensions may contain namespace-scoped resources to be applied in other namespaces. | ||
| // This namespace must exist. | ||
| // namespace selects the namespace that namespace-scoped resources for the extension | ||
| // are applied to. | ||
| // | ||
| // The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. | ||
| // <opcon:standard:description> | ||
| // In the standard configuration, namespace is required and must reference an existing | ||
| // namespace on the cluster. | ||
| // </opcon:standard:description> | ||
| // <opcon:experimental:description> | ||
| // BoxcutterRuntime feature set, namespace is optional. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit again, sorry: Ignore feature gates, experimental vs. standard, etc. Just plainly state what the field is/means, etc. for users of the experimental release. There should be no appearance of "when experimental is enabled" or "with the BoxcutterRuntime feature gate", etc. Imagine this feature has been promoted to standard and it just is this way in GA. Use that description. That way when we do promote to standard, all we need to do is drop the |
||
| // When set, it must reference an existing namespace. When omitted, operator-controller | ||
| // resolves and creates a managed namespace from bundle metadata. The mode (set vs omitted) | ||
| // is locked at creation time and cannot be changed. | ||
| // </opcon:experimental:description> | ||
| // | ||
| // The namespace field follows the DNS label standard as defined in [RFC 1123]. | ||
| // It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character, | ||
| // and be no longer than 63 characters. | ||
| // | ||
| // [RFC 1123]: https://tools.ietf.org/html/rfc1123 | ||
| // | ||
| // <opcon:standard:validation:Required> | ||
| // <opcon:standard:validation:XValidation:rule="self != ''",message="namespace is required"> | ||
| // <opcon:experimental:validation:XValidation:rule="oldSelf != '' || self == ''",message="namespace cannot be set after creation; mode is locked at creation time"> | ||
| // | ||
| // +kubebuilder:validation:MaxLength:=63 | ||
| // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="namespace is immutable" | ||
| // +kubebuilder:validation:XValidation:rule="self.matches(\"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$\")",message="namespace must be a valid DNS1123 label" | ||
| // +required | ||
| // +kubebuilder:validation:XValidation:rule="self == '' || self.matches(\"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$\")",message="namespace must be a valid DNS1123 label" | ||
| // +kubebuilder:validation:XValidation:rule="oldSelf == '' || self == oldSelf",message="namespace is immutable once set" | ||
| // +optional | ||
| Namespace string `json:"namespace"` | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // serviceAccount is a deprecated field and is completely ignored. | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -507,6 +507,7 @@ func run() error { | |
| IsWebhookSupportEnabled: certProvider != nil, | ||
| IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport), | ||
| IsDeploymentConfigEnabled: features.OperatorControllerFeatureGate.Enabled(features.DeploymentConfig), | ||
| IsBoxcutterRuntimeEnabled: features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime), | ||
| } | ||
| var cerCfg reconcilerConfigurator | ||
| if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { | ||
|
|
@@ -659,6 +660,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl | |
| controllers.RetrieveRevisionStates(revisionStatesGetter), | ||
| controllers.ResolveBundle(c.resolver, c.mgr.GetClient()), | ||
| controllers.UnpackBundle(c.imagePuller, c.imageCache), | ||
| controllers.ValidateInstallNamespace(coreClient), | ||
| controllers.ApplyBundleWithBoxcutter(appl.Apply), | ||
| } | ||
|
|
||
|
|
@@ -746,6 +748,7 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster | |
| controllers.RetrieveRevisionStates(revisionStatesGetter), | ||
| controllers.ResolveBundle(c.resolver, c.mgr.GetClient()), | ||
| controllers.UnpackBundle(c.imagePuller, c.imageCache), | ||
| controllers.ValidateInstallNamespace(coreClient), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought we decided on no changes to the helm reconciler/applier path? |
||
| controllers.ApplyBundle(appl), | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry didn't notice this on my first comment about the CRD changes: Let's leave the standard configuration CRD completely unchanged. When we re-gen the CRDs, I'd expect
helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yamlto be unchanged.