diff --git a/internal/config/reboot.go b/internal/config/reboot.go index 6feef98..20daa67 100644 --- a/internal/config/reboot.go +++ b/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 } diff --git a/internal/config/reboot_test.go b/internal/config/reboot_test.go index 74e1c75..814a183 100644 --- a/internal/config/reboot_test.go +++ b/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") + } + }) +} diff --git a/internal/init/run.go b/internal/init/run.go index dd0435d..56e4525 100644 --- a/internal/init/run.go +++ b/internal/init/run.go @@ -471,7 +471,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. diff --git a/internal/node/providers.go b/internal/node/providers.go index ef5b086..f96996e 100644 --- a/internal/node/providers.go +++ b/internal/node/providers.go @@ -268,8 +268,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