diff --git a/doc/user-guide/policies.md b/doc/user-guide/policies.md index 762ef004..f70231de 100644 --- a/doc/user-guide/policies.md +++ b/doc/user-guide/policies.md @@ -302,6 +302,16 @@ The policy state is not exported. | **Skip that policy** | The existing policy is kept unchanged. | | **Update the existing draft** | The existing policy's content is replaced, but only if it is a draft; a released or archived policy is an error. | + The same option applies to a binding in the file whose policy and + node group are already bound on the server, which is the usual case + when a file is imported a second time: + + | Option | Effect on an existing binding | + |---|---| + | **Fail the import (recommended)** | The document is an error and nothing is imported. | + | **Skip that policy** | The existing binding is kept unchanged. | + | **Update the existing draft** | The priority is set to the one in the file. A binding that already has that priority is reported as `skipped`. | + Changing the option re-runs the preview. 4. If the preview is valid, select **Import N resources**. Otherwise fix the file; if any document has an error, nothing is imported. @@ -312,6 +322,9 @@ Rules: yourself. - Imported bindings are always created **disabled**. Enable them on [Policy Bindings](bindings.md) after releasing the policy. +- An import never enables or disables a binding that already exists; + only its priority can change. +- A file may bind a policy to a node group only once. - A binding's `spec.policy` refers to a `metadata.name` in the same file or to the name of a policy that already exists on the server. Its `spec.group` must be the name of an existing node group; node groups are diff --git a/server/internal/api/export.go b/server/internal/api/export.go index c693431c..4b0d7710 100644 --- a/server/internal/api/export.go +++ b/server/internal/api/export.go @@ -30,6 +30,9 @@ type ExportHandler struct { // az backs the systemd:exec check on import; nil refuses every // command-running Systemd policy in a bundle (fail closed). az authz.Authorizer + // OnBindingChange is called after an import changes the priority of an + // enabled binding, so the nodes of that group pick up the new order. + OnBindingChange func(b *models.PolicyBinding) } // NewExportHandler wires the export/import endpoints to their services. @@ -334,12 +337,48 @@ func (s scopedPolicyStore) UpdatePolicy(ctx context.Context, id string, req *mod } // scopedBindingStore only creates bindings to in-scope groups, for policies -// the caller can see. +// the caller can see. Existing bindings are matched among the ones the caller +// can view, and changing one needs the same reach as the bindings API. type scopedBindingStore struct { h *ExportHandler r *http.Request } +func (s scopedBindingStore) ListBindings(ctx context.Context) ([]*models.PolicyBindingWithDetails, error) { + all, err := s.h.bindingSvc.ListBindings(ctx) + if err != nil { + return nil, err + } + scope := requestScope(s.r, "binding", "view") + visible := make([]*models.PolicyBindingWithDetails, 0, len(all)) + for _, b := range all { + if scope.Allows(b.GroupID) { + visible = append(visible, b) + } + } + return visible, nil +} + +func (s scopedBindingStore) UpdateBinding(ctx context.Context, id string, req *models.UpdatePolicyBindingRequest) (*models.PolicyBinding, error) { + existing, err := s.h.bindingSvc.GetBinding(ctx, id) + if err != nil || existing == nil || !requestScope(s.r, "binding", "view").Allows(existing.GroupID) { + return nil, fmt.Errorf("policy binding not found") + } + if !requestScope(s.r, "binding", "toggle").Allows(existing.GroupID) { + return nil, errOutOfScope + } + updated, err := s.h.bindingSvc.UpdateBinding(ctx, id, req) + if err != nil { + return nil, err + } + // A disabled binding delivers nothing, so its priority is not yet + // visible to any node. + if s.h.OnBindingChange != nil && updated != nil && updated.State == models.BindingStateEnabled { + s.h.OnBindingChange(updated) + } + return updated, nil +} + func (s scopedBindingStore) CreateBinding(ctx context.Context, req *models.CreatePolicyBindingRequest) (*models.PolicyBinding, error) { if !requestScope(s.r, "binding", "create").Allows(req.GroupID) { return nil, errOutOfScope diff --git a/server/internal/export/export_test.go b/server/internal/export/export_test.go index ae0c5f3d..25970c15 100644 --- a/server/internal/export/export_test.go +++ b/server/internal/export/export_test.go @@ -188,7 +188,20 @@ func (f *fakePolicies) UpdatePolicy(_ context.Context, id string, req *models.Up } type fakeBindings struct { - created []*models.CreatePolicyBindingRequest + existing []*models.PolicyBindingWithDetails + created []*models.CreatePolicyBindingRequest + updated map[string]*models.UpdatePolicyBindingRequest +} + +func (f *fakeBindings) ListBindings(_ context.Context) ([]*models.PolicyBindingWithDetails, error) { + return f.existing, nil +} +func (f *fakeBindings) UpdateBinding(_ context.Context, id string, req *models.UpdatePolicyBindingRequest) (*models.PolicyBinding, error) { + if f.updated == nil { + f.updated = map[string]*models.UpdatePolicyBindingRequest{} + } + f.updated[id] = req + return &models.PolicyBinding{ID: id}, nil } func (f *fakeBindings) CreateBinding(_ context.Context, req *models.CreatePolicyBindingRequest) (*models.PolicyBinding, error) { @@ -307,6 +320,106 @@ func TestImportConflictModes(t *testing.T) { } } +// A bundle that is imported again meets the bindings it created the first +// time; they follow the conflict mode instead of being inserted twice. +func TestImportExistingBinding(t *testing.T) { + nameRefBundle := `apiVersion: bor.dev/v1 +kind: PolicyBinding +metadata: + name: firefox-eng +spec: + policy: "Firefox Baseline" + group: "Engineering" + priority: 10 +` + tests := []struct { + name string + bundle string + onConflict string + priority int // priority of the binding already on the server + wantOk bool + wantStatus string // status of the PolicyBinding document + wantMessage string // substring of its message + wantPriority int // priority written by UpdateBinding; 0 = no update + }{ + {name: "new-version updates a different priority", bundle: importBundle, onConflict: OnConflictNewVersion, priority: 50, wantOk: true, wantStatus: "updated", wantMessage: "from 50 to 10", wantPriority: 10}, + {name: "new-version leaves an equal priority", bundle: importBundle, onConflict: OnConflictNewVersion, priority: 10, wantOk: true, wantStatus: "skipped", wantMessage: "same priority"}, + {name: "skip leaves the binding", bundle: importBundle, onConflict: OnConflictSkip, priority: 50, wantOk: true, wantStatus: "skipped", wantMessage: "binding exists"}, + {name: "error rejects the bundle", bundle: nameRefBundle, onConflict: OnConflictError, priority: 50, wantOk: false, wantStatus: "error", wantMessage: "already exists"}, + {name: "new-version by policy name", bundle: nameRefBundle, onConflict: OnConflictNewVersion, priority: 50, wantOk: true, wantStatus: "updated", wantMessage: "from 50 to 10", wantPriority: 10}, + {name: "skip by policy name", bundle: nameRefBundle, onConflict: OnConflictSkip, priority: 50, wantOk: true, wantStatus: "skipped", wantMessage: "binding exists"}, + } + for _, tc := range tests { + for _, dryRun := range []bool{true, false} { + t.Run(fmt.Sprintf("%s/dry_run=%t", tc.name, dryRun), func(t *testing.T) { + fp, fb, fg := importFixture() + fp.existing = []*models.Policy{{ID: "p1", Name: "Firefox Baseline", Type: "Firefox", State: "draft"}} + fb.existing = []*models.PolicyBindingWithDetails{ + {PolicyBinding: models.PolicyBinding{ID: "b1", PolicyID: "p1", GroupID: "g1", State: models.BindingStateEnabled, Priority: tc.priority}}, + // Same group, other policy: must not be mistaken for a match. + {PolicyBinding: models.PolicyBinding{ID: "b2", PolicyID: "p2", GroupID: "g1", Priority: 1}}, + } + report, err := Import(context.Background(), fp, fb, fg, nil, []byte(tc.bundle), Options{DryRun: dryRun, OnConflict: tc.onConflict}) + if err != nil { + t.Fatal(err) + } + if report.Ok != tc.wantOk { + t.Fatalf("ok = %t, want %t: %+v", report.Ok, tc.wantOk, report) + } + var got *DocResult + for i := range report.Results { + if report.Results[i].Kind == "PolicyBinding" { + got = &report.Results[i] + } + } + if got == nil || got.Status != tc.wantStatus || !strings.Contains(got.Message, tc.wantMessage) { + t.Fatalf("binding result = %+v, want status %q with %q", got, tc.wantStatus, tc.wantMessage) + } + if len(fb.created) != 0 { + t.Fatalf("existing binding inserted again: %+v", fb.created) + } + wantUpdates := 0 + if tc.wantPriority != 0 && !dryRun { + wantUpdates = 1 + } + if len(fb.updated) != wantUpdates { + t.Fatalf("got %d binding updates, want %d", len(fb.updated), wantUpdates) + } + if wantUpdates == 1 { + req := fb.updated["b1"] + if req == nil || req.Priority == nil || *req.Priority != tc.wantPriority { + t.Fatalf("priority not updated on b1: %+v", fb.updated) + } + if req.State != nil { + t.Fatalf("import must not change the binding state: %v", *req.State) + } + } + }) + } + } +} + +func TestImportDuplicateBindingInBundle(t *testing.T) { + bundle := importBundle + `--- +apiVersion: bor.dev/v1 +kind: PolicyBinding +metadata: + name: firefox-eng-again +spec: + policy: firefox-baseline + group: "Engineering" + priority: 20 +` + fp, fb, fg := importFixture() + report, err := Import(context.Background(), fp, fb, fg, nil, []byte(bundle), Options{}) + if err != nil { + t.Fatal(err) + } + if report.Ok || report.Errors != 1 || len(fp.created) != 0 || len(fb.created) != 0 { + t.Fatalf("duplicate binding must reject the bundle: %+v", report) + } +} + func TestImportMissingGroupRejectsBundle(t *testing.T) { fp, fb, fg := importFixture() fg.groups = nil diff --git a/server/internal/export/import.go b/server/internal/export/import.go index e622db83..694e5459 100644 --- a/server/internal/export/import.go +++ b/server/internal/export/import.go @@ -23,6 +23,8 @@ type PolicyStore interface { // BindingStore is the slice of PolicyBindingService the importer needs. type BindingStore interface { + ListBindings(ctx context.Context) ([]*models.PolicyBindingWithDetails, error) + UpdateBinding(ctx context.Context, id string, req *models.UpdatePolicyBindingRequest) (*models.PolicyBinding, error) CreateBinding(ctx context.Context, req *models.CreatePolicyBindingRequest) (*models.PolicyBinding, error) } @@ -31,7 +33,8 @@ type GroupStore interface { ListNodeGroups(ctx context.Context) ([]*models.NodeGroup, error) } -// Conflict handling for policies whose display name already exists. +// Conflict handling for policies whose display name already exists and for +// bindings whose policy and node group are already bound. const ( OnConflictError = "error" OnConflictSkip = "skip" @@ -88,8 +91,14 @@ type pendingBinding struct { priority int policySlug bool // ref resolved against an in-file slug policyID string + existing *models.PolicyBinding // binding already on the server for this policy and group + skip bool // leave the existing binding untouched + skipReason string } +// bindingKey identifies a binding by the pair the database keeps unique. +type bindingKey struct{ policyID, groupID string } + // Import validates the whole bundle first and creates resources only when // every document is valid ("validate all, then execute"): nothing is written // for a bundle containing any invalid document. Execution-stage failures @@ -131,11 +140,20 @@ func Import(ctx context.Context, policies PolicyStore, bindings BindingStore, gr for _, g := range groupList { groupByName[g.Name] = g.ID } + bindingList, err := bindings.ListBindings(ctx) + if err != nil { + return nil, fmt.Errorf("failed to list policy bindings: %w", err) + } + bindingByKey := make(map[bindingKey]*models.PolicyBinding, len(bindingList)) + for _, eb := range bindingList { + bindingByKey[bindingKey{eb.PolicyID, eb.GroupID}] = &eb.PolicyBinding + } // ── Pass 1: parse + validate every document ── var pendingPolicies []*pendingPolicy var pendingBindings []*pendingBinding slugs := make(map[string]*pendingPolicy) + bindingsInFile := make(map[bindingKey]bool) namesInFile := make(map[string]bool) for i, doc := range docs { @@ -238,14 +256,47 @@ func Import(ctx context.Context, policies PolicyStore, bindings BindingStore, gr continue } b.groupID = gid - if _, ok := slugs[b.policyRef]; ok { + // existingPolicyID is the server-side policy the binding will + // attach to, when there is one already: a policy that is updated + // or skipped keeps its ID, a policy the bundle creates has none. + existingPolicyID := "" + if p, ok := slugs[b.policyRef]; ok { b.policySlug = true + if prev, exists := byName[p.name]; exists { + existingPolicyID = prev.ID + } } else if prev, ok := byName[b.policyRef]; ok { b.policyID = prev.ID + existingPolicyID = prev.ID } else { fail(n, res.Kind, name, fmt.Sprintf("policy %q not found in bundle or on this server", b.policyRef)) continue } + fileKey := bindingKey{existingPolicyID, gid} + if existingPolicyID == "" { + fileKey.policyID = "slug:" + b.policyRef + } + if bindingsInFile[fileKey] { + fail(n, res.Kind, name, "duplicate binding for this policy and node group in bundle") + continue + } + bindingsInFile[fileKey] = true + if prev, ok := bindingByKey[bindingKey{existingPolicyID, gid}]; ok && existingPolicyID != "" { + switch opts.OnConflict { + case OnConflictError: + fail(n, res.Kind, name, "a binding for this policy and node group already exists (on_conflict=error)") + continue + case OnConflictSkip: + b.skip = true + b.skipReason = "binding exists" + case OnConflictNewVersion: + b.existing = prev + if prev.Priority == b.priority { + b.skip = true + b.skipReason = "binding exists with the same priority" + } + } + } pendingBindings = append(pendingBindings, b) } } @@ -272,8 +323,18 @@ func Import(ctx context.Context, policies PolicyStore, bindings BindingStore, gr } } for _, b := range pendingBindings { - report.Created++ - report.Results = append(report.Results, DocResult{Doc: b.doc, Kind: "PolicyBinding", Name: b.name, Status: "created", Message: "would create"}) + switch { + case b.skip: + report.Skipped++ + report.Results = append(report.Results, DocResult{Doc: b.doc, Kind: "PolicyBinding", Name: b.name, Status: "skipped", Message: b.skipReason}) + case b.existing != nil: + report.Updated++ + report.Results = append(report.Results, DocResult{Doc: b.doc, Kind: "PolicyBinding", Name: b.name, Status: "updated", + Message: fmt.Sprintf("would change priority from %d to %d", b.existing.Priority, b.priority)}) + default: + report.Created++ + report.Results = append(report.Results, DocResult{Doc: b.doc, Kind: "PolicyBinding", Name: b.name, Status: "created", Message: "would create"}) + } } report.Ok = true return report, nil @@ -324,6 +385,23 @@ func Import(ctx context.Context, policies PolicyStore, bindings BindingStore, gr } } for _, b := range pendingBindings { + if b.skip { + report.Skipped++ + report.Results = append(report.Results, DocResult{Doc: b.doc, Kind: "PolicyBinding", Name: b.name, Status: "skipped", Message: b.skipReason}) + continue + } + if b.existing != nil { + // Only the priority follows the bundle; whether the binding is + // enabled stays an explicit decision made on the server. + if _, err := bindings.UpdateBinding(ctx, b.existing.ID, &models.UpdatePolicyBindingRequest{Priority: &b.priority}); err != nil { + fail(b.doc, "PolicyBinding", b.name, err.Error()) + continue + } + report.Updated++ + report.Results = append(report.Results, DocResult{Doc: b.doc, Kind: "PolicyBinding", Name: b.name, Status: "updated", + Message: fmt.Sprintf("priority changed from %d to %d", b.existing.Priority, b.priority)}) + continue + } id := b.policyID if b.policySlug { id = slugToID[b.policyRef] diff --git a/server/pkg/app/app.go b/server/pkg/app/app.go index cf9710fd..47875ed6 100644 --- a/server/pkg/app/app.go +++ b/server/pkg/app/app.go @@ -585,6 +585,9 @@ func Run(opts Options) { // import creates drafts and needs create. Both are audit-logged by the // handler itself (exports are GETs, which the audit middleware skips). exportHandler := api.NewExportHandler(policySvc, policyBindingSvc, nodeGroupSvc, auditSvc).WithAuthorizer(az) + exportHandler.OnBindingChange = func(b *models.PolicyBinding) { + policyHub.PublishResync(b.GroupID) + } mux.Handle("/api/v1/policies/export", authMiddleware(gate.RequireScoped("policy", "view")(http.HandlerFunc(exportHandler.Export)))) mux.Handle("/api/v1/policies/import", authMiddleware(gate.RequireScoped("policy", "create")(http.HandlerFunc(exportHandler.Import))))