(PE-37634) ensure sign-all with empty list succeeds

Prior to this change, if the sign-all behavior was invoked for the
CA service, an exception would be thrown because the function returns
a result that wasn't expected. This adds a test that demonstrates the
failure, and adds a fix to get the test to pass.

Author: jonathannewman

src/clj/puppetlabs/puppetserver/certificate_authority.clj

@@ -2483,35 +2483,35 @@
            serial-lock serial-lock-timeout-seconds] :as ca-settings} :- CaSettings
    report-activity]
   (let [;; if part of a CA bundle, the intermediate CA will be first in the chain
-        cacert      (utils/pem->ca-cert cacert cakey)
+        cacert (utils/pem->ca-cert cacert cakey)
         casubject (utils/get-subject-from-x509-certificate cacert)
-        ca-private-key  (utils/pem->private-key cakey)]
-    (when-not (empty? subjects)
-      ;; since we are going to be manipulating the serial file and the inventory file for multiple entries,
-      ;; acquire the locks to prevent lock thrashing
-      (common/with-safe-write-lock serial-lock serial-lock-descriptor serial-lock-timeout-seconds
-        (common/with-safe-write-lock inventory-lock inventory-lock-descriptor inventory-lock-timeout-seconds
-          (let [results
-                ;; loop through the subjects, one at a time, and collect the results for success or failure.
-                (loop [s subjects
-                       result {:signed []
-                               :no-csr []
-                               :signing-errors[]}]
-                  (if-not (empty? s)
-                    (let [subject (first s)
-                          csr-path (path-to-cert-request csrdir subject)]
-                      (if (fs/exists? csr-path)
-                        (let [_ (log/trace (i18n/trs "File exists at {0}" csr-path))
-                              one-result (maybe-sign-one subject csr-path cacert casubject ca-private-key ca-settings)]
-                          ;; one-result is either :signed or :signing-errors
-                          (recur (rest s)
-                                 (update result one-result conj subject)))
-                        (do
-                          (log/trace (i18n/trs "File does not exist at {0}" csr-path))
-                          (recur (rest s)
-                                 (update result :no-csr conj subject)))))
-                    result))]
-            ;; submit the signing activity as one entry for all the hosts.
-            (when-not (empty? (:signed results))
-              (report-activity (:signed results) "signed"))
-            results))))))
+        ca-private-key (utils/pem->private-key cakey)]
+
+    ;; since we are going to be manipulating the serial file and the inventory file for multiple entries,
+    ;; acquire the locks to prevent lock thrashing
+    (common/with-safe-write-lock serial-lock serial-lock-descriptor serial-lock-timeout-seconds
+      (common/with-safe-write-lock inventory-lock inventory-lock-descriptor inventory-lock-timeout-seconds
+        (let [results
+              ;; loop through the subjects, one at a time, and collect the results for success or failure.
+              (loop [s subjects
+                     result {:signed         []
+                             :no-csr         []
+                             :signing-errors []}]
+                (if-not (empty? s)
+                  (let [subject (first s)
+                        csr-path (path-to-cert-request csrdir subject)]
+                    (if (fs/exists? csr-path)
+                      (let [_ (log/trace (i18n/trs "File exists at {0}" csr-path))
+                            one-result (maybe-sign-one subject csr-path cacert casubject ca-private-key ca-settings)]
+                        ;; one-result is either :signed or :signing-errors
+                        (recur (rest s)
+                               (update result one-result conj subject)))
+                      (do
+                        (log/trace (i18n/trs "File does not exist at {0}" csr-path))
+                        (recur (rest s)
+                               (update result :no-csr conj subject)))))
+                  result))]
+          ;; submit the signing activity as one entry for all the hosts.
+          (when-not (empty? (:signed results)))
+            (report-activity (:signed results) "signed")
+          results)))))

test/integration/puppetlabs/services/certificate_authority/certificate_authority_int_test.clj

@@ -1227,6 +1227,19 @@
         :ssl-key (str bootstrap/server-conf-dir "/ssl/private_keys/localhost.pem")
         :ssl-ca-cert (str bootstrap/server-conf-dir "/ca/ca_crt.pem")
         :ssl-crl-path (str bootstrap/server-conf-dir "/ssl/crl.pem")}}
+      (testing "PE-37634 PE-sign-all with no pending certs returns 200 with expected payload"
+        (let [response (http-client/post
+                         "https://localhost:8140/puppet-ca/v1/sign/all"
+                         {:ssl-cert (str bootstrap/server-conf-dir "/ca/ca_crt.pem")
+                          :ssl-key (str bootstrap/server-conf-dir "/ca/ca_key.pem")
+                          :ssl-ca-cert (str bootstrap/server-conf-dir "/ca/ca_crt.pem")
+                          :as :text
+                          :headers {"Accept" "application/json"}})]
+          (is (= 200 (:status response)))
+          (is (= {:signed []
+                  :no-csr []
+                  :signing-errors []}
+                 (json/parse-string (:body response) true)))))
       (testing "returns 200 with valid payload"
         ;; note- more extensive testing of the behavior is done with the testing in sign-multiple-certificate-signing-requests!-test
         (let [certname (ks/rand-str :alpha-lower 16)