Skip to content

Commit

Permalink
Update CRD and fix minor issues
Browse files Browse the repository at this point in the history
This commit updates `RedpandaClusterSpec` to support the `TrustStore` option
that was added to the helm chart in
redpanda-data/helm-charts@f539d8f.

Additionally, it adds a regression test to find divergences between the CRD and
helm chart. The majority of this test case is currently disabled due to many
such divergences.

Finally, this commit also contains a handful of small fixes that were found in
the above test case:
- Added `omitempty` to `Auth.SASL`, `RedpandaMemory.{Memory,ReserveMemory}`.
- Deprecated `ListenerTLS.SecretRef` as it's not a real value.
- Deprecated `UsageStats.Organization` as it's been removed from the helm chart.
- Removed unused types `Limits`, `Requests`, `TopologySpreadConstraintsItems`.
- Corrected `TopologySpreadConstraint` to be a slice. (See note)

Technically, changing the type of `TopologySpreadConstraint` is a breaking
change. However, specifying it would have caused deployments to immediately
break. Therefore it's reasonably safe to assume this change will not affect any
users.

Changing the casing of `FullNameOverride` is being deferred as it is possible
to specify it without breaking deployments and changing the casing could result
in breakages as that would be seen as a field addition and removal from the API
perspective.
  • Loading branch information
