Prevent quota reduction via CEL validation in StorageConsumer CRD #3185

OdedViner · 2025-04-24T14:02:13Z

Added CEL validation to ensure StorageQuotaInGiB cannot be decreased via CR updates.

Tested manually:
1.Delete the old CRD, apply the new CRD, and verify that the CRD was created.

$ oc delete crd storageconsumers.ocs.openshift.io
customresourcedefinition.apiextensions.k8s.io "storageconsumers.ocs.openshift.io" deleted
$ oc get crd storageconsumers.ocs.openshift.io
Error from server (NotFound): customresourcedefinitions.apiextensions.k8s.io "storageconsumers.ocs.openshift.io" not found
$ oc apply -f config/crd/bases/ocs.openshift.io_storageconsumers.yaml
customresourcedefinition.apiextensions.k8s.io/storageconsumers.ocs.openshift.io created
$ oc get crd storageconsumers.ocs.openshift.io
NAME                                CREATED AT
storageconsumers.ocs.openshift.io   2025-04-28T09:07:44Z

2.Create a StorageConsumer Custom Resource

$ oc apply -f - <<EOF
apiVersion: ocs.openshift.io/v1alpha1
kind: StorageConsumer
metadata:
  name: test-consumer
  namespace: default
spec:
  enable: true
  storageQuotaInGiB: 100
EOF
storageconsumer.ocs.openshift.io/test-consumer created

$ oc get storageconsumers.ocs.openshift.io -n default -o yaml
apiVersion: v1
items:
- apiVersion: ocs.openshift.io/v1alpha1
  kind: StorageConsumer
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"ocs.openshift.io/v1alpha1","kind":"StorageConsumer","metadata":{"annotations":{},"name":"test-consumer","namespace":"default"},"spec":{"enable":true,"storageQuotaInGiB":100}}
    creationTimestamp: "2025-04-28T09:08:53Z"
    generation: 1
    name: test-consumer
    namespace: default
    resourceVersion: "3132911"
    uid: 025bdade-85eb-46f9-a9b8-2169f6649f27
  spec:
    enable: true
    storageQuotaInGiB: 100
kind: List
metadata:
  resourceVersion: ""

3.Test: Decrease the storageQuotaInGiB from 100 → 50 (Expect Rejection)

$ oc patch storageconsumer test-consumer --type=merge -p '{"spec": {"storageQuotaInGiB": 50}}'
The StorageConsumer "test-consumer" is invalid: spec: Invalid value: "object": storageQuotaInGiB cannot be decreased on update

$ oc get storageconsumers.ocs.openshift.io -n default -o yaml
apiVersion: v1
items:
- apiVersion: ocs.openshift.io/v1alpha1
  kind: StorageConsumer
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"ocs.openshift.io/v1alpha1","kind":"StorageConsumer","metadata":{"annotations":{},"name":"test-consumer","namespace":"default"},"spec":{"enable":true,"storageQuotaInGiB":100}}
    creationTimestamp: "2025-04-28T09:08:53Z"
    generation: 1
    name: test-consumer
    namespace: default
    resourceVersion: "3132911"
    uid: 025bdade-85eb-46f9-a9b8-2169f6649f27
  spec:
    enable: true
    storageQuotaInGiB: 100
kind: List
metadata:
  resourceVersion: ""

Test: Increase the storageQuotaInGiB from 100 → 110 (Expect Success)

$ oc patch storageconsumer test-consumer --type=merge -p '{"spec": {"storageQuotaInGiB": 110}}'
storageconsumer.ocs.openshift.io/test-consumer patched

$ oc get storageconsumers.ocs.openshift.io -n default -o yaml
apiVersion: v1
items:
- apiVersion: ocs.openshift.io/v1alpha1
  kind: StorageConsumer
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"ocs.openshift.io/v1alpha1","kind":"StorageConsumer","metadata":{"annotations":{},"name":"test-consumer","namespace":"default"},"spec":{"enable":true,"storageQuotaInGiB":100}}
    creationTimestamp: "2025-04-28T09:08:53Z"
    generation: 2
    name: test-consumer
    namespace: default
    resourceVersion: "3134674"
    uid: 025bdade-85eb-46f9-a9b8-2169f6649f27
  spec:
    enable: true
    storageQuotaInGiB: 110
