From 9ea81c861b1cc72b70ed87620c5879ad2320e1e7 Mon Sep 17 00:00:00 2001 From: Kevin McDermott Date: Mon, 19 Jan 2026 15:16:47 +0000 Subject: [PATCH] Use resource.Quantity for storageRequestSize Previously the resource.Quantity was stored as string which allowed invalid values to be created. This performs validation on the strings using the standard K8s resource mechanism. --- charts/k3k/templates/crds/k3k.io_clusters.yaml | 8 ++++++-- .../k3k/templates/crds/k3k.io_virtualclusterpolicies.yaml | 2 +- cli/cmds/cluster_create.go | 8 ++++++-- cli/cmds/cluster_create_test.go | 5 +++-- docs/crds/crds.adoc | 2 +- docs/crds/crds.md | 2 +- pkg/apis/k3k.io/v1beta1/types.go | 3 ++- pkg/apis/k3k.io/v1beta1/zz_generated.deepcopy.go | 5 +++++ pkg/controller/cluster/server/server.go | 3 +-- scripts/generate | 2 +- 10 files changed, 27 insertions(+), 13 deletions(-) diff --git a/charts/k3k/templates/crds/k3k.io_clusters.yaml b/charts/k3k/templates/crds/k3k.io_clusters.yaml index 0f27723..1e8b29e 100644 --- a/charts/k3k/templates/crds/k3k.io_clusters.yaml +++ b/charts/k3k/templates/crds/k3k.io_clusters.yaml @@ -4,7 +4,7 @@ kind: CustomResourceDefinition metadata: annotations: helm.sh/resource-policy: keep - controller-gen.kubebuilder.io/version: v0.18.0 + controller-gen.kubebuilder.io/version: v0.20.0 name: clusters.k3k.io spec: group: k3k.io @@ -434,11 +434,15 @@ spec: This field is only relevant in "dynamic" mode. type: string storageRequestSize: + anyOf: + - type: integer + - type: string default: 2G description: |- StorageRequestSize is the requested size for the PVC. This field is only relevant in "dynamic" mode. - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true type: default: dynamic description: Type specifies the persistence mode. diff --git a/charts/k3k/templates/crds/k3k.io_virtualclusterpolicies.yaml b/charts/k3k/templates/crds/k3k.io_virtualclusterpolicies.yaml index 5a81252..28a8790 100644 --- a/charts/k3k/templates/crds/k3k.io_virtualclusterpolicies.yaml +++ b/charts/k3k/templates/crds/k3k.io_virtualclusterpolicies.yaml @@ -4,7 +4,7 @@ kind: CustomResourceDefinition metadata: annotations: helm.sh/resource-policy: keep - controller-gen.kubebuilder.io/version: v0.18.0 + controller-gen.kubebuilder.io/version: v0.20.0 name: virtualclusterpolicies.k3k.io spec: group: k3k.io diff --git a/cli/cmds/cluster_create.go b/cli/cmds/cluster_create.go index 204eb35..f5a113c 100644 --- a/cli/cmds/cluster_create.go +++ b/cli/cmds/cluster_create.go @@ -20,6 +20,7 @@ import ( v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -211,7 +212,7 @@ func newCluster(name, namespace string, config *CreateConfig) *v1beta1.Cluster { Persistence: v1beta1.PersistenceConfig{ Type: v1beta1.PersistenceMode(config.persistenceType), StorageClassName: ptr.To(config.storageClassName), - StorageRequestSize: config.storageRequestSize, + StorageRequestSize: ptr.To(resource.MustParse(config.storageRequestSize)), }, MirrorHostNodes: config.mirrorHostNodes, }, @@ -433,7 +434,10 @@ func getClusterDetails(cluster *v1beta1.Cluster) (string, error) { data.Persistence.Type = cluster.Spec.Persistence.Type data.Persistence.StorageClassName = ptr.Deref(cluster.Spec.Persistence.StorageClassName, "") - data.Persistence.StorageRequestSize = cluster.Spec.Persistence.StorageRequestSize + + if srs := cluster.Spec.Persistence.StorageRequestSize; srs != nil { + data.Persistence.StorageRequestSize = srs.String() + } tmpl, err := template.New("clusterDetails").Parse(clusterDetailsTemplate) if err != nil { diff --git a/cli/cmds/cluster_create_test.go b/cli/cmds/cluster_create_test.go index 64b91f9..1c3153a 100644 --- a/cli/cmds/cluster_create_test.go +++ b/cli/cmds/cluster_create_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/utils/ptr" "github.com/rancher/k3k/pkg/apis/k3k.io/v1beta1" @@ -66,7 +67,7 @@ func Test_printClusterDetails(t *testing.T) { Persistence: v1beta1.PersistenceConfig{ Type: v1beta1.DynamicPersistenceMode, StorageClassName: ptr.To("local-path"), - StorageRequestSize: "3gb", + StorageRequestSize: ptr.To(resource.MustParse("3G")), }, }, Status: v1beta1.ClusterStatus{ @@ -81,7 +82,7 @@ func Test_printClusterDetails(t *testing.T) { Persistence: Type: dynamic StorageClass: local-path - Size: 3gb`, + Size: 3G`, }, } diff --git a/docs/crds/crds.adoc b/docs/crds/crds.adoc index 0bde2b3..6cf8024 100644 --- a/docs/crds/crds.adoc +++ b/docs/crds/crds.adoc @@ -414,7 +414,7 @@ _Appears In:_ | *`type`* __xref:{anchor_prefix}-github-com-rancher-k3k-pkg-apis-k3k-io-v1beta1-persistencemode[$$PersistenceMode$$]__ | Type specifies the persistence mode. + | dynamic | | *`storageClassName`* __string__ | StorageClassName is the name of the StorageClass to use for the PVC. + This field is only relevant in "dynamic" mode. + | | -| *`storageRequestSize`* __string__ | StorageRequestSize is the requested size for the PVC. + +| *`storageRequestSize`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#quantity-resource-api[$$Quantity$$]__ | StorageRequestSize is the requested size for the PVC. + This field is only relevant in "dynamic" mode. + | 2G | |=== diff --git a/docs/crds/crds.md b/docs/crds/crds.md index a02eaff..c74c7c2 100644 --- a/docs/crds/crds.md +++ b/docs/crds/crds.md @@ -313,7 +313,7 @@ _Appears in:_ | --- | --- | --- | --- | | `type` _[PersistenceMode](#persistencemode)_ | Type specifies the persistence mode. | dynamic | | | `storageClassName` _string_ | StorageClassName is the name of the StorageClass to use for the PVC.
This field is only relevant in "dynamic" mode. | | | -| `storageRequestSize` _string_ | StorageRequestSize is the requested size for the PVC.
This field is only relevant in "dynamic" mode. | 2G | | +| `storageRequestSize` _[Quantity](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#quantity-resource-api)_ | StorageRequestSize is the requested size for the PVC.
This field is only relevant in "dynamic" mode. | 2G | | #### PersistenceMode diff --git a/pkg/apis/k3k.io/v1beta1/types.go b/pkg/apis/k3k.io/v1beta1/types.go index 44a6b22..560fe87 100644 --- a/pkg/apis/k3k.io/v1beta1/types.go +++ b/pkg/apis/k3k.io/v1beta1/types.go @@ -2,6 +2,7 @@ package v1beta1 import ( v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -363,7 +364,7 @@ type PersistenceConfig struct { // // +kubebuilder:default="2G" // +optional - StorageRequestSize string `json:"storageRequestSize,omitempty"` + StorageRequestSize *resource.Quantity `json:"storageRequestSize,omitempty"` } // ExposeConfig specifies options for exposing the API server. diff --git a/pkg/apis/k3k.io/v1beta1/zz_generated.deepcopy.go b/pkg/apis/k3k.io/v1beta1/zz_generated.deepcopy.go index d359555..a624e9c 100644 --- a/pkg/apis/k3k.io/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/k3k.io/v1beta1/zz_generated.deepcopy.go @@ -418,6 +418,11 @@ func (in *PersistenceConfig) DeepCopyInto(out *PersistenceConfig) { *out = new(string) **out = **in } + if in.StorageRequestSize != nil { + in, out := &in.StorageRequestSize, &out.StorageRequestSize + x := (*in).DeepCopy() + *out = &x + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PersistenceConfig. diff --git a/pkg/controller/cluster/server/server.go b/pkg/controller/cluster/server/server.go index 16f491e..5e92ebb 100644 --- a/pkg/controller/cluster/server/server.go +++ b/pkg/controller/cluster/server/server.go @@ -8,7 +8,6 @@ import ( "strings" "text/template" - "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" @@ -406,7 +405,7 @@ func (s *Server) setupDynamicPersistence() v1.PersistentVolumeClaim { StorageClassName: s.cluster.Spec.Persistence.StorageClassName, Resources: v1.VolumeResourceRequirements{ Requests: v1.ResourceList{ - "storage": resource.MustParse(s.cluster.Spec.Persistence.StorageRequestSize), + "storage": *s.cluster.Spec.Persistence.StorageRequestSize, }, }, }, diff --git a/scripts/generate b/scripts/generate index 5c4a191..f806f9d 100755 --- a/scripts/generate +++ b/scripts/generate @@ -3,7 +3,7 @@ set -eou pipefail -CONTROLLER_TOOLS_VERSION=v0.18.0 +CONTROLLER_TOOLS_VERSION=v0.20.0 # This will return non-zero until all of our objects in ./pkg/apis can generate valid crds. # allowDangerousTypes is needed for struct that use floats