[Bugfix] Prevent unexpected rotation in case of SecurityContext change (#1649)

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [master](https://github.com/arangodb/kube-arangodb/tree/master) (N/A)
 - (Maintenance) Bump Prometheus API Version
+- (Bugfix) Prevent unexpected rotation in case of SecurityContext change
 
 ## [1.2.40](https://github.com/arangodb/kube-arangodb/tree/1.2.40) (2024-04-10)
 - (Feature) Add Core fields to the Scheduler Container Spec

pkg/deployment/rotation/arangod_empty.go

@@ -0,0 +1,68 @@
+//
+// DISCLAIMER
+//
+// Copyright 2024 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+
+package rotation
+
+import (
+	core "k8s.io/api/core/v1"
+
+	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
+	"github.com/arangodb/kube-arangodb/pkg/util"
+	"github.com/arangodb/kube-arangodb/pkg/util/compare"
+)
+
+func compareAndAssignEmptyField[T interface{}](spec, status *T) (*T, bool, error) {
+	if equal, err := util.CompareJSON(spec, status); err != nil {
+		return nil, false, err
+	} else if !equal {
+		if equal, err := util.CompareJSONP(spec, status); err != nil {
+			return nil, false, err
+		} else if equal {
+			return spec, true, nil
+		}
+	}
+
+	return nil, false, nil
+}
+
+func comparePodEmptyFields(_ api.DeploymentSpec, _ api.ServerGroup, spec, status *core.PodTemplateSpec) compare.Func {
+	return func(builder api.ActionBuilder) (mode compare.Mode, plan api.Plan, e error) {
+		if obj, replace, err := compareAndAssignEmptyField(spec.Spec.SecurityContext, status.Spec.SecurityContext); err != nil {
+			e = err
+			return
+		} else if replace {
+			mode = mode.And(compare.SilentRotation)
+			status.Spec.SecurityContext = obj.DeepCopy()
+		}
+		if equal, err := util.CompareJSON(spec.Spec.SecurityContext, status.Spec.SecurityContext); err != nil {
+			e = err
+			return
+		} else if !equal {
+			if equal, err := util.CompareJSONP(spec.Spec.SecurityContext, status.Spec.SecurityContext); err != nil {
+				e = err
+				return
+			} else if equal {
+				mode = mode.And(compare.SilentRotation)
+				status.Spec.SecurityContext = spec.Spec.SecurityContext.DeepCopy()
+			}
+		}
+		return
+	}
+}

pkg/deployment/rotation/arangod_empty_test.go

@@ -0,0 +1,177 @@
+//
+// DISCLAIMER
+//
+// Copyright 2024 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+
+package rotation
+
+import (
+	"testing"
+
+	core "k8s.io/api/core/v1"
+
+	api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
+	"github.com/arangodb/kube-arangodb/pkg/util"
+	"github.com/arangodb/kube-arangodb/pkg/util/compare"
+)
+
+func Test_ArangoD_PodSecurityContext(t *testing.T) {
+	testCases := []TestCase{
+		{
+			name:   "With deployment",
+			spec:   buildPodSpec(),
+			status: buildPodSpec(),
+
+			deploymentSpec: buildDeployment(func(depl *api.DeploymentSpec) {
+
+			}),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.SkippedRotation,
+			},
+		},
+		{
+			name:   "Nil to Nil SecurityContext",
+			spec:   buildPodSpec(),
+			status: buildPodSpec(),
+
+			deploymentSpec: buildDeployment(),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.SkippedRotation,
+			},
+		},
+		{
+			name:   "Nil to Empty SecurityContext",
+			spec:   buildPodSpec(addPodSecurityContext(nil)),
+			status: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{})),
+
+			deploymentSpec: buildDeployment(),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.SilentRotation,
+			},
+		},
+		{
+			name:   "Empty to nil SecurityContext",
+			spec:   buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{})),
+			status: buildPodSpec(addPodSecurityContext(nil)),
+
+			deploymentSpec: buildDeployment(),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.SilentRotation,
+			},
+		},
+		{
+			name:   "Empty to Empty SecurityContext",
+			spec:   buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{})),
+			status: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{})),
+
+			deploymentSpec: buildDeployment(),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.SkippedRotation,
+			},
+		},
+		{
+			name: "Empty to NonEmpty SecurityContext",
+			spec: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{})),
+			status: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{
+				RunAsGroup: util.NewType[int64](1000),
+			})),
+
+			deploymentSpec: buildDeployment(),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.GracefulRotation,
+			},
+		},
+		{
+			name: "Nil to NonEmpty SecurityContext",
+			spec: buildPodSpec(addPodSecurityContext(nil)),
+			status: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{
+				RunAsGroup: util.NewType[int64](1000),
+			})),
+
+			deploymentSpec: buildDeployment(),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.GracefulRotation,
+			},
+		},
+		{
+			name: "NonEmpty to Nil SecurityContext",
+			spec: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{
+				RunAsGroup: util.NewType[int64](1000),
+			})),
+			status: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{})),
+
+			deploymentSpec: buildDeployment(),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.GracefulRotation,
+			},
+		},
+		{
+			name: "NonEmpty to Nil SecurityContext",
+			spec: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{
+				RunAsGroup: util.NewType[int64](1000),
+			})),
+			status: buildPodSpec(addPodSecurityContext(nil)),
+
+			deploymentSpec: buildDeployment(),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.GracefulRotation,
+			},
+		},
+		{
+			name: "NonEmpty to NonEmpty SecurityContext",
+			spec: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{
+				RunAsGroup: util.NewType[int64](1000),
+			})),
+			status: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{
+				RunAsGroup: util.NewType[int64](1000),
+			})),
+
+			deploymentSpec: buildDeployment(),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.SkippedRotation,
+			},
+		},
+		{
+			name: "NonEmpty to NonEmpty Changed SecurityContext",
+			spec: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{
+				RunAsGroup: util.NewType[int64](1000),
+			})),
+			status: buildPodSpec(addPodSecurityContext(&core.PodSecurityContext{
+				RunAsGroup: util.NewType[int64](1001),
+			})),
+
+			deploymentSpec: buildDeployment(),
+
+			TestCaseOverride: TestCaseOverride{
+				expectedMode: compare.GracefulRotation,
+			},
+		},
+	}
+
+	runTestCases(t)(testCases...)
+}

