fix(config): treat allow_unverified_revocation_url as live in requires_reboot (#307)

The signer reads pki.allow_unverified_revocation_url from the stored
config on every issuance, and nothing reads it at boot, so a change to
it alone takes effect immediately. NeedsReboot still reported
requires_reboot=true for it. Class it as hot next to pki.profiles and
pki.root_leaf_issuance.

Add tests that drive NeedsReboot through the real apply path: the
running config read back from the store, and the incoming config after
a protobuf wire round trip. They cover an identical re-apply and a
profiles-only change (no reboot) and a network change (reboot).

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
Co-authored-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>

Author: Bugs5382

internal/config/reboot.go

@@ -28,13 +28,14 @@ import (
 // requires a reboot for the change to take effect.
 //
 // The CA signer reads the live config on every operation, so a change limited
-// to the cert profiles (pki.profiles) or the irreversible root-leaf-issuance
-// acknowledgement (pki.root_leaf_issuance) takes effect immediately for
-// signing — no reboot. Every other field (network, install disk, role, state
-// key, the revocation endpoint that gates the boot-time CRL/OCSP listener,
-// hostname, management link) is consumed at boot, so a change there still needs
-// a reboot. This is deliberately conservative: only the two proven-hot fields
-// are treated as hot; anything else falls through to "reboot".
+// to the cert profiles (pki.profiles), the irreversible root-leaf-issuance
+// acknowledgement (pki.root_leaf_issuance) or the revocation preflight override
+// (pki.allow_unverified_revocation_url) takes effect immediately for signing —
+// no reboot. Every other field (network, install disk, role, state key, the
+// revocation endpoint that gates the boot-time CRL/OCSP listener, hostname,
+// management link) is consumed at boot, so a change there still needs a
+// reboot. This is deliberately conservative: only the proven-hot fields are
+// treated as hot; anything else falls through to "reboot".
 func NeedsReboot(oldCfg, newCfg *Config) bool {
 	if oldCfg == nil || newCfg == nil {
 		return true
@@ -61,11 +62,12 @@ func Equivalent(a, b any) bool {
 }
 
 // hotNormalized returns a copy of c with the hot-reconfigurable fields blanked,
-// so DeepEqual on two normalized configs is true exactly when the only
-// differences are hot fields.
+// so two normalized configs are Equivalent exactly when the only differences
+// are hot fields.
 func hotNormalized(c *Config) Config {
 	n := *c
 	n.PKI.Profiles = nil
 	n.PKI.RootLeafIssuance = ""
+	n.PKI.AllowUnverifiedRevocationURL = false
 	return n
 }

internal/config/reboot_test.go

@@ -18,7 +18,12 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import "testing"
+import (
+	"testing"
+
+	cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1"
+	"google.golang.org/protobuf/proto"
+)
 
 func baseConfig() *Config {
 	return &Config{
@@ -112,3 +117,78 @@ func TestNeedsRebootIgnoresNilVersusEmpty(t *testing.T) {
 		t.Error("a real protocol change was classified as live")
 	}
 }
+
+// roundTrip returns the two views of a config that ApplyConfig compares: the
+// running config as the node reads it back from its store, and the incoming
+// config, after mutate, as it arrives over the wire. The wire matters: an
+// empty list is written to the store as [] and reads back as an empty slice,
+// while the same list crossing gRPC decodes as nil.
+func roundTrip(t *testing.T, mutate func(*Config)) (*Config, *Config) {
+	t.Helper()
+	stored, err := Parse(validYAML(t))
+	if err != nil {
+		t.Fatalf("Parse: %v", err)
+	}
+	raw, err := stored.Marshal()
+	if err != nil {
+		t.Fatalf("Marshal: %v", err)
+	}
+	old, err := Parse(raw)
+	if err != nil {
+		t.Fatalf("Parse stored: %v", err)
+	}
+	incoming, err := Parse(raw)
+	if err != nil {
+		t.Fatalf("Parse incoming: %v", err)
+	}
+	mutate(incoming)
+	wire, err := proto.Marshal(incoming.ToProto())
+	if err != nil {
+		t.Fatalf("proto.Marshal: %v", err)
+	}
+	var pb cryptosv1.MachineConfig
+	if err := proto.Unmarshal(wire, &pb); err != nil {
+		t.Fatalf("proto.Unmarshal: %v", err)
+	}
+	next, err := FromProto(&pb)
+	if err != nil {
+		t.Fatalf("FromProto: %v", err)
+	}
+	return old, next
+}
+
+func TestNeedsRebootThroughTheStoreAndTheWire(t *testing.T) {
+	t.Run("identical re-apply through the store and the API needs no reboot", func(t *testing.T) {
+		old, next := roundTrip(t, func(*Config) {})
+		if NeedsReboot(old, next) {
+			t.Error("re-applying the running config must not require a reboot")
+		}
+	})
+
+	t.Run("profile-only change through the store and the API needs no reboot", func(t *testing.T) {
+		old, next := roundTrip(t, func(c *Config) { c.PKI.Profiles = sampleProfiles() })
+		if NeedsReboot(old, next) {
+			t.Error("a profiles-only change must not require a reboot")
+		}
+	})
+
+	t.Run("network change through the store and the API requires a reboot", func(t *testing.T) {
+		old, next := roundTrip(t, func(c *Config) { c.Network.Nameservers = []string{"192.0.2.53"} })
+		if !NeedsReboot(old, next) {
+			t.Error("adding a nameserver must require a reboot")
+		}
+	})
+}
+
+func TestNeedsRebootLiveFields(t *testing.T) {
+	t.Run("allow_unverified_revocation_url is read live by the signer (no reboot)", func(t *testing.T) {
+		old, next := roundTrip(t, func(c *Config) {
+			c.PKI.RevocationBaseURL = "http://192.0.2.10"
+			c.PKI.AllowUnverifiedRevocationURL = true
+		})
+		old.PKI.RevocationBaseURL = "http://192.0.2.10"
+		if NeedsReboot(old, next) {
+			t.Error("an allow_unverified_revocation_url-only change must not require a reboot")
+		}
+	})
+}

internal/init/run.go

@@ -472,7 +472,8 @@ func boot(ctx context.Context, shutdown *shutdownRequests) (err error) {
 	}
 	// The signer reads the LIVE on-disk config, not the boot snapshot, so an
 	// ApplyConfig change on a running node (a new/updated cert profile, the
-	// root-leaf-issuance acknowledgement) takes effect for signing immediately
+	// root-leaf-issuance acknowledgement, the revocation preflight override)
+	// takes effect for signing immediately
 	// without a reboot. Fall back to the boot config if the store is briefly
 	// unreadable; install-level fields (network/disk/role/state key) are still
 	// only consumed at boot, so reading them live here is harmless.

internal/node/providers.go

@@ -289,8 +289,9 @@ func (c *ConfigStore) Apply(ctx context.Context, cfg *cryptosv1.MachineConfig) (
 	// the node, and classifying whether the change needs a reboot.
 	//
 	// Classify BEFORE overwriting: a change limited to the hot-reconfigurable
-	// fields (cert profiles, root-leaf-issuance acknowledgement) takes effect
-	// live for signing, so the caller need not reboot. Any other change — or a
+	// fields (cert profiles, root-leaf-issuance acknowledgement, revocation
+	// preflight override) takes effect live for signing, so the caller need
+	// not reboot. Any other change — or a
 	// first apply with no prior config — requires a reboot. Fail safe to reboot
 	// if the current config cannot be read or parsed.
 	var oldCfg *config.Config