red-hat-storage / ocs-operator

Operator for RHOCS
Apache License 2.0
85 stars 184 forks source link

fix panic in provider server while onboarding #2826

Closed rewantsoni closed 1 month ago

rewantsoni commented 1 month ago

Fix provider server panic while onboarding Consumers when the value of the storageQuota in the onboardingTicket is set to nil

Introduced with: https://github.com/red-hat-storage/ocs-operator/pull/2669

rewantsoni commented 1 month ago

What is the issue with converting the unit to int? We are not expecting very large numbers here. How does moving the conversion from outside the function into the function solves the problem?

@nb-ohad The issue is with converting the pointer to unit to int. If the storageQuotaInGib is nil it will panic. I moved everything inside a inline function to make it clean and return 0 as storageQuotaInGib if the value is nil (we could have the same check outside the function as well)

nb-ohad commented 1 month ago

@rewantsoni You should not convert a pointer. You should convert the value is not the same thing. Converting a pointer means that you are trying to reference the same underlying value but at the same time interpreting it as something else. That is wrong in this case.

You should convert the value (outside of the function), and send the converted value into the function.

nb-ohad commented 1 month ago

A good fix for the bug will be


size := ptr.Deref(onboardingTicket.StorageQuotaInGiB, 0)
if size > math.MaxInt {
    // return some error
} 
storageConsumerUUID, err := s.consumerManager.Create(ctx, req, int(size))
nb-ohad commented 1 month ago

/lgtm

openshift-ci[bot] commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leelavg, nb-ohad, rewantsoni

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/red-hat-storage/ocs-operator/blob/main/OWNERS)~~ [nb-ohad] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment