added opt-in managed nginx/TLS container (ADR 0005, plan step 13): server.nginx.* config serves GitTally over HTTPS on hosts without a reverse proxy — two-phase startup (ACME webroot via certbot container, then full HTTPS config), daily certificate renewal with nginx reload, labelled container removed on shutdown; all failures are non-fatal, the plain HTTP server keeps running
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Michael Hoennig
co-authored by
Claude Fable 5
Claude Fable 5
parent
fdbbd0516a
commit
b104eeee05
@@ -120,6 +120,36 @@ class ConfigLoaderTest : FunSpec() {
|
||||
config.branches["release"]!!.buildCommand shouldBe "./mvnw -P release test"
|
||||
}
|
||||
|
||||
test("empty publicBaseUrl defaults to https://<nginx.serverName>/ when set") {
|
||||
val dir = Files.createTempDirectory("gittally-test")
|
||||
dir.resolve(".gittally.yml").toFile().writeText(
|
||||
"""
|
||||
server:
|
||||
nginx:
|
||||
serverName: ci.example.org
|
||||
""".trimIndent(),
|
||||
)
|
||||
loader.load(dir).server.publicBaseUrl shouldBe "https://ci.example.org/"
|
||||
}
|
||||
|
||||
test("explicit publicBaseUrl wins over the nginx.serverName default") {
|
||||
val dir = Files.createTempDirectory("gittally-test")
|
||||
dir.resolve(".gittally.yml").toFile().writeText(
|
||||
"""
|
||||
server:
|
||||
publicBaseUrl: https://other.example.org/
|
||||
nginx:
|
||||
serverName: ci.example.org
|
||||
""".trimIndent(),
|
||||
)
|
||||
loader.load(dir).server.publicBaseUrl shouldBe "https://other.example.org/"
|
||||
}
|
||||
|
||||
test("publicBaseUrl stays empty without an nginx.serverName") {
|
||||
val dir = Files.createTempDirectory("gittally-test")
|
||||
loader.load(dir).server.publicBaseUrl shouldBe ""
|
||||
}
|
||||
|
||||
test("toYaml serializes GitTallyConfig with all sections") {
|
||||
val yaml = loader.toYaml(GitTallyConfig())
|
||||
yaml shouldContain "server:"
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
package de.hoennig.gittally.server
|
||||
|
||||
import io.kotest.core.spec.style.FunSpec
|
||||
import io.kotest.matchers.string.shouldContain
|
||||
import io.kotest.matchers.string.shouldNotContain
|
||||
|
||||
class NginxConfigFilesTest : FunSpec() {
|
||||
init {
|
||||
test("init config serves the ACME challenge and redirects to HTTPS, without a TLS server") {
|
||||
val conf = NginxConfigFiles.nginxConf("ci.example.org", "ci.example.org", 18080, full = false)
|
||||
|
||||
conf shouldContain "listen 80;"
|
||||
conf shouldContain "server_name ci.example.org;"
|
||||
conf shouldContain "location /.well-known/acme-challenge/"
|
||||
conf shouldContain "root /var/www/certbot;"
|
||||
conf shouldContain "return 301 https://\$host\$request_uri;"
|
||||
conf shouldNotContain "listen 443"
|
||||
conf shouldNotContain "proxy_pass"
|
||||
conf shouldNotContain "ssl_certificate"
|
||||
}
|
||||
|
||||
test("full config adds the TLS server with certificate paths and the proxy") {
|
||||
val conf = NginxConfigFiles.nginxConf("ci.example.org", "upstream.example.org", 18081, full = true)
|
||||
|
||||
conf shouldContain "listen 80;"
|
||||
conf shouldContain "listen 443 ssl;"
|
||||
conf shouldContain "ssl_certificate /etc/letsencrypt/live/ci.example.org/fullchain.pem;"
|
||||
conf shouldContain "ssl_certificate_key /etc/letsencrypt/live/ci.example.org/privkey.pem;"
|
||||
conf shouldContain "include /etc/letsencrypt/options-ssl-nginx.conf;"
|
||||
conf shouldContain "proxy_pass http://upstream.example.org:18081;"
|
||||
conf shouldContain "proxy_set_header Host \$host;"
|
||||
conf shouldContain "proxy_set_header X-Forwarded-Proto \$scheme;"
|
||||
conf shouldContain "add_header Cache-Control \"no-store, max-age=0\" always;"
|
||||
}
|
||||
|
||||
test("ssl options reference the mounted dhparams and modern protocols") {
|
||||
NginxConfigFiles.SSL_OPTIONS shouldContain "ssl_protocols TLSv1.2 TLSv1.3;"
|
||||
NginxConfigFiles.SSL_OPTIONS shouldContain "ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem;"
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,312 @@
|
||||
package de.hoennig.gittally.server
|
||||
|
||||
import de.hoennig.gittally.build.ArtifactKeys
|
||||
import de.hoennig.gittally.config.ConfigLoader
|
||||
import de.hoennig.gittally.config.GitTallyConfig
|
||||
import de.hoennig.gittally.config.NginxConfig
|
||||
import de.hoennig.gittally.config.ServerConfig
|
||||
import de.hoennig.gittally.git.GitCommandResult
|
||||
import de.hoennig.gittally.git.GitCommandRunner
|
||||
import io.kotest.core.spec.style.FunSpec
|
||||
import io.kotest.matchers.booleans.shouldBeTrue
|
||||
import io.kotest.matchers.collections.shouldBeEmpty
|
||||
import io.kotest.matchers.collections.shouldContain
|
||||
import io.kotest.matchers.nulls.shouldBeNull
|
||||
import io.kotest.matchers.nulls.shouldNotBeNull
|
||||
import io.kotest.matchers.shouldBe
|
||||
import io.kotest.matchers.string.shouldContain
|
||||
import io.kotest.matchers.string.shouldEndWith
|
||||
import io.kotest.matchers.string.shouldNotContain
|
||||
import io.mockk.clearMocks
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import java.nio.file.Files
|
||||
import java.nio.file.Path
|
||||
|
||||
class NginxProxyManagerTest : FunSpec() {
|
||||
private val commandRunner = mockk<GitCommandRunner>()
|
||||
private val configLoader = mockk<ConfigLoader>()
|
||||
private lateinit var manager: NginxProxyManager
|
||||
private lateinit var repoDir: Path
|
||||
private lateinit var stateDir: Path
|
||||
private val captured = mutableListOf<List<String>>()
|
||||
private val configsAtContainerRun = mutableListOf<String>()
|
||||
private var sleepCount = 0
|
||||
|
||||
private fun nginxConfig(
|
||||
enabled: Boolean = true,
|
||||
serverName: String = "ci.example.org",
|
||||
letsencryptEmail: String = "",
|
||||
containerName: String = "test-nginx",
|
||||
httpPort: Int = 8080,
|
||||
httpsPort: Int = 8443,
|
||||
explicitStateDir: Boolean = true,
|
||||
serverPort: Int = 18080,
|
||||
): GitTallyConfig =
|
||||
GitTallyConfig(
|
||||
server =
|
||||
ServerConfig(
|
||||
port = serverPort,
|
||||
nginx =
|
||||
NginxConfig(
|
||||
enabled = enabled,
|
||||
serverName = serverName,
|
||||
httpPort = httpPort,
|
||||
httpsPort = httpsPort,
|
||||
containerName = containerName,
|
||||
stateDir = if (explicitStateDir) stateDir.toString() else "",
|
||||
letsencryptEmail = letsencryptEmail,
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
private fun expectedRunArgs(): List<String> =
|
||||
listOf(
|
||||
"docker",
|
||||
"run",
|
||||
"-d",
|
||||
"--name",
|
||||
"test-nginx",
|
||||
"--publish",
|
||||
"8080:80",
|
||||
"--publish",
|
||||
"8443:443",
|
||||
"--network",
|
||||
"bridge",
|
||||
"--volume",
|
||||
"$stateDir/certbot/conf:/etc/letsencrypt",
|
||||
"--volume",
|
||||
"$stateDir/certbot/www:/var/www/certbot",
|
||||
"--volume",
|
||||
"$stateDir/nginx/log:/var/log/nginx",
|
||||
"--volume",
|
||||
"$stateDir/nginx/nginx.conf:/etc/nginx/nginx.conf:ro",
|
||||
"--label",
|
||||
"org.hoennig.gittally=true",
|
||||
"--label",
|
||||
"org.hoennig.gittally.repository=${ArtifactKeys.repoKey(repoDir)}",
|
||||
"--label",
|
||||
"org.hoennig.gittally.role=nginx",
|
||||
"nginx",
|
||||
)
|
||||
|
||||
private fun expectedCertbotPrefix(): List<String> =
|
||||
listOf(
|
||||
"docker",
|
||||
"run",
|
||||
"--rm",
|
||||
"--volume",
|
||||
"$stateDir/certbot/conf:/etc/letsencrypt",
|
||||
"--volume",
|
||||
"$stateDir/certbot/www:/var/www/certbot",
|
||||
"--volume",
|
||||
"$stateDir/certbot/log:/var/log/letsencrypt",
|
||||
"certbot/certbot",
|
||||
)
|
||||
|
||||
private fun preCreateCertificate() {
|
||||
val certFile = stateDir.resolve("certbot/conf/live/ci.example.org/fullchain.pem")
|
||||
Files.createDirectories(certFile.parent)
|
||||
Files.writeString(certFile, "certificate")
|
||||
}
|
||||
|
||||
init {
|
||||
beforeEach {
|
||||
clearMocks(commandRunner, configLoader)
|
||||
captured.clear()
|
||||
configsAtContainerRun.clear()
|
||||
sleepCount = 0
|
||||
repoDir = Files.createTempDirectory("gittally-nginx-repo")
|
||||
stateDir = Files.createTempDirectory("gittally-nginx-state")
|
||||
every { commandRunner.run(capture(captured), any(), any()) } answers {
|
||||
if (captured.last().take(3) == listOf("docker", "run", "-d")) {
|
||||
configsAtContainerRun += Files.readString(stateDir.resolve("nginx/nginx.conf"))
|
||||
}
|
||||
GitCommandResult(0, "", "")
|
||||
}
|
||||
manager = NginxProxyManager(commandRunner, configLoader)
|
||||
manager.workingDir = repoDir
|
||||
manager.dhParamsDownloader = { Files.writeString(it, "dh-params") }
|
||||
manager.sleeper = { sleepCount++ }
|
||||
}
|
||||
|
||||
test("start makes no docker calls when disabled") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig(enabled = false)
|
||||
|
||||
manager.start()
|
||||
|
||||
captured.shouldBeEmpty()
|
||||
}
|
||||
|
||||
test("resolves the legacy defaults for upstream host, container name, and state dir") {
|
||||
every { configLoader.load(repoDir) } returns
|
||||
nginxConfig(containerName = "", explicitStateDir = false)
|
||||
|
||||
val settings = manager.resolveSettings().shouldNotBeNull()
|
||||
|
||||
settings.upstreamHost shouldBe "ci.example.org"
|
||||
settings.upstreamPort shouldBe 18080
|
||||
settings.containerName shouldBe "gittally-nginx-${repoDir.fileName}"
|
||||
settings.stateDir.toString() shouldEndWith "gittally/nginx/${ArtifactKeys.repoKey(repoDir)}"
|
||||
}
|
||||
|
||||
test("rejects a server name that could inject nginx directives") {
|
||||
every { configLoader.load(repoDir) } returns
|
||||
nginxConfig(serverName = "evil.example.org;\n} inject {")
|
||||
|
||||
manager.resolveSettings().shouldBeNull()
|
||||
manager.start()
|
||||
captured.shouldBeEmpty()
|
||||
}
|
||||
|
||||
test("rejects a missing server name") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig(serverName = "")
|
||||
|
||||
manager.resolveSettings().shouldBeNull()
|
||||
}
|
||||
|
||||
test("rejects a server.port collision with the nginx ports") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig(serverPort = 8080)
|
||||
|
||||
manager.resolveSettings().shouldBeNull()
|
||||
}
|
||||
|
||||
test("first start runs the init config, obtains a certificate, and restarts with the full config") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig()
|
||||
|
||||
manager.start()
|
||||
|
||||
// phase 1 runs on the HTTP-only init config, phase 2 on the full HTTPS config
|
||||
configsAtContainerRun.size shouldBe 2
|
||||
configsAtContainerRun[0] shouldNotContain "listen 443"
|
||||
configsAtContainerRun[1] shouldContain "listen 443 ssl;"
|
||||
captured shouldContain expectedRunArgs()
|
||||
captured shouldContain expectedCertbotPrefix() +
|
||||
listOf(
|
||||
"certonly",
|
||||
"--webroot",
|
||||
"--webroot-path",
|
||||
"/var/www/certbot",
|
||||
"--cert-name",
|
||||
"ci.example.org",
|
||||
"-d",
|
||||
"ci.example.org",
|
||||
"--rsa-key-size",
|
||||
"4096",
|
||||
"--non-interactive",
|
||||
"--agree-tos",
|
||||
"--register-unsafely-without-email",
|
||||
)
|
||||
Files.readString(stateDir.resolve("certbot/conf/options-ssl-nginx.conf")) shouldBe NginxConfigFiles.SSL_OPTIONS
|
||||
Files.readString(stateDir.resolve("certbot/conf/ssl-dhparams.pem")) shouldBe "dh-params"
|
||||
}
|
||||
|
||||
test("start with an existing certificate uses the full config immediately and renews") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig()
|
||||
preCreateCertificate()
|
||||
|
||||
manager.start()
|
||||
|
||||
configsAtContainerRun.size shouldBe 2
|
||||
configsAtContainerRun[0] shouldContain "listen 443 ssl;"
|
||||
captured shouldContain expectedCertbotPrefix() + listOf("renew", "-q")
|
||||
}
|
||||
|
||||
test("a configured letsencryptEmail registers with --email instead of unsafely") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig(letsencryptEmail = "admin@example.org")
|
||||
|
||||
val settings = manager.resolveSettings().shouldNotBeNull()
|
||||
val args = manager.obtainCertificateArgs(settings)
|
||||
|
||||
args shouldContain "--email"
|
||||
args shouldContain "admin@example.org"
|
||||
(args.contains("--register-unsafely-without-email")) shouldBe false
|
||||
}
|
||||
|
||||
test("stale nginx containers of this repository are removed by label before the start") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig()
|
||||
|
||||
manager.start()
|
||||
|
||||
captured shouldContain
|
||||
listOf(
|
||||
"docker",
|
||||
"ps",
|
||||
"-aq",
|
||||
"--filter",
|
||||
"label=org.hoennig.gittally=true",
|
||||
"--filter",
|
||||
"label=org.hoennig.gittally.repository=${ArtifactKeys.repoKey(repoDir)}",
|
||||
"--filter",
|
||||
"label=org.hoennig.gittally.role=nginx",
|
||||
)
|
||||
captured shouldContain listOf("docker", "rm", "-f", "test-nginx")
|
||||
}
|
||||
|
||||
test("does not start while a foreign container occupies an nginx port") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig()
|
||||
every {
|
||||
commandRunner.run(match { it.take(2) == listOf("docker", "ps") && it.contains("--format") }, any(), any())
|
||||
} returns GitCommandResult(0, "abc123\tother-app\t0.0.0.0:8080->80/tcp\tvendor=other", "")
|
||||
|
||||
manager.start()
|
||||
|
||||
captured.none { it.take(3) == listOf("docker", "run", "-d") }.shouldBeTrue()
|
||||
sleepCount shouldBe 4
|
||||
}
|
||||
|
||||
test("removes a stale gittally-named container occupying an nginx port") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig()
|
||||
every {
|
||||
commandRunner.run(match { it.take(2) == listOf("docker", "ps") && it.contains("--format") }, any(), any())
|
||||
} returnsMany
|
||||
listOf(
|
||||
GitCommandResult(0, "abc123\tgittally-nginx-old\t0.0.0.0:8080->80/tcp\t", ""),
|
||||
GitCommandResult(0, "", ""),
|
||||
)
|
||||
|
||||
manager.start()
|
||||
|
||||
captured shouldContain listOf("docker", "rm", "-f", "abc123")
|
||||
captured shouldContain expectedRunArgs()
|
||||
}
|
||||
|
||||
test("renewCertificateAndReload renews the certificate and reloads nginx") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig()
|
||||
manager.start()
|
||||
captured.clear()
|
||||
|
||||
manager.renewCertificateAndReload()
|
||||
|
||||
captured shouldBe
|
||||
listOf(
|
||||
expectedCertbotPrefix() + listOf("renew", "-q"),
|
||||
listOf("docker", "exec", "test-nginx", "nginx", "-s", "reload"),
|
||||
)
|
||||
}
|
||||
|
||||
test("renewCertificateAndReload is a no-op while no container is managed") {
|
||||
manager.renewCertificateAndReload()
|
||||
|
||||
captured.shouldBeEmpty()
|
||||
}
|
||||
|
||||
test("stop removes the managed container exactly once") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig()
|
||||
manager.start()
|
||||
captured.clear()
|
||||
|
||||
manager.stop()
|
||||
manager.stop()
|
||||
|
||||
captured shouldBe listOf(listOf("docker", "rm", "-f", "test-nginx"))
|
||||
}
|
||||
|
||||
test("a docker failure never throws — the HTTP server keeps running") {
|
||||
every { configLoader.load(repoDir) } returns nginxConfig()
|
||||
every { commandRunner.run(any(), any(), any()) } throws RuntimeException("docker: command not found")
|
||||
|
||||
manager.start()
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,69 @@
|
||||
package de.hoennig.gittally.server
|
||||
|
||||
import io.kotest.core.spec.style.FunSpec
|
||||
import io.kotest.matchers.shouldBe
|
||||
import io.mockk.clearMocks
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import io.mockk.slot
|
||||
import io.mockk.verify
|
||||
import java.util.concurrent.ScheduledExecutorService
|
||||
import java.util.concurrent.TimeUnit
|
||||
|
||||
class ServerNginxLifecycleTest : FunSpec() {
|
||||
private val manager = mockk<NginxProxyManager>(relaxUnitFun = true)
|
||||
private val scheduler = mockk<ScheduledExecutorService>(relaxed = true)
|
||||
private lateinit var lifecycle: ServerNginxLifecycle
|
||||
private var schedulerCreated = 0
|
||||
|
||||
init {
|
||||
beforeEach {
|
||||
clearMocks(manager, scheduler)
|
||||
schedulerCreated = 0
|
||||
lifecycle = ServerNginxLifecycle(manager)
|
||||
lifecycle.schedulerFactory = {
|
||||
schedulerCreated++
|
||||
scheduler
|
||||
}
|
||||
}
|
||||
|
||||
test("nothing is scheduled and no container is touched when disabled") {
|
||||
every { manager.isEnabled() } returns false
|
||||
|
||||
lifecycle.onApplicationReady()
|
||||
|
||||
schedulerCreated shouldBe 0
|
||||
verify(exactly = 0) { manager.start() }
|
||||
}
|
||||
|
||||
test("starts nginx on the scheduler thread and schedules the daily renewal check") {
|
||||
every { manager.isEnabled() } returns true
|
||||
every { scheduler.execute(any()) } answers { firstArg<Runnable>().run() }
|
||||
val renewalTask = slot<Runnable>()
|
||||
every { scheduler.scheduleWithFixedDelay(capture(renewalTask), 24L, 24L, TimeUnit.HOURS) } returns mockk()
|
||||
|
||||
lifecycle.onApplicationReady()
|
||||
|
||||
verify(exactly = 1) { manager.start() }
|
||||
renewalTask.captured.run()
|
||||
verify(exactly = 1) { manager.renewCertificateAndReload() }
|
||||
}
|
||||
|
||||
test("shutdown stops the scheduler and removes the container") {
|
||||
every { manager.isEnabled() } returns true
|
||||
lifecycle.onApplicationReady()
|
||||
|
||||
lifecycle.onShutdown()
|
||||
|
||||
verify(exactly = 1) { scheduler.shutdownNow() }
|
||||
verify(exactly = 1) { manager.stop() }
|
||||
}
|
||||
|
||||
test("shutdown without a prior start still asks the manager to stop") {
|
||||
lifecycle.onShutdown()
|
||||
|
||||
verify(exactly = 1) { manager.stop() }
|
||||
verify(exactly = 0) { scheduler.shutdownNow() }
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user