Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions internal/config/reboot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
82 changes: 81 additions & 1 deletion internal/config/reboot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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")
}
})
}
3 changes: 2 additions & 1 deletion internal/init/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions internal/node/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down