Repository navigation
fix(config): treat allow_unverified_revocation_url as live in requires_reboot - #307
Merged
Merged
Conversation
Bugs5382
marked this pull request as ready for review
September 30, 2026 16:28
…s_reboot 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>
Bugs5382
force-pushed
the
fix/301-requires-reboot-nil-empty
branch
from
September 30, 2026 16:41
e2757a0 to
547fb28
Compare
2 tasks done
Contributor
Author
|
Merging for the v0.1.0 test run: CI green and mergeable at merge time. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
config applyreportedrequires_reboot=truefor changes that are actually live. The nil-versus-empty part of #301 (identical re-apply and profiles-only changes reporting a reboot) was fixed onmainby #286, which compares configs by their stored YAML. This PR is rebased on that and covers what's left:pki.allow_unverified_revocation_urlis now classed as live. The signer reads it from the stored config on every issuance (applyRevocationininternal/node/signer.go, through the liveconfigFuncininternal/init/run.go). Nothing reads it at boot:run.goonly mentions it in a log line, and the revocation preflight and listener are built fromrevocation_base_url, which stays reboot-relevant. A change to the override alone now reportsrequires_reboot=false, next topki.profilesandpki.root_leaf_issuance.NeedsRebootthrough the real apply path. The running config goes throughMarshal/Parselike the store. The incoming config goes throughToProto, a protobuf wire round trip andFromProtolike the API. They cover an identical re-apply (no reboot), a profiles-only change (no reboot) and a network change (reboot). On the pre-feat(config): switch ACME and EST on and off through machine config #286main, the wire round trip reproduced the bug; an in-processFromProto(ToProto())did not, because it keeps the empty slice.Follow-up, not in this PR:
pki.allow_unsynced_clockisn't onmainyet (it arrives with the SNTP client work, #271). The docs already say the signer reads it live but the apply still reports a reboot for it. Once #271 merges, it should join the live list the same way, with a test.The
Reboot pendingline incryptosctl image statuscomes from image upgrades, not fromApplyConfig, so it isn't affected.Docs: CryptOS-PKI/docs#59 lists
allow_unverified_revocation_urlamong the live fields in the machine config reference, "Apply config" and "Declarative config". Merge the two together.Closes #301
Verification
How this was verified
allow_unverified_revocation_urltest failed onmain(requires_reboot=true) and passes with the change.mainand pass on currentmainand on this branch. The existingTestNeedsReboot*cases are unchanged and pass.task ci(fmt, lint, vet, test, build) passes locally on the rebased branch.Before merging: add a closing comment summarizing what was actually done in this PR
(not just the checked boxes).