pkg/deployment/rotation/builder_utils_security_context_test.go

@@ -0,0 +1,29 @@
+//
+// DISCLAIMER
+//
+// Copyright 2024 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+
+package rotation
+
+import core "k8s.io/api/core/v1"
+
+func addPodSecurityContext(context *core.PodSecurityContext) podSpecBuilder {
+	return func(pod *core.PodTemplateSpec) {
+		pod.Spec.SecurityContext = context.DeepCopy()
+	}
+}

pkg/deployment/rotation/compare.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2016-2023 ArangoDB GmbH, Cologne, Germany
+// Copyright 2016-2024 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -48,5 +48,5 @@ func compareFunc(deploymentSpec api.DeploymentSpec, member api.MemberStatus, gro
 			return checksum, nil
 		},
 		spec, status,
-		podCompare, affinityCompare, comparePodVolumes, containersCompare, initContainersCompare, comparePodTolerations)
+		podCompare, affinityCompare, comparePodVolumes, containersCompare, initContainersCompare, comparePodTolerations, comparePodEmptyFields)
 }

pkg/deployment/rotation/testdata/pod_lifecycle_change.000.status.json

@@ -289,6 +289,7 @@
       }
     ],
     "restartPolicy": "Never",
+    "securityContext": {},
     "serviceAccountName": "deployment-pod",
     "subdomain": "deployment-int",
     "terminationGracePeriodSeconds": 3600,

pkg/deployment/rotation/utils_test.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2016-2023 ArangoDB GmbH, Cologne, Germany
+// Copyright 2016-2024 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -111,7 +111,7 @@ func runTestCasesForModeAndGroup(t *testing.T, m api.DeploymentMode, g api.Serve
 			require.Error(t, err)
 			require.EqualError(t, err, q.expectedErr)
 		} else {
-			require.Equal(t, q.expectedMode, mode)
+			require.Equalf(t, q.expectedMode, mode, "Expected %s, got %s", q.expectedMode.String(), mode.String())
 
 			switch mode {
 			case compare2.InPlaceRotation:

pkg/util/checksum.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany
+// Copyright 2016-2024 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -35,7 +35,7 @@ func SHA256(data []byte) string {
 	return fmt.Sprintf("%0x", sha256.Sum256(data))
 }
 
-func SHA256FromJSON(a interface{}) (string, error) {
+func SHA256FromJSON[T interface{}](a T) (string, error) {
 	d, err := json.Marshal(a)
 	if err != nil {
 		return "", err
@@ -44,7 +44,7 @@ func SHA256FromJSON(a interface{}) (string, error) {
 	return SHA256(d), nil
 }
 
-func CompareJSON(a, b interface{}) (bool, error) {
+func CompareJSON[T interface{}](a, b T) (bool, error) {
 	ad, err := SHA256FromJSON(a)
 	if err != nil {
 		return false, err
@@ -56,3 +56,17 @@ func CompareJSON(a, b interface{}) (bool, error) {
 
 	return ad == bd, nil
 }
+
+func CompareJSONP[T interface{}](a, b *T) (bool, error) {
+	var a1, b1 T
+
+	if a != nil {
+		a1 = *a
+	}
+
+	if b != nil {
+		b1 = *b
+	}
+
+	return CompareJSON(a1, b1)
+}

pkg/util/compare/mode.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2023 ArangoDB GmbH, Cologne, Germany
+// Copyright 2023-2024 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -38,6 +38,31 @@ const (
 	EnforcedRotation
 )
 
+const (
+	SkippedRotationString  = "Skipped"
+	SilentRotationString   = "Silent"
+	InPlaceRotationString  = "InPlace"
+	GracefulRotationString = "Graceful"
+	EnforcedRotationString = "EnforcedSkipped"
+)
+
+func (m Mode) String() string {
+	switch m {
+	case SkippedRotation:
+		return SkippedRotationString
+	case SilentRotation:
+		return SilentRotationString
+	case InPlaceRotation:
+		return InPlaceRotationString
+	case GracefulRotation:
+		return GracefulRotationString
+	case EnforcedRotation:
+		return EnforcedRotationString
+	}
+
+	return ""
+}
+
 // And returns the higher value of the rotation mode.
 func (m Mode) And(b Mode) Mode {
 	if m > b {