Merge pull request #243 from arangodb/bugfix/reject-critical-args

Reject critical options during validation fixes #207

Author: ewoutp

pkg/apis/deployment/v1alpha/server_group_spec.go

@@ -23,11 +23,15 @@
 package v1alpha
 
 import (
+	"strings"
+
 	"github.com/pkg/errors"
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 
 	"github.com/arangodb/kube-arangodb/pkg/util"
+	arangod_options "github.com/arangodb/kube-arangodb/pkg/util/arangod/options"
+	arangosync_options "github.com/arangodb/kube-arangodb/pkg/util/arangosync/options"
 	"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
 )
 
@@ -115,6 +119,19 @@ func (s ServerGroupSpec) Validate(group ServerGroup, used bool, mode DeploymentM
 				return maskAny(errors.Wrapf(ValidationError, "Invalid storageClassName: %s", err))
 			}
 		}
+		for _, arg := range s.Args {
+			parts := strings.Split(arg, "=")
+			optionKey := strings.TrimSpace(parts[0])
+			if group.IsArangod() {
+				if arangod_options.IsCriticalOption(optionKey) {
+					return maskAny(errors.Wrapf(ValidationError, "Critical option '%s' cannot be overriden", optionKey))
+				}
+			} else if group.IsArangosync() {
+				if arangosync_options.IsCriticalOption(optionKey) {
+					return maskAny(errors.Wrapf(ValidationError, "Critical option '%s' cannot be overriden", optionKey))
+				}
+			}
+		}
 	} else if s.GetCount() != 0 {
 		return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d for un-used group. Expected 0", s.GetCount()))
 	}

pkg/apis/deployment/v1alpha/server_group_spec_test.go

@@ -107,3 +107,16 @@ func TestServerGroupSpecDefault(t *testing.T) {
 		assert.Equal(t, "", def(ServerGroupSpec{}, g, true, DeploymentModeSingle).GetStorageClassName())
 	}
 }
+
+func TestServerGroupSpecValidateArgs(t *testing.T) {
+	// Valid
+	assert.Nil(t, ServerGroupSpec{Count: util.NewInt(1), Args: []string{}}.Validate(ServerGroupSingle, true, DeploymentModeSingle, EnvironmentDevelopment))
+	assert.Nil(t, ServerGroupSpec{Count: util.NewInt(1), Args: []string{"--master.endpoint"}}.Validate(ServerGroupSingle, true, DeploymentModeSingle, EnvironmentDevelopment))
+	assert.Nil(t, ServerGroupSpec{Count: util.NewInt(1), Args: []string{}}.Validate(ServerGroupSyncMasters, true, DeploymentModeCluster, EnvironmentDevelopment))
+	assert.Nil(t, ServerGroupSpec{Count: util.NewInt(1), Args: []string{"--server.authentication=true"}}.Validate(ServerGroupSyncMasters, true, DeploymentModeCluster, EnvironmentDevelopment))
+	// Invalid
+	assert.Error(t, ServerGroupSpec{Count: util.NewInt(1), Args: []string{"--server.authentication=true"}}.Validate(ServerGroupSingle, true, DeploymentModeSingle, EnvironmentDevelopment))
+	assert.Error(t, ServerGroupSpec{Count: util.NewInt(1), Args: []string{"--server.authentication", "true"}}.Validate(ServerGroupSingle, true, DeploymentModeSingle, EnvironmentDevelopment))
+	assert.Error(t, ServerGroupSpec{Count: util.NewInt(1), Args: []string{"--master.endpoint=http://something"}}.Validate(ServerGroupSyncMasters, true, DeploymentModeCluster, EnvironmentDevelopment))
+	assert.Error(t, ServerGroupSpec{Count: util.NewInt(1), Args: []string{"--mq.type=strange"}}.Validate(ServerGroupSyncMasters, true, DeploymentModeCluster, EnvironmentDevelopment))
+}

pkg/util/arangod/options/options.go

@@ -0,0 +1,60 @@
+//
+// DISCLAIMER
+//
+// Copyright 2018 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
+//
+// Author Ewout Prangsma
+//
+
+package options
+
+import "strings"
+
+var (
+	criticalOptionKeys = map[string]struct{}{
+		"agency.activate":                struct{}{},
+		"agency.disaster-recovery-id":    struct{}{},
+		"agency.endpoint":                struct{}{},
+		"agency.my-address":              struct{}{},
+		"agency.size":                    struct{}{},
+		"agency.supervision":             struct{}{},
+		"cluster.agency-endpoint":        struct{}{},
+		"cluster.my-address":             struct{}{},
+		"cluster.my-role":                struct{}{},
+		"database.directory":             struct{}{},
+		"database.auto-upgrade":          struct{}{},
+		"foxx.queues":                    struct{}{},
+		"replication.automatic-failover": struct{}{},
+		"rocksdb.encryption-keyfile":     struct{}{},
+		"server.authentication":          struct{}{},
+		"server.endpoint":                struct{}{},
+		"server.jwt-secret":              struct{}{},
+		"server.storage-engine":          struct{}{},
+		"ssl.keyfile":                    struct{}{},
+		"ssl.ecdh-curve":                 struct{}{},
+	}
+)
+
+// IsCriticalOption returns true if the given string is the key of
+// an option of arangod that cannot be overwritten.
+func IsCriticalOption(optionKey string) bool {
+	if strings.HasPrefix(optionKey, "--") {
+		optionKey = optionKey[2:]
+	}
+	_, found := criticalOptionKeys[optionKey]
+	return found
+}

pkg/util/arangosync/options/options.go

@@ -0,0 +1,49 @@
+//
+// DISCLAIMER
+//
+// Copyright 2018 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
+//
+// Author Ewout Prangsma
+//
+
+package options
+
+import "strings"
+
+var (
+	criticalOptionKeys = map[string]struct{}{
+		"cluster.jwt-secret":   struct{}{},
+		"cluster.endpoint":     struct{}{},
+		"master.endpoint":      struct{}{},
+		"master.jwt-secret":    struct{}{},
+		"mq.type":              struct{}{},
+		"server.client-cafile": struct{}{},
+		"server.endpoint":      struct{}{},
+		"server.keyfile":       struct{}{},
+		"server.port":          struct{}{},
+	}
+)
+
+// IsCriticalOption returns true if the given string is the key of
+// an option of arangosync that cannot be overwritten.
+func IsCriticalOption(optionKey string) bool {
+	if strings.HasPrefix(optionKey, "--") {
+		optionKey = optionKey[2:]
+	}
+	_, found := criticalOptionKeys[optionKey]
+	return found
+}