kind: List
metadata:
  resourceVersion: ""

5.Test: Decrease the storageQuotaInGiB from 100 → 0 (Expect Success)
When StorageQuotaInGiB=0, a ClusterResourceQuota is not created because the function checks if consumer.Spec.StorageQuotaInGiB > 0 before adding a quota resource.

ocs-operator/services/provider/server/server.go

Line 2060 in 91f2ec4

if consumer.Spec.StorageQuotaInGiB > 0 {

$  oc patch storageconsumer test-consumer --type=merge -p '{"spec": {"storageQuotaInGiB": 0}}'
storageconsumer.ocs.openshift.io/test-consumer patched

$ oc get storageconsumers.ocs.openshift.io -n default -o yaml
apiVersion: v1
items:
- apiVersion: ocs.openshift.io/v1alpha1
  kind: StorageConsumer
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"ocs.openshift.io/v1alpha1","kind":"StorageConsumer","metadata":{"annotations":{},"name":"test-consumer","namespace":"default"},"spec":{"enable":true,"storageQuotaInGiB":100}}
    creationTimestamp: "2025-04-28T11:37:09Z"
    generation: 3
    name: test-consumer
    namespace: default
    resourceVersion: "3227178"
    uid: 7f2a32e1-6272-4840-83f8-fced2dcfa600
  spec:
    enable: true
    storageQuotaInGiB: 0
kind: List
metadata:
  resourceVersion: ""

6.Test: Increase the storageQuotaInGiB from 0 → 34 (Expect Success)

$  oc patch storageconsumer test-consumer --type=merge -p '{"spec": {"storageQuotaInGiB": 34}}'
storageconsumer.ocs.openshift.io/test-consumer patched

oviner~/DEV_REPOS/ocs-operator(cel_validate_quota_reduction)$ oc get storageconsumers.ocs.openshift.io -n default -o yaml
apiVersion: v1
items:
- apiVersion: ocs.openshift.io/v1alpha1
  kind: StorageConsumer
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"ocs.openshift.io/v1alpha1","kind":"StorageConsumer","metadata":{"annotations":{},"name":"test-consumer","namespace":"default"},"spec":{"enable":true,"storageQuotaInGiB":100}}
    creationTimestamp: "2025-04-28T11:37:09Z"
    generation: 4
    name: test-consumer
    namespace: default
    resourceVersion: "3227976"
    uid: 7f2a32e1-6272-4840-83f8-fced2dcfa600
  spec:
    enable: true
    storageQuotaInGiB: 34
kind: List
metadata:
  resourceVersion: ""

iamniting · 2025-04-24T15:35:37Z

api/v1alpha1/storageconsumer_types.go

@@ -104,6 +104,7 @@ type StorageConsumerStatus struct {
 }

 // ClientStatus is the information pushed from connected storage client
+// +kubebuilder:validation:XValidation:rule="!(has(oldSelf.storageQuotaUtilizationRatio) && self.storageQuotaUtilizationRatio < oldSelf.storageQuotaUtilizationRatio)",message="storageQuotaUtilizationRatio cannot be decreased on update"


Lets keep the message and rule in different line if possible to have a better readibility.

controller-gen v0.16.1 does not support splitting rule and message across two XValidation: lines. It treats them as two separate markers, which is invalid.

$ ./bin/controller-gen --version Version: v0.16.1

