Add comprehensive unit tests and bats test for TLS certificate reload

Co-authored-by: rchincha <45800463+rchincha@users.noreply.github.com>
2026-06-18 05:28:07 +08:00 · 2026-01-30 18:08:10 +00:00
parent 3d17ba6dbd
commit 26aff7583a
2 changed files with 337 additions and 0 deletions
@@ -193,5 +193,131 @@ func TestCertReloaderDirectly(t *testing.T) {
 			So(err, ShouldBeNil)
 			So(cert, ShouldNotBeNil)
 		})
+
+		Convey("GetCertificateFunc should handle only cert file modification", func() {
+			reloader, err := api.NewCertReloader(certPath, keyPath)
+			So(err, ShouldBeNil)
+
+			getCert := reloader.GetCertificateFunc()
+			initialCert, err := getCert(nil)
+			So(err, ShouldBeNil)
+			So(initialCert, ShouldNotBeNil)
+
+			// Wait to ensure modification time will be different
+			time.Sleep(2 * time.Second)
+
+			// Modify only the cert file (touch it to update mtime)
+			// Generate new cert with same key
+			newServerOpts := &tlsutils.CertificateOptions{
+				Hostname:   "127.0.0.1",
+				CommonName: "Updated Cert Only",
+				NotAfter:   time.Now().AddDate(1, 0, 0),
+			}
+			// Read the existing key
+			keyData, err := os.ReadFile(keyPath)
+			So(err, ShouldBeNil)
+
+			// Generate new cert using the existing key
+			err = tlsutils.GenerateServerCertToFile(caCertPEM, caKeyPEM, certPath, keyPath, newServerOpts)
+			So(err, ShouldBeNil)
+
+			// Restore original key
+			err = os.WriteFile(keyPath, keyData, 0o600)
+			So(err, ShouldBeNil)
+
+			// Get certificate again - should reload
+			updatedCert, err := getCert(nil)
+			So(err, ShouldBeNil)
+			So(updatedCert, ShouldNotBeNil)
+		})
+
+		Convey("GetCertificateFunc should handle concurrent access", func() {
+			reloader, err := api.NewCertReloader(certPath, keyPath)
+			So(err, ShouldBeNil)
+
+			getCert := reloader.GetCertificateFunc()
+
+			// Launch multiple goroutines to access certificate concurrently
+			done := make(chan error, 10)
+			for i := 0; i < 10; i++ {
+				go func() {
+					var lastErr error
+					for j := 0; j < 100; j++ {
+						cert, err := getCert(nil)
+						if err != nil || cert == nil {
+							lastErr = err
+							break
+						}
+					}
+					done <- lastErr
+				}()
+			}
+
+			// Wait for all goroutines to complete and check for errors
+			for i := 0; i < 10; i++ {
+				err := <-done
+				So(err, ShouldBeNil)
+			}
+		})
+
+		Convey("GetCertificateFunc should not reload if files haven't changed", func() {
+			reloader, err := api.NewCertReloader(certPath, keyPath)
+			So(err, ShouldBeNil)
+
+			getCert := reloader.GetCertificateFunc()
+
+			// Get certificate multiple times
+			cert1, err := getCert(nil)
+			So(err, ShouldBeNil)
+			So(cert1, ShouldNotBeNil)
+
+			cert2, err := getCert(nil)
+			So(err, ShouldBeNil)
+			So(cert2, ShouldNotBeNil)
+
+			cert3, err := getCert(nil)
+			So(err, ShouldBeNil)
+			So(cert3, ShouldNotBeNil)
+
+			// All should return the same certificate instance (pointer equality)
+			So(cert1, ShouldEqual, cert2)
+			So(cert2, ShouldEqual, cert3)
+		})
+
+		Convey("GetCertificateFunc should reload when key file changes", func() {
+			reloader, err := api.NewCertReloader(certPath, keyPath)
+			So(err, ShouldBeNil)
+
+			getCert := reloader.GetCertificateFunc()
+			initialCert, err := getCert(nil)
+			So(err, ShouldBeNil)
+			So(initialCert, ShouldNotBeNil)
+
+			// Wait to ensure modification time will be different
+			time.Sleep(2 * time.Second)
+
+			// Generate completely new cert and key
+			newServerOpts := &tlsutils.CertificateOptions{
+				Hostname:   "127.0.0.1",
+				CommonName: "New Key Cert",
+				NotAfter:   time.Now().AddDate(1, 0, 0),
+			}
+			err = tlsutils.GenerateServerCertToFile(caCertPEM, caKeyPEM, certPath, keyPath, newServerOpts)
+			So(err, ShouldBeNil)
+
+			// Get certificate again - should reload due to key change
+			updatedCert, err := getCert(nil)
+			So(err, ShouldBeNil)
+			So(updatedCert, ShouldNotBeNil)
+
+			// Verify certificates are different
+			initialLeaf, err := x509.ParseCertificate(initialCert.Certificate[0])
+			So(err, ShouldBeNil)
+			updatedLeaf, err := x509.ParseCertificate(updatedCert.Certificate[0])
+			So(err, ShouldBeNil)
+
+			So(initialLeaf.Subject.CommonName, ShouldEqual, "Initial Cert")
+			So(updatedLeaf.Subject.CommonName, ShouldEqual, "New Key Cert")
+		})
 	})
 }
