505: detect nil pointer for replicas

streamnative · Dec 16, 2022 · bd8b2c0 · bd8b2c0
1 parent 698546e
commit bd8b2c0
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 3 deletions.
diff --git a/controllers/common.go b/controllers/common.go
@@ -133,3 +133,9 @@ func applyVPA(ctx context.Context, r client.Client, logger logr.Logger, conditio
 	}
 	return nil
 }
+
+func panicIfNil(value interface{}, message string) {
+	if value == nil {
+		panic(message)
+	}
+}
diff --git a/controllers/function.go b/controllers/function.go
@@ -211,6 +211,10 @@ func (r *FunctionReconciler) ApplyFunctionHPA(ctx context.Context, function *v1a
 		// HPA not enabled, skip further action
 		return nil
 	}
+
+	panicIfNil(function.Spec.MinReplicas, "MinReplicas should not be nil but is nil; This is likely because the webhook is not installed properly so it does not have a default value")
+	panicIfNil(function.Spec.Replicas, "Replicas should not be nil but is nil; This is likely because the webhook is not installed properly so it does not have a default value")
+
 	condition := function.Status.Conditions[v1alpha1.HPA]
 	if condition.Status == metav1.ConditionTrue && !newGeneration {
 		return nil

diff --git a/controllers/sink.go b/controllers/sink.go
@@ -211,6 +211,10 @@ func (r *SinkReconciler) ApplySinkHPA(ctx context.Context, sink *v1alpha1.Sink,
 		// HPA not enabled, skip further action
 		return nil
 	}
+
+	panicIfNil(sink.Spec.MinReplicas, "MinReplicas should not be nil but is nil; This is likely because the webhook is not installed properly so it does not have a default value")
+	panicIfNil(sink.Spec.Replicas, "Replicas should not be nil but is nil; This is likely because the webhook is not installed properly so it does not have a default value")
+
 	condition := sink.Status.Conditions[v1alpha1.HPA]
 	if condition.Status == metav1.ConditionTrue && !newGeneration {
 		return nil

diff --git a/controllers/source.go b/controllers/source.go
@@ -211,6 +211,10 @@ func (r *SourceReconciler) ApplySourceHPA(ctx context.Context, source *v1alpha1.
 		// HPA not enabled, skip further action
 		return nil
 	}
+
+	panicIfNil(source.Spec.MinReplicas, "MinReplicas should not be nil but is nil; This is likely because the webhook is not installed properly so it does not have a default value")
+	panicIfNil(source.Spec.Replicas, "Replicas should not be nil but is nil; This is likely because the webhook is not installed properly so it does not have a default value")
+
 	condition := source.Status.Conditions[v1alpha1.HPA]
 	if condition.Status == metav1.ConditionTrue && !newGeneration {
 		return nil

diff --git a/controllers/spec/hpa.go b/controllers/spec/hpa.go
@@ -154,11 +154,13 @@ func makeBuiltinHPA(objectMeta *metav1.ObjectMeta, minReplicas, maxReplicas int3
 	}
 }
 
-func makeHPA(objectMeta *metav1.ObjectMeta, minReplicas, maxReplicas int32, podPolicy v1alpha1.PodPolicy, targetRef autov2beta2.CrossVersionObjectReference) *autov2beta2.HorizontalPodAutoscaler {
+func makeHPA(objectMeta *metav1.ObjectMeta, minReplicas, maxReplicas *int32, podPolicy v1alpha1.PodPolicy, targetRef autov2beta2.CrossVersionObjectReference) *autov2beta2.HorizontalPodAutoscaler {
+	min := *minReplicas
+	max := *maxReplicas
 	spec := autov2beta2.HorizontalPodAutoscalerSpec{
 		ScaleTargetRef: targetRef,
-		MinReplicas:    &minReplicas,
-		MaxReplicas:    maxReplicas,
+		MinReplicas:    &min,
+		MaxReplicas:    max,
 		Metrics:        podPolicy.AutoScalingMetrics,
 		Behavior:       podPolicy.AutoScalingBehavior,
 	}