oviner~/DEV_REPOS/ocs-operator(cel_validate_quota_reduction)$ make gen-latest-csv Ensuring operator-sdk hack/ensure-operator-sdk.sh Using operator-sdk CLI present at /home/oviner/DEV_REPOS/ocs-operator/bin/operator-sdk-v1.25.4 Using existing [email protected] at /home/oviner/DEV_REPOS/ocs-operator/bin/controller-gen Updating generated manifests /home/oviner/DEV_REPOS/ocs-operator/bin/controller-gen rbac:roleName=manager-role crd:generateEmbeddedObjectMeta=true,allowDangerousTypes=true paths=./api/... webhook paths="./..." output:crd:artifacts:config=config/crd/bases /home/oviner/DEV_REPOS/ocs-operator/api/v1alpha1/storageconsumer_types.go:108:1: missing argument "rule" (at <input>:1:69) /home/oviner/DEV_REPOS/ocs-operator/api/v1alpha1/storageconsumer_types.go:108:1: missing argument "rule" (at <input>:1:69) /home/oviner/DEV_REPOS/ocs-operator/api/v1alpha1/storageconsumer_types.go:108:1: missing argument "rule" (at <input>:1:69) /home/oviner/DEV_REPOS/ocs-operator/api/v1alpha1/storageconsumer_types.go:108:1: missing argument "rule" (at <input>:1:69) Error: not all generators ran successfully run `controller-gen rbac:roleName=manager-role crd:generateEmbeddedObjectMeta=true,allowDangerousTypes=true paths=./api/... webhook paths=./... output:crd:artifacts:config=config/crd/bases -w` to see all available markers, or `controller-gen rbac:roleName=manager-role crd:generateEmbeddedObjectMeta=true,allowDangerousTypes=true paths=./api/... webhook paths=./... output:crd:artifacts:config=config/crd/bases -h` for usage make: *** [Makefile:139: manifests] Error 1

openshift-ci · 2025-04-25T15:34:34Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: OdedViner
Once this PR has been reviewed and has the lgtm label, please ask for approval from iamniting. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

iamniting

Why do you need a not condition, instead you can use the >

iamniting · 2025-04-28T09:59:45Z

@OdedViner Have you tested this new rule? Is it working as expected?

OdedViner · 2025-04-28T10:03:29Z

@OdedViner Have you tested this new rule? Is it working as expected?

yes, I changed the PR description with new test #3185 (comment)

iamniting · 2025-04-28T10:05:12Z

@OdedViner Have you tested this new rule? Is it working as expected?

I saw the latest results in the description. Can you also test to delete it or move it to 0 and adjust the rule accordingly?

FYR https://issues.redhat.com/browse/DFBUGS-1301?focusedId=27054192&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-27054192

OdedViner · 2025-04-28T11:57:35Z

@OdedViner Have you tested this new rule? Is it working as expected?

I saw the latest results in the description. Can you also test to delete it or move it to 0 and adjust the rule accordingly?

FYR https://issues.redhat.com/browse/DFBUGS-1301?focusedId=27054192&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-27054192

I saw the latest results in the description. Can you also test to delete it or move it to 0 and adjust the rule accordingly?

Hi @iamniting ,

I updated the rule to:
self.storageQuotaInGiB == 0 || self.storageQuotaInGiB >= oldSelf.storageQuotaInGiB
and added two additional steps (steps 5 and 6) to my manual test procedure to cover these cases.
#3185 (comment)

Thanks for the review!

Signed-off-by: Oded Viner <[email protected]>

iamniting · 2025-04-28T15:41:36Z

@OdedViner Have you tested this new rule? Is it working as expected?

I saw the latest results in the description. Can you also test to delete it or move it to 0 and adjust the rule accordingly?
FYR https://issues.redhat.com/browse/DFBUGS-1301?focusedId=27054192&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-27054192

I saw the latest results in the description. Can you also test to delete it or move it to 0 and adjust the rule accordingly?

Hi @iamniting ,

I updated the rule to: self.storageQuotaInGiB == 0 || self.storageQuotaInGiB >= oldSelf.storageQuotaInGiB and added two additional steps (steps 5 and 6) to my manual test procedure to cover these cases. #3185 (comment)

Thanks for the review!

Can you also check if this allows you to delete the field?

OdedViner · 2025-04-29T11:47:07Z

Can you also check if this allows you to delete the field?

@iamniting Is this the expected behavior?

$ oc patch storageconsumer test-consumer -n default --type=json -p='[{"op": "remove", "path": "/spec/storageQuotaInGiB"}]'
The StorageConsumer "test-consumer" is invalid: spec: Invalid value: "object": no such key: storageQuotaInGiB evaluating rule: storageQuotaInGiB cannot be decreased unless setting to 0

OdedViner · 2025-04-29T11:48:13Z

@OdedViner: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ocs-operator-bundle-e2e-aws a650d4b link true /test ocs-operator-bundle-e2e-aws
Full PR test history. Your PR dashboard.

