fix: gate cluster-wide IPsec on every chassis being configured (#953)

This commit is contained in:
Julian Sommer
2026-08-31 15:31:59 +10:00
committed by GitHub
parent e9404f1d70
commit ce9b6e5b41
7 changed files with 1084 additions and 93 deletions
+5 -4
View File
@@ -1530,10 +1530,11 @@ func (d *Daemon) startCluster() error {
}
// Reconcile OVN native IPsec both ways (idempotent): a node that does not
// use it should not be left running charon on 500/4500 either.
if err := host.ReconcileOVNIPSec(d.configPath, d.clusterConfig); err != nil {
slog.Warn("Failed to reconcile OVN native IPsec", "err", err)
}
// use it should not be left running charon on 500/4500 either. Runs in the
// background because the first attempt routinely loses the race with
// ovn-central accepting connections, and a node that gives up there stays
// unconfigured while a peer may already require encryption cluster-wide.
go host.MaintainIPSec(d.ctx, d.configPath, d.clusterConfig, d.ipsecBarrier())
// Keep the host firewall's peer sets in step with cluster membership. Every
// formation path reaches this, which none of the installers do. Runs in the
+174
View File
@@ -0,0 +1,174 @@
package daemon
import (
"context"
"encoding/json"
"errors"
"fmt"
"time"
"github.com/mulgadc/spinifex/spinifex/network/host"
"github.com/nats-io/nats.go/jetstream"
)
// ipsecStatusPrefix is the key prefix for a node's IPsec state record in the
// cluster-state bucket.
const ipsecStatusPrefix = "ipsecstatus."
// ipsecStatusFreshness bounds how long a status record counts for. A node that
// has stopped publishing is not evidence that it is still configured. Must stay
// a large multiple of the interval that refreshes these records.
const ipsecStatusFreshness = 5 * time.Minute
// ipsecLivenessWindow bounds how long a node counts as live on its heartbeat
// alone. Heartbeats run at heartbeatInterval, so this is six of them: long
// enough to ride out a slow pass, short enough that a node taken out of service
// stops holding the cluster's encryption decision hostage.
const ipsecLivenessWindow = 60 * time.Second
// ipsecStatusRecord is what a node publishes about its own IPsec state. The
// timestamp is the point of the record: state that cannot go stale cannot
// distinguish a configured peer from one that stopped answering.
type ipsecStatusRecord struct {
Node string `json:"node"`
Ready bool `json:"ready"`
NBReachable bool `json:"nb_reachable"`
Written time.Time `json:"written"`
}
// KVIPSecBarrier carries per-node IPsec state over the cluster-state KV.
type KVIPSecBarrier struct {
kv jetstream.KeyValue
// now is a var for tests; staleness is otherwise untestable without sleeping.
now func() time.Time
}
var _ host.IPSecBarrier = (*KVIPSecBarrier)(nil)
// NewKVIPSecBarrier returns a barrier over kv, or nil if kv is nil. A nil
// barrier is meaningful to the caller: it falls back to local knowledge, which
// is the right answer for a cluster with no peers to be out of step with.
func NewKVIPSecBarrier(kv jetstream.KeyValue) *KVIPSecBarrier {
if kv == nil {
return nil
}
return &KVIPSecBarrier{kv: kv, now: time.Now}
}
// ipsecBarrier returns the cluster readiness barrier, or a nil interface when
// there is no cluster KV to carry it. Returning the concrete nil pointer would
// give host a non-nil interface holding nil, which is not the same thing.
func (d *Daemon) ipsecBarrier() host.IPSecBarrier {
if d.jsManager == nil || d.jsManager.clusterKV == nil {
return nil
}
return NewKVIPSecBarrier(d.jsManager.clusterKV)
}
// Publish records this node's own IPsec state.
func (b *KVIPSecBarrier) Publish(ctx context.Context, node string, status host.IPSecNodeStatus) error {
if node == "" {
return errors.New("node name unset")
}
data, err := json.Marshal(ipsecStatusRecord{
Node: node,
Ready: status.Ready,
NBReachable: status.NBReachable,
Written: b.now().UTC(),
})
if err != nil {
return fmt.Errorf("marshal IPsec status: %w", err)
}
if _, err := b.kv.Put(ctx, ipsecStatusPrefix+node, data); err != nil {
return fmt.Errorf("put IPsec status for %s: %w", node, err)
}
return nil
}
// Cluster returns the last published status of every live node. A node with a
// fresh record is live by definition; one without is live only if it is still
// heartbeating, and otherwise drops out of the set entirely.
//
// The two-step matters. A node whose KV writes are failing stops heartbeating
// too, so it leaves the set rather than being read as a chassis that lost its
// configuration — which would strip encryption from the peers still talking.
func (b *KVIPSecBarrier) Cluster(ctx context.Context, nodes []string) (map[string]host.IPSecNodeStatus, error) {
cluster := make(map[string]host.IPSecNodeStatus, len(nodes))
for _, node := range nodes {
rec, err := b.readStatus(ctx, node)
if err != nil {
return nil, err
}
if rec != nil && b.now().UTC().Sub(rec.Written) <= ipsecStatusFreshness {
cluster[node] = host.IPSecNodeStatus{Ready: rec.Ready, NBReachable: rec.NBReachable}
continue
}
alive, err := b.heartbeating(ctx, node)
if err != nil {
return nil, err
}
if alive {
cluster[node] = host.IPSecNodeStatus{}
}
}
return cluster, nil
}
// readStatus returns nil for a node that has published nothing, and for a record
// that cannot be parsed — both mean "no usable claim of readiness".
func (b *KVIPSecBarrier) readStatus(ctx context.Context, node string) (*ipsecStatusRecord, error) {
entry, err := b.kv.Get(ctx, ipsecStatusPrefix+node)
if errors.Is(err, jetstream.ErrKeyNotFound) {
return nil, nil
}
if err != nil {
return nil, fmt.Errorf("get IPsec status for %s: %w", node, err)
}
rec, ok := decodeStatus(entry.Value())
if !ok {
return nil, nil
}
return &rec, nil
}
// decodeStatus reports usability rather than an error: a record that cannot be
// parsed is not a claim of readiness, which is a state this cares about and not
// a failure to read the KV.
func decodeStatus(data []byte) (ipsecStatusRecord, bool) {
var rec ipsecStatusRecord
if json.Unmarshal(data, &rec) != nil {
return ipsecStatusRecord{}, false
}
return rec, true
}
// heartbeating reports whether the node has written a heartbeat recently enough
// to still count as part of the cluster.
func (b *KVIPSecBarrier) heartbeating(ctx context.Context, node string) (bool, error) {
entry, err := b.kv.Get(ctx, "heartbeat."+node)
if errors.Is(err, jetstream.ErrKeyNotFound) {
return false, nil
}
if err != nil {
return false, fmt.Errorf("get heartbeat for %s: %w", node, err)
}
written, ok := decodeHeartbeatTime(entry.Value())
if !ok {
return false, nil
}
return b.now().UTC().Sub(written) <= ipsecLivenessWindow, nil
}
// decodeHeartbeatTime returns when the heartbeat was written. A beat that cannot
// be parsed or dated vouches for nothing, which is an answer rather than a fault.
func decodeHeartbeatTime(data []byte) (time.Time, bool) {
var h Heartbeat
if json.Unmarshal(data, &h) != nil {
return time.Time{}, false
}
written, err := time.Parse(time.RFC3339, h.Timestamp)
if err != nil {
return time.Time{}, false
}
return written.UTC(), true
}
+153
View File
@@ -0,0 +1,153 @@
//test:in-package the barrier's staleness rules are only testable from inside:
// they need the clock seam, the freshness constants, the shared JetStream test
// server, and jsManager.clusterKV to seed heartbeats against.
package daemon
import (
"testing"
"time"
"github.com/mulgadc/spinifex/spinifex/network/host"
"github.com/nats-io/nats.go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func newTestIPSecBarrier(t *testing.T) (*KVIPSecBarrier, *JetStreamManager) {
t.Helper()
nc, err := nats.Connect(sharedJSNATSURL)
require.NoError(t, err)
t.Cleanup(nc.Close)
jsm, err := NewJetStreamManager(nc, 1)
require.NoError(t, err)
require.NoError(t, jsm.InitClusterStateBucket())
barrier := NewKVIPSecBarrier(jsm.clusterKV)
require.NotNil(t, barrier)
return barrier, jsm
}
// beat marks a node live without saying anything about its IPsec state.
func beat(t *testing.T, jsm *JetStreamManager, node string) {
t.Helper()
require.NoError(t, jsm.WriteHeartbeat(&Heartbeat{
Node: node,
Timestamp: time.Now().UTC().Format(time.RFC3339),
}))
}
func TestKVIPSecBarrier_ReportsEveryLiveNodesState(t *testing.T) {
barrier, jsm := newTestIPSecBarrier(t)
nodes := []string{"ipsec-a", "ipsec-b"}
for _, n := range nodes {
beat(t, jsm, n)
}
require.NoError(t, barrier.Publish(t.Context(), "ipsec-a", host.IPSecNodeStatus{Ready: true, NBReachable: true}))
cluster, err := barrier.Cluster(t.Context(), nodes)
require.NoError(t, err)
assert.Equal(t, map[string]host.IPSecNodeStatus{
"ipsec-a": {Ready: true, NBReachable: true},
"ipsec-b": {},
}, cluster)
require.NoError(t, barrier.Publish(t.Context(), "ipsec-b", host.IPSecNodeStatus{Ready: true}))
cluster, err = barrier.Cluster(t.Context(), nodes)
require.NoError(t, err)
assert.Equal(t, map[string]host.IPSecNodeStatus{
"ipsec-a": {Ready: true, NBReachable: true},
"ipsec-b": {Ready: true},
}, cluster)
}
// A node that publishes Ready=false has said its own setup is incomplete, which
// is exactly the case the flag must not be asserted over.
func TestKVIPSecBarrier_ExplicitlyUnreadyNodeIsReported(t *testing.T) {
barrier, jsm := newTestIPSecBarrier(t)
beat(t, jsm, "ipsec-unready")
require.NoError(t, barrier.Publish(t.Context(), "ipsec-unready", host.IPSecNodeStatus{}))
cluster, err := barrier.Cluster(t.Context(), []string{"ipsec-unready"})
require.NoError(t, err)
assert.Equal(t, map[string]host.IPSecNodeStatus{"ipsec-unready": {}}, cluster)
}
// A node taken out of service has no chassis registered, so there is no tunnel
// to it to black-hole. Reporting it as unconfigured would strip encryption from
// the peers that are still talking to each other.
func TestKVIPSecBarrier_NodeThatStoppedHeartbeatingDropsOutOfTheSet(t *testing.T) {
barrier, jsm := newTestIPSecBarrier(t)
beat(t, jsm, "ipsec-gone")
require.NoError(t, barrier.Publish(t.Context(), "ipsec-gone", host.IPSecNodeStatus{Ready: true}))
// Far enough ahead that both the status record and the heartbeat are stale.
barrier.now = func() time.Time { return time.Now().Add(ipsecStatusFreshness + time.Hour) }
cluster, err := barrier.Cluster(t.Context(), []string{"ipsec-gone"})
require.NoError(t, err)
assert.Empty(t, cluster)
}
// Still heartbeating but its status has gone stale: the node is up and can no
// longer vouch for its own configuration, so it counts as not ready.
func TestKVIPSecBarrier_LiveNodeWithAStaleStatusIsNotReady(t *testing.T) {
barrier, jsm := newTestIPSecBarrier(t)
require.NoError(t, barrier.Publish(t.Context(), "ipsec-stale", host.IPSecNodeStatus{Ready: true}))
// The status is stale, but a heartbeat lands at the shifted clock.
shifted := time.Now().Add(ipsecStatusFreshness + time.Minute)
barrier.now = func() time.Time { return shifted }
require.NoError(t, jsm.WriteHeartbeat(&Heartbeat{
Node: "ipsec-stale",
Timestamp: shifted.UTC().Format(time.RFC3339),
}))
cluster, err := barrier.Cluster(t.Context(), []string{"ipsec-stale"})
require.NoError(t, err)
assert.Equal(t, map[string]host.IPSecNodeStatus{"ipsec-stale": {}}, cluster)
}
// A node that has never published anything and never heartbeated is not part of
// the running cluster yet.
func TestKVIPSecBarrier_UnknownNodeIsAbsent(t *testing.T) {
barrier, _ := newTestIPSecBarrier(t)
cluster, err := barrier.Cluster(t.Context(), []string{"ipsec-never-seen"})
require.NoError(t, err)
assert.Empty(t, cluster)
}
func TestKVIPSecBarrier_PublishRequiresANodeName(t *testing.T) {
barrier, _ := newTestIPSecBarrier(t)
err := barrier.Publish(t.Context(), "", host.IPSecNodeStatus{Ready: true})
require.Error(t, err)
assert.Contains(t, err.Error(), "node name unset")
}
// A nil KV means no cluster to be out of step with, and the caller relies on a
// nil interface rather than a typed nil to tell.
func TestNewKVIPSecBarrier_NilKVYieldsNoBarrier(t *testing.T) {
assert.Nil(t, NewKVIPSecBarrier(nil))
}
// assert.Nil passes for a typed nil, so the interface conversion is checked with
// == instead: a non-nil interface holding a nil pointer panics in host.
func TestDaemonIPSecBarrier_NilManagerYieldsANilInterface(t *testing.T) {
d := &Daemon{}
assert.Equal(t, host.IPSecBarrier(nil), d.ipsecBarrier())
}
// The heartbeat that keeps a node in the set must be refreshed far more often
// than it is allowed to age, or a healthy node drops out between beats.
func TestIPSecLivenessWindowExceedsHeartbeatInterval(t *testing.T) {
assert.Greater(t, ipsecLivenessWindow, 3*heartbeatInterval)
}
+50 -2
View File
@@ -1,6 +1,8 @@
package host
import (
"bytes"
"errors"
"fmt"
"strings"
@@ -35,15 +37,61 @@ func EnableIPSecEncapsulation() error {
return nil
}
// errNBUnreachable marks the one failure that means "this node has no local NB
// DB" rather than "the read broke". A permission change on the socket, a missing
// binary or a timed-out transaction are all real faults on a management node,
// and lumping them in with this would silence them.
var errNBUnreachable = errors.New("no local OVN NB DB")
// nbUnreachablePatterns are what ovsdb's client library prints when it cannot
// open or complete a handshake on the socket.
var nbUnreachablePatterns = []string{
"database connection failed",
"connection refused",
"no such file or directory",
}
// GetNBGlobalIPSec reads NB_Global.ipsec from the local OVN NB DB. The error
// doubles as the reachability answer: a present socket file says nothing about
// whether the database behind it accepts connections yet.
//
// Reads stdout alone. ovn-nbctl writes vlog lines to stderr on a successful run,
// and folding those into the value parses a live "true" as false.
func GetNBGlobalIPSec() (bool, error) {
cmd := utils.SudoCommand("ovn-nbctl", "--timeout=5", "get", "NB_Global", ".", "ipsec")
var stderr bytes.Buffer
cmd.Stderr = &stderr
out, err := cmd.Output()
if err != nil {
msg := strings.TrimSpace(stderr.String())
wrapped := fmt.Errorf("get NB_Global ipsec: %s: %w", msg, err)
if isNBUnreachable(msg) {
return false, fmt.Errorf("%w: %w", errNBUnreachable, wrapped)
}
return false, wrapped
}
return strings.Trim(strings.TrimSpace(string(out)), `"`) == "true", nil
}
func isNBUnreachable(msg string) bool {
lower := strings.ToLower(msg)
for _, pattern := range nbUnreachablePatterns {
if strings.Contains(lower, pattern) {
return true
}
}
return false
}
// SetNBGlobalIPSec writes NB_Global.ipsec on the local OVN NB DB, triggering
// ovn-controller to add options:remote_name to Geneve tunnels for strongSwan.
// Only the management node has a local NB socket; callers gate on presence.
// Only the management node has a reachable NB DB; callers gate on that.
func SetNBGlobalIPSec(enable bool) error {
val := "false"
if enable {
val = "true"
}
out, err := utils.SudoCommand("ovn-nbctl", "set", "NB_Global", ".", "ipsec="+val).CombinedOutput()
out, err := utils.SudoCommand("ovn-nbctl", "--timeout=5", "set", "NB_Global", ".", "ipsec="+val).CombinedOutput()
if err != nil {
return fmt.Errorf("set NB_Global ipsec=%s: %s: %w", val, strings.TrimSpace(string(out)), err)
}
+252 -21
View File
@@ -1,23 +1,63 @@
package host
import (
"context"
"errors"
"fmt"
"log/slog"
"maps"
"os"
"path/filepath"
"slices"
"strings"
"time"
"github.com/mulgadc/spinifex/spinifex/admin"
"github.com/mulgadc/spinifex/spinifex/config"
"github.com/mulgadc/spinifex/spinifex/otelsetup"
"github.com/mulgadc/spinifex/spinifex/utils"
)
// systemctlActiveTimeout bounds the wait for openvswitch-ipsec.service to become active.
var systemctlActiveTimeout = 5 * time.Second
// ovnNBSocketPath gates the NB_Global ipsec write to the management node.
var ovnNBSocketPath = "/run/ovn/ovnnb_db.sock"
// ipsecRetryDelay is the first gap after a failed pass; it doubles up to
// ipsecReconcileInterval. Short, because until every chassis has finished its
// local setup the cluster cannot require encryption at all.
var ipsecRetryDelay = 3 * time.Second
// ipsecReconcileInterval re-runs a successful pass. It doubles as the readiness
// heartbeat, so it must stay well inside whatever freshness window the barrier
// applies to the records this publishes.
var ipsecReconcileInterval = 60 * time.Second
// IPSecNodeStatus is one node's published view of its own IPsec state.
type IPSecNodeStatus struct {
// Ready reports that this node's local IPsec configuration is complete.
Ready bool
// NBReachable reports that this node can read and write NB_Global. Only
// management nodes can, and which nodes those are is not in the config, so
// they say so here and every node elects the same writer from the answer.
NBReachable bool
}
// IPSecBarrier carries each node's IPsec state across the cluster.
// NB_Global.ipsec is cluster-wide, so asserting it from one node's local
// knowledge is what lets a partially configured mesh black-hole guest traffic.
// The interface keeps the transport (JetStream KV) out of L0.
type IPSecBarrier interface {
// Publish records this node's own state. Callers publish on every pass, so
// a live node's record stays fresh and a stopped one's goes stale.
Publish(ctx context.Context, node string, status IPSecNodeStatus) error
// Cluster returns the last published status of every named node that is
// still live. A node that is not live is absent from the map rather than
// unready: it has no chassis registered, so there is no tunnel to it to
// black-hole, and demanding plaintext on its behalf would only strip
// encryption from the traffic that is still flowing.
Cluster(ctx context.Context, nodes []string) (map[string]IPSecNodeStatus, error)
}
const (
// ovsIPSecUnit owns charon: ovs-monitor-ipsec execs the strongSwan starter
@@ -34,26 +74,108 @@ const (
// root-equivalent, so unit changes go through this instead.
var ipsecStateHelper = "/usr/local/lib/spinifex/spinifex-set-ipsec-state"
// MaintainIPSec schedules ReconcileOVNIPSec until it succeeds, then re-runs it
// at ipsecReconcileInterval for the lifetime of ctx. A scheduler only.
//
// A single attempt at startup routinely loses the race with ovn-central
// accepting connections, and dropping that attempt leaves the node's IPsec
// unconfigured until something restarts the daemon — while another node may
// already have made encryption mandatory cluster-wide.
func MaintainIPSec(ctx context.Context, configPath string, clusterConfig *config.ClusterConfig, barrier IPSecBarrier) {
// Said once, because a pass that reconciles to nothing logs nothing, which in
// a journal is indistinguishable from a node that armed.
switch {
case clusterConfig == nil:
slog.Info("ipsec: cluster membership unknown, leaving the host's IPsec services alone")
case !ipsecWanted(clusterConfig):
slog.Info("ipsec: not in use on this cluster, IKE and NAT-T listeners stay stopped")
}
delay, announced := ipsecRetryDelay, false
for {
err := ReconcileOVNIPSec(ctx, configPath, clusterConfig, barrier)
switch {
case err != nil:
delay = min(delay*2, ipsecReconcileInterval)
slog.Warn("Failed to reconcile OVN native IPsec, will retry", "err", err, "retry_in_ms", otelsetup.Millis(delay))
default:
// Only the first success is news. The steady state re-runs every
// interval for the lifetime of the daemon, and saying so each time
// buries the passes that changed something.
if !announced && ipsecWanted(clusterConfig) {
slog.Info("ipsec: local IPsec configuration complete on this node")
announced = true
}
delay = ipsecReconcileInterval
}
select {
case <-ctx.Done():
return
case <-time.After(delay):
}
}
}
// ipsecWanted reports the cluster's intent. Single-node clusters have no Geneve
// tunnels to protect, so charon must not be left listening on 500/4500 there
// even though ipsec_enabled defaults to true.
func ipsecWanted(clusterConfig *config.ClusterConfig) bool {
return clusterConfig != nil && clusterConfig.Network.IPSecEnabled && len(clusterConfig.Nodes) > 1
}
// ReconcileOVNIPSec brings the host's IPsec services in line with the cluster
// config, then enables OVN IPsec if it is wanted. Runs on every startup so the
// disabled case is reached too, which EnableOVNIPSec alone never is.
func ReconcileOVNIPSec(configPath string, clusterConfig *config.ClusterConfig) error {
func ReconcileOVNIPSec(ctx context.Context, configPath string, clusterConfig *config.ClusterConfig, barrier IPSecBarrier) error {
// A nil config means the intent is unknown; leave the host's services alone
// rather than guessing and tearing down working tunnels.
if clusterConfig == nil {
return nil
}
want := clusterConfig.Network.IPSecEnabled && len(clusterConfig.Nodes) > 1
want := ipsecWanted(clusterConfig)
if err := ensureIPSecServices(want); err != nil {
return err
}
if !want {
slog.Info("ipsec: not in use, IKE and NAT-T listeners stopped")
return DisableOVNIPSec(ctx, clusterConfig, barrier)
}
return EnableOVNIPSec(ctx, configPath, clusterConfig, barrier)
}
// DisableOVNIPSec is the off direction, and it has to reach NB_Global. Charon is
// already stopped by the time this runs, so a flag left asserted demands
// encryption that no chassis can perform — the same black hole as a partial
// enable, reached by turning IPsec off instead of on.
func DisableOVNIPSec(ctx context.Context, clusterConfig *config.ClusterConfig, barrier IPSecBarrier) error {
if barrier != nil && clusterConfig != nil {
if err := barrier.Publish(ctx, clusterConfig.Node, IPSecNodeStatus{}); err != nil {
return fmt.Errorf("publish IPsec state: %w", err)
}
}
current, err := GetNBGlobalIPSec()
if err != nil {
return skipUnlessNBReadable(err)
}
if !current {
return nil
}
return EnableOVNIPSec(configPath, clusterConfig)
slog.Info("ipsec: switched off for this cluster, releasing the cluster-wide encryption requirement")
return SetNBGlobalIPSec(false)
}
// skipUnlessNBReadable turns "this node has no local NB DB" into a quiet skip
// and leaves every other failure — a socket it may not open, a missing binary,
// a timed-out transaction — as a real error, so the pass retries and says so.
func skipUnlessNBReadable(err error) error {
if errors.Is(err, errNBUnreachable) {
slog.Debug("ipsec: no local OVN NB DB, leaving NB_Global to a management node", "err", err)
return nil
}
return err
}
// ensureIPSecServices runs the helper only when the host does not already match
@@ -99,14 +221,47 @@ func unitIsActive(unit string) bool {
// EnableOVNIPSec wires the local IPsec peer cert and flips ipsec_encapsulation=true.
// Idempotent. Single-node clusters short-circuit (no Geneve tunnels to encrypt).
// Lives in L0 per ADR-0006 S8 (IPSec is OVN-native only; SA lifecycle invisible above L0).
func EnableOVNIPSec(configPath string, clusterConfig *config.ClusterConfig) error {
func EnableOVNIPSec(ctx context.Context, configPath string, clusterConfig *config.ClusterConfig, barrier IPSecBarrier) error {
if configPath == "" {
return fmt.Errorf("config path unset")
}
if clusterConfig != nil && len(clusterConfig.Nodes) <= 1 {
slog.Info("ipsec: single-node cluster, skipping enable (no peers)")
slog.Debug("ipsec: single-node cluster, skipping enable (no peers)")
return nil
}
// Probed before the local half, because the answer is published alongside it:
// which nodes can write NB_Global is not in the config, so they have to say.
current, nbErr := GetNBGlobalIPSec()
if nbErr != nil && !errors.Is(nbErr, errNBUnreachable) {
return nbErr
}
status := IPSecNodeStatus{NBReachable: nbErr == nil}
if err := configureLocalIPSec(configPath); err != nil {
// Said out loud, not left to expire. A record that only goes stale keeps
// this chassis counted as configured for a whole freshness window, which
// is the black hole again with a slower fuse.
publishStatus(ctx, clusterConfig, barrier, status)
return err
}
status.Ready = true
if barrier != nil && clusterConfig != nil {
if err := barrier.Publish(ctx, clusterConfig.Node, status); err != nil {
return fmt.Errorf("publish IPsec state: %w", err)
}
}
if nbErr != nil {
return skipUnlessNBReadable(nbErr)
}
return reconcileNBGlobalIPSec(ctx, clusterConfig, barrier, current)
}
// configureLocalIPSec does this node's own half: point OVS at the peer cert and
// turn on encapsulation, once ovs-monitor-ipsec is up to act on both.
func configureLocalIPSec(configPath string) error {
configDir := filepath.Dir(configPath)
certPath, keyPath := admin.IPSecCertPaths(configDir)
caCertPath := filepath.Join(configDir, "ca.pem")
@@ -124,26 +279,102 @@ func EnableOVNIPSec(configPath string, clusterConfig *config.ClusterConfig) erro
if err := SetIPSecCertPaths(certPath, keyPath, caCertPath); err != nil {
return err
}
return EnableIPSecEncapsulation()
}
if err := EnableIPSecEncapsulation(); err != nil {
// publishStatus is the best-effort form used on failure paths, where the error
// being reported matters more than the one this might add.
func publishStatus(ctx context.Context, clusterConfig *config.ClusterConfig, barrier IPSecBarrier, status IPSecNodeStatus) {
if barrier == nil || clusterConfig == nil {
return
}
if err := barrier.Publish(ctx, clusterConfig.Node, status); err != nil {
slog.Warn("ipsec: could not publish this node's state, peers will wait for it to go stale", "err", err)
}
}
// reconcileNBGlobalIPSec holds NB_Global.ipsec in step with the whole cluster.
// Without this flag ovn-controller skips options:remote_name on Geneve tunnels
// and ovs-monitor-ipsec materialises no strongSwan connections; with it, a
// chassis that has not finished its own setup silently drops guest traffic. So
// it tracks the slowest live chassis, not this one.
func reconcileNBGlobalIPSec(ctx context.Context, clusterConfig *config.ClusterConfig, barrier IPSecBarrier, current bool) error {
if barrier == nil || clusterConfig == nil {
if current {
return nil
}
return SetNBGlobalIPSec(true)
}
cluster, err := barrier.Cluster(ctx, nodeNames(clusterConfig))
if err != nil {
// Unreadable is not evidence that a chassis is unconfigured. Retracting on
// it would drop a working encrypted mesh to plaintext over a KV outage.
return fmt.Errorf("read IPsec cluster state: %w", err)
}
// One writer. Every management node can reach the NB DB, so without this they
// race on one cluster-global row from snapshots taken at different instants,
// and two that disagree for a single pass flip the flag — tearing down and
// rebuilding every strongSwan connection in the cluster on each flip.
writer := nbGlobalWriter(cluster)
if writer != clusterConfig.Node {
slog.Debug("ipsec: another node owns the NB_Global write this pass", "writer", writer)
return nil
}
pending := notReady(cluster)
switch {
case len(pending) == 0 && current:
return nil
case len(pending) == 0:
slog.Info("ipsec: every live chassis reports a complete configuration, requiring encryption cluster-wide")
return SetNBGlobalIPSec(true)
case !current:
slog.Info("ipsec: holding encryption off until every chassis is configured", "pending", pending)
return nil
}
// Plaintext Geneve is where the cluster sat before IPsec was asked for, and it
// is both recoverable and visible. A black hole is neither.
slog.Error("ipsec: retracting cluster-wide encryption, chassis are unconfigured and cross-chassis guest traffic would black-hole", "pending", pending)
if err := SetNBGlobalIPSec(false); err != nil {
slog.Error("ipsec: NB_Global still demands encryption that unconfigured chassis cannot perform, cross-chassis guest traffic is being dropped now",
"pending", pending, "err", err)
return err
}
return nil
}
// NB_Global.ipsec is cluster-wide; only the management node has a local NB socket.
// Without this flag, ovn-controller skips adding options:remote_name to Geneve
// tunnels and ovs-monitor-ipsec never materialises strongSwan connections.
if _, err := os.Stat(ovnNBSocketPath); err == nil {
if err := SetNBGlobalIPSec(true); err != nil {
return err
// nbGlobalWriter picks the one node that may write NB_Global this pass: the
// first live management node in name order. Every node computes it from the same
// published set, so they agree without a lock.
func nbGlobalWriter(cluster map[string]IPSecNodeStatus) string {
var writer string
for _, node := range slices.Sorted(maps.Keys(cluster)) {
if cluster[node].NBReachable {
writer = node
break
}
}
return writer
}
slog.Info("OVN native IPsec enabled on intra-AZ Geneve",
"cert", certPath,
"key", keyPath,
"ca", caCertPath,
)
return nil
// notReady names the live nodes whose own IPsec configuration is incomplete.
func notReady(cluster map[string]IPSecNodeStatus) []string {
var pending []string
for _, node := range slices.Sorted(maps.Keys(cluster)) {
if !cluster[node].Ready {
pending = append(pending, node)
}
}
return pending
}
// nodeNames returns cluster membership in a stable order so logged pending sets
// do not churn between passes.
func nodeNames(clusterConfig *config.ClusterConfig) []string {
return slices.Sorted(maps.Keys(clusterConfig.Nodes))
}
// ensureOVSMonitorIPSecActive polls openvswitch-ipsec.service for "active".
+391 -65
View File
@@ -1,10 +1,14 @@
package host
import (
"context"
"errors"
"os"
"os/exec"
"path/filepath"
"slices"
"strings"
"sync"
"testing"
"time"
@@ -29,10 +33,33 @@ type recordingSudo struct {
activeOutput string
enabledOutput map[string]string
activePerUnit map[string]string
// nbUnreachable stands in for a node whose ovn-central is not answering, or
// which is not a management node at all. The socket file exists in both
// cases, which is why its presence was never the right question.
nbUnreachable bool
// nbIPSec is what NB_Global.ipsec currently reads as.
nbIPSec string
// nbError is a failure that is not "there is no local NB DB here" — a
// permission change on the socket, a missing binary, a timed-out
// transaction. On a management node each of those is a real fault.
nbError string
}
func (r *recordingSudo) stub(name string, args ...string) *exec.Cmd {
r.runs = append(r.runs, append([]string{name}, args...))
if name == "ovn-nbctl" {
if r.nbError != "" {
return exec.Command("sh", "-c", `echo "`+r.nbError+`" >&2; exit 1`)
}
if r.nbUnreachable {
return exec.Command("sh", "-c",
`echo "ovn-nbctl: unix:/var/run/ovn/ovnnb_db.sock: database connection failed" >&2; exit 1`)
}
if slices.Contains(args, "get") {
return exec.Command("printf", "%s\n", r.nbIPSec)
}
return exec.Command("true")
}
if name != "systemctl" || len(args) < 2 {
return exec.Command("true")
}
@@ -74,64 +101,186 @@ func multiNodeIPSecConfig(enabled bool) *config.ClusterConfig {
return cfg
}
func TestEnableOVNIPSec(t *testing.T) {
recorder := &recordingSudo{}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
// fakeBarrier stands in for the cluster readiness channel.
type fakeBarrier struct {
published []IPSecNodeStatus
publishedNode string
publishErr error
cluster map[string]IPSecNodeStatus
clusterErr error
}
func (f *fakeBarrier) Publish(_ context.Context, node string, status IPSecNodeStatus) error {
f.publishedNode = node
f.published = append(f.published, status)
return f.publishErr
}
func (f *fakeBarrier) Cluster(_ context.Context, _ []string) (map[string]IPSecNodeStatus, error) {
return f.cluster, f.clusterErr
}
// allReady is the steady state: every chassis has finished its local setup and
// node1 is the management node that owns the NB_Global write.
func allReady() *fakeBarrier {
return &fakeBarrier{cluster: map[string]IPSecNodeStatus{
"node1": {Ready: true, NBReachable: true},
"node2": {Ready: true},
}}
}
// node2Pending is a cluster where this node is ready and its peer is not.
func node2Pending() *fakeBarrier {
return &fakeBarrier{cluster: map[string]IPSecNodeStatus{
"node1": {Ready: true, NBReachable: true},
"node2": {},
}}
}
// ipsecTestConfigDir lays out the credentials EnableOVNIPSec insists on.
func ipsecTestConfigDir(t *testing.T) string {
t.Helper()
configDir := t.TempDir()
configPath := filepath.Join(configDir, "spinifex.toml")
require.NoError(t, os.WriteFile(configPath, []byte("placeholder"), 0600))
for _, rel := range []string{"ca.pem", "ipsec/peer.pem", "ipsec/peer.key"} {
full := filepath.Join(configDir, rel)
require.NoError(t, os.MkdirAll(filepath.Dir(full), 0750))
require.NoError(t, os.WriteFile(full, []byte("x"), 0600))
}
return configDir
}
// Worker path: no local NB socket → no ovn-nbctl call.
origNBSock := ovnNBSocketPath
ovnNBSocketPath = filepath.Join(configDir, "no-such-socket")
t.Cleanup(func() { ovnNBSocketPath = origNBSock })
require.NoError(t, EnableOVNIPSec(configPath, multiNodeClusterConfig()))
require.Len(t, recorder.runs, 3)
assert.Equal(t, []string{"systemctl", "is-active", "openvswitch-ipsec.service"}, recorder.runs[0])
for _, run := range recorder.runs[1:] {
assert.Equal(t, "ovs-vsctl", run[0])
assert.Equal(t, "set", run[1])
assert.Equal(t, "Open_vSwitch", run[2])
// nbctlWrites returns the ovn-nbctl invocations that write, dropping the reads.
// Matched on content: keying off an argument index would silently return
// nothing the moment anyone adds a flag, turning every "no write happened"
// assertion into one that asserts nothing.
func (r *recordingSudo) nbctlWrites() [][]string {
var out [][]string
for _, run := range r.runs {
if run[0] == "ovn-nbctl" && slices.Contains(run, "set") {
out = append(out, run)
}
}
joined := strings.Join(recorder.runs[1], " ")
assert.Contains(t, joined, "other_config:certificate="+filepath.Join(configDir, "ipsec", "peer.pem"))
assert.Contains(t, joined, "other_config:private_key="+filepath.Join(configDir, "ipsec", "peer.key"))
assert.Contains(t, joined, "other_config:ca_cert="+filepath.Join(configDir, "ca.pem"))
assert.Contains(t, strings.Join(recorder.runs[2], " "), "other_config:ipsec_encapsulation=true")
return out
}
// ovsSets returns the ovs-vsctl writes, joined for substring assertions.
func (r *recordingSudo) ovsSets() []string {
var out []string
for _, run := range r.runs {
if run[0] == "ovs-vsctl" && slices.Contains(run, "set") {
out = append(out, strings.Join(run, " "))
}
}
return out
}
func TestEnableOVNIPSec(t *testing.T) {
recorder := &recordingSudo{nbUnreachable: true}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configDir := ipsecTestConfigDir(t)
configPath := filepath.Join(configDir, "spinifex.toml")
require.NoError(t, EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(), allReady()))
sets := recorder.ovsSets()
require.Len(t, sets, 2)
assert.Contains(t, sets[0], "other_config:certificate="+filepath.Join(configDir, "ipsec", "peer.pem"))
assert.Contains(t, sets[0], "other_config:private_key="+filepath.Join(configDir, "ipsec", "peer.key"))
assert.Contains(t, sets[0], "other_config:ca_cert="+filepath.Join(configDir, "ca.pem"))
assert.Contains(t, sets[1], "other_config:ipsec_encapsulation=true")
// The NB DB is unreachable, so the read is attempted and nothing is written.
assert.Empty(t, recorder.nbctlWrites())
}
func TestEnableOVNIPSec_Management(t *testing.T) {
recorder := &recordingSudo{}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configDir := t.TempDir()
configPath := filepath.Join(configDir, "spinifex.toml")
require.NoError(t, os.WriteFile(configPath, []byte("placeholder"), 0600))
for _, rel := range []string{"ca.pem", "ipsec/peer.pem", "ipsec/peer.key"} {
full := filepath.Join(configDir, rel)
require.NoError(t, os.MkdirAll(filepath.Dir(full), 0750))
require.NoError(t, os.WriteFile(full, []byte("x"), 0600))
}
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
sockPath := filepath.Join(configDir, "ovnnb_db.sock")
require.NoError(t, os.WriteFile(sockPath, []byte{}, 0600))
origNBSock := ovnNBSocketPath
ovnNBSocketPath = sockPath
t.Cleanup(func() { ovnNBSocketPath = origNBSock })
require.NoError(t, EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(), allReady()))
require.NoError(t, EnableOVNIPSec(configPath, multiNodeClusterConfig()))
assert.Equal(t, [][]string{{"ovn-nbctl", "--timeout=5", "set", "NB_Global", ".", "ipsec=true"}},
recorder.nbctlWrites())
}
require.Len(t, recorder.runs, 4)
assert.Equal(t, []string{"ovn-nbctl", "set", "NB_Global", ".", "ipsec=true"}, recorder.runs[3])
// The reported incident: one node asserts encryption cluster-wide while the
// others have not finished, and every guest crossing chassis black-holes.
func TestEnableOVNIPSec_HoldsFlagUntilEveryChassisIsReady(t *testing.T) {
recorder := &recordingSudo{}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
barrier := node2Pending()
require.NoError(t, EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(), barrier))
assert.Equal(t, "node1", barrier.publishedNode)
assert.Equal(t, []IPSecNodeStatus{{Ready: true, NBReachable: true}}, barrier.published,
"the local half completed, so this node must report itself ready")
assert.Empty(t, recorder.nbctlWrites(),
"NB_Global must not be asserted while a chassis is unconfigured")
}
// A flag left asserted over an unconfigured chassis drops guest traffic on the
// floor; plaintext is the state the cluster had before IPsec was asked for.
func TestEnableOVNIPSec_RetractsFlagWhenAChassisRegresses(t *testing.T) {
recorder := &recordingSudo{nbIPSec: "true"}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
require.NoError(t, EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(),
node2Pending()))
assert.Equal(t, [][]string{{"ovn-nbctl", "--timeout=5", "set", "NB_Global", ".", "ipsec=false"}},
recorder.nbctlWrites())
}
// An unreadable barrier is not evidence that a chassis is unconfigured, so it
// must never downgrade a working encrypted mesh to plaintext.
func TestEnableOVNIPSec_BarrierErrorLeavesTheFlagAlone(t *testing.T) {
recorder := &recordingSudo{nbIPSec: "true"}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
err := EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(),
&fakeBarrier{clusterErr: errors.New("kv unavailable")})
require.Error(t, err)
assert.Contains(t, err.Error(), "read IPsec cluster state")
assert.Empty(t, recorder.nbctlWrites())
}
// Already true and everyone ready: no write, so a steady-state pass is free.
func TestEnableOVNIPSec_AlreadyAssertedIsANoOp(t *testing.T) {
recorder := &recordingSudo{nbIPSec: "true"}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
require.NoError(t, EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(), allReady()))
assert.Empty(t, recorder.nbctlWrites())
}
// A node that cannot publish must fail its pass rather than proceed: silently
// dropping the record would make it pending forever to everyone else.
func TestEnableOVNIPSec_PublishFailureFailsThePass(t *testing.T) {
recorder := &recordingSudo{}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
err := EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(),
&fakeBarrier{cluster: allReady().cluster, publishErr: errors.New("kv unavailable")})
require.Error(t, err)
assert.Contains(t, err.Error(), "publish IPsec state")
assert.Empty(t, recorder.nbctlWrites())
}
func TestEnableOVNIPSec_SingleNodeSkip(t *testing.T) {
@@ -148,7 +297,7 @@ func TestEnableOVNIPSec_SingleNodeSkip(t *testing.T) {
Node: "node1",
Nodes: map[string]config.Config{"node1": {}},
}
require.NoError(t, EnableOVNIPSec(configPath, cfg))
require.NoError(t, EnableOVNIPSec(t.Context(), configPath, cfg, allReady()))
}
func TestEnableOVNIPSec_MonitorIPSecInactive(t *testing.T) {
@@ -168,7 +317,7 @@ func TestEnableOVNIPSec_MonitorIPSecInactive(t *testing.T) {
require.NoError(t, os.WriteFile(full, []byte("x"), 0600))
}
err := EnableOVNIPSec(configPath, multiNodeClusterConfig())
err := EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(), allReady())
require.Error(t, err)
assert.Contains(t, err.Error(), "ovs-monitor-ipsec")
assert.Contains(t, err.Error(), "not active")
@@ -180,23 +329,24 @@ func TestEnableOVNIPSec_MonitorIPSecInactive(t *testing.T) {
}
func TestEnableOVNIPSec_MissingCert(t *testing.T) {
t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd {
t.Fatalf("utils.SudoCommand must not run when cert files are absent")
return exec.Command("true")
}))
recorder := &recordingSudo{}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configDir := t.TempDir()
configPath := filepath.Join(configDir, "spinifex.toml")
require.NoError(t, os.WriteFile(configPath, []byte("placeholder"), 0600))
require.NoError(t, os.WriteFile(filepath.Join(configDir, "ca.pem"), []byte("x"), 0600))
err := EnableOVNIPSec(configPath, multiNodeClusterConfig())
err := EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(), allReady())
require.Error(t, err)
assert.Contains(t, err.Error(), "missing IPsec credential")
assert.Empty(t, recorder.ovsSets(), "OVS must not be touched when the credentials are absent")
assert.Empty(t, recorder.nbctlWrites())
}
func TestEnableOVNIPSec_NoConfigPath(t *testing.T) {
err := EnableOVNIPSec("", nil)
err := EnableOVNIPSec(t.Context(), "", nil, nil)
require.Error(t, err)
assert.Contains(t, err.Error(), "config path unset")
}
@@ -205,7 +355,7 @@ func TestReconcileOVNIPSec_DisabledStopsCharon(t *testing.T) {
recorder := &recordingSudo{}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false)))
require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false), allReady()))
assert.Equal(t, [][]string{{ipsecStateHelper, "off"}}, recorder.helperRuns())
@@ -220,7 +370,7 @@ func TestReconcileOVNIPSec_NeverCallsSystemctlDirectly(t *testing.T) {
recorder := &recordingSudo{}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false)))
require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false), allReady()))
for _, run := range recorder.runs {
if run[0] != "systemctl" {
@@ -240,7 +390,7 @@ func TestReconcileOVNIPSec_SingleNodeStopsCharon(t *testing.T) {
cfg := &config.ClusterConfig{Node: "node1", Nodes: map[string]config.Config{"node1": {}}}
cfg.Network.IPSecEnabled = true
require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", cfg))
require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", cfg, allReady()))
assert.Equal(t, [][]string{{ipsecStateHelper, "off"}}, recorder.helperRuns())
}
@@ -258,20 +408,10 @@ func TestReconcileOVNIPSec_EnabledTurnsTheServicesOn(t *testing.T) {
}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configDir := t.TempDir()
configPath := filepath.Join(configDir, "spinifex.toml")
require.NoError(t, os.WriteFile(configPath, []byte("placeholder"), 0600))
for _, rel := range []string{"ca.pem", "ipsec/peer.pem", "ipsec/peer.key"} {
full := filepath.Join(configDir, rel)
require.NoError(t, os.MkdirAll(filepath.Dir(full), 0750))
require.NoError(t, os.WriteFile(full, []byte("x"), 0600))
}
recorder.nbUnreachable = true
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
origNBSock := ovnNBSocketPath
ovnNBSocketPath = filepath.Join(configDir, "no-such-socket")
t.Cleanup(func() { ovnNBSocketPath = origNBSock })
require.NoError(t, ReconcileOVNIPSec(configPath, multiNodeIPSecConfig(true)))
require.NoError(t, ReconcileOVNIPSec(t.Context(), configPath, multiNodeIPSecConfig(true), allReady()))
assert.Equal(t, [][]string{{ipsecStateHelper, "on"}}, recorder.helperRuns())
@@ -292,7 +432,7 @@ func TestReconcileOVNIPSec_AlreadyInStateIsNoOp(t *testing.T) {
}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false)))
require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false), allReady()))
assert.Empty(t, recorder.helperRuns())
}
@@ -309,7 +449,7 @@ func TestReconcileOVNIPSec_UnknownUnitIsNotAnError(t *testing.T) {
}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false)))
require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false), allReady()))
assert.Empty(t, recorder.helperRuns())
}
@@ -321,5 +461,191 @@ func TestReconcileOVNIPSec_NilConfigLeavesUnitsAlone(t *testing.T) {
return exec.Command("true")
}))
require.NoError(t, ReconcileOVNIPSec("/etc/spinifex/spinifex.toml", nil))
require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", nil, allReady()))
}
// A single attempt is what left two of three chassis unconfigured for minutes
// while a peer already required encryption; the pass has to be retried.
func TestMaintainIPSec_RetriesAFailedPass(t *testing.T) {
recorder := &recordingSudo{}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
origRetry, origInterval := ipsecRetryDelay, ipsecReconcileInterval
ipsecRetryDelay, ipsecReconcileInterval = time.Millisecond, time.Millisecond
t.Cleanup(func() { ipsecRetryDelay, ipsecReconcileInterval = origRetry, origInterval })
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
barrier := &flakyBarrier{failFor: 2}
ctx, cancel := context.WithCancel(t.Context())
defer cancel()
done := make(chan struct{})
go func() {
MaintainIPSec(ctx, configPath, multiNodeIPSecConfig(true), barrier)
close(done)
}()
require.Eventually(t, func() bool { return barrier.succeeded() }, 5*time.Second, 5*time.Millisecond,
"MaintainIPSec gave up after the first failure")
cancel()
select {
case <-done:
case <-time.After(5 * time.Second):
t.Fatal("MaintainIPSec did not stop on context cancellation")
}
}
// flakyBarrier fails its first failFor reads, then reports the cluster ready.
type flakyBarrier struct {
mu sync.Mutex
failFor int
calls int
}
func (f *flakyBarrier) Publish(context.Context, string, IPSecNodeStatus) error { return nil }
func (f *flakyBarrier) Cluster(context.Context, []string) (map[string]IPSecNodeStatus, error) {
f.mu.Lock()
defer f.mu.Unlock()
f.calls++
if f.calls <= f.failFor {
return nil, errors.New("kv unavailable")
}
return allReady().cluster, nil
}
func (f *flakyBarrier) succeeded() bool {
f.mu.Lock()
defer f.mu.Unlock()
return f.calls > f.failFor
}
// Every management node can reach the NB DB, so without a single elected writer
// two of them disagreeing for one pass flip the flag and rebuild every
// strongSwan connection in the cluster.
func TestEnableOVNIPSec_OnlyTheElectedWriterTouchesNBGlobal(t *testing.T) {
recorder := &recordingSudo{}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
// node1 is this node, but node0 sorts first and is also NB-reachable.
barrier := &fakeBarrier{cluster: map[string]IPSecNodeStatus{
"node0": {Ready: true, NBReachable: true},
"node1": {Ready: true, NBReachable: true},
}}
require.NoError(t, EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(), barrier))
assert.Empty(t, recorder.nbctlWrites(),
"a node that is not the elected writer must not race the flag")
}
// The elected writer is the first live management node, not merely the first
// node: a hypervisor cannot write the flag and must not veto the one that can.
func TestNBGlobalWriter_SkipsNodesWithoutANBDB(t *testing.T) {
writer := nbGlobalWriter(map[string]IPSecNodeStatus{
"node1": {Ready: true},
"node2": {Ready: true, NBReachable: true},
"node3": {Ready: true, NBReachable: true},
})
assert.Equal(t, "node2", writer)
}
// A node that fails its local half must say so, not leave its last "ready"
// record to expire: silence keeps it counted as configured for a whole
// freshness window, which is the black hole again with a slower fuse.
func TestEnableOVNIPSec_PublishesUnreadyWhenTheLocalHalfFails(t *testing.T) {
recorder := &recordingSudo{activeOutput: "inactive\n"}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
origTimeout := systemctlActiveTimeout
systemctlActiveTimeout = 100 * time.Millisecond
t.Cleanup(func() { systemctlActiveTimeout = origTimeout })
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
barrier := allReady()
err := EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(), barrier)
require.Error(t, err)
assert.Contains(t, err.Error(), "ovs-monitor-ipsec")
assert.Equal(t, "node1", barrier.publishedNode)
require.Len(t, barrier.published, 1)
assert.False(t, barrier.published[0].Ready,
"a node that cannot encrypt must publish that, not go quiet")
assert.True(t, barrier.published[0].NBReachable)
}
// Turning IPsec off stops charon on every node. A flag left asserted then
// demands encryption nothing can perform — the same black hole, reached from
// the other direction.
func TestReconcileOVNIPSec_DisabledRetractsNBGlobal(t *testing.T) {
recorder := &recordingSudo{nbIPSec: "true"}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
barrier := allReady()
require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml",
multiNodeIPSecConfig(false), barrier))
assert.Equal(t, [][]string{{"ovn-nbctl", "--timeout=5", "set", "NB_Global", ".", "ipsec=false"}},
recorder.nbctlWrites())
require.Len(t, barrier.published, 1)
assert.False(t, barrier.published[0].Ready)
}
// Already released: the off path must not rewrite a flag that is already false.
func TestReconcileOVNIPSec_DisabledIsANoOpWhenAlreadyReleased(t *testing.T) {
recorder := &recordingSudo{}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml",
multiNodeIPSecConfig(false), allReady()))
assert.Empty(t, recorder.nbctlWrites())
}
// A management node whose NB read fails for any reason other than "there is no
// local DB here" has a real fault. Swallowing it was how the node that holds
// the flag could go blind and never retract.
func TestEnableOVNIPSec_NBReadFailureFailsThePass(t *testing.T) {
recorder := &recordingSudo{nbError: "ovn-nbctl: unix:/var/run/ovn/ovnnb_db.sock: permission denied"}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
err := EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(), allReady())
require.Error(t, err)
assert.Contains(t, err.Error(), "permission denied")
assert.Empty(t, recorder.nbctlWrites())
}
// A node with no local NB DB is the ordinary hypervisor case, not a fault.
func TestEnableOVNIPSec_NoLocalNBDBIsNotAnError(t *testing.T) {
recorder := &recordingSudo{nbUnreachable: true}
t.Cleanup(utils.SetSudoCommandForTest(recorder.stub))
configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml")
barrier := allReady()
require.NoError(t, EnableOVNIPSec(t.Context(), configPath, multiNodeClusterConfig(), barrier))
require.Len(t, barrier.published, 1)
assert.True(t, barrier.published[0].Ready)
assert.False(t, barrier.published[0].NBReachable,
"a node that cannot read NB_Global must not offer itself as the writer")
}
// ovn-nbctl writes vlog lines to stderr on a successful run. Folding those into
// the value parses a live "true" as false, which sends the reconcile into the
// "holding encryption off" branch while the flag is in fact still asserted.
func TestGetNBGlobalIPSec_IgnoresStderrChatter(t *testing.T) {
t.Cleanup(utils.SetSudoCommandForTest(func(string, ...string) *exec.Cmd {
return exec.Command("sh", "-c", `echo "ovsdb-idl|WARN|reconnecting" >&2; echo true`)
}))
value, err := GetNBGlobalIPSec()
require.NoError(t, err)
assert.True(t, value)
}
+59 -1
View File
@@ -4,6 +4,7 @@ package multinode
import (
"context"
"errors"
"fmt"
"strings"
"testing"
@@ -43,6 +44,7 @@ func runIPSec(t *testing.T, fix *Fixture) {
harness.Step(t, "OVS DB carries cert pointers + ipsec_encapsulation=true on every node")
required := []string{"certificate=", "private_key=", "ca_cert=", "ipsec_encapsulation=\"true\""}
incomplete := map[string]string{}
for _, n := range fix.Cluster.Nodes {
c, cancel := context.WithTimeout(context.Background(), 10*time.Second)
raw, err := ssh.Run(c, n.Addr, "sudo ovs-vsctl get Open_vSwitch . other_config")
@@ -53,12 +55,15 @@ func runIPSec(t *testing.T, fix *Fixture) {
s := strings.TrimSpace(string(raw))
for _, key := range required {
if !strings.Contains(s, key) {
t.Fatalf("%s OVS other_config missing %q: %s", n.Name, key, s)
incomplete[n.Name] = s
t.Errorf("%s OVS other_config missing %q: %s", n.Name, key, s)
}
}
harness.Detail(t, "node", n.Name, "other_config", s)
}
assertNBGlobalMatchesChassis(t, fix, ssh, incomplete)
harness.Step(t, "xfrm SAs with AES-GCM established on every node")
harness.EventuallyErr(t, func() error {
for _, n := range fix.Cluster.Nodes {
@@ -132,3 +137,56 @@ func ipsecRequested(t *testing.T, ssh *harness.PeerSSH, n harness.Node) bool {
harness.Detail(t, "node", n.Name, "ipsec_enabled", value)
return value != "false"
}
// assertNBGlobalMatchesChassis checks the invariant a partial IPsec enable
// breaks: NB_Global.ipsec is cluster-wide, so asserting it while any chassis is
// still unconfigured black-holes every guest that crosses chassis, with no
// control-plane signal that anything is wrong.
func assertNBGlobalMatchesChassis(t *testing.T, fix *Fixture, ssh *harness.PeerSSH, incomplete map[string]string) {
harness.Step(t, "NB_Global ipsec is not asserted over an unconfigured chassis")
// The flag is asserted only once every node has published its own readiness,
// so it lags the per-node config by up to a reconcile interval. Polling for it
// is the difference between testing the invariant and testing the clock.
var asserted string
harness.EventuallyErr(t, func() error {
var unreadable []string
for _, n := range fix.Cluster.Nodes {
value, err := readNBGlobalIPSec(t, ssh, n)
if err != nil {
unreadable = append(unreadable, fmt.Sprintf("%s: %v", n.Name, err))
continue
}
harness.Detail(t, "node", n.Name, "nb_global_ipsec", value)
if value == "true" {
asserted = n.Name
return nil
}
}
if len(unreadable) == len(fix.Cluster.Nodes) {
return fmt.Errorf("no node could read NB_Global: %s", strings.Join(unreadable, "; "))
}
return errors.New("no node reports NB_Global ipsec=true — ovn-controller adds no options:remote_name, so intra-AZ Geneve is plaintext")
}, 120*time.Second, 5*time.Second)
if len(incomplete) > 0 {
t.Fatalf("NB_Global ipsec=true (read on %s) while %d chassis are unconfigured: %v — cross-chassis guest traffic is black-holing",
asserted, len(incomplete), incomplete)
}
}
// readNBGlobalIPSec returns the flag as the node reports it. A node with no
// local NB DB returns an error rather than "false": conflating the two is the
// defect the production change exists to remove, and the test must not reinstate
// it by discarding the exit status.
func readNBGlobalIPSec(t *testing.T, ssh *harness.PeerSSH, n harness.Node) (string, error) {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
raw, err := ssh.Run(ctx, n.Addr, "sudo ovn-nbctl --timeout=5 get NB_Global . ipsec")
if err != nil {
return "", fmt.Errorf("%w: %s", err, strings.TrimSpace(string(raw)))
}
return strings.Trim(strings.TrimSpace(string(raw)), `"`), nil
}