druid-io / druid-operator

Druid Kubernetes Operator
Other
205 stars 93 forks source link

Nil pointer dereference when storage class name not set #279

Open applike-ss opened 2 years ago

applike-ss commented 2 years ago

If one is using the default storage class name and therefore doesn't supply a storage class name, the operator does have issues coming up because of a nil pointer exception that is caused in isVolumeExpansionEnabled: controllers/druid/handler.go:881

        sc, err := readers.Get(context.TODO(), sdk, *nodeVCT.Spec.StorageClassName, m, func() object { return makeStorageClassEmptyObj() }, emitEvent)

StorageClassName is dereferenced without being checked for being nil.

Since i haven't read the whole code of the operator, i can not suggest a fix as i don't know the implications it would have.

AdheipSingh commented 2 years ago

@applike-ss thanks for reporting this. I'll take a look into this. Regardless, feel free to send in a patch.

AdheipSingh commented 2 years ago

This function is called only when scalePvcSts is kept to true, in that case operator needs the name of the storageclass to confirm if volume expansion is enabled or not.

applike-ss commented 2 years ago

While that may be true, we have seen a crash in the operator. My assumption is that you might need to make a nil check and if the value is indeed nil, then you'd need to fetch the default that would then be used by kubernetes. Just my 2 cents.

AdheipSingh commented 1 year ago

@applike-ss seems like the code requires some refactoring, i will track this issue in the new repo https://github.com/datainfrahq/druid-operator

Ill tag you in the issue . Thanks

applike-ss commented 1 year ago

Thanks for taking care!