Skip to content

Commit 1e640a9

Browse files
Pangjipingclaude
andcommitted
feat(egress): add DELETE /policy endpoint for removing egress rules by target
Add DELETE handler that accepts a JSON array of target strings, removes matching rules case-insensitively, and commits the updated policy. Targets not found are silently ignored (idempotent). Spec and README docs updated. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 395b5aa commit 1e640a9

7 files changed

Lines changed: 347 additions & 10 deletions

File tree

components/egress/policy_server.go

Lines changed: 89 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,10 @@ func (s *policyServer) handlePolicy(w http.ResponseWriter, r *http.Request) {
147147
s.handlePost(w, r)
148148
case http.MethodPatch:
149149
s.handlePatch(w, r)
150+
case http.MethodDelete:
151+
s.handleDelete(w, r)
150152
default:
151-
w.Header().Set("Allow", "GET, POST, PUT, PATCH")
153+
w.Header().Set("Allow", "GET, POST, PUT, PATCH, DELETE")
152154
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
153155
}
154156
}
@@ -222,15 +224,16 @@ func (s *policyServer) handlePatch(w http.ResponseWriter, r *http.Request) {
222224
defer s.mu.Unlock()
223225

224226
raw, err := readPolicyRequestBody(r)
225-
if err != nil || raw == "" {
226-
if err != nil {
227-
logEgressUpdateFailedWarn(fmt.Sprintf("failed to read body: %v", err))
228-
} else {
229-
logEgressUpdateFailedWarn("empty patch body")
230-
}
227+
if err != nil {
228+
logEgressUpdateFailedWarn(fmt.Sprintf("failed to read body: %v", err))
231229
http.Error(w, fmt.Sprintf("failed to read body: %v", err), http.StatusBadRequest)
232230
return
233231
}
232+
if raw == "" {
233+
logEgressUpdateFailedWarn("empty patch body")
234+
http.Error(w, "empty body", http.StatusBadRequest)
235+
return
236+
}
234237

235238
var patchRules []policy.EgressRule
236239
if err := json.Unmarshal([]byte(raw), &patchRules); err != nil {
@@ -268,6 +271,85 @@ func (s *policyServer) handlePatch(w http.ResponseWriter, r *http.Request) {
268271
})
269272
}
270273

274+
func (s *policyServer) handleDelete(w http.ResponseWriter, r *http.Request) {
275+
defer r.Body.Close()
276+
s.mu.Lock()
277+
defer s.mu.Unlock()
278+
279+
raw, err := readPolicyRequestBody(r)
280+
if err != nil {
281+
logEgressUpdateFailedWarn(fmt.Sprintf("failed to read body: %v", err))
282+
http.Error(w, fmt.Sprintf("failed to read body: %v", err), http.StatusBadRequest)
283+
return
284+
}
285+
if raw == "" {
286+
logEgressUpdateFailedWarn("empty delete body")
287+
http.Error(w, "empty body", http.StatusBadRequest)
288+
return
289+
}
290+
291+
var targets []string
292+
if err := json.Unmarshal([]byte(raw), &targets); err != nil {
293+
logEgressUpdateFailedWarn(fmt.Sprintf("invalid delete targets: %v", err))
294+
http.Error(w, fmt.Sprintf("invalid delete targets: %v", err), http.StatusBadRequest)
295+
return
296+
}
297+
if len(targets) == 0 {
298+
logEgressUpdateFailedWarn("empty delete targets array")
299+
http.Error(w, "invalid delete targets: empty array", http.StatusBadRequest)
300+
return
301+
}
302+
303+
base := s.proxy.CurrentPolicy()
304+
if base == nil {
305+
base = policy.DefaultDenyPolicy()
306+
}
307+
oldCount := len(base.Egress)
308+
newEgress, removedRules := removeRulesByTarget(base.Egress, targets)
309+
removed := oldCount - len(newEgress)
310+
311+
if removed == 0 {
312+
mode := modeFromPolicy(base)
313+
writeJSON(w, http.StatusOK, policyStatusResponse{
314+
Status: "ok",
315+
Mode: mode,
316+
EnforcementMode: s.enforcementMode,
317+
Reason: "no matching targets found",
318+
Policy: base,
319+
})
320+
return
321+
}
322+
323+
rawMerged, err := json.Marshal(policy.NetworkPolicy{
324+
DefaultAction: base.DefaultAction,
325+
Egress: newEgress,
326+
})
327+
if err != nil {
328+
logEgressUpdateFailedWarn(fmt.Sprintf("failed to marshal updated policy: %v", err))
329+
http.Error(w, fmt.Sprintf("internal error: %v", err), http.StatusInternalServerError)
330+
return
331+
}
332+
newPolicy, err := policy.ParsePolicy(string(rawMerged))
333+
if err != nil {
334+
logEgressUpdateFailedWarn(fmt.Sprintf("invalid policy after delete: %v", err))
335+
http.Error(w, fmt.Sprintf("invalid policy after delete: %v", err), http.StatusBadRequest)
336+
return
337+
}
338+
339+
mode := modeFromPolicy(newPolicy)
340+
log.Infof("policy API: deleting %d egress rule(s) by target, removed=%d, mode=%s, enforcement=%s", len(targets), removed, mode, s.enforcementMode)
341+
if !s.commitPolicy(r.Context(), w, newPolicy, "delete") {
342+
return
343+
}
344+
logEgressUpdated(newPolicy.DefaultAction, removedRules)
345+
log.Infof("policy API: delete applied successfully")
346+
writeJSON(w, http.StatusOK, policyStatusResponse{
347+
Status: "ok",
348+
Mode: mode,
349+
EnforcementMode: s.enforcementMode,
350+
})
351+
}
352+
271353
// commitPolicy applies one logical change: optional disk persist → merge always file rules → nft
272354
// static (with nameserver allow-IPs) → then update in-memory user policy (POST/PATCH/GET view).
273355
func (s *policyServer) commitPolicy(ctx context.Context, w http.ResponseWriter, pol *policy.NetworkPolicy, op string) bool {

components/egress/policy_server_test.go

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,150 @@ func TestHandlePatch_RejectsWhenOverMaxEgressRules(t *testing.T) {
245245
require.Len(t, proxy.updated.Egress, 2, "policy should be unchanged")
246246
}
247247

248+
func TestHandleDelete_RemovesMatchingTargets(t *testing.T) {
249+
initial := &policy.NetworkPolicy{
250+
DefaultAction: policy.ActionDeny,
251+
Egress: []policy.EgressRule{
252+
{Action: policy.ActionAllow, Target: "example.com"},
253+
{Action: policy.ActionDeny, Target: "blocked.com"},
254+
{Action: policy.ActionAllow, Target: "keep.com"},
255+
},
256+
}
257+
proxy := &stubProxy{updated: initial}
258+
nft := &stubNft{}
259+
srv := &policyServer{proxy: proxy, nft: nft, enforcementMode: "dns+nft"}
260+
261+
body := `["blocked.com","nonexistent.com"]`
262+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
263+
w := httptest.NewRecorder()
264+
265+
srv.handlePolicy(w, req)
266+
267+
resp := w.Result()
268+
require.Equal(t, http.StatusOK, resp.StatusCode, "expected 200 OK")
269+
require.Equal(t, 1, nft.calls, "expected nft ApplyStatic called once")
270+
require.NotNil(t, proxy.updated, "expected proxy policy updated")
271+
require.Equal(t, policy.ActionDeny, proxy.updated.DefaultAction, "defaultAction should be preserved")
272+
require.Len(t, proxy.updated.Egress, 2, "expected 2 rules remaining after delete")
273+
require.Equal(t, policy.ActionAllow, proxy.updated.Egress[0].Action)
274+
require.Equal(t, "example.com", proxy.updated.Egress[0].Target)
275+
require.Equal(t, policy.ActionAllow, proxy.updated.Egress[1].Action)
276+
require.Equal(t, "keep.com", proxy.updated.Egress[1].Target)
277+
}
278+
279+
func TestHandleDelete_CaseInsensitiveMatch(t *testing.T) {
280+
initial := &policy.NetworkPolicy{
281+
DefaultAction: policy.ActionDeny,
282+
Egress: []policy.EgressRule{
283+
{Action: policy.ActionAllow, Target: "Example.COM"},
284+
{Action: policy.ActionDeny, Target: "Blocked.COM"},
285+
},
286+
}
287+
proxy := &stubProxy{updated: initial}
288+
nft := &stubNft{}
289+
srv := &policyServer{proxy: proxy, nft: nft, enforcementMode: "dns+nft"}
290+
291+
body := `["example.com"]`
292+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
293+
w := httptest.NewRecorder()
294+
295+
srv.handlePolicy(w, req)
296+
297+
resp := w.Result()
298+
require.Equal(t, http.StatusOK, resp.StatusCode, "expected 200 OK")
299+
require.NotNil(t, proxy.updated)
300+
require.Len(t, proxy.updated.Egress, 1, "expected 1 rule remaining")
301+
require.Equal(t, "Blocked.COM", proxy.updated.Egress[0].Target, "unmatched rule should remain")
302+
}
303+
304+
func TestHandleDelete_NoMatchReturns200(t *testing.T) {
305+
initial := &policy.NetworkPolicy{
306+
DefaultAction: policy.ActionDeny,
307+
Egress: []policy.EgressRule{
308+
{Action: policy.ActionAllow, Target: "keep.com"},
309+
},
310+
}
311+
proxy := &stubProxy{updated: initial}
312+
nft := &stubNft{}
313+
srv := &policyServer{proxy: proxy, nft: nft, enforcementMode: "dns+nft"}
314+
315+
body := `["nonexistent.com"]`
316+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
317+
w := httptest.NewRecorder()
318+
319+
srv.handlePolicy(w, req)
320+
321+
resp := w.Result()
322+
require.Equal(t, http.StatusOK, resp.StatusCode, "expected 200 OK even when no targets match")
323+
require.Equal(t, 0, nft.calls, "nft should not be called when nothing changes")
324+
require.Len(t, proxy.updated.Egress, 1, "policy should be unchanged")
325+
}
326+
327+
func TestHandleDelete_EmptyBodyReturns400(t *testing.T) {
328+
proxy := &stubProxy{updated: policy.DefaultDenyPolicy()}
329+
srv := &policyServer{proxy: proxy, nft: nil, enforcementMode: "dns"}
330+
331+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(""))
332+
w := httptest.NewRecorder()
333+
334+
srv.handlePolicy(w, req)
335+
336+
resp := w.Result()
337+
require.Equal(t, http.StatusBadRequest, resp.StatusCode, "expected 400 for empty body")
338+
}
339+
340+
func TestHandleDelete_EmptyArrayReturns400(t *testing.T) {
341+
proxy := &stubProxy{updated: policy.DefaultDenyPolicy()}
342+
srv := &policyServer{proxy: proxy, nft: nil, enforcementMode: "dns"}
343+
344+
body := `[]`
345+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
346+
w := httptest.NewRecorder()
347+
348+
srv.handlePolicy(w, req)
349+
350+
resp := w.Result()
351+
require.Equal(t, http.StatusBadRequest, resp.StatusCode, "expected 400 for empty array")
352+
}
353+
354+
func TestHandleDelete_InvalidJSONReturns400(t *testing.T) {
355+
proxy := &stubProxy{updated: policy.DefaultDenyPolicy()}
356+
srv := &policyServer{proxy: proxy, nft: nil, enforcementMode: "dns"}
357+
358+
body := `not-json`
359+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
360+
w := httptest.NewRecorder()
361+
362+
srv.handlePolicy(w, req)
363+
364+
resp := w.Result()
365+
require.Equal(t, http.StatusBadRequest, resp.StatusCode, "expected 400 for invalid JSON")
366+
}
367+
368+
func TestHandleDelete_NftFailureReturns500(t *testing.T) {
369+
initial := &policy.NetworkPolicy{
370+
DefaultAction: policy.ActionDeny,
371+
Egress: []policy.EgressRule{
372+
{Action: policy.ActionAllow, Target: "example.com"},
373+
},
374+
}
375+
proxy := &stubProxy{updated: initial}
376+
nft := &stubNft{err: errors.New("nft apply failed")}
377+
srv := &policyServer{proxy: proxy, nft: nft, enforcementMode: "dns+nft"}
378+
379+
body := `["example.com"]`
380+
req := httptest.NewRequest(http.MethodDelete, "/policy", strings.NewReader(body))
381+
w := httptest.NewRecorder()
382+
383+
srv.handlePolicy(w, req)
384+
385+
resp := w.Result()
386+
require.Equal(t, http.StatusInternalServerError, resp.StatusCode, "expected 500 on nft failure")
387+
require.Equal(t, 1, nft.calls, "expected nft ApplyStatic called once")
388+
require.Len(t, proxy.updated.Egress, 1, "proxy should not be updated on nft failure")
389+
require.Equal(t, "example.com", proxy.updated.Egress[0].Target, "original rule should remain")
390+
}
391+
248392
func TestHandlePost_RejectsWhenOverMaxEgressRules(t *testing.T) {
249393
proxy := &stubProxy{}
250394
nft := &stubNft{}

components/egress/policy_utils.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,32 @@ func mergeEgressRules(base, additions []policy.EgressRule) []policy.EgressRule {
8383
return out
8484
}
8585

86+
// removeRulesByTarget returns a new slice with rules matching targets removed,
87+
// plus the removed rules. Domain targets are matched case-insensitively.
88+
// Targets not found are silently ignored.
89+
func removeRulesByTarget(rules []policy.EgressRule, targets []string) (kept, removed []policy.EgressRule) {
90+
if len(targets) == 0 || len(rules) == 0 {
91+
return rules, nil
92+
}
93+
removeSet := make(map[string]struct{}, len(targets))
94+
for _, t := range targets {
95+
key := strings.ToLower(strings.TrimSpace(t))
96+
if key == "" {
97+
continue
98+
}
99+
removeSet[key] = struct{}{}
100+
}
101+
kept = make([]policy.EgressRule, 0, len(rules))
102+
for _, r := range rules {
103+
if _, ok := removeSet[strings.ToLower(r.Target)]; ok {
104+
removed = append(removed, r)
105+
} else {
106+
kept = append(kept, r)
107+
}
108+
}
109+
return kept, removed
110+
}
111+
86112
// mergeKey: domain targets lowercased for dedupe; IP/CIDR left as-is.
87113
func mergeKey(r policy.EgressRule) string {
88114
if r.Target == "" {

components/egress/tests/smoke-nft.sh

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
#!/bin/bash
22

33
# Copyright 2026 Alibaba Group Holding Ltd.
4-
#
4+
#
55
# Licensed under the Apache License, Version 2.0 (the "License");
66
# you may not use this file except in compliance with the License.
77
# You may obtain a copy of the License at
8-
#
8+
#
99
# http://www.apache.org/licenses/LICENSE-2.0
10-
#
10+
#
1111
# Unless required by applicable law or agreed to in writing, software
1212
# distributed under the License is distributed on an "AS IS" BASIS,
1313
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -155,6 +155,47 @@ else
155155
pass "www.mozilla.org blocked after patch"
156156
fi
157157

158+
info "DELETE: deny two hosts, then delete one rule"
159+
curl -sSf -XPOST "http://127.0.0.1:${POLICY_PORT}/policy" \
160+
-d '{"defaultAction":"allow","egress":[{"action":"deny","target":"api.github.com"},{"action":"deny","target":"www.cloudflare.com"}]}'
161+
162+
info "Test: both hosts should be blocked before delete"
163+
if run_in_app -I https://api.github.com --max-time 8 >/dev/null 2>&1; then
164+
fail "api.github.com should be blocked before delete"
165+
fi
166+
if run_in_app -I https://www.cloudflare.com --max-time 8 >/dev/null 2>&1; then
167+
fail "www.cloudflare.com should be blocked before delete"
168+
fi
169+
pass "both hosts blocked before delete"
170+
171+
info "Deleting api.github.com rule"
172+
curl -sSf -XDELETE "http://127.0.0.1:${POLICY_PORT}/policy" \
173+
-d '["api.github.com"]'
174+
175+
info "Test: api.github.com allowed, www.cloudflare.com still blocked after delete"
176+
run_in_app -I https://api.github.com --max-time 20 >/dev/null 2>&1 || fail "api.github.com should be allowed after delete"
177+
pass "api.github.com allowed after delete"
178+
if run_in_app -I https://www.cloudflare.com --max-time 8 >/dev/null 2>&1; then
179+
fail "www.cloudflare.com should remain blocked after delete"
180+
fi
181+
pass "www.cloudflare.com still blocked"
182+
183+
info "Deleting non-existent target (idempotent)"
184+
resp="$(curl -sSf -XDELETE "http://127.0.0.1:${POLICY_PORT}/policy" -d '["nonexistent.com"]')"
185+
if echo "${resp}" | grep -q '"no matching targets found"'; then
186+
pass "idempotent delete returns no matching targets found"
187+
else
188+
fail "expected no matching targets found, got: ${resp}"
189+
fi
190+
191+
info "Deleting with empty body (expect 400)"
192+
http_code="$(curl -s -o /dev/null -w '%{http_code}' -XDELETE "http://127.0.0.1:${POLICY_PORT}/policy" -d '')"
193+
if [ "${http_code}" = "400" ]; then
194+
pass "empty body returns 400"
195+
else
196+
fail "empty body should return 400, got ${http_code}"
197+
fi
198+
158199
info "Always-rule dynamic check (single transition)"
159200
curl -sSf -XPOST "http://127.0.0.1:${POLICY_PORT}/policy" \
160201
-d '{"defaultAction":"deny","egress":[{"action":"allow","target":"api.github.com"}]}'

specs/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ the sandbox endpoint for the egress port and then calling the sidecar endpoint d
122122
**Main Endpoints:**
123123
- `GET /policy` - Get the current egress policy
124124
- `PATCH /policy` - Merge new egress rules into the current policy
125+
- `DELETE /policy` - Remove specific egress rules from the current policy by target
125126

126127
## Technical Features
127128

specs/README_zh.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@
121121
**主要端点:**
122122
- `GET /policy` - 获取当前 egress 策略
123123
- `PATCH /policy` - 将新的 egress 规则合并到当前策略
124+
- `DELETE /policy` - 按 target 删除当前策略中的指定 egress 规则
124125

125126
## 技术特性
126127

0 commit comments

Comments
 (0)