Resume the pipeline when a new zone is loaded#153
Conversation
If a zone's pipeline is soft-halted, and a new version of the zone is loaded, the pipeline doesn't resume. Part of the reason for this was to stop the zone signer from signing an unsigned zone that hasn't been approved. A secondary issue is that a 'SignZone' command (e.g. issued autonomously by the key manager), while the pipeline is halted, causes the zone signer to hard-halt. This accidentally turns soft-halts into hard-halts. Now, a new zonetree for ready-to-sign zones has been added. Unsigned zones are added to this tree when they are approved. This separates the pipeline mode from the unsigned zone approval state; the zone signer can be run at any time, and it will only touch approved zones. The zone signer has also been adjusted to leave the pipeline mode alone if it is already halted.
|
Contributes to solving #154. |
| if let Ok(mut zone_state) = zone.state.lock() { | ||
| match zone_state.pipeline_mode.clone() { | ||
| PipelineMode::Running => {} | ||
| PipelineMode::SoftHalt(message) => { |
There was a problem hiding this comment.
Is there a reason you chose to include the soft-halt message in your log message, but didn't also add it to the hard-halt log message?
|
So, I tried testing this but the zone gets stuck showing that signing has been requested but proceeding no further. The logs show "Starting signing operation" but nothing more. Reverting to 0.1.0-alpha fixes the problem. |
Ah, in trace level logging I see: But the system is not left in the correct state from the point of view of the I'll see why it thinks the zone is unavailable. |
|
Ahha, maybe it only adds the zone to the new signable zones collection if it is approved, but I have no approval required at present. |
|
@bal-e: I pushed a change that also promotes the zone if review is not required. It now works better for me but I'm still hitting a problem when the unsigned review passed but the signed review did not (in my case I forgot to chmod +x the signed review script). Then it gets stuck again. Investigating. Also, maybe nothing to do with this PR, if the unsigned review script lacks execute permission and cannot be run the zone is held. Attempting to approve it using |
In the case of a zone that failed review, on zone reload we should: a. Re-review it, because the content may have changed. b. Allow it to progress to the next stage in the pipeline (if review passes). Prior to this change it was neither re-reviewed nor progressed, new zone content triggering a re-review was just ignored.
|
Right, I think that solves it. The change makes me a little uncomfortable because I'm changing existing logic, but as far as I can tell it was just wrong, maybe a left over of the earlier HTTP API based approval (because then you could get multiple approvals for the same zone so ignoring a duplicate might make sense, but now with review script exit code based approval it can only be approved once). FYI manual approval won't work, e.g. |
There was a problem hiding this comment.
I also have some issues when doing (with local zone file):
- add policy with rejecting review script (and review required)
- add zone
- change policy to use different script (that still rejects)
- run
policy reload - run
zone status ...(now shows the new review script, and the existing error messsage (so is out of sync, but that is known, I think) - run
zone reload ... - change script to accept zone
- run
zone reload ... zone statussays loaded new version bla 1s ago, but is still waiting for approval and error that the review rejected the zone.
(I haven't looked further through the code yet, but want to get this out already)
I might be an idiot ... |
|
Seems to work |
|
Just a note: #197 still applies in this branch, but I assume that is fix unrelated to this PR, in that the fix touches on how policies are reloaded. |
|
Indeed I would say they are unrelated. |
|
@mozzieongit: See c71ee90. |
Looks good. Are you going to resolve the |
Done. |
If a zone's pipeline is soft-halted, and a new version of the zone is loaded, the pipeline doesn't resume. Part of the reason for this was to stop the zone signer from signing an unsigned zone that hasn't been approved. A secondary issue is that a 'SignZone' command (e.g. issued autonomously by the key manager), while the pipeline is halted, causes the zone signer to hard-halt. This accidentally turns soft-halts into hard-halts.
Now, a new zonetree for ready-to-sign zones has been added. Unsigned zones are added to this tree when they are approved. This separates the pipeline mode from the unsigned zone approval state; the zone signer can be run at any time, and it will only touch approved zones. The zone signer has also been adjusted to leave the pipeline mode alone if it is already halted.