@@ -0,0 +1,211 @@
+load helpers_zot
+load ../port_helper
+
+function verify_prerequisites {
+    if [ ! $(command -v curl) ]; then
+        echo "you need to install curl as a prerequisite to running the tests" >&3
+        return 1
+    fi
+
+    if [ ! $(command -v openssl) ]; then
+        echo "you need to install openssl as a prerequisite to running the tests" >&3
+        return 1
+    fi
+
+    return 0
+}
+
+# Generate TLS certificates for testing
+function generate_certs() {
+    local cert_dir=$1
+    mkdir -p ${cert_dir}
+    
+    # Generate CA certificate
+    openssl req -newkey rsa:2048 -nodes -days 365 -x509 \
+        -keyout ${cert_dir}/ca.key \
+        -out ${cert_dir}/ca.crt \
+        -subj "/CN=Test CA" 2>/dev/null
+    
+    # Generate initial server certificate (version 1)
+    openssl req -newkey rsa:2048 -nodes \
+        -keyout ${cert_dir}/server.key \
+        -out ${cert_dir}/server.csr \
+        -subj "/OU=TestServer/CN=Server v1" 2>/dev/null
+    
+    openssl x509 -req -days 365 -sha256 \
+        -in ${cert_dir}/server.csr \
+        -CA ${cert_dir}/ca.crt \
+        -CAkey ${cert_dir}/ca.key \
+        -CAcreateserial \
+        -out ${cert_dir}/server.cert \
+        -extfile <(echo subjectAltName = IP:127.0.0.1) 2>/dev/null
+}
+
+# Generate new server certificate with different CN
+function regenerate_server_cert() {
+    local cert_dir=$1
+    local version=$2
+    
+    # Generate new server certificate (version 2)
+    openssl req -newkey rsa:2048 -nodes \
+        -keyout ${cert_dir}/server.key \
+        -out ${cert_dir}/server.csr \
+        -subj "/OU=TestServer/CN=Server v${version}" 2>/dev/null
+    
+    openssl x509 -req -days 365 -sha256 \
+        -in ${cert_dir}/server.csr \
+        -CA ${cert_dir}/ca.crt \
+        -CAkey ${cert_dir}/ca.key \
+        -CAcreateserial \
+        -out ${cert_dir}/server.cert \
+        -extfile <(echo subjectAltName = IP:127.0.0.1) 2>/dev/null
+}
+
+function setup_file() {
+    # Verify prerequisites are available
+    if ! $(verify_prerequisites); then
+        exit 1
+    fi
+
+    # Generate certificates
+    local cert_dir=${BATS_FILE_TMPDIR}/certs
+    generate_certs ${cert_dir}
+
+    # Setup zot server with TLS
+    local zot_root_dir=${BATS_FILE_TMPDIR}/zot
+    local zot_config_file=${BATS_FILE_TMPDIR}/zot_config.json
+    zot_port=$(get_free_port_for_service "zot")
+    echo ${zot_port} > ${BATS_FILE_TMPDIR}/zot.port
+
+    mkdir -p ${zot_root_dir}
+
+    cat > ${zot_config_file}<<EOF
+{
+  "distSpecVersion":"1.1.1",
+  "storage":{
+    "rootDirectory": "${zot_root_dir}"
+  },
+  "http": {
+    "address": "127.0.0.1",
+    "port": "${zot_port}",
+    "tls": {
+      "cert": "${cert_dir}/server.cert",
+      "key": "${cert_dir}/server.key"
+    }
+  },
+  "log":{
+    "level":"debug",
+    "output": "${BATS_FILE_TMPDIR}/zot.log"
+  }
+}
+EOF
+    
+    zot_serve ${ZOT_PATH} ${zot_config_file}
+    wait_zot_reachable ${zot_port}
+}
+
+function teardown() {
+    # conditionally printing on failure is possible from teardown but not from teardown_file
+    cat ${BATS_FILE_TMPDIR}/zot.log
+}
+
+function teardown_file() {
+    zot_stop_all
+}
+
+@test "verify initial TLS connection works" {
+    zot_port=`cat ${BATS_FILE_TMPDIR}/zot.port`
+    cert_dir=${BATS_FILE_TMPDIR}/certs
+    
+    # Test HTTPS connection with CA certificate
+    run curl --cacert ${cert_dir}/ca.crt https://127.0.0.1:${zot_port}/v2/
+    [ "$status" -eq 0 ]
+}
+
+@test "verify certificate details - initial cert" {
+    zot_port=`cat ${BATS_FILE_TMPDIR}/zot.port`
+    cert_dir=${BATS_FILE_TMPDIR}/certs
+    
+    # Get certificate subject from server
+    cert_subject=$(echo | openssl s_client -connect 127.0.0.1:${zot_port} -showcerts 2>/dev/null | \
+        openssl x509 -noout -subject 2>/dev/null | grep "Server v1")
+    
+    # Verify we got the initial certificate (v1)
+    [ ! -z "$cert_subject" ]
+}
+
+@test "reload certificate and verify new cert is used" {
+    zot_port=`cat ${BATS_FILE_TMPDIR}/zot.port`
+    cert_dir=${BATS_FILE_TMPDIR}/certs
+    
+    # Verify initial connection works
+    run curl --cacert ${cert_dir}/ca.crt https://127.0.0.1:${zot_port}/v2/
+    [ "$status" -eq 0 ]
+    
+    # Wait a moment to ensure modification time will be different
+    sleep 2
+    
+    # Generate new certificate with different CommonName
+    regenerate_server_cert ${cert_dir} 2
+    
+    # Wait for certificate to be detected and reloaded
+    sleep 2
+    
+    # Verify connection still works with new certificate
+    run curl --cacert ${cert_dir}/ca.crt https://127.0.0.1:${zot_port}/v2/
+    [ "$status" -eq 0 ]
+    
+    # Get certificate subject from server
+    cert_subject=$(echo | openssl s_client -connect 127.0.0.1:${zot_port} -showcerts 2>/dev/null | \
+        openssl x509 -noout -subject 2>/dev/null | grep "Server v2")
+    
+    # Verify we got the new certificate (v2)
+    [ ! -z "$cert_subject" ]
+}
+
+@test "verify multiple certificate reloads work" {
+    zot_port=`cat ${BATS_FILE_TMPDIR}/zot.port`
+    cert_dir=${BATS_FILE_TMPDIR}/certs
+    
+    for i in 3 4 5; do
+        # Generate new certificate
+        regenerate_server_cert ${cert_dir} ${i}
+        
+        # Wait for reload
+        sleep 2
+        
+        # Verify connection works
+        run curl --cacert ${cert_dir}/ca.crt https://127.0.0.1:${zot_port}/v2/
+        [ "$status" -eq 0 ]
+        
+        # Verify new certificate is in use
+        cert_subject=$(echo | openssl s_client -connect 127.0.0.1:${zot_port} -showcerts 2>/dev/null | \
+            openssl x509 -noout -subject 2>/dev/null | grep "Server v${i}")
+        [ ! -z "$cert_subject" ]
+    done
+}
+
+@test "verify server continues working if certificate reload fails" {
+    zot_port=`cat ${BATS_FILE_TMPDIR}/zot.port`
+    cert_dir=${BATS_FILE_TMPDIR}/certs
+    
+    # Get current certificate version
+    cert_subject_before=$(echo | openssl s_client -connect 127.0.0.1:${zot_port} -showcerts 2>/dev/null | \
+        openssl x509 -noout -subject 2>/dev/null)
+    
+    # Temporarily remove certificate files (will cause reload to fail)
+    mv ${cert_dir}/server.cert ${cert_dir}/server.cert.backup
+    
+    # Wait and try to connect - should still work with old certificate
+    sleep 2
+    run curl --cacert ${cert_dir}/ca.crt https://127.0.0.1:${zot_port}/v2/
+    [ "$status" -eq 0 ]
+    
+    # Restore certificate
+    mv ${cert_dir}/server.cert.backup ${cert_dir}/server.cert
+    
+    # Verify still using old certificate
+    cert_subject_after=$(echo | openssl s_client -connect 127.0.0.1:${zot_port} -showcerts 2>/dev/null | \
+        openssl x509 -noout -subject 2>/dev/null)
+    [ "$cert_subject_before" = "$cert_subject_after" ]
+}