diff --git a/core/services/addons/hostgrants/document.go b/core/services/addons/hostgrants/document.go index 12fa369b2b..903997f465 100644 --- a/core/services/addons/hostgrants/document.go +++ b/core/services/addons/hostgrants/document.go @@ -89,6 +89,11 @@ type OpGrant struct { // BlanketRevision names the blanket switch revision that produced a // grant with Source blanket, so the group can be told apart and revoked. BlanketRevision uint64 `json:"blanketRevision,omitempty"` + // OperationID is set when the operation was added to an entry that already + // existed, by the lifecycle operation of an upgrade. It takes effect only + // once that operation completed, while the other operations of the entry + // keep working. Empty means the operation of the entry counts. + OperationID string `json:"operationId,omitempty"` } // AddonGrants is the UI-made grant entry of one add-on. @@ -343,6 +348,9 @@ func validate(s Snapshot) error { return invalid("operation %q of %q", op.Name, id) } seen[op.Name] = true + if op.OperationID != "" && !validOperationID(op.OperationID) { + return invalid("operation ID of %q for %q", op.Name, id) + } if !validText(op.By) || !validText(op.Version) || !validTime(op.At) { return invalid("metadata of %q for %q", op.Name, id) } diff --git a/core/services/addons/hostgrants/policy.go b/core/services/addons/hostgrants/policy.go index 9f3df3bff8..d671d410b3 100644 --- a/core/services/addons/hostgrants/policy.go +++ b/core/services/addons/hostgrants/policy.go @@ -119,12 +119,22 @@ func (f Floor) Decide(doc Snapshot, c Context, addonID, op string) Decision { if c.Identity == "" || entry.Identity != c.Identity { return Decision{Reason: ReasonIdentityMismatch} } - if c.Completed != nil && !c.Completed(entry.OperationID) { + if c.Completed != nil && !c.Completed(EffectiveOperationID(entry, entry.Ops[i])) { return Decision{Reason: ReasonNotGranted} } return Decision{Allowed: true, Source: entry.Ops[i].Source} } +// EffectiveOperationID is the lifecycle operation whose completion makes one +// operation of an entry effective: its own, when an upgrade added it to an +// existing entry, else the one that created the entry. +func EffectiveOperationID(entry AddonGrants, op OpGrant) string { + if op.OperationID != "" { + return op.OperationID + } + return entry.OperationID +} + // OpStatus is a Decision for one named operation. type OpStatus struct { Op string diff --git a/core/services/addons/hostgrants/reconcile.go b/core/services/addons/hostgrants/reconcile.go index ad65511160..3f619795e5 100644 --- a/core/services/addons/hostgrants/reconcile.go +++ b/core/services/addons/hostgrants/reconcile.go @@ -51,8 +51,30 @@ func PlanReconcile(snap Snapshot, in ReconcileInput) (Patch, []Removal) { reason = RemovedNotInstalled case identity != entry.Identity: reason = RemovedIdentityMismatch - case in.OperationCompleted != nil && !in.OperationCompleted(entry.OperationID): - reason = RemovedOperationIncomplete + } + if reason == "" && in.OperationCompleted != nil { + // An operation added by an upgrade is pending on its own; the + // operations the entry held before keep working. + var kept []OpGrant + var dropped []string + for _, op := range entry.Ops { + if in.OperationCompleted(EffectiveOperationID(entry, op)) { + kept = append(kept, op) + } else { + dropped = append(dropped, op.Name) + } + } + switch { + case len(dropped) == 0: + case len(kept) == 0: + reason = RemovedOperationIncomplete + default: + trimmed := entry + trimmed.Ops = kept + patch.Addons[id] = &trimmed + removals = append(removals, Removal{AddonID: id, Reason: RemovedOperationIncomplete, Ops: dropped}) + continue + } } if reason == "" { continue diff --git a/core/services/addons/hostgrants/reconcile_test.go b/core/services/addons/hostgrants/reconcile_test.go index d68634ee67..57ce38e4f4 100644 --- a/core/services/addons/hostgrants/reconcile_test.go +++ b/core/services/addons/hostgrants/reconcile_test.go @@ -161,3 +161,40 @@ func TestAuditAppendsFsyncedLines(t *testing.T) { t.Error("a symlink must be refused") } } + +func TestUpgradeAddedOperationIsPendingOnItsOwn(t *testing.T) { + entry := *grant("install-op", "workspace.materialize") + entry.Ops = append(entry.Ops, OpGrant{Name: "egress.check", Source: SourceUpgrade, OperationID: "upgrade-op"}) + snap := emptySnapshot() + snap.Grants["a"] = entry + completed := map[string]bool{"install-op": true} + ctx := Context{Identity: "id-1", Completed: func(id string) bool { return completed[id] }} + + // Before the upgrade completes, the old operation works and the new one does not. + var f Floor + if !f.Decide(snap, ctx, "a", "workspace.materialize").Allowed { + t.Fatal("the operation held before the upgrade stopped working") + } + if d := f.Decide(snap, ctx, "a", "egress.check"); d.Allowed || d.Reason != ReasonNotGranted { + t.Fatalf("%+v", d) + } + completed["upgrade-op"] = true + if !f.Decide(snap, ctx, "a", "egress.check").Allowed { + t.Fatal("the operation of a completed upgrade is not effective") + } + + // A crash before the upgrade completed removes only what it added. + delete(completed, "upgrade-op") + in := ReconcileInput{ + Installed: func(string) (string, bool) { return "id-1", true }, + OperationCompleted: func(id string) bool { return completed[id] }, + } + patch, removals := PlanReconcile(snap, in) + if len(removals) != 1 || !reflect.DeepEqual(removals[0].Ops, []string{"egress.check"}) || removals[0].Reason != RemovedOperationIncomplete { + t.Fatalf("%+v", removals) + } + kept := patch.Addons["a"] + if kept == nil || len(kept.Ops) != 1 || kept.Ops[0].Name != "workspace.materialize" { + t.Fatalf("%+v", kept) + } +}