refactor: review fixes, CI overhaul#7
Merged
Merged
Conversation
…stages Embed upstream GPG signing key in repo to eliminate --no-check-certificate. Add sha256 verification for ocserv-exporter binaries per architecture. Deduplicate apt install across s6-builder and exporter-builder via shared downloader stage. Remove apt-get upgrade from builder stages (only keep in final). Remove no-op apt purge from final stage. Fix hadolint warnings (wget -q, SHELL pipefail, quoting). Add .hadolint.yaml for DL3008/DL3003.
Add VPN_NETWORK env var to scope NAT MASQUERADE to the VPN CIDR subnet. Falls back to broad rule with a warning when unset (backwards compatible). Add teardown script called via oneshot down to reverse iptables rules on container stop. Mount config volume read-only in docker-compose.yml.
Add docker/setup-qemu-action for cross-platform builds. Add platforms: linux/amd64,linux/arm64 to build-push-action. Bump cosign-installer to v4.0.0, cosign binary to v3.0.4, setup-buildx-action to v3.12.0, login-action to v3.7.0, metadata-action to v5.10.0, build-push-action to v6.19.2.
…ant dep Convert ocserv/run from execlineb to bash for consistency with other services. Remove unnecessary exporter→iptables dependency. Add oneshot ocserv-exporter-gate service that checks EXPORTER_ENABLED and prevents ocserv-exporter from starting when disabled (replaces sleep infinity). Simplify ocserv-exporter/run to just exec the binary directly.
Remove noisy echo lines, only log on error. Parse both IP and port from EXPORTER_BIND instead of hardcoding 127.0.0.1. Silent on success.
Add build, push, lint, clean, help targets to Makefile. Add .pre-commit-config.yaml with trailing-whitespace, end-of-file-fixer, check-yaml, check-added-large-files, hadolint, and shellcheck hooks. Document VPN_NETWORK env var and pre-commit setup in README. Update docker run examples with read-only config volume mount.
VPN_NETWORK is now mandatory. The container refuses to start if it is not set, instead of silently falling back to a broad MASQUERADE rule that exposes the entire host network to NAT.
Check the full fingerprint in VALIDSIG output instead of just matching the status tag, preventing acceptance of signatures from any key in the keyring.
Create a dedicated system user so ocserv can drop privileges via run-as-user/run-as-group in ocserv.conf.
Prevent accidental commit of real secrets. Users copy .env.example to .env and fill in their values. Also ignore config/, *.pem, and *.key.
Track consecutive crashes and halt the container after 5 failures. A clean exit (code 0) resets the counter.
Ensure every RUN instruction fails immediately on error instead of silently continuing.
Allow overriding the image tag via IMAGE_TAG env variable, defaulting to latest.
Use find to discover all shell scripts so new services and finish scripts are automatically picked up.
Scan the published image for CRITICAL and HIGH vulnerabilities and upload SARIF results to the GitHub Security tab.
Attach a Software Bill of Materials to the built image via BuildKit.
- Add run-as-user/run-as-group to required ocserv.conf settings - Mark VPN_NETWORK as required in the environment table - Update .env references to .env.example with copy instructions - Replace hardcoded image tags with :latest
s6 oneshot up files must not be executable. Remove spurious trailing newlines from dependency marker files.
Address hadolint warnings inline instead of globally suppressing them.
- Bump pre-commit-hooks to v6.0.0, shellcheck-py to v0.11.0.1 - Add yamllint, conventional-pre-commit, and extra safety hooks
Logging is handled by the container runtime by default.
- Pin debian:bookworm-slim by digest for reproducible builds - Bump s6-overlay to 3.2.2.0 - Add SHELL pipefail to s6-builder stage - Use --status-file for GPG signature verification - Add libssl-dev and --enable-oidc-auth for OIDC support - Remove libhttp-parser-dev (unused) - Trim transitive runtime dependencies (apt resolves them) - Add non-root ocserv user comment
- lint target now runs pre-commit instead of manual hadolint/shellcheck - Remove oci-image alias, build uses --load for local daemon - Add build-multiarch target for linux/amd64,linux/arm64 - TAG defaults to git describe output
- Update ocserv-exporter checksums for v0.2.2 - Inherit ocserv-builder from downloader (reuse wget/ca-certificates) - Add /opt/ocserv/bin, /opt/ocserv/sbin, /opt/ocserv-exporter to PATH - Remove curl from runtime deps (no longer needed by healthcheck)
Healthcheck should only verify ocserv status. Exporter failure should not trigger container restart and kill VPN sessions.
Gate oneshot with exit 1 causes s6-rc to report partial failure. Move EXPORTER_ENABLED check directly into ocserv-exporter/run and use s6-svc -Od to cleanly stay down when disabled.
- Add FORWARD ACCEPT for VPN subnet (fixes DROP policy on host) - Add RELATED,ESTABLISHED rule for return traffic - Scope TCPMSS clamp to VPN subnet only (both -s and -d for hairpin) - Warn if net.ipv4.ip_forward is disabled - Teardown cleans up all new rules
Version pinning (DL3008) not practical with digest-pinned base image. cd /tmp (DL3003) is intentional inside tmpfs-mounted RUN layers.
- Add libcurl3-gnutls, libcjose0, libjansson4 for OIDC auth support - Switch HEALTHCHECK to exec-form to avoid unnecessary sh -c wrapper
Dockerfile, scripts, and configuration are independent works (aggregation), not derivative works of ocserv (GPLv2).
Self-contained test suite using Docker Compose bridge network: - Generates self-signed cert and test user automatically - Verifies TLS, VPN tunnel, IP assignment, gateway ping, exporter metrics - 6 assertions, cleanup via trap on exit
Moves apt-get install from runtime to Dockerfile.test-runner, eliminating ~30s dependency installation on each test run.
Mounts ./config as read-write for runtime files (e.g. occtl socket) while keeping ocserv.conf read-only to prevent accidental modification.
Crash counter added complexity without value — healthcheck already detects ocserv failures and Docker reports unhealthy status.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docker-publish.ymlwithci.yml(lint → build → test → publish), added scheduled vulnerability scan and Dependabot