Add PersistentKeepalive, consolidate resource rendering, fix docs (#4, #1)

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.

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

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.

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

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