mirror of
https://github.com/open-policy-agent/opa.git
synced 2026-08-12 19:32:48 -06:00
server: Add early exit on PUT /v1/policies
These changes update the server to compare the body of the HTTP request with the existing policy in the store. If the bytes are equal, the server returns immediately. These changes are being made to workaround poor parser performance given deeply nested JSON objects that are commonly found in Kubernetes use cases. Specifically, we've observed very high parse times (e.g., over a second) when processing policies in production environments. The problem becomes worse when kube-mgmt is being used because it periodically resyncs the policies causing load even if the policies are not changing. Signed-off-by: Torin Sandall <torinsandall@gmail.com>
This commit is contained in:
+22
-9
@@ -1422,10 +1422,30 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
m.Timer("server_read_bytes").Stop()
|
||||
|
||||
txn, err := s.store.NewTransaction(ctx, storage.WriteParams)
|
||||
if err != nil {
|
||||
writer.ErrorAuto(w, err)
|
||||
return
|
||||
}
|
||||
|
||||
if bs, err := s.store.GetPolicy(ctx, txn, path); err != nil {
|
||||
if !storage.IsNotFound(err) {
|
||||
s.abortAuto(ctx, txn, w, err)
|
||||
return
|
||||
}
|
||||
} else if bytes.Equal(buf, bs) {
|
||||
s.store.Abort(ctx, txn)
|
||||
response := types.PolicyPutResponseV1{}
|
||||
if includeMetrics {
|
||||
response.Metrics = m.All()
|
||||
}
|
||||
writer.JSON(w, http.StatusOK, response, pretty)
|
||||
return
|
||||
}
|
||||
|
||||
m.Timer(metrics.RegoModuleParse).Start()
|
||||
|
||||
parsedMod, err := ast.ParseModule(path, string(buf))
|
||||
|
||||
m.Timer(metrics.RegoModuleParse).Stop()
|
||||
|
||||
if err != nil {
|
||||
@@ -1443,13 +1463,6 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
txn, err := s.store.NewTransaction(ctx, storage.WriteParams)
|
||||
|
||||
if err != nil {
|
||||
writer.ErrorAuto(w, err)
|
||||
return
|
||||
}
|
||||
|
||||
if err := s.checkPolicyPackageScope(ctx, txn, parsedMod.Package); err != nil {
|
||||
s.abortAuto(ctx, txn, w, err)
|
||||
return
|
||||
|
||||
@@ -19,6 +19,7 @@ import (
|
||||
"net/http/httputil"
|
||||
"net/url"
|
||||
"reflect"
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -1598,6 +1599,49 @@ q[x] { p[x] }`,
|
||||
}
|
||||
}
|
||||
|
||||
func TestPoliciesPutV1Noop(t *testing.T) {
|
||||
f := newFixture(t)
|
||||
f.v1("PUT", "/policies/test?metrics", `package foo`, 200, "")
|
||||
f.reset()
|
||||
f.v1("PUT", "/policies/test?metrics", `package foo`, 200, "")
|
||||
|
||||
var resp types.PolicyPutResponseV1
|
||||
if err := json.NewDecoder(f.recorder.Body).Decode(&resp); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
exp := []string{"timer_server_read_bytes_ns"}
|
||||
|
||||
// Sort the metric keys and compare to expected value. We're assuming the
|
||||
// server skips parsing if the bytes are equal.
|
||||
result := []string{}
|
||||
|
||||
for k := range resp.Metrics {
|
||||
result = append(result, k)
|
||||
}
|
||||
|
||||
sort.Strings(result)
|
||||
|
||||
if !reflect.DeepEqual(exp, result) {
|
||||
t.Fatalf("Expected %v but got %v", exp, result)
|
||||
}
|
||||
|
||||
f.reset()
|
||||
|
||||
// Ensure subsequent update with changed policy parses the body.
|
||||
f.v1("PUT", "/policies/test?metrics", "package foo\np = 1", 200, "")
|
||||
|
||||
var resp2 types.PolicyPutResponseV1
|
||||
if err := json.NewDecoder(f.recorder.Body).Decode(&resp2); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if _, ok := resp2.Metrics["timer_rego_module_parse_ns"]; !ok {
|
||||
t.Fatalf("Expected parse module metric in response but got %v", resp2)
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
func TestPoliciesListV1(t *testing.T) {
|
||||
f := newFixture(t)
|
||||
put := newReqV1(http.MethodPut, "/policies/1", testMod)
|
||||
|
||||
Reference in New Issue
Block a user