Rules: placed in the same change they are created in

hc rule add --before/--after/--first created the rule at the end of its
chain and then moved it: two history entries, two probations, the rule
briefly in force in the wrong place, and in a changeset the place was
lost. POST /rules (and the other ordered kinds) now takes ?before=ID,
?after=ID or ?first=1, kept with the operation when staged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -58,6 +58,11 @@ All notable changes to Housecarl are documented here. The format follows
 
 ### Fixed
 
+- `hc rule add --before/--after/--first` made two changes (create at the
+  end, then move), so two history entries, two probations, and the rule
+  briefly in force at the end of its chain; staged in a changeset, the
+  place was lost. The create now takes the place (`POST
+  /rules?before=ID`, `after`, `first`) and makes one change.
 - `hc matrix` in a terminal: the colours' escape codes threw the columns
   out of line.
 - `hc counters` (`GET /counters`) kept listing the objects of a

cmd/hc/rules.go

@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"io"
+	"net/url"
 	"regexp"
 	"strconv"
 	"strings"
@@ -340,25 +341,20 @@ output (from it) or early (before connection tracking: cheap drops).
 			if err != nil {
 				return err
 			}
-			var resp api.MutationResponse
+			// Placed in the same change: one history entry, and never in
-			if err := c.Do(ctx(), "POST", "/rules", nil, r, &resp); err != nil {
+			// force at the end of the chain.
-				return err
+			q := url.Values{}
+			switch {
+			case rf.first:
+				q.Set("first", "1")
+			case rf.before != 0:
+				q.Set("before", fmt.Sprint(rf.before))
+			case rf.after != 0:
+				q.Set("after", fmt.Sprint(rf.after))
 			}
-			// Then place it.
+			var resp api.MutationResponse
-			if resp.Applied && (rf.first || rf.before != 0 || rf.after != 0) {
+			if err := c.Do(ctx(), "POST", "/rules", q, r, &resp); err != nil {
-				var created model.Rule
+				return err
-				if err := jsonUnmarshal(resp.Object, &created); err == nil {
-					mv := api.MoveInput{Before: rf.before, After: rf.after}
-					if rf.first {
-						mv.To = "start"
-					}
-					var moved api.MutationResponse
-					if err := c.Do(ctx(), "POST", objPath("rules", fmt.Sprint(created.ID))+"/move", nil, mv, &moved); err != nil {
-						return err
-					}
-					resp.Changes = append(resp.Changes, moved.Changes...)
-					resp.ConfirmBy = moved.ConfirmBy
-				}
 			}
 			return report(&resp)
 		},

docs/api.md

@@ -114,7 +114,7 @@ share one pattern, `/{kind}` with kind `zones`, `sets`, `services`,
 | | |
 |---|---|
 | `GET /{kind}` | all of them, in order; `GET /rules?chain=input` for one chain; `GET /services` includes the built-in ones |
-| `POST /{kind}` | create; rules, forwards and NAT rules go last unless given a `position` |
+| `POST /{kind}[?before=ID\|after=ID\|first=1]` | create; rules, forwards and NAT rules go last unless given a `position`, or placed before or after another of their chain, or first, in the same change (one history entry; staged in a changeset, the place goes with it) |
 | `GET /{kind}/{ref}` | by ID, or name for zones, sets, services and forwards |
 | `PATCH /{kind}/{ref}` | change the fields given (a JSON merge: `null` clears) |
 | `DELETE /{kind}/{ref}` | refused while something uses it |

internal/model/changeset.go

@@ -53,7 +53,10 @@ type Op struct {
 	ID int64 `json:"id,omitempty"`
 	// Data is the object (create), the fields to change (update,
 	// settings) or where to move it (move).
-	Data    json.RawMessage `json:"data,omitempty"`
+	Data json.RawMessage `json:"data,omitempty"`
+	// Place is where a create puts the object in its order (a move's
+	// data); none: at the end.
+	Place   json.RawMessage `json:"place,omitempty"`
 	Summary string          `json:"summary"`
 	AddedAt time.Time       `json:"added_at"`
 }

internal/server/h_config.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"net/netip"
 	"sort"
+	"strconv"
 	"strings"
 
 	udiff "github.com/aymanbagabas/go-udiff"
@@ -310,7 +311,19 @@ func (s *Server) createObject(w http.ResponseWriter, r *http.Request, p *auth.Pr
 	if _, err := decodeObject(kind, raw); err != nil {
 		return err
 	}
-	return s.change(w, r, p, model.Op{Op: model.OpCreate, Kind: kind, Data: raw})
+	op := model.Op{Op: model.OpCreate, Kind: kind, Data: raw}
+	// ?before=ID, ?after=ID or ?first=1: where it goes in its order.
+	q := r.URL.Query()
+	var place api.MoveInput
+	place.Before, _ = strconv.ParseInt(q.Get("before"), 10, 64)
+	place.After, _ = strconv.ParseInt(q.Get("after"), 10, 64)
+	if queryBool(r, "first") {
+		place.To = "start"
+	}
+	if place.Before != 0 || place.After != 0 || place.To != "" {
+		op.Place, _ = json.Marshal(place)
+	}
+	return s.change(w, r, p, op)
 }
 
 func (s *Server) updateObject(w http.ResponseWriter, r *http.Request, p *auth.Principal) error {

internal/server/ops.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/http"
 	"sort"
+	"strconv"
 	"strings"
 
 	"housecarl/internal/api"
@@ -63,7 +64,16 @@ func createOp(st *model.State, op *model.Op) (int64, error) {
 		*pos = lastPosition(st, op.Kind, chainOf(o))
 	}
 	data, _ := json.Marshal(o)
-	return op.ID, st.Put(op.Kind, op.ID, data)
+	if err := st.Put(op.Kind, op.ID, data); err != nil {
+		return 0, err
+	}
+	// Placed in the same change, so it is never in force anywhere else.
+	if len(op.Place) > 0 {
+		if _, err := moveOp(st, &model.Op{Op: model.OpMove, Kind: op.Kind, Ref: strconv.FormatInt(op.ID, 10), Data: op.Place}); err != nil {
+			return 0, err
+		}
+	}
+	return op.ID, nil
 }
 
 func updateOp(st *model.State, op *model.Op) (int64, error) {

internal/server/server_test.go

@@ -1184,3 +1184,49 @@ func (e *env) text(path string) string {
 	b, _ := io.ReadAll(resp.Body)
 	return string(b)
 }
+
+// TestCreatePlaced: a rule created ?before= another is placed in the same
+// change (one history entry), also when staged in a changeset.
+func TestCreatePlaced(t *testing.T) {
+	e := newEnv(t)
+	var a, b, c api.MutationResponse
+	e.call(e.token, "POST", "/rules", model.Rule{Chain: "input", Match: model.Match{Services: []string{"ssh"}}}, &a)
+	e.call(e.token, "POST", "/rules", model.Rule{Chain: "input", Match: model.Match{Services: []string{"dns"}}}, &b)
+	var first model.Rule
+	json.Unmarshal(a.Object, &first)
+	if code := e.call(e.token, "POST", "/rules?before="+itoa(first.ID), model.Rule{Chain: "input", Match: model.Match{Services: []string{"http"}}, Comment: "placed"}, &c); code != 201 {
+		t.Fatalf("create placed: %d %+v", code, c)
+	}
+	order := func() string {
+		var out []string
+		for _, r := range e.eng.State().Rules {
+			out = append(out, strings.Join(r.Services, ""))
+		}
+		return strings.Join(out, ",")
+	}
+	if o := order(); o != "http,ssh,dns" {
+		t.Errorf("order: %s", o)
+	}
+	var hs []model.HistoryEntry
+	e.call(e.token, "GET", "/history", nil, &hs)
+	if len(hs) != 3 || len(hs[0].Changes) != 1 || hs[0].Changes[0].Action() != "create" {
+		t.Errorf("history: %d entries, newest %+v", len(hs), hs[0])
+	}
+	// Staged, the place goes with it.
+	var cs model.Changeset
+	e.call(e.token, "POST", "/changesets", nil, &cs)
+	e.call(e.token, "POST", "/rules?first=1&changeset="+itoa(cs.ID), model.Rule{Chain: "input", Match: model.Match{Services: []string{"ntp"}}}, nil)
+	if code := e.call(e.token, "POST", "/changesets/"+itoa(cs.ID)+"/apply", nil, nil); code != 200 {
+		t.Fatalf("apply: %d", code)
+	}
+	if o := order(); o != "ntp,http,ssh,dns" {
+		t.Errorf("order after the changeset: %s", o)
+	}
+	// A bad place: refused, nothing created.
+	if code := e.call(e.token, "POST", "/rules?before=99999", model.Rule{Chain: "input"}, nil); code != 400 {
+		t.Errorf("before a rule that isn't there: %d", code)
+	}
+	if n := len(e.eng.State().Rules); n != 4 {
+		t.Errorf("%d rules", n)
+	}
+}