Skip to content

Commit

Permalink
Add dc.Spec.Config validation for properties that are not available i…
Browse files Browse the repository at this point in the history
…n defined version
  • Loading branch information
burmanm committed Nov 2, 2021
1 parent ed4a19b commit ffe1cc2
Show file tree
Hide file tree
Showing 14 changed files with 920 additions and 97 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Changelog for Cass Operator, new PRs should update the `main / unreleased` secti
* [CHANGE] #202 Support fetching FeatureSet from management-api if available. Return RequestError with StatusCode when endpoint has bad status.
* [ENHANCEMENT] #175 Add FQL reconciliation via parseFQLFromConfig and SetFullQueryLogging called from ReconcileAllRacks. CallIsFullQueryLogEnabledEndpoint and CallSetFullQueryLog functions to httphelper.
* [ENHANCEMENT] #185 Add more app.kubernetes.io labels to all the managed resources
* [ENHANCEMENT] Validate dc.Spec.Config properties against the Cassandra's allowed config value list

## v1.8.0
* [CHANGE] #178 If clusterName includes characters not allowed in the serviceName, strip those chars from service name.
Expand Down
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ 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="./..."

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

fmt: ## Run go fmt against code.
go fmt ./...

Expand All @@ -96,7 +99,7 @@ test: manifests generate fmt vet envtest ## Run tests.
# Old unit tests first - these use mocked client / fakeclient
go test ./pkg/... -coverprofile cover-operator.out
# Then the envtest ones
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./controllers/... -coverprofile cover.out
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./apis/... ./controllers/... -coverprofile cover.out

integ-test: kustomize cert-manager ## Run integration tests from directory M_INTEG_DIR or set M_INTEG_DIR=all to run all the integration tests.
ifeq ($(M_INTEG_DIR), all)
Expand Down
725 changes: 725 additions & 0 deletions apis/cassandra/v1beta1/cassandra_config_generated.go

Large diffs are not rendered by default.

40 changes: 40 additions & 0 deletions apis/cassandra/v1beta1/cassandradatacenter_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ func ValidateSingleDatacenter(dc CassandraDatacenter) error {
}
}

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

return nil
}

Expand Down Expand Up @@ -226,3 +233,36 @@ func (dc *CassandraDatacenter) ValidateUpdate(old runtime.Object) error {
func (dc *CassandraDatacenter) ValidateDelete() error {
return nil
}

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
}
42 changes: 42 additions & 0 deletions apis/cassandra/v1beta1/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,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
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.9"
8 changes: 8 additions & 0 deletions hack/config/generate.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!/usr/bin/env bash
OUTPUTFILE=../../apis/cassandra/v1beta1/cassandra_config_generated.go
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', 'trunk'])
10 changes: 0 additions & 10 deletions hack/kind-reload-operator.sh

This file was deleted.

29 changes: 0 additions & 29 deletions hack/release/build-kind-control-planes.sh

This file was deleted.

22 changes: 0 additions & 22 deletions hack/release/check-chart-sources.sh

This file was deleted.

18 changes: 0 additions & 18 deletions hack/release/make-yaml-bundle.sh

This file was deleted.

17 changes: 0 additions & 17 deletions hack/run_kubefwd.sh

This file was deleted.

3 changes: 3 additions & 0 deletions pkg/reconciliation/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ func (rc *ReconciliationContext) IsValid(dc *api.CassandraDatacenter) error {
// validate any other defined users
errs = append(errs, rc.validateCassandraUserSecrets()...)

// Validate spec.Config
errs = append(errs, api.ValidateConfig(dc))

// Validate Management API config
errs = append(errs, httphelper.ValidateManagementApiConfig(dc, rc.Client, rc.Ctx)...)
if len(errs) > 0 {
Expand Down

0 comments on commit ffe1cc2

Please sign in to comment.