chrisseto authored and RafalKorepta committed Jun 26, 2024
1 parent 9b21206 commit f7a44ae
Show file tree
Hide file tree
Showing 6 changed files with 1,458 additions and 342 deletions.
65 changes: 30 additions & 35 deletions src/go/k8s/api/redpanda/v1alpha2/redpanda_clusterspec_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ type ConnectorsCreateObj struct {
// Auth configures authentication in the Helm values. See https://docs.redpanda.com/current/manage/kubernetes/security/authentication/sasl-kubernetes/.
type Auth struct {
// Configures SASL authentication in the Helm values.
SASL *SASL `json:"sasl"`
SASL *SASL `json:"sasl,omitempty"`
}

// SASL configures SASL authentication in the Helm values.
Expand Down Expand Up @@ -356,16 +356,31 @@ type SecretRef struct {
Name string `json:"name"`
}

// TrustStore is a mapping from a value on either a Secret or ConfigMap to the
// `truststore_path` field of a listener.
// +kubebuilder:validation:MaxProperties=1
// +kubebuilder:validation:MinProperties=1
type TrustStore struct {
ConfigMapKeyRef *corev1.ConfigMapKeySelector `json:"configMapKeyRef,omitempty"`
SecretKeyRef *corev1.SecretKeySelector `json:"secretKeyRef,omitempty"`
}

// ListenerTLS configures TLS configuration for each listener in the Helm values.
type ListenerTLS struct {
// References a specific certificate for the listener.
Cert *string `json:"cert,omitempty"`
// Specifies whether TLS is enabled for the listener.
Enabled *bool `json:"enabled,omitempty"`
// References a Secret rsource containing TLS credentials for the listener.
// References a Secret resource containing TLS credentials for the listener.
//
// Deprecated: Setting SecretRef has no affect and will be removed in
// future releases.
SecretRef *string `json:"secretRef,omitempty"`
// Indicates whether client authentication (mTLS) is required.
RequireClientAuth *bool `json:"requireClientAuth,omitempty"`
// TrustStore allows setting the `truststore_path` on this listener. If
// specified, this field takes precedence over [Certificate.CAEnabled].
TrustStore *TrustStore `json:"trustStore,omitempty"`
}

// ExternalService allows you to enable or disable the creation of an external Service type.
Expand Down Expand Up @@ -407,8 +422,10 @@ type Logging struct {
// UsageStats configures the reporting of usage statistics. Redpanda Data uses these metrics to learn how the software is used, which can guide future improvements.
type UsageStats struct {
// Specifies whether usage reporting is enabled.
Enabled bool `json:"enabled"`
Enabled *bool `json:"enabled,omitempty"`
// Specifies the name of the organization using the software. This can be useful for identifying and segmenting usage data by organization, if usage reporting is enabled.
// Deprecated: This value is no longer respected in the redpanda helm chart
// and will be removed in a future version.
Organization *string `json:"organization,omitempty"`
// Specifies the ID of your Redpanda cluster.
ClusterID *string `json:"clusterId,omitempty"`
Expand All @@ -422,18 +439,6 @@ type Resources struct {
Memory *Memory `json:"memory,omitempty"`
}

// Limits specifies the maximum resources that each Pod can use.
type Limits struct {
CPU *int `json:"cpu,omitempty"`
Memory *string `json:"memory,omitempty"`
}

// Requests configures the minimum resources to be allocated to each Pod.
type Requests struct {
CPU *int `json:"cpu,omitempty"`
Memory *string `json:"memory,omitempty"`
}

// Storage configures storage-related settings in the Helm values. See https://docs.redpanda.com/current/manage/kubernetes/storage/.
type Storage struct {
// Specifies the absolute path on the worker node to store the Redpanda data directory. If unspecified, then an `emptyDir` volume is used. If specified but `persistentVolume.enabled` is true, `storage.hostPath` has no effect.
Expand Down Expand Up @@ -641,7 +646,7 @@ type Statefulset struct {
// Applies tolerations to allow Pods to be scheduled on nodes with matching taints, enabling control over where Pods can run.
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
// Defines topology spread constraints to control how Pods are spread across different topology domains.
TopologySpreadConstraints *TopologySpreadConstraints `json:"topologySpreadConstraints,omitempty"`
TopologySpreadConstraints []*TopologySpreadConstraints `json:"topologySpreadConstraints,omitempty"`
// Defines the update strategy for the StatefulSet to manage how updates are rolled out to the Pods.
UpdateStrategy *UpdateStrategy `json:"updateStrategy,omitempty"`
// Specifies the termination grace period in seconds to control the time delay before forcefully terminating a Pod.
Expand Down Expand Up @@ -692,14 +697,14 @@ type StartupProbe struct {

// PodAntiAffinity configures Pod anti-affinity rules to prevent Pods from being scheduled together on the same node.
type PodAntiAffinity struct {
// Specifies the topology key used to spread Pods across different nodes or other topologies.
TopologyKey string `json:"topologyKey"`
// Defines the type of anti-affinity, such as `soft` or `hard`.
Type string `json:"type"`
// Sets the weight associated with the soft anti-affinity rule.
Weight int `json:"weight"`
// TopologyKey specifies the topology key used to spread Pods across different nodes or other topologies.
TopologyKey *string `json:"topologyKey,omitempty"`
// Type defines the type of anti-affinity, such as `soft` or `hard`.
Type *string `json:"type,omitempty"`
// Weight sets the weight associated with the soft anti-affinity rule.
Weight *int `json:"weight,omitempty"`
// +kubebuilder:pruning:PreserveUnknownFields
// Configures additional custom anti-affinity rules.
// Custom configures additional custom anti-affinity rules.
Custom *runtime.RawExtension `json:"custom,omitempty"`
}

Expand Down Expand Up @@ -876,16 +881,6 @@ type RPControllers struct {
CreateRBAC *bool `json:"createRBAC,omitempty"`
}

// TopologySpreadConstraintsItems configures topology spread constraints to control how Pods are spread across different topology domains.
type TopologySpreadConstraintsItems struct {
// Defines the degree to which Pods may be unevenly distributed.
MaxSkew int `json:"maxSkew,omitempty"`
// Specifies the topology key used to spread Pods across different nodes or other topologies.
TopologyKey *string `json:"topologyKey,omitempty"`
// Sets the policy for how to handle unsatisfiable constraints, such as `DoNotSchedule` or `ScheduleAnyway`.
WhenUnsatisfiable *string `json:"whenUnsatisfiable,omitempty"`
}

// CPU configures CPU resources for containers. See https://docs.redpanda.com/current/manage/kubernetes/manage-resources/.
type CPU struct {
// Specifies the number of CPU cores available to the application. Redpanda makes use of a thread per core model. For details, see https://docs.redpanda.com/current/get-started/architecture/#thread-per-core-model. For this reason, Redpanda should only be given full cores. Note: You can increase cores, but decreasing cores is not currently supported. See the GitHub issue:https://github.com/redpanda-data/redpanda/issues/350. This setting is equivalent to `--smp`, `resources.requests.cpu`, and `resources.limits.cpu`. For production, use `4` or greater.
Expand Down Expand Up @@ -920,9 +915,9 @@ type Memory struct {
// 3. Other container processes (whatever small amount remains)
type RedpandaMemory struct {
// Memory for the Redpanda process. This must be lower than the container's memory (`resources.memory.container.min` if provided, otherwise `resources.memory.container.max`). Equivalent to `--memory`. For production, use 8Gi or greater.
Memory *resource.Quantity `json:"memory"`
Memory *resource.Quantity `json:"memory,omitempty"`
// Memory reserved for the Seastar subsystem. Any value above 1Gi will provide diminishing performance benefits. Equivalent to `--reserve-memory`. For production, use 1Gi.
ReserveMemory *resource.Quantity `json:"reserveMemory"`
ReserveMemory *resource.Quantity `json:"reserveMemory,omitempty"`
}

// RBAC configures role-based access control (RBAC).
Expand Down
103 changes: 103 additions & 0 deletions src/go/k8s/api/redpanda/v1alpha2/redpanda_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@ package v1alpha2_test
import (
"encoding/json"
"fmt"
"regexp"
"strings"
"testing"

fuzz "github.com/google/gofuzz"
"github.com/redpanda-data/helm-charts/charts/redpanda"
"github.com/redpanda-data/redpanda-operator/src/go/k8s/api/apiutil"
"github.com/redpanda-data/redpanda-operator/src/go/k8s/api/redpanda/v1alpha2"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/runtime"
)

// TestRedpanda_ValuesJSON asserts that .ValuesJSON appropriately coalesces the
Expand Down Expand Up @@ -56,3 +61,101 @@ func TestRedpanda_ValuesJSON(t *testing.T) {
require.JSONEq(t, expected, string(valuesJSON.Raw))
}
}

func MarshalThrough[T any](data []byte) ([]byte, error) {
var through T
if err := json.Unmarshal(data, &through); err != nil {
return nil, err
}
return json.Marshal(through)
}

func AssertJSONCompat[From, To any](t *testing.T, fuzzer *fuzz.Fuzzer) {
for i := 0; i < 10; i++ {
var from From
fuzzer.Fuzz(&from)

original, err := json.Marshal(from)
require.NoError(t, err)

through, err := MarshalThrough[To](original)
require.NoError(t, err)

require.JSONEq(t, string(original), string(through))
}
}

// TestHelmValuesCompat asserts that the JSON representation of the redpanda
// cluster spec is byte of byte compatible with the values that the helm chart
// accepts.
func TestHelmValuesCompat(t *testing.T) {
// There are integer sizing mismatches, so clamp all ints to 32
// bits.
intTruncation := func(n *int, c fuzz.Continue) { //nolint:staticcheck // fuzzing is weird
v := int(c.Int31())
*n = v
}

// Makes strings easier to read.
asciiStrs := func(s *string, c fuzz.Continue) {
var x []byte
for i := 0; i < c.Intn(20); i++ {
// Ascii range for printable characters is [32,126].
x = append(x, byte(32+c.Intn(127-32)))
}
*s = string(x)
}

t.Run("helm2crd", func(t *testing.T) {
t.Skipf("Too many issues to currently be useful")

fuzzer := fuzz.New().NilChance(0.5).Funcs(
asciiStrs,
intTruncation,
func(a *any, c fuzz.Continue) {},
)
AssertJSONCompat[redpanda.PartialValues, v1alpha2.RedpandaClusterSpec](t, fuzzer)
})

t.Run("crd2helm", func(t *testing.T) {
disabledFields := []string{
"Connectors", // Untyped in the CRD.
"Console", // Untyped in the CRD.
"External", // .Type is missing omitempty due to issues in genpartial.
"Force", // Missing from Helm
"FullNameOverride", // Incorrectly cased in the CRD's JSON tag (Should be fullnameOverride). Would be a breaking change to fix.
"ImagePullSecrets", // Missing from Helm.
"Listeners", // CRD uses homogeneous types for all listeners which can cause divergences.
"Logging", // Disabled due to issues in genpartial.
"Monitoring", // Disabled due to issues in genpartial.
"PostInstallJob", // Incorrectly typed in Helm.
"PostUpgradeJob", // Incorrectly typed in Helm.
"Resources", // Multiple issues, at least one due to genpartial.
"Service", // Disabled due to issues in genpartial.
"Statefulset", // Many divergences from helm. Needs further inspection.
"Storage", // Helm is missing nameOverwrite
"Tuning", // Disabled due to extraVolumeMounts being typed as a string in helm.

// Deprecated fields (IE fields that shouldn't be in the CRD but
// aren't removed for backwards compat).
"Organization", // UsageStats
}

fuzzer := fuzz.New().NilChance(0.5).SkipFieldsWithPattern(
regexp.MustCompile("^("+strings.Join(disabledFields, "|")+"$)"),
).Funcs(
asciiStrs,
intTruncation,
// There are some untyped values as runtime.RawExtension's within
// the CRD. We can't really generate those, so skip over them for
// now. Omitting this function will call .Fuzz to panic.
func(e **runtime.RawExtension, c fuzz.Continue) {},
// Ensure that certificates are never nil. There's a mild divergence
// in how the values unmarshal which is irrelevant to this test.
func(cert *v1alpha2.Certificate, c fuzz.Continue) {
c.Fuzz(cert)
},
)
AssertJSONCompat[v1alpha2.RedpandaClusterSpec, redpanda.PartialValues](t, fuzzer)
})
}
Loading

0 comments on commit f7a44ae

Please sign in to comment.