diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5882325..9907d1c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1 +1,92 @@ -Documentation will be added when the project gets more mature. Feel free to contribute +# Contributing + +This project is built with [Kubebuilder](https://github.com/kubernetes-sigs/kubebuilder); +read about that first. Fork the repository, make your changes, and open a PR. + +Planned work is scoped in [`docs/ROADMAP.md`](docs/ROADMAP.md), with each item tracked as +a GitHub issue. + +## Getting started + +```console +make test # runs manifests, generate, fmt, vet, then the envtest suite +make manifests # regenerate CRDs and RBAC after changing api/ +make generate # regenerate deepcopy functions after changing api/ +``` + +`make test` regenerates before running, so a stale CRD or an unformatted file fails +locally rather than in CI. Never hand-edit `zz_generated.deepcopy.go` or anything under +`config/crd/bases` — regenerate instead. `release.yaml` and `bundle/` are produced at +release time and should not be edited by hand either. + +## Engineering practices + +These apply to every change. + +### TDD + +Write the test first, watch it fail for the right reason, then make it pass. A test that +has never failed has proven nothing. + +For changes that add behaviour this is literal. For **pure refactors** it is not — there +is no new behaviour to drive out, so writing new tests first would be cargo-culting. +On a refactor it means: establish a green baseline before touching anything, keep it green +after every step, and if an existing test fails, the refactor changed behaviour. Fix the +code, never the assertion. + +### DRY + +The hardest-won lesson in this codebase: `internal/resources` and the inline reconciler +functions were the same code twice, and they silently drifted apart. Do not create a second +copy of anything — a constant, a rendering path, a default value. One source of truth, +referenced. + +Note the limit: two things that look alike but change for different reasons are not +duplication. Do not collapse them to satisfy the acronym. + +### 12-Factor + +Most factors are satisfied by Kubernetes itself. The ones that bite here: + +- **Config** — configuration comes from the CRD or the environment, never hardcoded in more + than one place. A default belongs in the CRD's `+kubebuilder:default`, with any Go + constant existing only as a nil-fallback. +- **Processes** — builders stay pure functions of their inputs. No caching on the + reconciler, no hidden state between calls. +- **Logs** — event streams to stdout via `logr`. No log files, no rotation. +- **Disposability** — fast startup, graceful shutdown. + +### YAGNI + +Build what the issue asks for. Do not add a config knob because someone might want it, and +do not generalise for a second implementation that does not exist. If you are writing an +abstraction with exactly one caller, stop. + +### KISS + +Prefer the boring solution. If a reviewer needs the design doc open to understand the diff, +it is too clever. Explicit repetition of three short lines beats a helper that takes four +parameters to avoid it. + +### Cold review before the PR + +Before opening a PR, get a review from a reviewer with none of the context that produced +the change — a fresh session, colleague, or agent — given only: + +- the diff +- the issue text, including its acceptance criteria +- the repository + +and explicitly **not** the implementation plan, the discussion that produced it, or any +reasoning about why choices were made. + +Ask it to answer: + +1. Does the diff do what the acceptance criteria say, no more and no less? +2. Is there scope creep — anything changed that the issue did not ask for? +3. Is anything wrong, unclear, or surprising to someone seeing it for the first time? +4. Are the tests testing behaviour, or asserting on implementation details? + +An author who has been reasoning about a change for an hour cannot see what is unexplained +in it. If the cold reviewer misreads the diff, that is a finding about the diff. Address +the findings, then open the PR. diff --git a/README.md b/README.md index c98d752..2d78e3e 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ Painless deployment of wireguard on kubernetes ## Support -If you are facing any problems please open an [issue](https://github.com/nccloud/wireguard-operator/issues) +If you are facing any problems please open an [issue](https://github.com/jacaudi/wireguard-operator/issues) ## Tested with - [x] IBM Cloud Kubernetes Service @@ -32,6 +32,8 @@ If you are facing any problems please open an [issue](https://github.com/nccloud * Does not need persistance. peer/server keys are stored as k8s secrets and loaded into the wireguard pod * Exposes a metrics endpoint * Supports tunneling/traffic obfuscation using [wstunnel](https://github.com/erebe/wstunnel) +* IPv6 support, including IPv6-only peers, through `spec.peerCIDRv6` and `spec.ipv6Only` +* Per-peer egress network policies through `WireguardPeer.spec.egressNetworkPolicies` ## Example @@ -78,9 +80,20 @@ MTU = 1380 PublicKey = sO3ZWhnIT8owcdsfwiMRu2D8LzKmae2gUAxAmhx5GTg= AllowedIPs = 0.0.0.0/0 Endpoint = 32.121.45.102:51820 +PersistentKeepalive = 25 ``` +`PersistentKeepalive` defaults to 25 seconds and is configurable through +`Wireguard.Spec.PersistentKeepalive`. Set it to `0` to omit it. + ## How to deploy + +> **Note:** this fork has not cut its own release yet, so the commands below install the +> upstream `nccloud` operator, which is diverging from this fork. Features added here — +> `spec.persistentKeepalive`, for example — will not exist in that build. Until a release +> is published (tracked in [#36](https://github.com/jacaudi/wireguard-operator/issues/36)), +> deploy from source with `make deploy`. + ### Using provided manifest file ``` kubectl apply -f https://github.com/nccloud/wireguard-operator/releases/download/v2.11.0/release.yaml diff --git a/api/v1alpha1/wireguard_types.go b/api/v1alpha1/wireguard_types.go index 970bb04..542fe9b 100644 --- a/api/v1alpha1/wireguard_types.go +++ b/api/v1alpha1/wireguard_types.go @@ -63,6 +63,15 @@ type WireguardSpec struct { EnableIpForwardOnPodInit bool `json:"enableIpForwardOnPodInit,omitempty"` // A boolean field that specifies whether to use the userspace implementation of Wireguard instead of the kernel one. UseWgUserspaceImplementation bool `json:"useWgUserspaceImplementation,omitempty"` + // PersistentKeepalive is the interval in seconds at which peers send keepalive packets + // to the server. This keeps NAT and conntrack entries alive so the server can reach the + // peer, and makes the peer re-handshake promptly after the server pod is rescheduled. + // Set to 0 to disable. Defaults to 25. + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=65535 + // +kubebuilder:default=25 + // +optional + PersistentKeepalive *int32 `json:"persistentKeepalive,omitempty"` NodeSelector map[string]string `json:"nodeSelector,omitempty"` // A list of Kubernetes taint tolerations applied to the Wireguard pod. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 33aa464..24c0c69 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -342,6 +342,11 @@ func (in *WireguardSpec) DeepCopyInto(out *WireguardSpec) { (*out)[key] = val } } + if in.PersistentKeepalive != nil { + in, out := &in.PersistentKeepalive, &out.PersistentKeepalive + *out = new(int32) + **out = **in + } if in.NodeSelector != nil { in, out := &in.NodeSelector, &out.NodeSelector *out = make(map[string]string, len(*in)) diff --git a/config/crd/bases/vpn.wireguard-operator.io_wireguards.yaml b/config/crd/bases/vpn.wireguard-operator.io_wireguards.yaml index eb12a1d..1d31a0b 100644 --- a/config/crd/bases/vpn.wireguard-operator.io_wireguards.yaml +++ b/config/crd/bases/vpn.wireguard-operator.io_wireguards.yaml @@ -219,6 +219,17 @@ spec: PeerCIDRv6 is the IPv6 CIDR range from which Wireguard peer IPv6 addresses will be allocated. When set, IPv6 support is enabled for this Wireguard instance. type: string + persistentKeepalive: + default: 25 + description: |- + PersistentKeepalive is the interval in seconds at which peers send keepalive packets + to the server. This keeps NAT and conntrack entries alive so the server can reach the + peer, and makes the peer re-handshake promptly after the server pod is rescheduled. + Set to 0 to disable. Defaults to 25. + format: int32 + maximum: 65535 + minimum: 0 + type: integer port: description: A field that specifies the value to use for a nodePort ServiceType diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 9efc073..806be75 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -148,18 +148,20 @@ nor blocks them. The killswitch does not interfere with port forwarding. Blocks everything else. -### 0.1 Consolidate the two divergent Deployment builders ([#1](https://github.com/jacaudi/wireguard-operator/issues/1)) +### 0.1 Consolidate resource rendering onto internal/resources builders ([#1](https://github.com/jacaudi/wireguard-operator/issues/1)) -Two paths render the Deployment and the newer one is dead code. -`internal/resources/deployment.go:54` (`DeploymentBuilder.ForWireguard`) is constructed -at `internal/controller/wireguard_controller.go:1000` but never called; the live path is -`deploymentForWireguard` at `:1110`, called from `:810`, `:825`, `:846`, `:863`, `:874`. -They have already drifted — the unused one adds a `metrics` port and uses `HTTPPort`. +The whole `internal/resources` package is dead code. All four builders are constructed in +`SetupWithManager` but no method is ever called — every resource is rendered by an inline +function on the reconciler instead. Twelve call sites across the two controllers, six +inline functions, and a duplicated set of port and image constants. -Every feature below touches Deployment rendering. Implementing in `internal/resources` -alone would have no runtime effect. +The two copies have already drifted: the unused `DeploymentBuilder` declares a `metrics` +container port the live path lacks. -- [ ] Exactly one function renders the Deployment +Every feature below touches this rendering. Implementing in `internal/resources` alone +would have no runtime effect at all. + +- [ ] Exactly one function renders each resource type - [ ] Drift resolved intentionally, not by arbitrarily picking a copy - [ ] Existing controller tests pass unchanged diff --git a/internal/controller/wireguard_controller.go b/internal/controller/wireguard_controller.go index 8275c79..c7897ad 100644 --- a/internal/controller/wireguard_controller.go +++ b/internal/controller/wireguard_controller.go @@ -40,18 +40,16 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/intstr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" ) const ( - port = 51820 - httpPort = 8080 - metricsPort = 9586 - defaultTunnelPort = 443 - defaultWstunnelImage = "ghcr.io/erebe/wstunnel:latest" + // defaultPersistentKeepalive is the fallback keepalive interval, in seconds, used when + // Wireguard.Spec.PersistentKeepalive is nil. The value users actually receive comes from + // the CRD default; this only covers objects stored before the field existed. + defaultPersistentKeepalive = 25 ) // Standard condition types for Wireguard @@ -76,24 +74,6 @@ type WireguardReconciler struct { ipAllocator *ipam.Allocator } -func labelsForWireguard(name string) map[string]string { - return resources.LabelsForWireguard(name) -} - -func (r *WireguardReconciler) ConfigmapForWireguard(m *v1alpha1.Wireguard, hostname string) *corev1.ConfigMap { - ls := labelsForWireguard(m.Name) - dep := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: m.Name + "-config", - Namespace: m.Namespace, - Labels: ls, - }, - } - - _ = ctrl.SetControllerReference(m, dep, r.Scheme) - return dep -} - func (r *WireguardReconciler) getWireguardPeers(ctx context.Context, req ctrl.Request) (*v1alpha1.WireguardPeerList, error) { peers := &v1alpha1.WireguardPeerList{} if err := r.List(ctx, peers, client.InNamespace(req.Namespace)); err != nil { @@ -204,7 +184,25 @@ func effectivePeerCIDR6(wg *v1alpha1.Wireguard) (string, bool) { return wg.Spec.PeerCIDRv6, true } +// persistentKeepalive returns the keepalive interval to write into peer configs. +// A nil value means the field was absent — either the object predates the field or +// defaulting is disabled — so fall back to the default rather than to zero, which +// WireGuard interprets as "disabled". +func persistentKeepalive(wg *v1alpha1.Wireguard) int32 { + if wg.Spec.PersistentKeepalive == nil { + return defaultPersistentKeepalive + } + return *wg.Spec.PersistentKeepalive +} + func (r *WireguardReconciler) updateWireguardPeers(ctx context.Context, req ctrl.Request, wireguard *v1alpha1.Wireguard, serverAddress string, dns string, dnsSearchDomain string, serverPublicKey string, serverMtu string) error { + // Rendered once and interpolated into every config flavor below. Empty when the + // interval is 0, so the line is omitted rather than written as an explicit zero. + keepaliveLine := "" + if ka := persistentKeepalive(wireguard); ka > 0 { + keepaliveLine = fmt.Sprintf("PersistentKeepalive = %d\n", ka) + } + peers, err := r.getWireguardPeers(ctx, req) if err != nil { return err @@ -329,7 +327,7 @@ DNS = %s`, strings.TrimSpace(string(v)), addressLine, dnsConfiguration) if wireguard.Spec.Tunnel.Enabled { tunnelPort := wireguard.Spec.Tunnel.Port if tunnelPort == 0 { - tunnelPort = defaultTunnelPort + tunnelPort = resources.DefaultTunnelPort } // Tunnel config with PreUp/PostDown hooks @@ -341,7 +339,7 @@ PostDown = killall wstunnel || true PublicKey = %s AllowedIPs = %s Endpoint = 127.0.0.1:%d -`, port, port, serverAddress, tunnelPort, serverPublicKey, allowIps, port) +%s`, resources.WireguardPort, resources.WireguardPort, serverAddress, tunnelPort, serverPublicKey, allowIps, resources.WireguardPort, keepaliveLine) if wireguard.Spec.Tunnel.DualMode { // In dual mode, store both configs: @@ -353,7 +351,7 @@ Endpoint = 127.0.0.1:%d PublicKey = %s AllowedIPs = %s Endpoint = %s:%s -`, serverPublicKey, allowIps, serverAddress, wireguard.Status.Port) +%s`, serverPublicKey, allowIps, serverAddress, wireguard.Status.Port, keepaliveLine) newPeerCfgData[peer.Name] = []byte(directCfg) newPeerCfgData[peer.Name+".tunnel"] = []byte(tunnelCfg) } else { @@ -367,7 +365,7 @@ Endpoint = %s:%s PublicKey = %s AllowedIPs = %s Endpoint = %s:%s -`, serverPublicKey, allowIps, serverAddress, wireguard.Status.Port) +%s`, serverPublicKey, allowIps, serverAddress, wireguard.Status.Port, keepaliveLine) newPeerCfgData[peer.Name] = []byte(pureCfg) } } @@ -492,7 +490,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( err = r.Get(ctx, types.NamespacedName{Name: wireguard.Name + "-metrics-svc", Namespace: wireguard.Namespace}, svcFound) if err != nil && errors.IsNotFound(err) { - svc := r.serviceForWireguardMetrics(wireguard) + svc, err := r.serviceBuilder.ForWireguardMetrics(wireguard) + if err != nil { + log.Error(err, "Failed to build metrics service", "wireguard.Name", wireguard.Name) + return ctrl.Result{}, err + } log.Info("Creating a new service", "service.Namespace", svc.Namespace, "service.Name", svc.Name) err = r.Create(ctx, svc) if err != nil { @@ -550,7 +552,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( err = r.Get(ctx, types.NamespacedName{Name: wireguard.Name + "-svc", Namespace: wireguard.Namespace}, svcFound) if err != nil && errors.IsNotFound(err) { - svc := r.serviceForWireguard(wireguard, serviceType) + svc, err := r.serviceBuilder.ForWireguard(wireguard, serviceType) + if err != nil { + log.Error(err, "Failed to build service", "wireguard.Name", wireguard.Name) + return ctrl.Result{}, err + } log.Info("Creating a new service", "service.Namespace", svc.Namespace, "service.Name", svc.Name) err = r.Create(ctx, svc) if err != nil { @@ -575,7 +581,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // Update service ports if tunnel configuration changed. // Compare port count and port numbers to detect tunnel/dualMode toggling. { - desiredSvc := r.serviceForWireguard(wireguard, serviceType) + desiredSvc, err := r.serviceBuilder.ForWireguard(wireguard, serviceType) + if err != nil { + log.Error(err, "Failed to build service", "wireguard.Name", wireguard.Name) + return ctrl.Result{}, err + } needsUpdate := len(svcFound.Spec.Ports) != len(desiredSvc.Spec.Ports) if !needsUpdate && len(svcFound.Spec.Ports) > 0 { if svcFound.Spec.Ports[0].Port != desiredSvc.Spec.Ports[0].Port { @@ -598,11 +608,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } // Compute the effective external port (tunnel port when enabled, WG port otherwise) - var port = fmt.Sprintf("%d", port) + var port = fmt.Sprintf("%d", resources.WireguardPort) if wireguard.Spec.Tunnel.Enabled { tp := wireguard.Spec.Tunnel.Port if tp == 0 { - tp = defaultTunnelPort + tp = resources.DefaultTunnelPort } port = fmt.Sprintf("%d", tp) } @@ -710,8 +720,12 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( log.Info("Updating secret with new config") publicKey := string(secret.Data["publicKey"]) - err := r.Update(ctx, r.secretForWireguard(wireguard, b, privateKey, publicKey)) + updatedSecret, err := r.secretBuilder.ForWireguard(wireguard, b, privateKey, publicKey) if err != nil { + log.Error(err, "Failed to build secret with new config") + return ctrl.Result{}, err + } + if err := r.Update(ctx, updatedSecret); err != nil { log.Error(err, "Failed to update secret with new config") return ctrl.Result{}, err } @@ -767,7 +781,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } - secret := r.secretForWireguard(wireguard, b, privateKey, publicKey) + secret, err := r.secretBuilder.ForWireguard(wireguard, b, privateKey, publicKey) + if err != nil { + log.Error(err, "Failed to build secret", "wireguard.Name", wireguard.Name) + return ctrl.Result{}, err + } log.Info("Creating a new secret", "secret.Namespace", secret.Namespace, "secret.Name", secret.Name) @@ -786,7 +804,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( configFound := &corev1.ConfigMap{} err = r.Get(ctx, types.NamespacedName{Name: wireguard.Name + "-config", Namespace: wireguard.Namespace}, configFound) if err != nil && errors.IsNotFound(err) { - config := r.ConfigmapForWireguard(wireguard, address) + config, err := r.configMapBuilder.ForWireguard(wireguard) + if err != nil { + log.Error(err, "Failed to build configmap", "wireguard.Name", wireguard.Name) + return ctrl.Result{}, err + } log.Info("Creating a new config", "config.Namespace", config.Namespace, "config.Name", config.Name) err = r.Create(ctx, config) if err != nil { @@ -807,7 +829,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( deploymentFound := &appsv1.Deployment{} err = r.Get(ctx, types.NamespacedName{Name: wireguard.Name + "-dep", Namespace: wireguard.Namespace}, deploymentFound) if err != nil && errors.IsNotFound(err) { - dep := r.deploymentForWireguard(wireguard) + dep, buildErr := r.deploymentBuilder.ForWireguard(wireguard) + if buildErr != nil { + log.Error(buildErr, "Failed to build deployment", "wireguard.Name", wireguard.Name) + return ctrl.Result{}, buildErr + } log.Info("Creating a new dep", "dep.Namespace", dep.Namespace, "dep.Name", dep.Name, "useUserspace", wireguard.Spec.UseWgUserspaceImplementation) err = r.Create(ctx, dep) if err != nil { @@ -822,7 +848,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } if deploymentFound.Spec.Template.Spec.Containers[0].Image != r.AgentImage { - dep := r.deploymentForWireguard(wireguard) + dep, buildErr := r.deploymentBuilder.ForWireguard(wireguard) + if buildErr != nil { + log.Error(buildErr, "Failed to build deployment", "wireguard.Name", wireguard.Name) + return ctrl.Result{}, buildErr + } err = r.Update(ctx, dep) if err != nil { log.Error(err, "unable to update deployment image", "dep.Namespace", dep.Namespace, "dep.Name", dep.Name) @@ -843,7 +873,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } if existingUserspace != desiredUserspace { log.Info("Updating deployment userspace flag", "desired", desiredUserspace, "existing", existingUserspace) - dep := r.deploymentForWireguard(wireguard) + dep, buildErr := r.deploymentBuilder.ForWireguard(wireguard) + if buildErr != nil { + log.Error(buildErr, "Failed to build deployment", "wireguard.Name", wireguard.Name) + return ctrl.Result{}, buildErr + } if err := r.Update(ctx, dep); err != nil { log.Error(err, "unable to update deployment userspace flag", "dep.Namespace", dep.Namespace, "dep.Name", dep.Name) return ctrl.Result{}, err @@ -860,7 +894,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } if hasWstunnel != wireguard.Spec.Tunnel.Enabled { log.Info("Updating deployment tunnel sidecar", "desired", wireguard.Spec.Tunnel.Enabled, "existing", hasWstunnel) - dep := r.deploymentForWireguard(wireguard) + dep, buildErr := r.deploymentBuilder.ForWireguard(wireguard) + if buildErr != nil { + log.Error(buildErr, "Failed to build deployment", "wireguard.Name", wireguard.Name) + return ctrl.Result{}, buildErr + } if err := r.Update(ctx, dep); err != nil { log.Error(err, "unable to update deployment tunnel sidecar", "dep.Namespace", dep.Namespace, "dep.Name", dep.Name) return ctrl.Result{}, err @@ -871,7 +909,11 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if !reflect.DeepEqual(deploymentFound.Spec.Template.Spec.NodeSelector, wireguard.Spec.NodeSelector) || !reflect.DeepEqual(deploymentFound.Spec.Template.Spec.Tolerations, wireguard.Spec.Tolerations) { log.Info("Updating deployment scheduling settings") - dep := r.deploymentForWireguard(wireguard) + dep, buildErr := r.deploymentBuilder.ForWireguard(wireguard) + if buildErr != nil { + log.Error(buildErr, "Failed to build deployment", "wireguard.Name", wireguard.Name) + return ctrl.Result{}, buildErr + } if err := r.Update(ctx, dep); err != nil { log.Error(err, "unable to update deployment scheduling settings", "dep.Namespace", dep.Namespace, "dep.Name", dep.Name) return ctrl.Result{}, err @@ -1010,277 +1052,3 @@ func (r *WireguardReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&corev1.Secret{}). Complete(r) } - -func (r *WireguardReconciler) serviceForWireguard(m *v1alpha1.Wireguard, serviceType corev1.ServiceType) *corev1.Service { - labels := labelsForWireguard(m.Name) - - svcPorts := []corev1.ServicePort{{ - Name: "wireguard", - Protocol: corev1.ProtocolUDP, - NodePort: m.Spec.NodePort, - Port: port, - TargetPort: intstr.FromInt(port), - }} - - if m.Spec.Tunnel.Enabled { - tunnelPort := m.Spec.Tunnel.Port - if tunnelPort == 0 { - tunnelPort = defaultTunnelPort - } - tunnelSvcPort := corev1.ServicePort{ - Name: "tunnel", - Protocol: corev1.ProtocolTCP, - Port: tunnelPort, - TargetPort: intstr.FromInt(int(tunnelPort)), - } - if m.Spec.Tunnel.DualMode { - svcPorts = append(svcPorts, tunnelSvcPort) - } else { - svcPorts = []corev1.ServicePort{tunnelSvcPort} - } - } - - dep := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: m.Name + "-svc", - Namespace: m.Namespace, - Annotations: m.Spec.ServiceAnnotations, - Labels: labels, - }, - Spec: corev1.ServiceSpec{ - Selector: labels, - Ports: svcPorts, - Type: serviceType, - }, - } - - if dep.Spec.Type == corev1.ServiceTypeLoadBalancer { - if m.Spec.Address != "" { - dep.Spec.LoadBalancerIP = m.Spec.Address - } - } - - _ = ctrl.SetControllerReference(m, dep, r.Scheme) - return dep -} - -func (r *WireguardReconciler) serviceForWireguardMetrics(m *v1alpha1.Wireguard) *corev1.Service { - labels := labelsForWireguard(m.Name) - - dep := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: m.Name + "-metrics-svc", - Namespace: m.Namespace, - Labels: labels, - }, - Spec: corev1.ServiceSpec{ - Selector: labels, - Ports: []corev1.ServicePort{{ - Name: "metrics", - Protocol: corev1.ProtocolTCP, - Port: metricsPort, - TargetPort: intstr.FromInt(metricsPort), - }}, - Type: corev1.ServiceTypeClusterIP, - }, - } - - _ = ctrl.SetControllerReference(m, dep, r.Scheme) - return dep -} - -func (r *WireguardReconciler) secretForWireguard(m *v1alpha1.Wireguard, state []byte, privateKey string, publicKey string) *corev1.Secret { - - ls := labelsForWireguard(m.Name) - dep := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: m.Name, - Namespace: m.Namespace, - Labels: ls, - }, - Data: map[string][]byte{"state.json": state, "privateKey": []byte(privateKey), "publicKey": []byte(publicKey)}, - } - - _ = ctrl.SetControllerReference(m, dep, r.Scheme) - - return dep - -} - -func (r *WireguardReconciler) deploymentForWireguard(m *v1alpha1.Wireguard) *appsv1.Deployment { - ls := labelsForWireguard(m.Name) - replicas := int32(1) - - readOnlyRootFilesystem := true - allowPrivilegeEscalation := false - automountServiceAccountToken := false - - dep := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: m.Name + "-dep", - Namespace: m.Namespace, - Labels: ls, - }, - Spec: appsv1.DeploymentSpec{ - Replicas: &replicas, - Selector: &metav1.LabelSelector{ - MatchLabels: ls, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: ls, - }, - Spec: corev1.PodSpec{ - NodeSelector: m.Spec.NodeSelector, - Tolerations: m.Spec.Tolerations, - SecurityContext: &corev1.PodSecurityContext{ - SeccompProfile: &corev1.SeccompProfile{ - Type: corev1.SeccompProfileType("RuntimeDefault"), - }, - }, - AutomountServiceAccountToken: &automountServiceAccountToken, - Volumes: []corev1.Volume{ - { - Name: "socket", - VolumeSource: corev1.VolumeSource{ - - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, - }, - { - - Name: "config", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: m.Name, - }, - }, - }}, - InitContainers: []corev1.Container{}, - Containers: []corev1.Container{ - { - SecurityContext: &corev1.SecurityContext{ - ReadOnlyRootFilesystem: &readOnlyRootFilesystem, - AllowPrivilegeEscalation: &allowPrivilegeEscalation, - Capabilities: &corev1.Capabilities{Add: []corev1.Capability{"NET_ADMIN"}}, - }, - Image: r.AgentImage, - ImagePullPolicy: r.AgentImagePullPolicy, - Name: "agent", - Command: []string{"agent", "--v", "11", "--wg-iface", "wg0", "--wg-listen-port", fmt.Sprintf("%d", port), "--state", "/tmp/wireguard/state.json", "--wg-userspace-implementation-fallback", "wireguard-go"}, - Ports: []corev1.ContainerPort{ - { - ContainerPort: port, - Name: "wireguard", - Protocol: corev1.ProtocolUDP, - }, - { - ContainerPort: port, - Name: "http", - Protocol: corev1.ProtocolTCP, - }, - }, - EnvFrom: []corev1.EnvFromSource{{ - ConfigMapRef: &corev1.ConfigMapEnvSource{ - LocalObjectReference: corev1.LocalObjectReference{Name: m.Name + "-config"}, - }, - }}, - ReadinessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(httpPort), - Path: "/health", - }, - }, - }, - LivenessProbe: &corev1.Probe{ - PeriodSeconds: 5, - ProbeHandler: corev1.ProbeHandler{ - TCPSocket: &corev1.TCPSocketAction{ - Port: intstr.FromInt(httpPort), - }, - }, - }, - VolumeMounts: []corev1.VolumeMount{ - { - Name: "socket", - MountPath: "/var/run/wireguard/", - }, - { - Name: "config", - MountPath: "/tmp/wireguard/", - }}, - Resources: m.Spec.Agent.Resources, - }}, - }, - }, - }, - } - - if m.Spec.Tunnel.Enabled { - image := m.Spec.Tunnel.Image - if image == "" { - image = defaultWstunnelImage - } - tunnelPort := m.Spec.Tunnel.Port - if tunnelPort == 0 { - tunnelPort = defaultTunnelPort - } - dep.Spec.Template.Spec.Containers = append(dep.Spec.Template.Spec.Containers, - corev1.Container{ - SecurityContext: &corev1.SecurityContext{ - ReadOnlyRootFilesystem: &readOnlyRootFilesystem, - AllowPrivilegeEscalation: &allowPrivilegeEscalation, - }, - Image: image, - Name: "wstunnel", - Command: []string{ - "/usr/bin/dumb-init", "--", - "/home/app/wstunnel", "server", - "--restrict-to", fmt.Sprintf("127.0.0.1:%d", port), - fmt.Sprintf("wss://0.0.0.0:%d", tunnelPort), - }, - Ports: []corev1.ContainerPort{ - { - ContainerPort: tunnelPort, - Name: "tunnel", - Protocol: corev1.ProtocolTCP, - }, - }, - ReadinessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - TCPSocket: &corev1.TCPSocketAction{ - Port: intstr.FromInt(int(tunnelPort)), - }, - }, - }, - Resources: m.Spec.Tunnel.Resources, - }) - } - - if m.Spec.EnableIpForwardOnPodInit { - privileged := true - dep.Spec.Template.Spec.InitContainers = append(dep.Spec.Template.Spec.InitContainers, - corev1.Container{ - SecurityContext: &corev1.SecurityContext{ - Privileged: &privileged, - }, - Image: r.AgentImage, - ImagePullPolicy: r.AgentImagePullPolicy, - Name: "sysctl", - Command: []string{"/bin/sh"}, - Args: []string{"-c", "echo 1 > /proc/sys/net/ipv4/ip_forward"}, - }) - } - - if m.Spec.UseWgUserspaceImplementation { - for i, c := range dep.Spec.Template.Spec.Containers { - if c.Name == "agent" { - dep.Spec.Template.Spec.Containers[i].Command = append(dep.Spec.Template.Spec.Containers[i].Command, "--wg-use-userspace-implementation") - } - } - } - - _ = ctrl.SetControllerReference(m, dep, r.Scheme) - return dep -} diff --git a/internal/controller/wireguard_controller_test.go b/internal/controller/wireguard_controller_test.go index 9842775..a4766ac 100644 --- a/internal/controller/wireguard_controller_test.go +++ b/internal/controller/wireguard_controller_test.go @@ -86,6 +86,31 @@ func reconcileServiceWithClusterIP(svcKey client.ObjectKey, port int32) error { return k8sClient.Update(context.Background(), svc) } +// peerConfigLines returns the lines of a rendered peer config that start with prefix. +// It returns nil when the aggregated secret or the peer's key is missing, so an absence +// assertion must first establish that the config was rendered at all — otherwise a +// missing secret would satisfy it vacuously. +func peerConfigLines(wgName, namespace, peerKey, prefix string) []string { + secret := &corev1.Secret{} + if err := k8sClient.Get(context.Background(), types.NamespacedName{ + Name: wgName + "-peer-configs", + Namespace: namespace, + }, secret); err != nil { + return nil + } + data, ok := secret.Data[peerKey] + if !ok { + return nil + } + var matched []string + for line := range strings.SplitSeq(string(data), "\n") { + if strings.HasPrefix(line, prefix) { + matched = append(matched, line) + } + } + return matched +} + var _ = Describe("wireguard controller", func() { // Define utility constants for object names and testing timeouts/durations and intervals. @@ -1459,6 +1484,98 @@ var _ = Describe("wireguard controller", func() { } }) + Context("PersistentKeepalive", func() { + + // Brings up a Wireguard with the given spec plus one peer, reconciles its + // LoadBalancer service, and returns the peer's key in the aggregated config secret. + createWireguardWithPeer := func(spec v1alpha1.WireguardSpec) string { + wgServer := &v1alpha1.Wireguard{ + ObjectMeta: metav1.ObjectMeta{ + Name: wgKey.Name, + Namespace: wgKey.Namespace, + }, + Spec: spec, + } + Expect(k8sClient.Create(context.Background(), wgServer)).Should(Succeed()) + + peerName := wgName + "-peer1" + wgPeer := &v1alpha1.WireguardPeer{ + ObjectMeta: metav1.ObjectMeta{ + Name: peerName, + Namespace: wgNamespace, + }, + Spec: v1alpha1.WireguardPeerSpec{ + WireguardRef: wgName, + }, + } + Expect(k8sClient.Create(context.Background(), wgPeer)).Should(Succeed()) + + serviceKey := types.NamespacedName{ + Namespace: wgKey.Namespace, + Name: wgKey.Name + "-svc", + } + expectedLabels := map[string]string{"app": "wireguard", "instance": wgKey.Name} + Eventually(func() map[string]string { + svc := &corev1.Service{} + if err := k8sClient.Get(context.Background(), serviceKey, svc); err != nil { + return map[string]string{} + } + return svc.Spec.Selector + }, Timeout, Interval).Should(BeEquivalentTo(expectedLabels)) + + Expect(reconcileServiceWithTypeLoadBalancer(serviceKey, "test-address")).Should(Succeed()) + + return peerName + } + + It("defaults PersistentKeepalive to 25 when unset", func() { + peerName := createWireguardWithPeer(v1alpha1.WireguardSpec{}) + + Eventually(func() []string { + return peerConfigLines(wgName, wgNamespace, peerName, "PersistentKeepalive") + }, Timeout, Interval).Should(Equal([]string{"PersistentKeepalive = 25"})) + }) + + It("uses the configured PersistentKeepalive interval", func() { + keepalive := int32(15) + peerName := createWireguardWithPeer(v1alpha1.WireguardSpec{PersistentKeepalive: &keepalive}) + + Eventually(func() []string { + return peerConfigLines(wgName, wgNamespace, peerName, "PersistentKeepalive") + }, Timeout, Interval).Should(Equal([]string{"PersistentKeepalive = 15"})) + }) + + It("renders keepalive in the tunnel and dual-mode direct configs", func() { + peerName := createWireguardWithPeer(v1alpha1.WireguardSpec{ + Tunnel: v1alpha1.TunnelSpec{Enabled: true, DualMode: true}, + }) + + // Dual mode stores the direct config under the peer name and the + // wstunnel config under ".tunnel". Both are separate templates, + // so both need asserting. + Eventually(func() []string { + return peerConfigLines(wgName, wgNamespace, peerName, "PersistentKeepalive") + }, Timeout, Interval).Should(Equal([]string{"PersistentKeepalive = 25"})) + + Eventually(func() []string { + return peerConfigLines(wgName, wgNamespace, peerName+".tunnel", "PersistentKeepalive") + }, Timeout, Interval).Should(Equal([]string{"PersistentKeepalive = 25"})) + }) + + It("omits PersistentKeepalive entirely when set to 0", func() { + keepalive := int32(0) + peerName := createWireguardWithPeer(v1alpha1.WireguardSpec{PersistentKeepalive: &keepalive}) + + // Establish the config was actually rendered before asserting on absence, + // so a missing secret cannot satisfy the assertion vacuously. + Eventually(func() []string { + return peerConfigLines(wgName, wgNamespace, peerName, "Endpoint") + }, Timeout, Interval).Should(HaveLen(1)) + + Expect(peerConfigLines(wgName, wgNamespace, peerName, "PersistentKeepalive")).To(BeEmpty()) + }) + }) + Context("Tunnel", func() { It("should add wstunnel sidecar and TCP service when tunnel is enabled", func() { expectedAddress := "test-tunnel-address" diff --git a/internal/controller/wireguardpeer_controller.go b/internal/controller/wireguardpeer_controller.go index d56ea58..fa52145 100644 --- a/internal/controller/wireguardpeer_controller.go +++ b/internal/controller/wireguardpeer_controller.go @@ -21,11 +21,11 @@ import ( "fmt" "github.com/nccloud/wireguard-operator/api/v1alpha1" + "github.com/nccloud/wireguard-operator/internal/resources" wgtypes "golang.zx2c4.com/wireguard/wgctrl/wgtypes" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -38,6 +38,8 @@ import ( type WireguardPeerReconciler struct { client.Client Scheme *runtime.Scheme + + secretBuilder *resources.SecretBuilder } func (r *WireguardPeerReconciler) updateStatus(ctx context.Context, peer *v1alpha1.WireguardPeer, status string, message string) error { @@ -53,23 +55,6 @@ func (r *WireguardPeerReconciler) updateStatus(ctx context.Context, peer *v1alph return nil } -func (r *WireguardPeerReconciler) secretForPeer(m *v1alpha1.WireguardPeer, privateKey string, publicKey string) *corev1.Secret { - ls := labelsForWireguard(m.Name) - dep := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: m.Name + "-peer", - Namespace: m.Namespace, - Labels: ls, - }, - Data: map[string][]byte{"privateKey": []byte(privateKey), "publicKey": []byte(publicKey)}, - } - // Set Nodered instance as the owner and controller - _ = ctrl.SetControllerReference(m, dep, r.Scheme) - - return dep - -} - //+kubebuilder:rbac:groups=vpn.wireguard-operator.io,resources=wireguardpeers,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=vpn.wireguard-operator.io,resources=wireguardpeers/status,verbs=get;update;patch //+kubebuilder:rbac:groups=vpn.wireguard-operator.io,resources=wireguardpeers/finalizers,verbs=update @@ -133,7 +118,11 @@ func (r *WireguardPeerReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Secret does not exist yet — generate new keys and create it. publicKey = key.PublicKey().String() - secret := r.secretForPeer(peer, key.String(), publicKey) + secret, buildErr := r.secretBuilder.ForPeer(peer, key.String(), publicKey) + if buildErr != nil { + log.Error(buildErr, "Failed to build peer secret", "secret.Name", secretName) + return ctrl.Result{}, buildErr + } log.Info("Creating a new secret", "secret.Namespace", secret.Namespace, "secret.Name", secret.Name) if err = r.Create(ctx, secret); err != nil { log.Error(err, "Failed to create new secret", "secret.Namespace", secret.Namespace, "secret.Name", secret.Name) @@ -257,6 +246,8 @@ func (r *WireguardPeerReconciler) checkDuplicateAddress(ctx context.Context, nam // SetupWithManager sets up the controller with the Manager. func (r *WireguardPeerReconciler) SetupWithManager(mgr ctrl.Manager) error { + r.secretBuilder = resources.NewSecretBuilder(r.Scheme) + return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.WireguardPeer{}). Complete(r) diff --git a/internal/resources/deployment.go b/internal/resources/deployment.go index 2151774..6b4903b 100644 --- a/internal/resources/deployment.go +++ b/internal/resources/deployment.go @@ -164,7 +164,7 @@ func (b *DeploymentBuilder) agentContainer(wg *v1alpha1.Wireguard, readOnlyRootF Protocol: corev1.ProtocolUDP, }, { - ContainerPort: WireguardPort, + ContainerPort: HTTPPort, Name: "http", Protocol: corev1.ProtocolTCP, }, diff --git a/release.yaml b/release.yaml index 2e47c93..1a299dd 100644 --- a/release.yaml +++ b/release.yaml @@ -479,6 +479,17 @@ spec: PeerCIDRv6 is the IPv6 CIDR range from which Wireguard peer IPv6 addresses will be allocated. When set, IPv6 support is enabled for this Wireguard instance. type: string + persistentKeepalive: + default: 25 + description: |- + PersistentKeepalive is the interval in seconds at which peers send keepalive packets + to the server. This keeps NAT and conntrack entries alive so the server can reach the + peer, and makes the peer re-handshake promptly after the server pod is rescheduled. + Set to 0 to disable. Defaults to 25. + format: int32 + maximum: 65535 + minimum: 0 + type: integer port: description: A field that specifies the value to use for a nodePort ServiceType