@iamniting Is this a temp issue?

iamniting · 2025-04-29T15:28:56Z

Can you also check if this allows you to delete the field?

@iamniting Is this the expected behavior?

$ oc patch storageconsumer test-consumer -n default --type=json -p='[{"op": "remove", "path": "/spec/storageQuotaInGiB"}]'
The StorageConsumer "test-consumer" is invalid: spec: Invalid value: "object": no such key: storageQuotaInGiB evaluating rule: storageQuotaInGiB cannot be decreased unless setting to 0

Can you try doing the same thing with oc edit?

iamniting · 2025-04-29T15:29:14Z

/test ocs-operator-bundle-e2e-aws

openshift-ci · 2025-04-29T17:59:10Z

@OdedViner: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/ocs-operator-bundle-e2e-aws	`a650d4b`	link	true	`/test ocs-operator-bundle-e2e-aws`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 24, 2025

OdedViner force-pushed the cel_validate_quota_reduction branch 5 times, most recently from c50497d to aa0b959 Compare April 24, 2025 14:25

iamniting requested changes Apr 24, 2025

View reviewed changes

openshift-ci bot assigned iamniting Apr 24, 2025

OdedViner force-pushed the cel_validate_quota_reduction branch from aa0b959 to 7067ecd Compare April 25, 2025 15:34

OdedViner force-pushed the cel_validate_quota_reduction branch 2 times, most recently from 9d8b330 to 56913a6 Compare April 25, 2025 15:45

OdedViner changed the title ~~WIP: Prevent quota reduction via CEL validation in StorageConsumer CRD~~ Prevent quota reduction via CEL validation in StorageConsumer CRD Apr 25, 2025

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 25, 2025

OdedViner requested a review from iamniting April 26, 2025 07:59

iamniting requested changes Apr 26, 2025

View reviewed changes

OdedViner force-pushed the cel_validate_quota_reduction branch 3 times, most recently from 3711373 to 734f0c7 Compare April 28, 2025 09:26

OdedViner force-pushed the cel_validate_quota_reduction branch from 734f0c7 to 8ae909b Compare April 28, 2025 11:38

Prevent quota reduction via CEL validation in StorageConsumer CRD

a650d4b

Signed-off-by: Oded Viner <[email protected]>

OdedViner force-pushed the cel_validate_quota_reduction branch from 8ae909b to a650d4b Compare April 28, 2025 12:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent quota reduction via CEL validation in StorageConsumer CRD #3185

Prevent quota reduction via CEL validation in StorageConsumer CRD #3185

OdedViner commented Apr 24, 2025 •

edited

Loading

iamniting Apr 24, 2025

OdedViner Apr 25, 2025 •

edited

Loading

openshift-ci bot commented Apr 25, 2025

iamniting left a comment

iamniting commented Apr 28, 2025

OdedViner commented Apr 28, 2025

iamniting commented Apr 28, 2025

OdedViner commented Apr 28, 2025

iamniting commented Apr 28, 2025

OdedViner commented Apr 29, 2025

OdedViner commented Apr 29, 2025

iamniting commented Apr 29, 2025

iamniting commented Apr 29, 2025

openshift-ci bot commented Apr 29, 2025

Prevent quota reduction via CEL validation in StorageConsumer CRD #3185

Are you sure you want to change the base?

Prevent quota reduction via CEL validation in StorageConsumer CRD #3185

Conversation

OdedViner commented Apr 24, 2025 • edited Loading

iamniting Apr 24, 2025

Choose a reason for hiding this comment

OdedViner Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

openshift-ci bot commented Apr 25, 2025

iamniting left a comment

Choose a reason for hiding this comment

iamniting commented Apr 28, 2025

OdedViner commented Apr 28, 2025

iamniting commented Apr 28, 2025

OdedViner commented Apr 28, 2025

iamniting commented Apr 28, 2025

OdedViner commented Apr 29, 2025

OdedViner commented Apr 29, 2025

iamniting commented Apr 29, 2025

iamniting commented Apr 29, 2025

openshift-ci bot commented Apr 29, 2025

OdedViner commented Apr 24, 2025 •

edited

Loading

OdedViner Apr 25, 2025 •

edited

Loading