Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dc.Spec.Config validation for properties #224

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Changelog for Cass Operator, new PRs should update the `main / unreleased` secti
## unreleased

* [CHANGE] [#354](https://github.com/k8ssandra/cass-operator/issues/354) Remove oldDefunctLabel support since we recreate StS. Fix #335 created-by value to match expected value.
* [ENHANCEMENT] [#224](https://github.com/k8ssandra/cass-operator/pull/224) Validate dc.Spec.Config properties against the Cassandra's allowed config value list
* [ENHANCEMENT] [#383](https://github.com/k8ssandra/cass-operator/pull/383) Add UpgradeSSTables, Compaction and Scrub to management-api client. Improve CassandraTasks to have the ability to validate input parameters, filter target pods and do processing outside of pods.
* [ENHANCEMENT] [#384](https://github.com/k8ssandra/cass-operator/issues/384) Add a new CassandraTask operation "replacenode" that removes the existing PVCs from the pod, deletes the pod and starts a replacement process.
* [ENHANCEMENT] [#387](https://github.com/k8ssandra/cass-operator/issues/387) Add a new CassandraTask operation "upgradesstables" that allows to do SSTable upgrades after Cassandra version upgrade.
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and Cust
generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."

.PHONY: generate-configs
generate-configs:
cd hack/config && ./generate.sh

.PHONY: fmt
fmt: ## Run go fmt against code.
go fmt ./...
Expand Down
1,521 changes: 1,521 additions & 0 deletions apis/cassandra/v1beta1/cassandra_config_generated.go

Large diffs are not rendered by default.

44 changes: 43 additions & 1 deletion apis/cassandra/v1beta1/cassandradatacenter_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,17 @@ func ValidateSingleDatacenter(dc CassandraDatacenter) error {
return err
}

return ValidateFQLConfig(dc)
if err := ValidateFQLConfig(dc); err != nil {
return err
}
if !isDse {
err := ValidateConfig(&dc)
if err != nil {
return err
}
}

return nil
}

// ValidateDatacenterFieldChanges checks that no values are improperly changing while updating
Expand Down Expand Up @@ -238,6 +248,38 @@ var (
ErrFQLNotSupported = fmt.Errorf("full query logging is only supported on OSS Cassandra 4.0+")
)

func ValidateConfig(dc *CassandraDatacenter) error {
// TODO Cleanup to more common processing after ModelValues is moved to apis
if dc.Spec.Config != nil {
var dcConfig map[string]interface{}
if err := json.Unmarshal(dc.Spec.Config, &dcConfig); err != nil {
return err
}
casYaml, found := dcConfig["cassandra-yaml"]
if !found {
return nil
}

casYamlMap, ok := casYaml.(map[string]interface{})
if !ok {
err := fmt.Errorf("failed to parse cassandra-yaml")
return err
}

configValues, err := GetCassandraConfigValues(dc.Spec.ServerVersion)
if err != nil {
return err
}
for k := range casYamlMap {
if !configValues.HasProperty(k) {
// We should probably add an event to tell the user that they're using old values
return fmt.Errorf("property %s is not valid for serverVersion %s", k, dc.Spec.ServerVersion)
}
}
}
return nil
}

func ValidateFQLConfig(dc CassandraDatacenter) error {
if dc.Spec.Config != nil {
enabled, err := dc.FullQueryEnabled()
Expand Down
4 changes: 2 additions & 2 deletions apis/cassandra/v1beta1/groupversion_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ limitations under the License.
*/

// Package v1beta1 contains API Schema definitions for the cassandra.datastax.com v1beta1 API group
//+kubebuilder:object:generate=true
//+groupName=cassandra.datastax.com
// +kubebuilder:object:generate=true
// +groupName=cassandra.datastax.com
package v1beta1

import (
Expand Down
42 changes: 42 additions & 0 deletions apis/cassandra/v1beta1/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,48 @@ func Test_ValidateSingleDatacenter(t *testing.T) {
},
errString: "attempted to define config jvm-server-options with cassandra-3.11.7",
},
{
name: "Cassandra 3.11 invalid cassandra-yaml full_query_options",
dc: &CassandraDatacenter{
ObjectMeta: metav1.ObjectMeta{
Name: "exampleDC",
},
Spec: CassandraDatacenterSpec{
ServerType: "cassandra",
ServerVersion: "3.11.11",
Config: json.RawMessage(`
{
"cassandra-yaml": {
"full_query_logging_options": {
"log_dir": "/var/log/cassandra/fql"
}
}
}
`),
},
},
errString: "property full_query_logging_options is not valid for serverVersion 3.11.11",
},
{
name: "Cassandra 4.0.1 invalid cassandra-yaml thrift_max_message_length_in_mb",
dc: &CassandraDatacenter{
ObjectMeta: metav1.ObjectMeta{
Name: "exampleDC",
},
Spec: CassandraDatacenterSpec{
ServerType: "cassandra",
ServerVersion: "4.0.1",
Config: json.RawMessage(`
{
"cassandra-yaml": {
"thrift_max_message_length_in_mb": "256"
}
}
`),
},
},
errString: "property thrift_max_message_length_in_mb is not valid for serverVersion 4.0.1",
},
{
name: "DSE 6.8 invalid config file jvm-options",
dc: &CassandraDatacenter{
Expand Down
4 changes: 2 additions & 2 deletions apis/config/v1beta1/groupversion_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ limitations under the License.
*/

// Package v1 contains API Schema definitions for the config v1 API group
//+kubebuilder:object:generate=true
//+groupName=config.k8ssandra.io
// +kubebuilder:object:generate=true
// +groupName=config.k8ssandra.io
package v1beta1

import (
Expand Down
4 changes: 2 additions & 2 deletions apis/control/v1alpha1/groupversion_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ limitations under the License.
*/

// Package v1alpha1 contains API Schema definitions for the control.k8ssandra.io v1alpha1 API group
//+kubebuilder:object:generate=true
//+groupName=control.k8ssandra.io
// +kubebuilder:object:generate=true
// +groupName=control.k8ssandra.io
package v1alpha1

import (
Expand Down
12 changes: 12 additions & 0 deletions hack/config/Pipfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
javalang = "*"

[dev-packages]

[requires]
python_version = "3.10"
10 changes: 10 additions & 0 deletions hack/config/generate.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/usr/bin/env bash
OUTPUTFILE=../../apis/cassandra/v1beta1/cassandra_config_generated.go
curl -sL https://raw.githubusercontent.com/apache/cassandra/cassandra-4.0.6/src/java/org/apache/cassandra/config/Config.java --output Config-406.java
curl -sL https://raw.githubusercontent.com/apache/cassandra/cassandra-4.0.0/src/java/org/apache/cassandra/config/Config.java --output Config-400.java
curl -sL https://raw.githubusercontent.com/apache/cassandra/cassandra-4.0/src/java/org/apache/cassandra/config/Config.java --output Config-40.java
curl -sL https://raw.githubusercontent.com/apache/cassandra/cassandra-3.11/src/java/org/apache/cassandra/config/Config.java --output Config-311.java
curl -sL https://raw.githubusercontent.com/apache/cassandra/trunk/src/java/org/apache/cassandra/config/Config.java --output Config-trunk.java
pipenv run python parse.py > $OUTPUTFILE
go fmt $OUTPUTFILE
rm -f *.java
85 changes: 85 additions & 0 deletions hack/config/parse.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# This script generates golang structs for Cassandra configurations
import javalang
from pathlib import Path
from typing import List

def parse_file(file: str):
config_file = Path(file).read_text()
tree = javalang.parse.parse(config_file)
for path, node in tree.filter(javalang.tree.ClassDeclaration):
if node.name == "Config":
for field in node.fields:
if 'public' in field.modifiers and not 'static' in field.modifiers:
for declarator in field.declarators:
print(f'"{declarator.name}": true,')
# for annotation in field.annotations:
# if annotation.name == "Deprecated":
# print('Deprecated!')

def generate_configs(versions: List[str]):
print("""
//go:build !ignore_autogenerated
// +build !ignore_autogenerated

// Code is generated with hack/config. DO NOT EDIT.
package v1beta1

import (
"strconv"
"strings"
)

type CassandraConfigValues struct {
accepted map[string]interface{}
}

func (c *CassandraConfigValues) HasProperty(propertyName string) bool {
_, found := c.accepted[propertyName]
return found
}

func GetCassandraConfigValues(serverVersion string) (*CassandraConfigValues, error) {
versionParts := strings.Split(serverVersion, ".")
serverMajorVersion, err := strconv.ParseInt(versionParts[0], 10, 8)
if err != nil {
return nil, err
}

if serverMajorVersion == 3 {
// Return configuration for 3.11, regardless of the real version (we don't support <3.11)
return &config311, nil
}

if serverMajorVersion == 4 {
serverMinorVersion, err := strconv.ParseInt(versionParts[1], 10, 8)
if err != nil {
return nil, err
}
if serverMinorVersion == 0 {
// Return config40
return &config40, nil
}
}
// Something brand new..
return &configtrunk, nil
}

var(
""")

for ver in versions:
create_config_values(ver)

print(')')

def create_config_values(version: str):
print(f"""
config{version} = CassandraConfigValues{{
accepted: map[string]interface{{}}{{""")
parse_file(f'Config-{version}.java')
print("""
},
}
""")

generate_configs(['311', '40', '400', '406', 'trunk', 'DSE'])
14 changes: 7 additions & 7 deletions pkg/generated/clientset/versioned/fake/register.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions pkg/generated/clientset/versioned/scheme/register.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion pkg/httphelper/security.go
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,6 @@ func (provider *ManualManagementApiSecurityProvider) BuildHttpClient(client clie
// Below implementation modified from:
//
// https://go-review.googlesource.com/c/go/+/193620/5/src/crypto/tls/example_test.go#210
//
func buildVerifyPeerCertificateNoHostCheck(rootCAs *x509.CertPool) func([][]byte, [][]*x509.Certificate) error {
f := func(certificates [][]byte, _ [][]*x509.Certificate) error {
certs := make([]*x509.Certificate, len(certificates))
Expand Down
3 changes: 2 additions & 1 deletion pkg/oplabels/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import (
"testing"
)

/**
/*
*
A valid label must be an empty string or consist of alphanumeric characters,
'-', '_' or '.', and must start and end with an alphanumeric.
*/
Expand Down
8 changes: 0 additions & 8 deletions pkg/psp/emm.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,7 @@ type EMMService interface {
getLogger() logr.Logger
}

//
// Util
//
func getPodsRackNameSet(pods []*corev1.Pod) utils.StringSet {
names := utils.StringSet{}
for _, pod := range pods {
Expand All @@ -128,9 +126,7 @@ func filterPodsByRackName(pods []*corev1.Pod, rackName string) []*corev1.Pod {
return utils.FilterPodsWithLabel(pods, api.RackLabel, rackName)
}

//
// EMMOperations impl
//
type EMMServiceImpl struct {
EMMSPI
}
Expand Down Expand Up @@ -386,9 +382,7 @@ func (impl *EMMServiceImpl) getLogger() logr.Logger {
return impl.GetLogger()
}

//
// EMMChecks impl
//
func (impl *EMMServiceImpl) getPodNameSetWithVolumeHealthInaccessiblePVC(rackName string) (utils.StringSet, error) {
pods := impl.GetDCPods()
result := []*corev1.Pod{}
Expand Down Expand Up @@ -434,9 +428,7 @@ func (impl *EMMServiceImpl) getInProgressNodeReplacements() []string {
return impl.GetInProgressNodeReplacements()
}

//
// Helper methods
//
func (impl *EMMServiceImpl) getNodesWithTaintKeyValueEffect(taintKey, value string, effect corev1.TaintEffect) ([]*corev1.Node, error) {
nodes, err := impl.GetAllNodesInDC()
if err != nil {
Expand Down
Loading