mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-11 18:21:29 +02:00
Compare commits
8 Commits
l10n_main
...
agent-netw
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6c9267c7d3 | ||
|
|
29eff3b207 | ||
|
|
35137326f7 | ||
|
|
5085a2f96b | ||
|
|
30c2010c09 | ||
|
|
9b06290240 | ||
|
|
829156f53d | ||
|
|
4f6caa1110 |
@@ -11,6 +11,7 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/netbirdio/netbird/e2e/harness"
|
||||
"github.com/netbirdio/netbird/shared/management/client/rest"
|
||||
"github.com/netbirdio/netbird/shared/management/http/api"
|
||||
)
|
||||
|
||||
@@ -177,3 +178,89 @@ func TestSettingsBootstrapSelfAddressed(t *testing.T) {
|
||||
require.NoError(t, err, "bootstrap after delete must succeed")
|
||||
assert.Equal(t, "gw2.e2e.netbird.selfhosted", recreated.Endpoint, "the fresh bootstrap claims the new hostname")
|
||||
}
|
||||
|
||||
// TestSettingsConditionalWrites covers the lost-update guard end to end, over
|
||||
// the same REST client the Terraform provider uses: read the settings, take
|
||||
// the entity-tag, and have a write refused when the row moved underneath it.
|
||||
//
|
||||
// The scenario is the one that motivates the feature. A client reads the
|
||||
// settings and computes an update. An operator turns PII redaction on in the
|
||||
// dashboard in the meantime. Without a precondition the client's write puts
|
||||
// redaction straight back off — no error, no drift warning, a
|
||||
// compliance-relevant control silently disabled. With one, the write is
|
||||
// refused and the client can read again.
|
||||
func TestSettingsConditionalWrites(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
fresh, err := harnessStartFresh(ctx, t)
|
||||
require.NoError(t, err, "start dedicated combined server")
|
||||
|
||||
const cluster = "eu.e2e.netbird.selfhosted"
|
||||
bootstrapped, err := fresh.CreateSettings(ctx, api.AgentNetworkSettingsCreateRequest{
|
||||
ProxyAddress: ptr(cluster),
|
||||
})
|
||||
require.NoError(t, err, "bootstrap must succeed")
|
||||
|
||||
// What the client plans against.
|
||||
planned, etag, err := fresh.GetSettingsWithETag(ctx)
|
||||
require.NoError(t, err, "read must succeed")
|
||||
require.NotEmpty(t, etag, "the read must carry a validator")
|
||||
assert.Equal(t, bootstrapped.Endpoint, planned.Endpoint)
|
||||
|
||||
_, again, err := fresh.GetSettingsWithETag(ctx)
|
||||
require.NoError(t, err, "second read must succeed")
|
||||
assert.Equal(t, etag, again, "an unchanged row must read as the same validator")
|
||||
|
||||
update := func(redactPii bool, retention int) api.AgentNetworkSettingsRequest {
|
||||
return api.AgentNetworkSettingsRequest{
|
||||
Endpoint: planned.Endpoint,
|
||||
ProxyAddress: planned.ProxyAddress,
|
||||
EnableLogCollection: true,
|
||||
EnablePromptCollection: true,
|
||||
RedactPii: redactPii,
|
||||
AccessLogRetentionDays: retention,
|
||||
}
|
||||
}
|
||||
|
||||
// The operator's change, which the planning client never saw.
|
||||
_, err = fresh.UpdateSettings(ctx, update(true, 21))
|
||||
require.NoError(t, err, "the intervening update must succeed")
|
||||
|
||||
// The client's write, planned against the earlier read, would have turned
|
||||
// redaction back off. It is refused instead.
|
||||
_, _, err = fresh.UpdateSettingsIfMatch(ctx, update(false, 7), etag)
|
||||
require.Error(t, err, "a stale precondition must be refused")
|
||||
require.True(t, rest.IsPreconditionFailed(err),
|
||||
"the refusal must be a precondition failure, got: %v", err)
|
||||
|
||||
intact, current, err := fresh.GetSettingsWithETag(ctx)
|
||||
require.NoError(t, err, "read after the refusal must succeed")
|
||||
assert.True(t, intact.RedactPii, "the refused write must not have turned redaction off")
|
||||
require.NotNil(t, intact.AccessLogRetentionDays)
|
||||
assert.Equal(t, 21, *intact.AccessLogRetentionDays, "the refused write must not have changed retention")
|
||||
assert.NotEqual(t, etag, current, "the validator must have moved with the intervening update")
|
||||
|
||||
// Retrying against the current validator goes through, and hands back the
|
||||
// validator for the write after it.
|
||||
updated, next, err := fresh.UpdateSettingsIfMatch(ctx, update(true, 7), current)
|
||||
require.NoError(t, err, "a matching precondition must be honoured")
|
||||
require.NotNil(t, updated.AccessLogRetentionDays)
|
||||
assert.Equal(t, 7, *updated.AccessLogRetentionDays, "the conditional write must apply")
|
||||
assert.NotEmpty(t, next, "the write must return a validator")
|
||||
assert.NotEqual(t, current, next, "the write must move the validator")
|
||||
|
||||
// The delete is conditional too, and refusing a stale one leaves the
|
||||
// endpoint claimed.
|
||||
err = fresh.DeleteSettingsIfMatch(ctx, etag)
|
||||
require.Error(t, err, "a stale precondition must refuse the delete")
|
||||
require.True(t, rest.IsPreconditionFailed(err),
|
||||
"the delete must be refused for staleness rather than for a state guard or a server error, got: %v", err)
|
||||
stillThere, err := fresh.GetSettings(ctx)
|
||||
require.NoError(t, err, "read after the refused delete must succeed")
|
||||
assert.Equal(t, planned.Endpoint, stillThere.Endpoint, "the refused delete must leave the endpoint claimed")
|
||||
|
||||
require.NoError(t, fresh.DeleteSettingsIfMatch(ctx, next), "a matching precondition must be honoured")
|
||||
gone, err := fresh.GetSettings(ctx)
|
||||
require.NoError(t, err, "read after the delete must succeed")
|
||||
assert.Empty(t, gone.Endpoint, "the row must be gone")
|
||||
}
|
||||
|
||||
@@ -153,6 +153,37 @@ func (c *Combined) DeleteSettings(ctx context.Context) error {
|
||||
return anDelete(ctx, c, "/api/agent-network/settings")
|
||||
}
|
||||
|
||||
// The conditional-request wrappers go through the typed REST client rather
|
||||
// than anRequest, so the e2e run exercises the client's own header handling —
|
||||
// the quoting on the way out and the unquoting on the way back — against a
|
||||
// real server, which is the path the Terraform provider takes.
|
||||
|
||||
// GetSettingsWithETag reads the settings along with the entity-tag that makes
|
||||
// a following write conditional.
|
||||
func (c *Combined) GetSettingsWithETag(ctx context.Context) (api.AgentNetworkSettings, string, error) {
|
||||
settings, etag, err := c.api.AgentNetwork.GetSettingsWithETag(ctx)
|
||||
if err != nil {
|
||||
return api.AgentNetworkSettings{}, "", err
|
||||
}
|
||||
return *settings, etag, nil
|
||||
}
|
||||
|
||||
// UpdateSettingsIfMatch applies the update only if etag is still current,
|
||||
// returning the entity-tag of the row it wrote.
|
||||
func (c *Combined) UpdateSettingsIfMatch(ctx context.Context, req api.AgentNetworkSettingsRequest, etag string) (api.AgentNetworkSettings, string, error) {
|
||||
settings, newETag, err := c.api.AgentNetwork.UpdateSettingsIfMatch(ctx, req, etag)
|
||||
if err != nil {
|
||||
return api.AgentNetworkSettings{}, "", err
|
||||
}
|
||||
return *settings, newETag, nil
|
||||
}
|
||||
|
||||
// DeleteSettingsIfMatch deletes the settings row only if etag is still
|
||||
// current.
|
||||
func (c *Combined) DeleteSettingsIfMatch(ctx context.Context, etag string) error {
|
||||
return c.api.AgentNetwork.DeleteSettingsIfMatch(ctx, etag)
|
||||
}
|
||||
|
||||
// ListConsumption returns the account's consumption rows (possibly empty).
|
||||
func (c *Combined) ListConsumption(ctx context.Context) ([]api.AgentNetworkConsumption, error) {
|
||||
return anRequest[[]api.AgentNetworkConsumption](ctx, c, http.MethodGet, "/api/agent-network/consumption", nil)
|
||||
|
||||
@@ -92,6 +92,13 @@ func newAgentNetworkHandlerFixture(t *testing.T) *agentNetworkHandlerFixture {
|
||||
}
|
||||
|
||||
func (f *agentNetworkHandlerFixture) do(t *testing.T, method, path, body string) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
return f.doWithHeaders(t, method, path, body, nil)
|
||||
}
|
||||
|
||||
// doWithHeaders is do with request headers, for the cases where the header is
|
||||
// the thing under test (conditional requests).
|
||||
func (f *agentNetworkHandlerFixture) doWithHeaders(t *testing.T, method, path, body string, headers map[string]string) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
var reader io.Reader
|
||||
if body != "" {
|
||||
@@ -101,6 +108,9 @@ func (f *agentNetworkHandlerFixture) do(t *testing.T, method, path, body string)
|
||||
if body != "" {
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
}
|
||||
for name, value := range headers {
|
||||
req.Header.Set(name, value)
|
||||
}
|
||||
req = nbcontext.SetUserAuthInRequest(req, auth.UserAuth{
|
||||
UserId: testUserID,
|
||||
AccountId: testAccountID,
|
||||
|
||||
@@ -60,12 +60,20 @@ func (h *handler) createSettings(w http.ResponseWriter, r *http.Request) {
|
||||
util.WriteError(r.Context(), err, w)
|
||||
return
|
||||
}
|
||||
// Emitting the validator here lets a client that just bootstrapped issue a
|
||||
// conditional PUT without an intervening GET.
|
||||
util.SetETag(w, created.ETag())
|
||||
util.WriteJSONObject(r.Context(), w, created.ToAPIResponse())
|
||||
}
|
||||
|
||||
// updateSettings replaces the mutable settings fields on the account's row.
|
||||
// A request carrying a cluster bootstraps the row when the account doesn't
|
||||
// have one yet.
|
||||
//
|
||||
// An If-Match header makes the update conditional: it is honoured against the
|
||||
// stored row inside the write's transaction, and a stale validator is refused
|
||||
// with 412 rather than overwriting what changed since the client read. Omitting
|
||||
// the header keeps the pre-existing last-write-wins behaviour.
|
||||
func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
|
||||
userAuth, err := nbcontext.GetUserAuthFromContext(r.Context())
|
||||
if err != nil {
|
||||
@@ -82,11 +90,12 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
|
||||
settings := &types.Settings{AccountID: userAuth.AccountId}
|
||||
settings.FromAPIRequest(&req)
|
||||
|
||||
updated, err := h.manager.UpdateSettings(r.Context(), userAuth.UserId, settings)
|
||||
updated, err := h.manager.UpdateSettings(r.Context(), userAuth.UserId, settings, util.IfMatch(r))
|
||||
if err != nil {
|
||||
util.WriteError(r.Context(), err, w)
|
||||
return
|
||||
}
|
||||
util.SetETag(w, updated.ETag())
|
||||
util.WriteJSONObject(r.Context(), w, updated.ToAPIResponse())
|
||||
}
|
||||
|
||||
@@ -94,6 +103,11 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
|
||||
// The manager refuses (412) while providers exist or a proxy is actively
|
||||
// serving the endpoint; a later POST bootstraps fresh, allocating a new
|
||||
// endpoint.
|
||||
//
|
||||
// An If-Match header makes the delete conditional, and is worth sending here
|
||||
// even more than on update: both existing guards are about state rather than
|
||||
// staleness, so nothing else stops a client from deleting a row that was
|
||||
// replaced since it read one.
|
||||
func (h *handler) deleteSettings(w http.ResponseWriter, r *http.Request) {
|
||||
userAuth, err := nbcontext.GetUserAuthFromContext(r.Context())
|
||||
if err != nil {
|
||||
@@ -101,7 +115,7 @@ func (h *handler) deleteSettings(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
if err := h.manager.DeleteSettings(r.Context(), userAuth.AccountId, userAuth.UserId); err != nil {
|
||||
if err := h.manager.DeleteSettings(r.Context(), userAuth.AccountId, userAuth.UserId, util.IfMatch(r)); err != nil {
|
||||
util.WriteError(r.Context(), err, w)
|
||||
return
|
||||
}
|
||||
@@ -123,5 +137,9 @@ func (h *handler) getSettings(w http.ResponseWriter, r *http.Request) {
|
||||
util.WriteError(r.Context(), err, w)
|
||||
return
|
||||
}
|
||||
// The pre-bootstrap defaults are a representation like any other and carry
|
||||
// a validator too, so an If-Match taken before bootstrap cannot silently
|
||||
// match the row that appeared since.
|
||||
util.SetETag(w, settings.ETag())
|
||||
util.WriteJSONObject(r.Context(), w, settings.ToAPIResponse())
|
||||
}
|
||||
|
||||
@@ -393,3 +393,202 @@ func TestSettingsHandler_DeleteReleasesEndpointForFreshBootstrap(t *testing.T) {
|
||||
"the fresh row must carry bootstrap defaults, not the deleted row's toggles")
|
||||
assert.NotNil(t, second.CreatedAt, "the fresh row is persisted and carries timestamps")
|
||||
}
|
||||
|
||||
// bootstrapForETag bootstraps a settings row and returns the response body
|
||||
// alongside the validator the bootstrap emitted, which is what a client would
|
||||
// carry into its first conditional write.
|
||||
func bootstrapForETag(t *testing.T, f *agentNetworkHandlerFixture) (api.AgentNetworkSettings, string) {
|
||||
t.Helper()
|
||||
|
||||
rec := f.do(t, http.MethodPost, "/agent-network/settings",
|
||||
`{"proxy_address": "eu.proxy.netbird.io", "enable_prompt_collection": true, "redact_pii": true, "access_log_retention_days": 14}`)
|
||||
require.Equal(t, http.StatusOK, rec.Code, "bootstrap POST must succeed: %s", rec.Body.String())
|
||||
|
||||
var settings api.AgentNetworkSettings
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &settings))
|
||||
|
||||
etag := rec.Header().Get("ETag")
|
||||
require.NotEmpty(t, etag, "bootstrap must emit a validator so a client can PUT without an intervening GET")
|
||||
return settings, etag
|
||||
}
|
||||
|
||||
// putBody renders a complete settings update — every field, with the identity
|
||||
// echo the endpoint requires — so the conditional-request tests differ only in
|
||||
// their headers.
|
||||
func putBody(settings api.AgentNetworkSettings, redactPii bool, retention int) string {
|
||||
return fmt.Sprintf(
|
||||
`{"endpoint": %q, "proxy_address": %q, "enable_log_collection": true, "enable_prompt_collection": true, "redact_pii": %t, "access_log_retention_days": %d}`,
|
||||
settings.Endpoint, settings.ProxyAddress, redactPii, retention)
|
||||
}
|
||||
|
||||
// TestSettingsHandler_EmitsETag pins that every read and every write hands the
|
||||
// client back a validator, quoted as a strong entity-tag. Without one on the
|
||||
// write responses a client would have to re-GET after every update to stay
|
||||
// able to make the next one conditional.
|
||||
func TestSettingsHandler_EmitsETag(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
|
||||
// The pre-bootstrap defaults are a representation too, and validate like
|
||||
// one — an If-Match taken here must not match the row that appears later.
|
||||
rec := f.do(t, http.MethodGet, "/agent-network/settings", "")
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
defaultsETag := rec.Header().Get("ETag")
|
||||
assert.NotEmpty(t, defaultsETag, "the unbootstrapped view must carry a validator")
|
||||
|
||||
settings, bootstrapETag := bootstrapForETag(t, f)
|
||||
assert.Regexp(t, `^"[0-9a-f]+"$`, bootstrapETag, "the validator must be a quoted strong entity-tag")
|
||||
assert.NotEqual(t, defaultsETag, bootstrapETag, "bootstrapping must move the validator")
|
||||
|
||||
rec = f.do(t, http.MethodGet, "/agent-network/settings", "")
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
assert.Equal(t, bootstrapETag, rec.Header().Get("ETag"),
|
||||
"reading an unchanged row must derive the same validator the bootstrap returned")
|
||||
|
||||
rec = f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, false, 7))
|
||||
require.Equal(t, http.StatusOK, rec.Code, "update must succeed: %s", rec.Body.String())
|
||||
assert.NotEqual(t, bootstrapETag, rec.Header().Get("ETag"),
|
||||
"an update that changed the representation must return a different validator")
|
||||
}
|
||||
|
||||
// TestSettingsHandler_PutIfMatch walks the conditional-update contract. The
|
||||
// stale case is the one the feature exists for: a client that planned against
|
||||
// an earlier read must be refused rather than silently reverting whatever
|
||||
// changed in between — RedactPii above all, where a silent revert turns a
|
||||
// compliance control off with no error and no drift warning.
|
||||
func TestSettingsHandler_PutIfMatch(t *testing.T) {
|
||||
t.Run("matching validator succeeds", func(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
settings, etag := bootstrapForETag(t, f)
|
||||
|
||||
rec := f.doWithHeaders(t, http.MethodPut, "/agent-network/settings",
|
||||
putBody(settings, false, 7), map[string]string{"If-Match": etag})
|
||||
require.Equal(t, http.StatusOK, rec.Code,
|
||||
"a matching precondition must be honoured: got %d body=%s", rec.Code, rec.Body.String())
|
||||
assert.NotEqual(t, etag, rec.Header().Get("ETag"),
|
||||
"the response must carry the new validator, not the one that was matched")
|
||||
})
|
||||
|
||||
t.Run("stale validator is refused and changes nothing", func(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
settings, stale := bootstrapForETag(t, f)
|
||||
|
||||
// Someone else writes in between — the dashboard operator enabling
|
||||
// something the planning client never saw.
|
||||
rec := f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, true, 21))
|
||||
require.Equal(t, http.StatusOK, rec.Code, "the intervening update must succeed: %s", rec.Body.String())
|
||||
var intervened api.AgentNetworkSettings
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &intervened))
|
||||
|
||||
rec = f.doWithHeaders(t, http.MethodPut, "/agent-network/settings",
|
||||
putBody(settings, false, 7), map[string]string{"If-Match": stale})
|
||||
require.Equal(t, http.StatusPreconditionFailed, rec.Code,
|
||||
"a stale precondition must be refused: got %d body=%s", rec.Code, rec.Body.String())
|
||||
|
||||
// Asserting the state, not just the status: a partial write would pass
|
||||
// a status-only check.
|
||||
rec = f.do(t, http.MethodGet, "/agent-network/settings", "")
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
var after api.AgentNetworkSettings
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &after))
|
||||
assert.Equal(t, intervened, after, "the refused update must leave the row byte-identical")
|
||||
})
|
||||
|
||||
t.Run("star matches the existing row", func(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
settings, _ := bootstrapForETag(t, f)
|
||||
|
||||
rec := f.doWithHeaders(t, http.MethodPut, "/agent-network/settings",
|
||||
putBody(settings, false, 7), map[string]string{"If-Match": "*"})
|
||||
assert.Equal(t, http.StatusOK, rec.Code,
|
||||
"* must match any current representation: got %d body=%s", rec.Code, rec.Body.String())
|
||||
})
|
||||
|
||||
t.Run("no precondition still succeeds", func(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
settings, _ := bootstrapForETag(t, f)
|
||||
|
||||
// The back-compatibility guarantee: clients that predate conditional
|
||||
// requests — the dashboard among them — keep last-write-wins.
|
||||
rec := f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, false, 7))
|
||||
assert.Equal(t, http.StatusOK, rec.Code,
|
||||
"an unconditional update must keep working: got %d body=%s", rec.Code, rec.Body.String())
|
||||
})
|
||||
|
||||
t.Run("precondition is checked before the immutability echo", func(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
settings, stale := bootstrapForETag(t, f)
|
||||
|
||||
rec := f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, true, 21))
|
||||
require.Equal(t, http.StatusOK, rec.Code, "the intervening update must succeed: %s", rec.Body.String())
|
||||
|
||||
// A client stale enough to hold an old validator may be stale in its
|
||||
// identity echo too. Answering 412 tells it the useful thing — go and
|
||||
// read again — where 422 would send it hunting an immutability bug.
|
||||
body := fmt.Sprintf(
|
||||
`{"endpoint": "other.gateway.example.com", "proxy_address": %q, "enable_log_collection": true, "enable_prompt_collection": true, "redact_pii": false, "access_log_retention_days": 7}`,
|
||||
settings.ProxyAddress)
|
||||
rec = f.doWithHeaders(t, http.MethodPut, "/agent-network/settings", body,
|
||||
map[string]string{"If-Match": stale})
|
||||
assert.Equal(t, http.StatusPreconditionFailed, rec.Code,
|
||||
"staleness must be reported ahead of the identity mismatch: got %d body=%s", rec.Code, rec.Body.String())
|
||||
})
|
||||
}
|
||||
|
||||
// TestSettingsHandler_DeleteIfMatch covers the conditional delete, which
|
||||
// carries more weight than the conditional update: both existing delete guards
|
||||
// are about state — no providers, no serving proxy — so nothing else stops a
|
||||
// client from deleting a row that was replaced since it read one.
|
||||
func TestSettingsHandler_DeleteIfMatch(t *testing.T) {
|
||||
t.Run("stale validator is refused and the row survives", func(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
settings, stale := bootstrapForETag(t, f)
|
||||
|
||||
rec := f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, true, 21))
|
||||
require.Equal(t, http.StatusOK, rec.Code, "the intervening update must succeed: %s", rec.Body.String())
|
||||
|
||||
rec = f.doWithHeaders(t, http.MethodDelete, "/agent-network/settings", "",
|
||||
map[string]string{"If-Match": stale})
|
||||
require.Equal(t, http.StatusPreconditionFailed, rec.Code,
|
||||
"a stale precondition must refuse the delete: got %d body=%s", rec.Code, rec.Body.String())
|
||||
|
||||
rec = f.do(t, http.MethodGet, "/agent-network/settings", "")
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
var after api.AgentNetworkSettings
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &after))
|
||||
assert.Equal(t, settings.Endpoint, after.Endpoint, "the refused delete must leave the row in place")
|
||||
})
|
||||
|
||||
t.Run("matching validator deletes", func(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
_, etag := bootstrapForETag(t, f)
|
||||
|
||||
rec := f.doWithHeaders(t, http.MethodDelete, "/agent-network/settings", "",
|
||||
map[string]string{"If-Match": etag})
|
||||
require.Equal(t, http.StatusOK, rec.Code,
|
||||
"a matching precondition must be honoured: got %d body=%s", rec.Code, rec.Body.String())
|
||||
|
||||
rec = f.do(t, http.MethodGet, "/agent-network/settings", "")
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
var after api.AgentNetworkSettings
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &after))
|
||||
assert.Empty(t, after.Endpoint, "the row must be gone")
|
||||
})
|
||||
|
||||
t.Run("precondition is checked before the state guards", func(t *testing.T) {
|
||||
f := newAgentNetworkHandlerFixture(t)
|
||||
settings, stale := bootstrapForETag(t, f)
|
||||
|
||||
rec := f.do(t, http.MethodPut, "/agent-network/settings", putBody(settings, true, 21))
|
||||
require.Equal(t, http.StatusOK, rec.Code, "the intervening update must succeed: %s", rec.Body.String())
|
||||
f.seedProvider(t, "prov-precondition")
|
||||
|
||||
// Both refusals are 412, so the status cannot tell them apart — the
|
||||
// message must, or a stale client is sent to delete providers it may
|
||||
// not even know about.
|
||||
rec = f.doWithHeaders(t, http.MethodDelete, "/agent-network/settings", "",
|
||||
map[string]string{"If-Match": stale})
|
||||
require.Equal(t, http.StatusPreconditionFailed, rec.Code, "the delete must be refused: %s", rec.Body.String())
|
||||
assert.Contains(t, rec.Body.String(), "if-match",
|
||||
"staleness must be reported ahead of the provider guard: %s", rec.Body.String())
|
||||
})
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@ import (
|
||||
"github.com/netbirdio/netbird/management/server/permissions/modules"
|
||||
"github.com/netbirdio/netbird/management/server/permissions/operations"
|
||||
"github.com/netbirdio/netbird/management/server/store"
|
||||
httputil "github.com/netbirdio/netbird/shared/management/http/util"
|
||||
"github.com/netbirdio/netbird/shared/management/status"
|
||||
)
|
||||
|
||||
@@ -71,8 +72,8 @@ type Manager interface {
|
||||
|
||||
GetSettings(ctx context.Context, accountID, userID string) (*types.Settings, error)
|
||||
CreateSettings(ctx context.Context, userID string, settings *types.Settings, proxyAddress, endpoint string) (*types.Settings, error)
|
||||
UpdateSettings(ctx context.Context, userID string, settings *types.Settings) (*types.Settings, error)
|
||||
DeleteSettings(ctx context.Context, accountID, userID string) error
|
||||
UpdateSettings(ctx context.Context, userID string, settings *types.Settings, precondition *httputil.Precondition) (*types.Settings, error)
|
||||
DeleteSettings(ctx context.Context, accountID, userID string, precondition *httputil.Precondition) error
|
||||
|
||||
ListConsumption(ctx context.Context, accountID, userID string) ([]*types.Consumption, error)
|
||||
ListAccessLogs(ctx context.Context, accountID, userID string, filter types.AgentNetworkAccessLogFilter) ([]*types.AgentNetworkAccessLog, int64, error)
|
||||
@@ -544,6 +545,13 @@ func (m *managerImpl) DeleteBudgetRule(ctx context.Context, accountID, userID, r
|
||||
return nil
|
||||
}
|
||||
|
||||
// stalePreconditionMsg is the refusal both conditional settings writes return.
|
||||
// Shared so the two cannot drift: DeleteSettings answers 412 for its state
|
||||
// guards as well, so the message is the only thing telling a client that it is
|
||||
// working from an old read rather than tripping over providers or a serving
|
||||
// proxy.
|
||||
const stalePreconditionMsg = "if-match precondition failed: the settings have changed since they were read; GET them again and retry"
|
||||
|
||||
// UpdateSettings replaces the mutable account-level settings — the collection
|
||||
// toggles and retention — on the account's row. The identity fields (Domain,
|
||||
// ProxyAddress) are assigned at bootstrap (CreateSettings) and immutable: the
|
||||
@@ -554,7 +562,11 @@ func (m *managerImpl) DeleteBudgetRule(ctx context.Context, accountID, userID, r
|
||||
// Because the collection toggles change the synthesised service config
|
||||
// (prompt-capture gating, access-log emission), a reconcile is triggered so
|
||||
// the proxy and peer network maps converge on the new state.
|
||||
func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, settings *types.Settings) (*types.Settings, error) {
|
||||
//
|
||||
// precondition carries the caller's If-Match, and is nil for an unconditional
|
||||
// update — last write wins, which is what the dashboard wants and what every
|
||||
// client that predates conditional requests gets.
|
||||
func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, settings *types.Settings, precondition *httputil.Precondition) (*types.Settings, error) {
|
||||
if err := m.requirePermission(ctx, settings.AccountID, userID, modules.AgentNetworkSettings, operations.Update); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -573,6 +585,20 @@ func (m *managerImpl) UpdateSettings(ctx context.Context, userID string, setting
|
||||
return fmt.Errorf("get agent network settings: %w", err)
|
||||
}
|
||||
|
||||
// Evaluated here, under the row lock and inside the write's own
|
||||
// transaction, rather than in the handler: comparing before the
|
||||
// transaction only narrows the race, since two requests can both pass
|
||||
// the check before either writes. Locking the row first makes it a
|
||||
// genuine compare-and-set.
|
||||
//
|
||||
// It comes before the identity comparison because a client holding a
|
||||
// stale validator is stale in its identity echo too, and "you are
|
||||
// working from an old read" is the more accurate answer than "the
|
||||
// endpoint is immutable".
|
||||
if !precondition.Matches(existing.ETag()) {
|
||||
return status.Errorf(status.PreconditionFailed, "%s", stalePreconditionMsg)
|
||||
}
|
||||
|
||||
// The identity echo is compared leniently (trimmed, case-insensitive):
|
||||
// the stored values are normalized lowercase, and a client replaying a
|
||||
// GET response must never be rejected over casing it didn't choose.
|
||||
@@ -635,7 +661,12 @@ func hostnamesEquivalent(supplied, stored string) bool {
|
||||
// is not reserved. That full-reset semantic is what gives clients that model
|
||||
// immutability as replace-on-change (e.g. Terraform's RequiresReplace) a real
|
||||
// path: tear down providers, delete, re-create.
|
||||
func (m *managerImpl) DeleteSettings(ctx context.Context, accountID, userID string) error {
|
||||
//
|
||||
// precondition carries the caller's If-Match, and is nil for an unconditional
|
||||
// delete. It matters more here than on update: the two guards above are about
|
||||
// state rather than staleness, so without it nothing stops a client from
|
||||
// deleting a row that was replaced since it last read one.
|
||||
func (m *managerImpl) DeleteSettings(ctx context.Context, accountID, userID string, precondition *httputil.Precondition) error {
|
||||
if err := m.requirePermission(ctx, accountID, userID, modules.AgentNetworkSettings, operations.Delete); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -651,6 +682,13 @@ func (m *managerImpl) DeleteSettings(ctx context.Context, accountID, userID stri
|
||||
return fmt.Errorf("get agent network settings: %w", err)
|
||||
}
|
||||
|
||||
// Under the row lock, for the same reason as in UpdateSettings, and
|
||||
// before the state guards: a caller working from an old read should
|
||||
// learn that first, not be told about providers it may not know exist.
|
||||
if !precondition.Matches(existing.ETag()) {
|
||||
return status.Errorf(status.PreconditionFailed, "%s", stalePreconditionMsg)
|
||||
}
|
||||
|
||||
providers, err := tx.GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, accountID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("get agent network providers: %w", err)
|
||||
@@ -1100,11 +1138,13 @@ func (*mockManager) CreateSettings(_ context.Context, _ string, s *types.Setting
|
||||
return s, nil
|
||||
}
|
||||
|
||||
func (*mockManager) UpdateSettings(_ context.Context, _ string, s *types.Settings) (*types.Settings, error) {
|
||||
func (*mockManager) UpdateSettings(_ context.Context, _ string, s *types.Settings, _ *httputil.Precondition) (*types.Settings, error) {
|
||||
return s, nil
|
||||
}
|
||||
|
||||
func (*mockManager) DeleteSettings(_ context.Context, _, _ string) error { return nil }
|
||||
func (*mockManager) DeleteSettings(_ context.Context, _, _ string, _ *httputil.Precondition) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (*mockManager) ListConsumption(_ context.Context, _, _ string) ([]*types.Consumption, error) {
|
||||
return nil, nil
|
||||
|
||||
199
management/internals/modules/agentnetwork/settings_etag_test.go
Normal file
199
management/internals/modules/agentnetwork/settings_etag_test.go
Normal file
@@ -0,0 +1,199 @@
|
||||
package agentnetwork
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strconv"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types"
|
||||
"github.com/netbirdio/netbird/management/server/permissions/modules"
|
||||
"github.com/netbirdio/netbird/management/server/permissions/operations"
|
||||
"github.com/netbirdio/netbird/management/server/store"
|
||||
httputil "github.com/netbirdio/netbird/shared/management/http/util"
|
||||
"github.com/netbirdio/netbird/shared/management/status"
|
||||
)
|
||||
|
||||
// ifMatch builds the precondition a client sending this validator would
|
||||
// produce, by going through the same header parse the handler uses rather than
|
||||
// reaching past it.
|
||||
func ifMatch(t *testing.T, etag string) *httputil.Precondition {
|
||||
t.Helper()
|
||||
|
||||
r := httptest.NewRequest(http.MethodPut, "/", nil)
|
||||
r.Header.Set("If-Match", strconv.Quote(etag))
|
||||
return httputil.IfMatch(r)
|
||||
}
|
||||
|
||||
// updateFor renders a complete update for the given row, echoing the identity
|
||||
// fields the endpoint requires and setting retention to tell writers apart.
|
||||
func updateFor(settings *types.Settings, retention int) *types.Settings {
|
||||
return &types.Settings{
|
||||
AccountID: settings.AccountID,
|
||||
Domain: settings.Domain,
|
||||
ProxyAddress: settings.ProxyAddress,
|
||||
EnableLogCollection: true,
|
||||
EnablePromptCollection: true,
|
||||
RedactPii: true,
|
||||
AccessLogRetentionDays: retention,
|
||||
}
|
||||
}
|
||||
|
||||
// TestUpdateSettingsPreconditionSerializesConcurrentWriters is the test the
|
||||
// design rests on. Two writers start from the same validator and race; exactly
|
||||
// one may win.
|
||||
//
|
||||
// An implementation that compares the validator before opening the write
|
||||
// transaction passes every sequential test in this suite and fails here: both
|
||||
// writers read the same row, both find their precondition satisfied, and both
|
||||
// then write — which is the lost update the feature exists to prevent, merely
|
||||
// narrowed to a smaller window. Holding the row under LockingStrengthUpdate
|
||||
// and comparing inside the write's own transaction is what makes it a genuine
|
||||
// compare-and-set.
|
||||
//
|
||||
// The test store is sqlite, which serializes writers of its own accord, so
|
||||
// what this pins directly is the outcome — exactly one success — rather than
|
||||
// the mechanism. It still has teeth against the check-before-transaction
|
||||
// shape, whose two reads interleave freely before either write. Running it
|
||||
// against postgres (NB_STORE_ENGINE_POSTGRES_DSN) exercises real concurrent
|
||||
// transactions.
|
||||
func TestUpdateSettingsPreconditionSerializesConcurrentWriters(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
f := newBootstrapFixture(t)
|
||||
|
||||
const accountID, userID = "account1", "user1"
|
||||
f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Create, true)
|
||||
f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Update, true)
|
||||
f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Update, true)
|
||||
|
||||
created, err := f.createSettings(ctx, accountID, userID, "cluster1.example.com", "")
|
||||
require.NoError(t, err, "bootstrap must succeed")
|
||||
|
||||
// Both writers plan against this one read, as a client that read, computed
|
||||
// a diff and is about to write the whole object back would.
|
||||
shared := created.ETag()
|
||||
|
||||
// noWrite is a retention value neither writer sends and the API would
|
||||
// never store, so an assertion that lands on it is a test bug rather than
|
||||
// a silently satisfied comparison. Zero would not do: the API documents 0
|
||||
// as "keep indefinitely", so it is a value the row could legitimately hold.
|
||||
const noWrite = -1
|
||||
|
||||
var (
|
||||
wg sync.WaitGroup
|
||||
start = make(chan struct{})
|
||||
errs = make([]error, 2)
|
||||
wrote = []int{7, 21}
|
||||
returned = []int{noWrite, noWrite}
|
||||
)
|
||||
for i := range 2 {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
<-start
|
||||
updated, err := f.manager.UpdateSettings(ctx, userID, updateFor(created, wrote[i]), ifMatch(t, shared))
|
||||
errs[i] = err
|
||||
if err == nil {
|
||||
returned[i] = updated.AccessLogRetentionDays
|
||||
}
|
||||
}()
|
||||
}
|
||||
close(start)
|
||||
wg.Wait()
|
||||
|
||||
succeeded, winner := 0, noWrite
|
||||
for i, err := range errs {
|
||||
if err == nil {
|
||||
succeeded++
|
||||
winner = wrote[i]
|
||||
assert.Equal(t, wrote[i], returned[i], "the winner's response must carry what it sent")
|
||||
continue
|
||||
}
|
||||
assert.Truef(t, isPreconditionFailed(err),
|
||||
"the losing writer must be refused for staleness, got: %v (writer %d)", err, i)
|
||||
}
|
||||
require.Equal(t, 1, succeeded, "exactly one writer may win: %v", errs)
|
||||
|
||||
// The row must carry the winner's value and nothing blended.
|
||||
stored, err := f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID)
|
||||
require.NoError(t, err, "the row must survive the race")
|
||||
assert.Equal(t, winner, stored.AccessLogRetentionDays,
|
||||
"the stored row must be exactly what the winning writer sent")
|
||||
assert.NotEqual(t, shared, stored.ETag(), "the surviving row must derive a new validator")
|
||||
}
|
||||
|
||||
// TestUpdateSettingsUnconditionalIgnoresStaleness pins the back-compatibility
|
||||
// half: without a precondition the manager keeps last-write-wins, which is
|
||||
// what the dashboard relies on and what any client that predates conditional
|
||||
// requests does.
|
||||
func TestUpdateSettingsUnconditionalIgnoresStaleness(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
f := newBootstrapFixture(t)
|
||||
|
||||
const accountID, userID = "account1", "user1"
|
||||
f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Create, true)
|
||||
f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Update, true)
|
||||
f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Update, true)
|
||||
|
||||
created, err := f.createSettings(ctx, accountID, userID, "cluster1.example.com", "")
|
||||
require.NoError(t, err, "bootstrap must succeed")
|
||||
|
||||
_, err = f.manager.UpdateSettings(ctx, userID, updateFor(created, 21), nil)
|
||||
require.NoError(t, err, "the first unconditional update must succeed")
|
||||
|
||||
// The second writer is working from a read that is now stale, and with no
|
||||
// precondition it overwrites regardless.
|
||||
updated, err := f.manager.UpdateSettings(ctx, userID, updateFor(created, 7), nil)
|
||||
require.NoError(t, err, "an unconditional update must not be refused for staleness")
|
||||
assert.Equal(t, 7, updated.AccessLogRetentionDays, "last write wins without a precondition")
|
||||
}
|
||||
|
||||
// TestDeleteSettingsPreconditionRefusesStale pins the conditional delete at
|
||||
// the manager level: a stale validator refuses, and the row is still there
|
||||
// afterwards. Deletion is the destructive operation and its two other guards
|
||||
// are about state rather than staleness, so this is the only thing standing
|
||||
// between a client working from an old read and a released endpoint.
|
||||
func TestDeleteSettingsPreconditionRefusesStale(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
f := newBootstrapFixture(t)
|
||||
|
||||
const accountID, userID = "account1", "user1"
|
||||
f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Create, true)
|
||||
f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Update, true)
|
||||
f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Delete, true)
|
||||
f.expectPermission(accountID, userID, modules.AgentNetworkSettings, operations.Delete, true)
|
||||
|
||||
created, err := f.createSettings(ctx, accountID, userID, "cluster1.example.com", "")
|
||||
require.NoError(t, err, "bootstrap must succeed")
|
||||
stale := created.ETag()
|
||||
|
||||
updated, err := f.manager.UpdateSettings(ctx, userID, updateFor(created, 21), nil)
|
||||
require.NoError(t, err, "the intervening update must succeed")
|
||||
|
||||
err = f.manager.DeleteSettings(ctx, accountID, userID, ifMatch(t, stale))
|
||||
require.Error(t, err, "a stale precondition must refuse the delete")
|
||||
assert.True(t, isPreconditionFailed(err), "the refusal must be a precondition failure, got: %v", err)
|
||||
|
||||
stored, err := f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID)
|
||||
require.NoError(t, err, "the refused delete must leave the row in place")
|
||||
assert.Equal(t, created.Domain, stored.Domain, "the endpoint must not have been released")
|
||||
|
||||
// The validator the intervening update returned is the current one, and
|
||||
// deleting with it goes through.
|
||||
require.NoError(t, f.manager.DeleteSettings(ctx, accountID, userID, ifMatch(t, updated.ETag())),
|
||||
"a matching precondition must be honoured")
|
||||
_, err = f.store.GetAgentNetworkSettings(ctx, store.LockingStrengthNone, accountID)
|
||||
assert.Error(t, err, "the row must be gone")
|
||||
}
|
||||
|
||||
// isPreconditionFailed reports whether err is the 412-mapped status error.
|
||||
func isPreconditionFailed(err error) bool {
|
||||
var sErr *status.Error
|
||||
return errors.As(err, &sErr) && sErr.Type() == status.PreconditionFailed
|
||||
}
|
||||
@@ -1,6 +1,8 @@
|
||||
package types
|
||||
|
||||
import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"fmt"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -67,6 +69,64 @@ func DefaultSettings(accountID string) *Settings {
|
||||
}
|
||||
}
|
||||
|
||||
// etagLength is how much of the hash the validator carries. 16 hex characters
|
||||
// — 64 bits — is far more than enough to make an accidental collision between
|
||||
// two representations of one account's settings unreachable, and keeps the
|
||||
// header short enough to read in a log line.
|
||||
const etagLength = 16
|
||||
|
||||
// ETag returns a strong validator over the settings representation, for
|
||||
// conditional requests (RFC 9110 If-Match). The value is unquoted; applying
|
||||
// the quoting is the transport layer's job.
|
||||
//
|
||||
// The hash covers an explicit field tuple rather than the marshalled API
|
||||
// representation: field ordering in the generated API types is not a contract,
|
||||
// so hashing serialized output would make the validator churn with codegen.
|
||||
// Two exclusions are deliberate:
|
||||
//
|
||||
// - AccountID identifies the resource — it is the URL, not the
|
||||
// representation. Including it would make the validator differ between
|
||||
// accounts whose settings are genuinely identical, which no client can
|
||||
// observe and no precondition needs.
|
||||
// - UpdatedAt is excluded so that equal representations always yield equal
|
||||
// validators. A write that changes nothing must not invalidate a
|
||||
// precondition another client is holding.
|
||||
//
|
||||
// Everything else is in, including the identity fields and CreatedAt. A
|
||||
// validator that covered only the mutable toggles would survive a delete
|
||||
// followed by a fresh bootstrap onto the same toggle values, and an If-Match
|
||||
// held across that gap would then authorize a write against what is really a
|
||||
// different resource. CreatedAt is what distinguishes the re-bootstrapped row.
|
||||
//
|
||||
// CreatedAt is hashed at whole-second precision because the validator has to
|
||||
// agree across a store round-trip. A freshly bootstrapped row derives its
|
||||
// validator in memory, from a time.Time carrying nanoseconds, while every
|
||||
// later comparison derives it from a row read back out of the store — and the
|
||||
// engines truncate: PostgreSQL to microseconds, MySQL DATETIME to whole
|
||||
// seconds without an fsp. At nanosecond precision the two never agree again,
|
||||
// so the validator a bootstrap hands out is permanently unusable. Seconds is
|
||||
// the floor every supported engine preserves. The cost is that a delete and
|
||||
// re-bootstrap within the same second, onto the same endpoint and the same
|
||||
// toggles, derives the same validator; a labeled bootstrap draws a fresh
|
||||
// random label, so that needs a self-addressed endpoint reclaimed inside one
|
||||
// second.
|
||||
//
|
||||
// Adding a field to Settings means deciding whether it belongs here; the
|
||||
// field-count guard in the tests is what forces that decision.
|
||||
func (s *Settings) ETag() string {
|
||||
h := sha256.New()
|
||||
fmt.Fprintf(h, "%s\x00%s\x00%t\x00%t\x00%t\x00%d\x00%d",
|
||||
s.Domain,
|
||||
s.ProxyAddress,
|
||||
s.EnableLogCollection,
|
||||
s.EnablePromptCollection,
|
||||
s.RedactPii,
|
||||
s.AccessLogRetentionDays,
|
||||
s.CreatedAt.Unix(),
|
||||
)
|
||||
return hex.EncodeToString(h.Sum(nil))[:etagLength]
|
||||
}
|
||||
|
||||
// Endpoint returns the bare hostname agents reach this account at — the
|
||||
// Domain column. Empty until the row is bootstrapped.
|
||||
func (s *Settings) Endpoint() string { return s.Domain }
|
||||
|
||||
184
management/internals/modules/agentnetwork/types/settings_test.go
Normal file
184
management/internals/modules/agentnetwork/types/settings_test.go
Normal file
@@ -0,0 +1,184 @@
|
||||
package types
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// etagSettings is a fully populated settings row — every hashed field set to a
|
||||
// distinctive value — so a mutation test can flip exactly one thing at a time.
|
||||
// The timestamp carries sub-second precision on purpose: a whole-second value
|
||||
// would make the precision test below pass without proving anything.
|
||||
func etagSettings() *Settings {
|
||||
created := time.Date(2026, 8, 11, 9, 30, 0, 123456789, time.UTC)
|
||||
return &Settings{
|
||||
AccountID: "acc-1",
|
||||
Domain: "cool-otter.eu.proxy.netbird.io",
|
||||
ProxyAddress: "eu.proxy.netbird.io",
|
||||
EnableLogCollection: true,
|
||||
EnablePromptCollection: true,
|
||||
RedactPii: true,
|
||||
AccessLogRetentionDays: 30,
|
||||
CreatedAt: created,
|
||||
UpdatedAt: created,
|
||||
}
|
||||
}
|
||||
|
||||
// TestSettings_ETagShape pins the wire shape of the validator: a bare
|
||||
// lowercase hex string of the documented length, with no quoting — quoting is
|
||||
// the transport layer's job, and a validator that arrived pre-quoted would be
|
||||
// double-quoted on the way out.
|
||||
func TestSettings_ETagShape(t *testing.T) {
|
||||
etag := etagSettings().ETag()
|
||||
|
||||
assert.Len(t, etag, etagLength, "the validator must be exactly etagLength characters")
|
||||
assert.NotContains(t, etag, `"`, "the derived validator must not carry its own quoting")
|
||||
for _, r := range etag {
|
||||
require.Truef(t, (r >= '0' && r <= '9') || (r >= 'a' && r <= 'f'),
|
||||
"the validator must be lowercase hex, got %q in %q", r, etag)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSettings_ETagIsStable covers the guarantee every conditional request
|
||||
// rests on: an unchanged row derives the same validator every time, including
|
||||
// across a fresh struct built from the same values. A validator that varied
|
||||
// per derivation would fail every If-Match and make the feature unusable.
|
||||
func TestSettings_ETagIsStable(t *testing.T) {
|
||||
s := etagSettings()
|
||||
|
||||
first := s.ETag()
|
||||
assert.Equal(t, first, s.ETag(), "repeated derivation from one value must agree")
|
||||
assert.Equal(t, first, etagSettings().ETag(), "an equal row must derive an equal validator")
|
||||
}
|
||||
|
||||
// TestSettings_ETagSensitivity is the other half of the contract: every field
|
||||
// the validator covers must actually move it. The cases are also what makes
|
||||
// the field-count guard meaningful — a new field that belongs in the tuple but
|
||||
// is missing from it has no case here, and the guard is what catches that.
|
||||
//
|
||||
// The mutations are checked to be pairwise distinct, not merely different from
|
||||
// the baseline: that is what catches an ambiguous concatenation, where moving
|
||||
// a character across a field boundary would hash identically without the
|
||||
// delimiter.
|
||||
func TestSettings_ETagSensitivity(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
mutate func(*Settings)
|
||||
}{
|
||||
{"domain", func(s *Settings) { s.Domain = "brave-otter.eu.proxy.netbird.io" }},
|
||||
{"proxy address", func(s *Settings) { s.ProxyAddress = "us.proxy.netbird.io" }},
|
||||
{"log collection", func(s *Settings) { s.EnableLogCollection = false }},
|
||||
{"prompt collection", func(s *Settings) { s.EnablePromptCollection = false }},
|
||||
{"redact pii", func(s *Settings) { s.RedactPii = false }},
|
||||
{"retention", func(s *Settings) { s.AccessLogRetentionDays = 14 }},
|
||||
{"created at", func(s *Settings) { s.CreatedAt = s.CreatedAt.Add(time.Second) }},
|
||||
// Moving characters across the Domain/ProxyAddress boundary leaves
|
||||
// the two fields' concatenation byte-identical, so this case passes
|
||||
// only because the tuple is delimited.
|
||||
{"identity boundary shifted", func(s *Settings) {
|
||||
joined := s.Domain + s.ProxyAddress
|
||||
split := len(s.Domain) - 3
|
||||
s.Domain, s.ProxyAddress = joined[:split], joined[split:]
|
||||
}},
|
||||
}
|
||||
|
||||
baseline := etagSettings().ETag()
|
||||
seen := map[string]string{"baseline": baseline}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
s := etagSettings()
|
||||
tc.mutate(s)
|
||||
|
||||
etag := s.ETag()
|
||||
assert.NotEqual(t, baseline, etag, "changing %s must change the validator", tc.name)
|
||||
|
||||
if other, clash := seen[etag]; clash {
|
||||
t.Fatalf("changing %s derives the same validator as %s (%s) — the field tuple is ambiguous", tc.name, other, etag)
|
||||
}
|
||||
seen[etag] = tc.name
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestSettings_ETagExclusions pins the two deliberate omissions. AccountID is
|
||||
// the resource's identity rather than its representation. UpdatedAt is left
|
||||
// out so that a write which changes nothing observable does not invalidate a
|
||||
// precondition another client is holding — equal representations must always
|
||||
// derive equal validators.
|
||||
func TestSettings_ETagExclusions(t *testing.T) {
|
||||
baseline := etagSettings().ETag()
|
||||
|
||||
other := etagSettings()
|
||||
other.AccountID = "acc-2"
|
||||
assert.Equal(t, baseline, other.ETag(), "the account id must not reach the validator")
|
||||
|
||||
touched := etagSettings()
|
||||
touched.UpdatedAt = touched.UpdatedAt.Add(time.Hour)
|
||||
assert.Equal(t, baseline, touched.ETag(), "a write that changed nothing must not move the validator")
|
||||
}
|
||||
|
||||
// TestSettings_ETagSurvivesTimestampTruncation pins the store round-trip the
|
||||
// validator has to survive. A freshly bootstrapped row derives its validator
|
||||
// in memory, from a time.Time carrying nanoseconds; every later comparison
|
||||
// derives it from a row read back out of the store, and the engines truncate
|
||||
// on the way through — PostgreSQL to microseconds, MySQL DATETIME to whole
|
||||
// seconds without an fsp. If the hash is sensitive below its coarsest engine's
|
||||
// precision, the validator a bootstrap hands out never matches again and the
|
||||
// documented "conditional PUT without an intervening GET" is a permanent 412.
|
||||
//
|
||||
// Asserted on the type rather than through a store, so it holds without running
|
||||
// the suite against every engine. The sqlite test store preserves nanoseconds,
|
||||
// so a sqlite-only suite cannot observe the truncation at all.
|
||||
func TestSettings_ETagSurvivesTimestampTruncation(t *testing.T) {
|
||||
inMemory := etagSettings()
|
||||
require.NotZero(t, inMemory.CreatedAt.Nanosecond(), "the fixture must carry sub-second precision to prove anything")
|
||||
|
||||
for name, truncation := range map[string]time.Duration{
|
||||
"postgres (microseconds)": time.Microsecond,
|
||||
"mysql (milliseconds)": time.Millisecond,
|
||||
"mysql datetime (seconds)": time.Second,
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
roundTripped := etagSettings()
|
||||
roundTripped.CreatedAt = roundTripped.CreatedAt.Truncate(truncation)
|
||||
|
||||
assert.Equal(t, inMemory.ETag(), roundTripped.ETag(),
|
||||
"a validator derived before the write must still match one derived after reading the row back")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestSettings_ETagOfDefaults covers the pre-bootstrap view, which GET serves
|
||||
// as a real representation and therefore validates like one. It must derive
|
||||
// without panicking on the zero CreatedAt, and it must not collide with a
|
||||
// bootstrapped row — otherwise an If-Match taken before bootstrap would
|
||||
// authorize a write against the row that appeared since.
|
||||
func TestSettings_ETagOfDefaults(t *testing.T) {
|
||||
defaults := DefaultSettings("acc-1").ETag()
|
||||
|
||||
assert.Len(t, defaults, etagLength, "the default view must derive a well-formed validator")
|
||||
assert.NotEqual(t, etagSettings().ETag(), defaults,
|
||||
"the unbootstrapped view must not validate as a bootstrapped row")
|
||||
}
|
||||
|
||||
// etagFieldCount is the number of fields Settings carries. ETag hashes an
|
||||
// explicit tuple rather than the struct, so a field added here is silently
|
||||
// outside the validator until someone decides otherwise — the worst kind of
|
||||
// gap, because the mechanism looks present and works for every other field.
|
||||
//
|
||||
// If this constant needs updating, that is the decision point: either add the
|
||||
// new field to ETag and give it a case in TestSettings_ETagSensitivity, or
|
||||
// record here why it stays out.
|
||||
const etagFieldCount = 9
|
||||
|
||||
// TestSettings_ETagFieldCountGuard fails when a field is added to or removed
|
||||
// from Settings, forcing the question of whether it belongs in the validator.
|
||||
func TestSettings_ETagFieldCountGuard(t *testing.T) {
|
||||
assert.Equal(t, etagFieldCount, reflect.TypeFor[Settings]().NumField(),
|
||||
"Settings gained or lost a field: decide whether it belongs in ETag(), then update etagFieldCount")
|
||||
}
|
||||
@@ -117,7 +117,7 @@ func TestAgentNetwork_UpdateSettings_PreservesImmutableAndTogglesCollection(t *t
|
||||
EnablePromptCollection: true,
|
||||
RedactPii: true,
|
||||
AccessLogRetentionDays: before.AccessLogRetentionDays,
|
||||
})
|
||||
}, nil)
|
||||
require.NoError(t, err, "UpdateSettings must succeed")
|
||||
assert.Equal(t, before.Domain, updated.Domain, "domain is immutable and must be preserved")
|
||||
assert.Equal(t, before.ProxyAddress, updated.ProxyAddress, "proxy address is immutable and must be preserved")
|
||||
@@ -147,7 +147,7 @@ func TestAgentNetwork_UpdateSettings_PreservesImmutableAndTogglesCollection(t *t
|
||||
EnablePromptCollection: false,
|
||||
RedactPii: false,
|
||||
AccessLogRetentionDays: before.AccessLogRetentionDays,
|
||||
})
|
||||
}, nil)
|
||||
assert.Error(t, err, "a mismatched identity echo must be rejected")
|
||||
assert.ErrorContains(t, err, "immutable", "the rejection must name the immutability rule")
|
||||
})
|
||||
|
||||
@@ -98,7 +98,7 @@ func NewAPIHandler(ctx context.Context, router *mux.Router, accountManager accou
|
||||
isValidChildAccount,
|
||||
)
|
||||
|
||||
corsMiddleware := cors.AllowAll()
|
||||
corsMiddleware := newCORSMiddleware()
|
||||
|
||||
metricsMiddleware := appMetrics.HTTPMiddleware()
|
||||
|
||||
@@ -145,3 +145,32 @@ func NewAPIHandler(ctx context.Context, router *mux.Router, accountManager accou
|
||||
|
||||
return router, nil
|
||||
}
|
||||
|
||||
// newCORSMiddleware builds the API's CORS policy: cors.AllowAll() plus ETag in
|
||||
// ExposedHeaders.
|
||||
//
|
||||
// The addition is what makes conditional requests usable from a browser. A
|
||||
// response header that is not CORS-safelisted is invisible to JavaScript
|
||||
// unless it is named in Access-Control-Expose-Headers, and ETag is not on that
|
||||
// list — so without this the server can hand a browser client a validator it
|
||||
// has no way to read, leaving conditional requests to non-browser clients
|
||||
// only. If-Match needs nothing further, since AllowedHeaders is already "*".
|
||||
//
|
||||
// Everything else mirrors cors.AllowAll() exactly. It is spelled out rather
|
||||
// than called because the library offers no way to extend it.
|
||||
func newCORSMiddleware() *cors.Cors {
|
||||
return cors.New(cors.Options{
|
||||
AllowedOrigins: []string{"*"},
|
||||
AllowedMethods: []string{
|
||||
http.MethodHead,
|
||||
http.MethodGet,
|
||||
http.MethodPost,
|
||||
http.MethodPut,
|
||||
http.MethodPatch,
|
||||
http.MethodDelete,
|
||||
},
|
||||
AllowedHeaders: []string{"*"},
|
||||
ExposedHeaders: []string{"ETag"},
|
||||
AllowCredentials: false,
|
||||
})
|
||||
}
|
||||
|
||||
88
management/server/http/handler_test.go
Normal file
88
management/server/http/handler_test.go
Normal file
@@ -0,0 +1,88 @@
|
||||
package http
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestCORSExposesETag pins the reason this policy is spelled out instead of
|
||||
// being cors.AllowAll(). ETag is not a CORS-safelisted response header, so
|
||||
// without it named in Access-Control-Expose-Headers a browser client is handed
|
||||
// a validator it cannot read — conditional requests would work for the CLI,
|
||||
// the REST client and Terraform, and silently not for the dashboard.
|
||||
//
|
||||
// Collapsing this back to cors.AllowAll() is exactly the simplification that
|
||||
// would reintroduce that, which is what this test is here to catch.
|
||||
func TestCORSExposesETag(t *testing.T) {
|
||||
handler := newCORSMiddleware().Handler(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("ETag", `"9f86d081884c7d65"`)
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/agent-network/settings", nil)
|
||||
req.Header.Set("Origin", "https://app.netbird.io")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
|
||||
// Compared canonicalized: the library normalizes the name it echoes, so
|
||||
// this reads "Etag" rather than "ETag". Browsers match the exposed-header
|
||||
// list case-insensitively, so the spelling does not matter — but asserting
|
||||
// it byte-exactly would fail for a reason that has nothing to do with the
|
||||
// behaviour being pinned.
|
||||
assert.Equal(t, http.CanonicalHeaderKey("ETag"),
|
||||
http.CanonicalHeaderKey(rec.Header().Get("Access-Control-Expose-Headers")),
|
||||
"browser clients must be allowed to read the validator they are sent")
|
||||
}
|
||||
|
||||
// TestCORSAllowsIfMatchPreflight covers the request half. It needs nothing
|
||||
// beyond the wildcard AllowedHeaders that was already there, so this is a
|
||||
// regression guard rather than a new grant: narrowing AllowedHeaders to a list
|
||||
// later must not drop If-Match and leave writes readable but not conditional.
|
||||
func TestCORSAllowsIfMatchPreflight(t *testing.T) {
|
||||
handler := newCORSMiddleware().Handler(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
req := httptest.NewRequest(http.MethodOptions, "/api/agent-network/settings", nil)
|
||||
req.Header.Set("Origin", "https://app.netbird.io")
|
||||
req.Header.Set("Access-Control-Request-Method", http.MethodPut)
|
||||
req.Header.Set("Access-Control-Request-Headers", "If-Match")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
assert.Contains(t, rec.Header().Get("Access-Control-Allow-Headers"), "If-Match",
|
||||
"a conditional write must survive preflight")
|
||||
assert.Contains(t, rec.Header().Get("Access-Control-Allow-Methods"), http.MethodPut,
|
||||
"the conditional write's method must survive preflight")
|
||||
}
|
||||
|
||||
// TestCORSMatchesAllowAllOtherwise pins the rest of the policy, which is a
|
||||
// verbatim copy of cors.AllowAll(). Spelling the options out is what let ETag
|
||||
// be added; it also means a change to the library's defaults no longer reaches
|
||||
// this API, so the settings that matter are asserted here rather than assumed.
|
||||
func TestCORSMatchesAllowAllOtherwise(t *testing.T) {
|
||||
handler := newCORSMiddleware().Handler(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
req := httptest.NewRequest(http.MethodOptions, "/api/peers", nil)
|
||||
req.Header.Set("Origin", "https://anywhere.example.com")
|
||||
req.Header.Set("Access-Control-Request-Method", http.MethodDelete)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
assert.Equal(t, "*", rec.Header().Get("Access-Control-Allow-Origin"), "any origin must still be allowed")
|
||||
assert.Empty(t, rec.Header().Get("Access-Control-Allow-Credentials"),
|
||||
"credentials must stay disallowed — allowing them alongside a wildcard origin would be a real weakening")
|
||||
assert.Contains(t, rec.Header().Get("Access-Control-Allow-Methods"), http.MethodDelete,
|
||||
"the full method set must still be allowed")
|
||||
}
|
||||
@@ -6,6 +6,8 @@ import (
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/netbirdio/netbird/shared/management/http/api"
|
||||
)
|
||||
@@ -336,25 +338,35 @@ func (a *AgentNetworkAPI) DeleteBudgetRule(ctx context.Context, ruleID string) e
|
||||
// to an APIError matchable via IsNotFound rather than fabricating defaults
|
||||
// the server never stated.
|
||||
func (a *AgentNetworkAPI) GetSettings(ctx context.Context) (*api.AgentNetworkSettings, error) {
|
||||
settings, _, err := a.GetSettingsWithETag(ctx)
|
||||
return settings, err
|
||||
}
|
||||
|
||||
// GetSettingsWithETag is GetSettings, additionally returning the entity-tag
|
||||
// the server derived for the settings it returned. Hand that validator to
|
||||
// UpdateSettingsIfMatch or DeleteSettingsIfMatch to make the write conditional
|
||||
// on nothing having changed in between — the read-modify-write cycle that
|
||||
// otherwise silently reverts a concurrent change.
|
||||
func (a *AgentNetworkAPI) GetSettingsWithETag(ctx context.Context) (*api.AgentNetworkSettings, string, error) {
|
||||
resp, err := a.c.NewRequest(ctx, "GET", "/api/agent-network/settings", nil, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, "", err
|
||||
}
|
||||
if resp.Body != nil {
|
||||
defer resp.Body.Close()
|
||||
}
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, "", err
|
||||
}
|
||||
if trimmed := bytes.TrimSpace(body); len(trimmed) == 0 || bytes.Equal(trimmed, []byte("null")) {
|
||||
return nil, &APIError{StatusCode: http.StatusNotFound, Message: "agent network settings not found"}
|
||||
return nil, "", &APIError{StatusCode: http.StatusNotFound, Message: "agent network settings not found"}
|
||||
}
|
||||
var ret api.AgentNetworkSettings
|
||||
if err := json.Unmarshal(body, &ret); err != nil {
|
||||
return nil, err
|
||||
return nil, "", err
|
||||
}
|
||||
return &ret, nil
|
||||
return &ret, etagFrom(resp), nil
|
||||
}
|
||||
|
||||
// CreateSettings bootstraps the account's Agent Network settings row,
|
||||
@@ -363,19 +375,30 @@ func (a *AgentNetworkAPI) GetSettings(ctx context.Context) (*api.AgentNetworkSet
|
||||
// request.Endpoint (self-addressed dedicated endpoint, claimed verbatim) must
|
||||
// be set. Returns a conflict when the account already has a settings row.
|
||||
func (a *AgentNetworkAPI) CreateSettings(ctx context.Context, request api.PostApiAgentNetworkSettingsJSONRequestBody) (*api.AgentNetworkSettings, error) {
|
||||
settings, _, err := a.CreateSettingsWithETag(ctx, request)
|
||||
return settings, err
|
||||
}
|
||||
|
||||
// CreateSettingsWithETag is CreateSettings, additionally returning the
|
||||
// entity-tag of the row it bootstrapped, so a client can follow the bootstrap
|
||||
// with a conditional write without an intervening read.
|
||||
func (a *AgentNetworkAPI) CreateSettingsWithETag(ctx context.Context, request api.PostApiAgentNetworkSettingsJSONRequestBody) (*api.AgentNetworkSettings, string, error) {
|
||||
requestBytes, err := json.Marshal(request)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, "", err
|
||||
}
|
||||
resp, err := a.c.NewRequest(ctx, "POST", "/api/agent-network/settings", bytes.NewReader(requestBytes), nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, "", err
|
||||
}
|
||||
if resp.Body != nil {
|
||||
defer resp.Body.Close()
|
||||
}
|
||||
ret, err := parseResponse[api.AgentNetworkSettings](resp)
|
||||
return &ret, err
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
return &ret, etagFrom(resp), nil
|
||||
}
|
||||
|
||||
// UpdateSettings updates the account's Agent Network settings; the request
|
||||
@@ -385,19 +408,35 @@ func (a *AgentNetworkAPI) CreateSettings(ctx context.Context, request api.PostAp
|
||||
// a request carrying different values is rejected. Returns not-found until
|
||||
// the account is bootstrapped.
|
||||
func (a *AgentNetworkAPI) UpdateSettings(ctx context.Context, request api.PutApiAgentNetworkSettingsJSONRequestBody) (*api.AgentNetworkSettings, error) {
|
||||
settings, _, err := a.UpdateSettingsIfMatch(ctx, request, "")
|
||||
return settings, err
|
||||
}
|
||||
|
||||
// UpdateSettingsIfMatch is UpdateSettings made conditional on etag — the
|
||||
// validator from an earlier read — still being current, and returns the
|
||||
// validator of the row it wrote. This is what closes the read-modify-write
|
||||
// window: a settings change made between the read and this write makes the
|
||||
// request fail with a precondition-failed APIError instead of reverting it.
|
||||
//
|
||||
// An empty etag sends no precondition and updates unconditionally, which is
|
||||
// what UpdateSettings does.
|
||||
func (a *AgentNetworkAPI) UpdateSettingsIfMatch(ctx context.Context, request api.PutApiAgentNetworkSettingsJSONRequestBody, etag string) (*api.AgentNetworkSettings, string, error) {
|
||||
requestBytes, err := json.Marshal(request)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, "", err
|
||||
}
|
||||
resp, err := a.c.NewRequest(ctx, "PUT", "/api/agent-network/settings", bytes.NewReader(requestBytes), nil)
|
||||
resp, err := a.c.newRequest(ctx, "PUT", "/api/agent-network/settings", bytes.NewReader(requestBytes), nil, ifMatchHeader(etag))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
return nil, "", err
|
||||
}
|
||||
if resp.Body != nil {
|
||||
defer resp.Body.Close()
|
||||
}
|
||||
ret, err := parseResponse[api.AgentNetworkSettings](resp)
|
||||
return &ret, err
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
return &ret, etagFrom(resp), nil
|
||||
}
|
||||
|
||||
// DeleteSettings deletes the account's Agent Network settings row, releasing
|
||||
@@ -405,7 +444,20 @@ func (a *AgentNetworkAPI) UpdateSettings(ctx context.Context, request api.PutApi
|
||||
// exists for the account or while a proxy is actively serving the endpoint.
|
||||
// Bootstrapping again afterwards allocates a new endpoint.
|
||||
func (a *AgentNetworkAPI) DeleteSettings(ctx context.Context) error {
|
||||
resp, err := a.c.NewRequest(ctx, "DELETE", "/api/agent-network/settings", nil, nil)
|
||||
return a.DeleteSettingsIfMatch(ctx, "")
|
||||
}
|
||||
|
||||
// DeleteSettingsIfMatch is DeleteSettings made conditional on etag — the
|
||||
// validator from an earlier read — still being current. Sending it matters
|
||||
// more here than on update: the server's other two refusals are about state
|
||||
// (no providers, no serving proxy), so this is the only thing that stops a
|
||||
// client working from an old read of one row from releasing the endpoint of
|
||||
// the row that replaced it.
|
||||
//
|
||||
// An empty etag sends no precondition and deletes unconditionally, which is
|
||||
// what DeleteSettings does.
|
||||
func (a *AgentNetworkAPI) DeleteSettingsIfMatch(ctx context.Context, etag string) error {
|
||||
resp, err := a.c.newRequest(ctx, "DELETE", "/api/agent-network/settings", nil, nil, ifMatchHeader(etag))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -415,3 +467,20 @@ func (a *AgentNetworkAPI) DeleteSettings(ctx context.Context) error {
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// etagFrom returns the bare validator from a response, with the transport's
|
||||
// quoting stripped so a caller can hand it straight back to an If-Match
|
||||
// parameter without knowing the wire syntax.
|
||||
func etagFrom(resp *http.Response) string {
|
||||
return strings.Trim(resp.Header.Get("ETag"), `"`)
|
||||
}
|
||||
|
||||
// ifMatchHeader renders the precondition headers for a bare validator,
|
||||
// re-applying the quoting etagFrom stripped. An empty validator yields no
|
||||
// headers at all — an unconditional request.
|
||||
func ifMatchHeader(etag string) map[string]string {
|
||||
if etag == "" {
|
||||
return nil
|
||||
}
|
||||
return map[string]string{"If-Match": strconv.Quote(etag)}
|
||||
}
|
||||
|
||||
@@ -559,3 +559,123 @@ func TestAgentNetwork_DeleteSettings_Guarded(t *testing.T) {
|
||||
assert.Contains(t, err.Error(), "cannot be deleted")
|
||||
})
|
||||
}
|
||||
|
||||
// TestAgentNetwork_GetSettings_ETag pins that the validator surfaces to the
|
||||
// caller with the transport's quoting stripped, so it can be handed straight
|
||||
// back to a conditional write without the caller knowing the wire syntax.
|
||||
func TestAgentNetwork_GetSettings_ETag(t *testing.T) {
|
||||
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
|
||||
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("ETag", `"9f86d081884c7d65"`)
|
||||
retBytes, _ := json.Marshal(testAgentNetworkSettings)
|
||||
_, err := w.Write(retBytes)
|
||||
require.NoError(t, err)
|
||||
})
|
||||
ret, etag, err := c.AgentNetwork.GetSettingsWithETag(context.Background())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, testAgentNetworkSettings, *ret)
|
||||
assert.Equal(t, "9f86d081884c7d65", etag, "the validator must arrive unquoted")
|
||||
})
|
||||
}
|
||||
|
||||
// TestAgentNetwork_UpdateSettings_IfMatch covers the round trip that makes the
|
||||
// whole feature usable: a validator taken from a read goes back out quoted on
|
||||
// the write, and the write's own validator comes back for the next one.
|
||||
func TestAgentNetwork_UpdateSettings_IfMatch(t *testing.T) {
|
||||
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
|
||||
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Equal(t, `"9f86d081884c7d65"`, r.Header.Get("If-Match"),
|
||||
"the precondition must go out quoted as a strong entity-tag")
|
||||
w.Header().Set("ETag", `"0011223344556677"`)
|
||||
retBytes, _ := json.Marshal(testAgentNetworkSettings)
|
||||
_, err := w.Write(retBytes)
|
||||
require.NoError(t, err)
|
||||
})
|
||||
_, etag, err := c.AgentNetwork.UpdateSettingsIfMatch(context.Background(), api.PutApiAgentNetworkSettingsJSONRequestBody{
|
||||
Endpoint: "brave-otter.eu.proxy.netbird.io",
|
||||
ProxyAddress: "eu.proxy.netbird.io",
|
||||
EnableLogCollection: true,
|
||||
}, "9f86d081884c7d65")
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "0011223344556677", etag, "the write must return the new validator")
|
||||
})
|
||||
}
|
||||
|
||||
// TestAgentNetwork_UpdateSettings_NoPrecondition pins the back-compatible
|
||||
// path: the plain method sends no If-Match at all, rather than an empty or
|
||||
// wildcard one, so it stays the unconditional update it has always been.
|
||||
func TestAgentNetwork_UpdateSettings_NoPrecondition(t *testing.T) {
|
||||
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
|
||||
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Empty(t, r.Header.Values("If-Match"), "an unconditional update must send no precondition")
|
||||
retBytes, _ := json.Marshal(testAgentNetworkSettings)
|
||||
_, err := w.Write(retBytes)
|
||||
require.NoError(t, err)
|
||||
})
|
||||
_, err := c.AgentNetwork.UpdateSettings(context.Background(), api.PutApiAgentNetworkSettingsJSONRequestBody{
|
||||
Endpoint: "brave-otter.eu.proxy.netbird.io",
|
||||
ProxyAddress: "eu.proxy.netbird.io",
|
||||
EnableLogCollection: true,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
// TestAgentNetwork_UpdateSettings_StalePrecondition pins how a refused write
|
||||
// reaches the caller: as an APIError a client can recognise as staleness and
|
||||
// answer by reading again, rather than as an opaque failure.
|
||||
func TestAgentNetwork_UpdateSettings_StalePrecondition(t *testing.T) {
|
||||
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
|
||||
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
|
||||
retBytes, _ := json.Marshal(util.ErrorResponse{Message: "if-match precondition failed: the settings have changed since they were read; get them again and retry", Code: 412})
|
||||
w.WriteHeader(412)
|
||||
_, err := w.Write(retBytes)
|
||||
require.NoError(t, err)
|
||||
})
|
||||
_, _, err := c.AgentNetwork.UpdateSettingsIfMatch(context.Background(), api.PutApiAgentNetworkSettingsJSONRequestBody{
|
||||
Endpoint: "brave-otter.eu.proxy.netbird.io",
|
||||
ProxyAddress: "eu.proxy.netbird.io",
|
||||
EnableLogCollection: true,
|
||||
}, "9f86d081884c7d65")
|
||||
require.Error(t, err)
|
||||
assert.True(t, rest.IsPreconditionFailed(err), "a refused precondition must be recognisable as one")
|
||||
assert.False(t, rest.IsNotFound(err), "it must not be confused with an unbootstrapped account")
|
||||
})
|
||||
}
|
||||
|
||||
// TestAgentNetwork_CreateSettings_ETag pins that the bootstrap hands back a
|
||||
// validator, which is what lets a client follow it with a conditional write
|
||||
// without an intervening read.
|
||||
func TestAgentNetwork_CreateSettings_ETag(t *testing.T) {
|
||||
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
|
||||
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("ETag", `"9f86d081884c7d65"`)
|
||||
retBytes, _ := json.Marshal(testAgentNetworkSettings)
|
||||
_, err := w.Write(retBytes)
|
||||
require.NoError(t, err)
|
||||
})
|
||||
_, etag, err := c.AgentNetwork.CreateSettingsWithETag(context.Background(), api.PostApiAgentNetworkSettingsJSONRequestBody{
|
||||
ProxyAddress: ptr("eu.proxy.netbird.io"),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "9f86d081884c7d65", etag, "the bootstrap must return a validator")
|
||||
})
|
||||
}
|
||||
|
||||
// TestAgentNetwork_DeleteSettings_IfMatch covers the conditional delete on the
|
||||
// wire, and that the plain method still sends nothing.
|
||||
func TestAgentNetwork_DeleteSettings_IfMatch(t *testing.T) {
|
||||
withMockClient(func(c *rest.Client, mux *http.ServeMux) {
|
||||
var seen []string
|
||||
mux.HandleFunc("/api/agent-network/settings", func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Equal(t, "DELETE", r.Method)
|
||||
seen = append(seen, r.Header.Get("If-Match"))
|
||||
_, err := w.Write([]byte("{}"))
|
||||
require.NoError(t, err)
|
||||
})
|
||||
require.NoError(t, c.AgentNetwork.DeleteSettingsIfMatch(context.Background(), "9f86d081884c7d65"))
|
||||
require.NoError(t, c.AgentNetwork.DeleteSettings(context.Background()))
|
||||
assert.Equal(t, []string{`"9f86d081884c7d65"`, ""}, seen,
|
||||
"the conditional delete must carry the quoted validator and the plain one must carry nothing")
|
||||
})
|
||||
}
|
||||
|
||||
@@ -31,6 +31,19 @@ func IsNotFound(err error) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// IsPreconditionFailed returns true if the error represents a 412 Precondition
|
||||
// Failed response — an If-Match the server refused, or an endpoint's own
|
||||
// precondition. A caller that sent a conditional request can use this to tell
|
||||
// "someone else changed it, read again and retry" apart from a real failure;
|
||||
// the message distinguishes it from an endpoint's other 412s.
|
||||
func IsPreconditionFailed(err error) bool {
|
||||
var apiErr *APIError
|
||||
if ok := errors.As(err, &apiErr); ok {
|
||||
return apiErr.StatusCode == http.StatusPreconditionFailed
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// Client Management service HTTP REST API Client
|
||||
type Client struct {
|
||||
managementURL string
|
||||
@@ -218,6 +231,12 @@ func (c *Client) initialize() {
|
||||
|
||||
// NewRequest creates and executes new management API request
|
||||
func (c *Client) NewRequest(ctx context.Context, method, path string, body io.Reader, query map[string]string) (*http.Response, error) {
|
||||
return c.newRequest(ctx, method, path, body, query, nil)
|
||||
}
|
||||
|
||||
// newRequest is NewRequest with request headers, for the endpoints whose
|
||||
// contract includes one — conditional requests carrying If-Match.
|
||||
func (c *Client) newRequest(ctx context.Context, method, path string, body io.Reader, query, headers map[string]string) (*http.Response, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, method, c.managementURL+path, body)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -231,6 +250,9 @@ func (c *Client) NewRequest(ctx context.Context, method, path string, body io.Re
|
||||
if c.userAgent != "" {
|
||||
req.Header.Set("User-Agent", c.userAgent)
|
||||
}
|
||||
for name, value := range headers {
|
||||
req.Header.Set(name, value)
|
||||
}
|
||||
|
||||
if len(query) != 0 {
|
||||
q := req.URL.Query()
|
||||
|
||||
@@ -6438,6 +6438,15 @@ components:
|
||||
schema:
|
||||
type: string
|
||||
example: cot7r4n3l3vh3qj4qveg
|
||||
ETag:
|
||||
description: |
|
||||
Strong entity-tag identifying the returned representation. Send it back
|
||||
in `If-Match` on a subsequent write to make that write conditional, so
|
||||
a change made between the read and the write is refused with `412`
|
||||
rather than silently overwritten.
|
||||
schema:
|
||||
type: string
|
||||
example: '"9f86d081884c7d65"'
|
||||
securitySchemes:
|
||||
BearerAuth:
|
||||
type: http
|
||||
@@ -13733,6 +13742,9 @@ paths:
|
||||
responses:
|
||||
'200':
|
||||
description: Agent Network settings for the account
|
||||
headers:
|
||||
ETag:
|
||||
$ref: '#/components/headers/ETag'
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
@@ -13760,6 +13772,9 @@ paths:
|
||||
responses:
|
||||
'200':
|
||||
description: The freshly bootstrapped Agent Network settings
|
||||
headers:
|
||||
ETag:
|
||||
$ref: '#/components/headers/ETag'
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
@@ -13778,11 +13793,25 @@ paths:
|
||||
"$ref": "#/components/responses/internal_error"
|
||||
put:
|
||||
summary: Update Agent Network settings
|
||||
description: Updates the account-level Agent Network settings; the request carries every field, replacing the mutable ones (collection toggles and retention). Returns 404 when the account has no settings row yet — bootstrap it with POST first. The endpoint and proxy address are assigned at bootstrap and immutable; the request must carry them unchanged, and a request carrying different values is rejected.
|
||||
description: Updates the account-level Agent Network settings; the request carries every field, replacing the mutable ones (collection toggles and retention). Returns 404 when the account has no settings row yet — bootstrap it with POST first. The endpoint and proxy address are assigned at bootstrap and immutable; the request must carry them unchanged, and a request carrying different values is rejected. Supply `If-Match` to make the update conditional; without it the update is unconditional and the last write wins.
|
||||
tags: [ Agent Network ]
|
||||
security:
|
||||
- BearerAuth: [ ]
|
||||
- TokenAuth: [ ]
|
||||
parameters:
|
||||
- name: If-Match
|
||||
in: header
|
||||
required: false
|
||||
description: |
|
||||
Makes the update conditional on the settings not having changed since
|
||||
they were read. Send the `ETag` from an earlier `GET`, `POST` or `PUT`,
|
||||
or `*` to require only that a settings row exists. The precondition is
|
||||
evaluated against the stored row inside the update's own transaction,
|
||||
so two clients starting from the same `ETag` cannot both succeed.
|
||||
Omitting the header leaves the update unconditional.
|
||||
schema:
|
||||
type: string
|
||||
example: '"9f86d081884c7d65"'
|
||||
requestBody:
|
||||
description: Settings update request
|
||||
content:
|
||||
@@ -13792,6 +13821,9 @@ paths:
|
||||
responses:
|
||||
'200':
|
||||
description: Updated Agent Network settings
|
||||
headers:
|
||||
ETag:
|
||||
$ref: '#/components/headers/ETag'
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
@@ -13804,17 +13836,34 @@ paths:
|
||||
"$ref": "#/components/responses/forbidden"
|
||||
'404':
|
||||
"$ref": "#/components/responses/not_found"
|
||||
'412':
|
||||
description: The `If-Match` precondition failed — the settings changed since they were read. The stored settings are unmodified; read them again and retry.
|
||||
content: { }
|
||||
'422':
|
||||
"$ref": "#/components/responses/validation_failed"
|
||||
'500':
|
||||
"$ref": "#/components/responses/internal_error"
|
||||
delete:
|
||||
summary: Delete Agent Network settings
|
||||
description: Deletes the account's Agent Network settings row, releasing the endpoint. Guarded — the delete is refused with 412 while any Agent Network provider exists for the account or while a proxy is actively serving the endpoint. Bootstrapping again after a delete allocates a new endpoint; the released hostname is not reserved.
|
||||
description: Deletes the account's Agent Network settings row, releasing the endpoint. Guarded — the delete is refused with 412 while any Agent Network provider exists for the account or while a proxy is actively serving the endpoint. Bootstrapping again after a delete allocates a new endpoint; the released hostname is not reserved. Supply `If-Match` to make the delete conditional, which is worth doing here even more than on update — the other two guards are about state rather than staleness, so nothing else stops a client from deleting a row that was replaced since it read one.
|
||||
tags: [ Agent Network ]
|
||||
security:
|
||||
- BearerAuth: [ ]
|
||||
- TokenAuth: [ ]
|
||||
parameters:
|
||||
- name: If-Match
|
||||
in: header
|
||||
required: false
|
||||
description: |
|
||||
Makes the delete conditional on the settings not having changed since
|
||||
they were read. Send the `ETag` from an earlier `GET`, `POST` or `PUT`,
|
||||
or `*` to require only that a settings row exists. The precondition is
|
||||
evaluated inside the delete's own transaction, ahead of the provider
|
||||
and serving-proxy guards. Omitting the header leaves the delete
|
||||
unconditional.
|
||||
schema:
|
||||
type: string
|
||||
example: '"9f86d081884c7d65"'
|
||||
responses:
|
||||
'200':
|
||||
description: Settings deleted
|
||||
@@ -13825,7 +13874,7 @@ paths:
|
||||
'404':
|
||||
"$ref": "#/components/responses/not_found"
|
||||
'412':
|
||||
description: Delete refused — Agent Network providers still exist for the account, or a proxy is actively serving the endpoint
|
||||
description: Delete refused — the `If-Match` precondition failed, or Agent Network providers still exist for the account, or a proxy is actively serving the endpoint. The stored settings are unmodified in every case; the response message distinguishes them.
|
||||
content: { }
|
||||
'500':
|
||||
"$ref": "#/components/responses/internal_error"
|
||||
|
||||
@@ -5939,6 +5939,28 @@ type GetApiAgentNetworkAccessLogsParamsSortBy string
|
||||
// GetApiAgentNetworkAccessLogsParamsSortOrder defines parameters for GetApiAgentNetworkAccessLogs.
|
||||
type GetApiAgentNetworkAccessLogsParamsSortOrder string
|
||||
|
||||
// DeleteApiAgentNetworkSettingsParams defines parameters for DeleteApiAgentNetworkSettings.
|
||||
type DeleteApiAgentNetworkSettingsParams struct {
|
||||
// IfMatch Makes the delete conditional on the settings not having changed since
|
||||
// they were read. Send the `ETag` from an earlier `GET`, `POST` or `PUT`,
|
||||
// or `*` to require only that a settings row exists. The precondition is
|
||||
// evaluated inside the delete's own transaction, ahead of the provider
|
||||
// and serving-proxy guards. Omitting the header leaves the delete
|
||||
// unconditional.
|
||||
IfMatch *string `json:"If-Match,omitempty"`
|
||||
}
|
||||
|
||||
// PutApiAgentNetworkSettingsParams defines parameters for PutApiAgentNetworkSettings.
|
||||
type PutApiAgentNetworkSettingsParams struct {
|
||||
// IfMatch Makes the update conditional on the settings not having changed since
|
||||
// they were read. Send the `ETag` from an earlier `GET`, `POST` or `PUT`,
|
||||
// or `*` to require only that a settings row exists. The precondition is
|
||||
// evaluated against the stored row inside the update's own transaction,
|
||||
// so two clients starting from the same `ETag` cannot both succeed.
|
||||
// Omitting the header leaves the update unconditional.
|
||||
IfMatch *string `json:"If-Match,omitempty"`
|
||||
}
|
||||
|
||||
// GetApiAgentNetworkUsageOverviewParams defines parameters for GetApiAgentNetworkUsageOverview.
|
||||
type GetApiAgentNetworkUsageOverviewParams struct {
|
||||
// Granularity Time bucket width. Defaults to day.
|
||||
|
||||
103
shared/management/http/util/etag.go
Normal file
103
shared/management/http/util/etag.go
Normal file
@@ -0,0 +1,103 @@
|
||||
package util
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
const (
|
||||
etagHeader = "ETag"
|
||||
ifMatchHeader = "If-Match"
|
||||
|
||||
// matchAny is the If-Match value that matches any current representation
|
||||
// of the resource (RFC 9110 §13.1.1).
|
||||
matchAny = "*"
|
||||
|
||||
// weakPrefix marks a weak validator. If-Match is defined in terms of the
|
||||
// strong comparison function, under which a weak validator never matches.
|
||||
weakPrefix = "W/"
|
||||
)
|
||||
|
||||
// SetETag writes etag as a strong ETag response header, quoted per RFC 9110.
|
||||
// The value passed in is the bare validator — callers derive it (typically
|
||||
// from the type being served) and this applies the wire syntax, so the quoting
|
||||
// is decided in one place rather than at every handler.
|
||||
//
|
||||
// Call it before writing the body: once the response is committed the header
|
||||
// no longer reaches the client. An empty etag writes no header at all, so a
|
||||
// caller with nothing to validate against does not have to special-case it.
|
||||
func SetETag(w http.ResponseWriter, etag string) {
|
||||
if etag == "" {
|
||||
return
|
||||
}
|
||||
w.Header().Set(etagHeader, strconv.Quote(etag))
|
||||
}
|
||||
|
||||
// Precondition is a parsed If-Match request precondition. The zero value
|
||||
// matches nothing; a nil *Precondition is an unconditional request and matches
|
||||
// everything, so a handler can pass the result of IfMatch straight through
|
||||
// without a presence check.
|
||||
type Precondition struct {
|
||||
// tags are the strong entity-tags the client will accept, unquoted.
|
||||
tags []string
|
||||
|
||||
// any records the "*" form, which matches any current representation.
|
||||
any bool
|
||||
}
|
||||
|
||||
// IfMatch parses the request's If-Match precondition. It returns nil when the
|
||||
// header is absent — an unconditional request, which is the back-compatible
|
||||
// default: clients that know nothing of conditional requests keep working.
|
||||
//
|
||||
// A header that is present but carries nothing usable — empty, or nothing but
|
||||
// weak validators — yields a precondition that matches nothing rather than
|
||||
// nil. Failing closed is the only safe direction: a client that meant to send
|
||||
// a precondition must not have it silently dropped and its write let through
|
||||
// unguarded.
|
||||
func IfMatch(r *http.Request) *Precondition {
|
||||
values := r.Header.Values(ifMatchHeader)
|
||||
if len(values) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
p := &Precondition{}
|
||||
for _, value := range values {
|
||||
for raw := range strings.SplitSeq(value, ",") {
|
||||
candidate := strings.TrimSpace(raw)
|
||||
switch {
|
||||
case candidate == "":
|
||||
// Tolerated rather than rejected: a stray comma changes
|
||||
// nothing about what the client is willing to accept.
|
||||
case candidate == matchAny:
|
||||
p.any = true
|
||||
case strings.HasPrefix(candidate, weakPrefix):
|
||||
// Dropped, not unwrapped. If-Match uses strong comparison, so
|
||||
// a weak validator cannot satisfy it — and unwrapping one into
|
||||
// a strong tag would quietly grant the match the client's own
|
||||
// header said it could not have.
|
||||
default:
|
||||
p.tags = append(p.tags, strings.Trim(candidate, `"`))
|
||||
}
|
||||
}
|
||||
}
|
||||
return p
|
||||
}
|
||||
|
||||
// Matches reports whether etag — the bare validator of the resource as it
|
||||
// currently stands — satisfies the precondition. A nil precondition matches
|
||||
// everything.
|
||||
//
|
||||
// Callers must establish that the resource exists before consulting this: the
|
||||
// "*" form asks whether there is any current representation, a question only
|
||||
// the caller can answer, and this reports true for it.
|
||||
func (p *Precondition) Matches(etag string) bool {
|
||||
if p == nil {
|
||||
return true
|
||||
}
|
||||
if p.any {
|
||||
return true
|
||||
}
|
||||
return slices.Contains(p.tags, etag)
|
||||
}
|
||||
161
shared/management/http/util/etag_test.go
Normal file
161
shared/management/http/util/etag_test.go
Normal file
@@ -0,0 +1,161 @@
|
||||
package util
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestSetETag covers the wire syntax: the bare validator goes in, a quoted
|
||||
// strong entity-tag comes out. Handlers pass what the type derived, so the
|
||||
// quoting has to happen here or every handler re-decides it.
|
||||
func TestSetETag(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
SetETag(rec, "9f86d081884c7d65")
|
||||
|
||||
assert.Equal(t, `"9f86d081884c7d65"`, rec.Header().Get("ETag"),
|
||||
"the validator must be emitted quoted")
|
||||
}
|
||||
|
||||
// TestSetETagEmpty pins the no-op: a caller with nothing to validate against
|
||||
// must not emit an empty entity-tag, which would be a validator that every
|
||||
// later request could match.
|
||||
func TestSetETagEmpty(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
SetETag(rec, "")
|
||||
|
||||
assert.Empty(t, rec.Header().Values("ETag"), "an empty validator must write no header")
|
||||
}
|
||||
|
||||
// TestSetETagRoundTrip closes the loop between the two halves of the helper:
|
||||
// what SetETag emits is what IfMatch accepts back. A client echoing the header
|
||||
// it was given must match, or conditional requests never succeed in practice.
|
||||
func TestSetETagRoundTrip(t *testing.T) {
|
||||
const etag = "9f86d081884c7d65"
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
SetETag(rec, etag)
|
||||
|
||||
r := httptest.NewRequest(http.MethodPut, "/", nil)
|
||||
r.Header.Set("If-Match", rec.Header().Get("ETag"))
|
||||
|
||||
assert.True(t, IfMatch(r).Matches(etag), "an echoed ETag header must satisfy the precondition")
|
||||
}
|
||||
|
||||
// TestIfMatchAbsent pins the back-compatibility guarantee: a request with no
|
||||
// If-Match is unconditional, and the nil precondition it yields matches
|
||||
// anything so handlers need no presence check.
|
||||
func TestIfMatchAbsent(t *testing.T) {
|
||||
p := IfMatch(httptest.NewRequest(http.MethodPut, "/", nil))
|
||||
|
||||
require.Nil(t, p, "an absent header must yield no precondition")
|
||||
assert.True(t, p.Matches("9f86d081884c7d65"), "a nil precondition must match anything")
|
||||
assert.True(t, p.Matches(""), "a nil precondition must not depend on the validator")
|
||||
}
|
||||
|
||||
// TestIfMatchParsing walks the header forms a client can send. The weak and
|
||||
// unusable cases are the ones that matter: each must yield a precondition that
|
||||
// exists and refuses, never one that is absent and waves the write through.
|
||||
func TestIfMatchParsing(t *testing.T) {
|
||||
const current = "9f86d081884c7d65"
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
header string
|
||||
match bool
|
||||
reason string
|
||||
}{
|
||||
{
|
||||
name: "quoted current validator",
|
||||
header: `"9f86d081884c7d65"`,
|
||||
match: true,
|
||||
reason: "the ordinary conditional request must be honoured",
|
||||
},
|
||||
{
|
||||
name: "unquoted current validator",
|
||||
header: "9f86d081884c7d65",
|
||||
match: true,
|
||||
reason: "a client that omits the quoting means the same thing, and only an exact value can match",
|
||||
},
|
||||
{
|
||||
name: "stale validator",
|
||||
header: `"0000000000000000"`,
|
||||
match: false,
|
||||
reason: "a validator from an earlier read must not match",
|
||||
},
|
||||
{
|
||||
name: "star",
|
||||
header: "*",
|
||||
match: true,
|
||||
reason: "* matches any current representation",
|
||||
},
|
||||
{
|
||||
name: "list containing the current validator",
|
||||
header: `"0000000000000000", "9f86d081884c7d65"`,
|
||||
match: true,
|
||||
reason: "If-Match is a list; any member matching is a match",
|
||||
},
|
||||
{
|
||||
name: "list of stale validators",
|
||||
header: `"0000000000000000", "1111111111111111"`,
|
||||
match: false,
|
||||
reason: "a list none of whose members match must not match",
|
||||
},
|
||||
{
|
||||
name: "surrounding whitespace",
|
||||
header: ` "9f86d081884c7d65" `,
|
||||
match: true,
|
||||
reason: "list whitespace is not part of the entity-tag",
|
||||
},
|
||||
{
|
||||
name: "stray comma",
|
||||
header: `"9f86d081884c7d65", `,
|
||||
match: true,
|
||||
reason: "an empty list element says nothing about what the client accepts",
|
||||
},
|
||||
{
|
||||
name: "weak validator of the current representation",
|
||||
header: `W/"9f86d081884c7d65"`,
|
||||
match: false,
|
||||
reason: "If-Match uses strong comparison, so a weak validator never satisfies it",
|
||||
},
|
||||
{
|
||||
name: "weak validator alongside a strong one",
|
||||
header: `W/"0000000000000000", "9f86d081884c7d65"`,
|
||||
match: true,
|
||||
reason: "dropping the weak member must not discard the rest of the list",
|
||||
},
|
||||
{
|
||||
name: "empty header",
|
||||
header: "",
|
||||
match: false,
|
||||
reason: "a precondition the server cannot make sense of must fail closed, not vanish",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodPut, "/", nil)
|
||||
r.Header.Set("If-Match", tc.header)
|
||||
|
||||
p := IfMatch(r)
|
||||
require.NotNil(t, p, "a header that was sent must yield a precondition: %s", tc.reason)
|
||||
assert.Equal(t, tc.match, p.Matches(current), tc.reason)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestIfMatchRepeatedHeader covers the same list split across header lines,
|
||||
// which is semantically identical to the comma form and which a proxy is free
|
||||
// to produce.
|
||||
func TestIfMatchRepeatedHeader(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodPut, "/", nil)
|
||||
r.Header.Add("If-Match", `"0000000000000000"`)
|
||||
r.Header.Add("If-Match", `"9f86d081884c7d65"`)
|
||||
|
||||
assert.True(t, IfMatch(r).Matches("9f86d081884c7d65"),
|
||||
"entity-tags split across header lines must be read as one list")
|
||||
}
|
||||
Reference in New Issue
Block a user