Bug 1707323: Operator may attempt to write into nil map

panic: assignment to entry in nil map [recovered]
	panic: assignment to entry in nil map

Author: Josef Karasek

pkg/k8shandler/defaults.go

@@ -57,6 +57,9 @@ func calculateReplicaCount(dpl *api.Elasticsearch) int {
 	case api.ZeroRedundancy:
 		return 0
 	default:
+		if dataNodeCount == 1 {
+			return 0
+		}
 		return 1
 	}
 }

pkg/k8shandler/deployment.go

@@ -538,6 +538,14 @@ func (node *deploymentNode) isChanged() bool {
 		nodeContainer := node.self.Spec.Template.Spec.Containers[index]
 		desiredContainer := desired.Spec.Template.Spec.Containers[index]
 
+		if nodeContainer.Resources.Requests == nil {
+			nodeContainer.Resources.Requests = v1.ResourceList{}
+		}
+
+		if nodeContainer.Resources.Limits == nil {
+			nodeContainer.Resources.Limits = v1.ResourceList{}
+		}
+
 		// check that both exist
 
 		if nodeContainer.Image != desiredContainer.Image {

pkg/k8shandler/statefulset.go

@@ -475,6 +475,14 @@ func (node *statefulSetNode) isChanged() bool {
 		nodeContainer := node.self.Spec.Template.Spec.Containers[index]
 		desiredContainer := desired.Spec.Template.Spec.Containers[index]
 
+		if nodeContainer.Resources.Requests == nil {
+			nodeContainer.Resources.Requests = v1.ResourceList{}
+		}
+
+		if nodeContainer.Resources.Limits == nil {
+			nodeContainer.Resources.Limits = v1.ResourceList{}
+		}
+
 		// check that both exist
 
 		if nodeContainer.Image != desiredContainer.Image {

pkg/k8shandler/util.go

@@ -121,6 +121,16 @@ func isValidDataCount(dpl *api.Elasticsearch) bool {
 func isValidRedundancyPolicy(dpl *api.Elasticsearch) bool {
 	dataCount := int(getDataCount(dpl))
 
+	switch dpl.Spec.RedundancyPolicy {
+	case "":
+	case api.ZeroRedundancy:
+	case api.SingleRedundancy:
+	case api.MultipleRedundancy:
+	case api.FullRedundancy:
+	default:
+		return false
+	}
+
 	return !(dataCount == 1 && dpl.Spec.RedundancyPolicy == api.SingleRedundancy)
 }
 
@@ -149,7 +159,7 @@ func isValidConf(dpl *api.Elasticsearch) error {
 		if err := updateConditionWithRetry(dpl, v1.ConditionTrue, updateInvalidReplicationCondition); err != nil {
 			return err
 		}
-		return fmt.Errorf("Wrong RedundancyPolicy selected. Choose different RedundancyPolicy or add more nodes with data roles")
+		return fmt.Errorf("Wrong RedundancyPolicy selected '%s'. Choose different RedundancyPolicy or add more nodes with data roles", dpl.Spec.RedundancyPolicy)
 	} else {
 		if err := updateConditionWithRetry(dpl, v1.ConditionFalse, updateInvalidReplicationCondition); err != nil {
 			return err

pkg/k8shandler/util_test.go

@@ -2,6 +2,8 @@ package k8shandler
 
 import (
 	"testing"
+
+	api "github.com/openshift/elasticsearch-operator/pkg/apis/elasticsearch/v1"
 )
 
 func TestSelectorsBothUndefined(t *testing.T) {
@@ -104,3 +106,121 @@ func TestSelectorsCommonOverwritten(t *testing.T) {
 		t.Errorf("Expected %v but got %v", expected, actual)
 	}
 }
+
+func TestNoRedundancyPolicySpecified(t *testing.T) {
+
+	esCR := &api.Elasticsearch{
+		Spec: api.ElasticsearchSpec{},
+	}
+
+	isValid := isValidRedundancyPolicy(esCR)
+
+	if !isValid {
+		t.Errorf("Expected default policy of SingleRedundancy to be used, incorrectly flagged as invalid")
+	}
+
+}
+
+func TestValidRedundancyPolicySpecified(t *testing.T) {
+
+	esCR := &api.Elasticsearch{
+		Spec: api.ElasticsearchSpec{
+			RedundancyPolicy: api.FullRedundancy,
+		},
+	}
+
+	isValid := isValidRedundancyPolicy(esCR)
+
+	if !isValid {
+		t.Error("Expected FullRedundancy to be valid, flagged as invalid")
+	}
+
+}
+
+func TestInvalidRedundancyPolicySpecified(t *testing.T) {
+
+	esCR := &api.Elasticsearch{
+		Spec: api.ElasticsearchSpec{
+			RedundancyPolicy: "NoRedundancy",
+		},
+	}
+
+	isValid := isValidRedundancyPolicy(esCR)
+
+	if isValid {
+		t.Error("Expected NoRedundancy to be flagged as invalid, was found to be valid")
+	}
+
+}
+
+func TestValidReplicaCount(t *testing.T) {
+
+	dataNodeCount := 5
+
+	esNode := api.ElasticsearchNode{
+		Roles:     []api.ElasticsearchNodeRole{"data"},
+		NodeCount: int32(dataNodeCount),
+	}
+
+	esCR := &api.Elasticsearch{
+		Spec: api.ElasticsearchSpec{
+			RedundancyPolicy: api.FullRedundancy,
+			Nodes:            []api.ElasticsearchNode{esNode},
+		},
+	}
+
+	rc := calculateReplicaCount(esCR)
+
+	// FullRedundancy = dataNodeCount - 1
+	if rc != dataNodeCount-1 {
+		t.Errorf("Expected 4 replica shards for 5 data nodes and FullRedundancy policy, got %d", rc)
+	}
+
+}
+
+func TestNoReplicaCount(t *testing.T) {
+
+	dataNodeCount := 5
+
+	esNode := api.ElasticsearchNode{
+		Roles:     []api.ElasticsearchNodeRole{"data"},
+		NodeCount: int32(dataNodeCount),
+	}
+
+	esCR := &api.Elasticsearch{
+		Spec: api.ElasticsearchSpec{
+			Nodes: []api.ElasticsearchNode{esNode},
+		},
+	}
+
+	rc := calculateReplicaCount(esCR)
+
+	// we default to 1
+	if rc != 1 {
+		t.Errorf("Expected SingleRedundancy, when no policy is specified and cluster has 2 or more data nodes, got %d replica shards", rc)
+	}
+
+}
+
+func TestSingleNodeNoReplicaCount(t *testing.T) {
+
+	dataNodeCount := 1
+
+	esNode := api.ElasticsearchNode{
+		Roles:     []api.ElasticsearchNodeRole{"data"},
+		NodeCount: int32(dataNodeCount),
+	}
+
+	esCR := &api.Elasticsearch{
+		Spec: api.ElasticsearchSpec{
+			Nodes: []api.ElasticsearchNode{esNode},
+		},
+	}
+
+	rc := calculateReplicaCount(esCR)
+
+	if rc != 0 {
+		t.Errorf("Expected ZeroRedundancy, when no policy is specified and cluster has only 1 data node, got %d replica shards", rc)
+	}
+
+}