Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Configure Cassandra nodes to lookup their IP address by their fully qualified domain name #334

Merged
Merged
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
29 changes: 29 additions & 0 deletions pkg/controllers/cassandra/nodepool/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ func StatefulSetForCluster(
Type: apps.RollingUpdateStatefulSetStrategyType,
},
PodManagementPolicy: apps.OrderedReadyPodManagement,
ServiceName: statefulSetName,
Template: apiv1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: nodePoolLabels,
Expand Down Expand Up @@ -195,6 +196,34 @@ func StatefulSetForCluster(
Name: "HEAP_NEWSIZE",
Value: "100M",
},
// Deliberately set to a single space to force Cassandra to do a host name lookup.
// See https://github.com/apache/cassandra/blob/cassandra-3.11.2/conf/cassandra.yaml#L592
{
Name: "CASSANDRA_LISTEN_ADDRESS",
Value: " ",
},
{
Name: "CASSANDRA_BROADCAST_ADDRESS",
Value: " ",
},
{
Name: "CASSANDRA_RPC_ADDRESS",
Value: " ",
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we use the Cassandra RPC interface, and do we want to listen on the default pod IP for RPC connections? It seems to me like we'd only want to listen on 127.0.0.1?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use it, but I'm not sure whether Cassandra nodes talk to each other with this protocol.
The default is to listen on all addresses:

So setting it here may not be ideal, but I can improve or remove it later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

// Set a non-existent default seed.
// The Kubernetes Seed Provider will fall back to a default seed host if it can't look up seeds via the CASSANDRA_SERVICE.
// And if the CASSANDRA_SEEDS environment variable is not set, it defaults to localhost.
// Which could cause confusion if a non-seed node is temporarily unable to lookup the seed nodes from the service.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would happen if we were to set this to localhost instead? Would the node fail to boot, or would it cause a split brain of some sort?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would mean that new nodes might consider themselves seeds in the event that the KubernetesSeedProvider plugin encounters an error.
So yeah, a split brain situation I think.

// We want the list of seeds to be strictly controlled by the service.
// See:
// https://github.com/docker-library/cassandra/blame/master/3.11/docker-entrypoint.sh#L31 and
// https://github.com/apache/cassandra/blob/cassandra-3.11.2/conf/cassandra.yaml#L416 and
// https://github.com/kubernetes/examples/blob/cabf8b8e4739e576837111e156763d19a64a3591/cassandra/java/src/main/java/io/k8s/cassandra/KubernetesSeedProvider.java#L69 and
// https://github.com/kubernetes/examples/blob/cabf8b8e4739e576837111e156763d19a64a3591/cassandra/go/main.go#L51
{
Name: "CASSANDRA_SEEDS",
Value: "black-hole-dns-name",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly concerned this could become an attack vector. What if someone creates a service named black-hole-dns-name? Could an attacker gain access to the contents of the Cassandra DB as a result?

If possible, I'd rather set this to a value that causes Cassandra to fail hard/crash (as this value should only be referenced in the event of a DNS resolution failure)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the answer is to remove the fallback code from the Kubernetes Seed Provider.
If it can't lookup the seeds from the Kubernetes it should log an error and return an empty list,
rather than returning a default.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do this in a follow up branch, where I remove the reliance on the Docker image entrypoint and instead create a Navigator default config.

},
{
Name: "CASSANDRA_ENDPOINT_SNITCH",
Value: cassSnitch,
Expand Down
15 changes: 13 additions & 2 deletions pkg/controllers/cassandra/service/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import (
const (
SeedLabelKey = "navigator.jetstack.io/cassandra-seed"
SeedLabelValue = "true"

// This field is deprecated. v1.Service.PublishNotReadyAddresses will replace it.
// See https://github.com/kubernetes/kubernetes/blob/v1.10.0/pkg/controller/endpoint/endpoints_controller.go#L68
tolerateUnreadyEndpointsAnnotationKey = "service.alpha.kubernetes.io/tolerate-unready-endpoints"
)

type serviceFactory func(*v1alpha1.CassandraCluster) *apiv1.Service
Expand Down Expand Up @@ -93,8 +97,11 @@ func SeedsServiceForCluster(cluster *v1alpha1.CassandraCluster) *apiv1.Service {
labels := util.ClusterLabels(cluster)
service := &apiv1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: util.SeedsServiceName(cluster),
Namespace: cluster.Namespace,
Name: util.SeedsServiceName(cluster),
Namespace: cluster.Namespace,
Annotations: map[string]string{
tolerateUnreadyEndpointsAnnotationKey: "true",
},
OwnerReferences: []metav1.OwnerReference{util.NewControllerRef(cluster)},
Labels: labels,
},
Expand All @@ -106,6 +113,10 @@ func SeedsServiceForCluster(cluster *v1alpha1.CassandraCluster) *apiv1.Service {
// But without it, DNS records are not registered.
// See https://github.com/kubernetes/kubernetes/issues/55158
Ports: []apiv1.ServicePort{{Port: 65535}},
// This ensures that seed node addresses are published immediately.
// And are available during C* node start up.
// Even for seed nodes them selves.
PublishNotReadyAddresses: true,
},
}
// Only mark nodes explicitly labeled as seeds as seed nodes
Expand Down