Skip to content

Commit 1bb5b67

Browse files
committed
authz: Add onPolicyUpdate callback to file watcher.
Signed-off-by: Keith Collister <kcollister@google.com>
1 parent 5013974 commit 1bb5b67

3 files changed

Lines changed: 106 additions & 19 deletions

File tree

authz/grpc_authz_end2end_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ var authzTests = map[string]struct {
9595
]
9696
}`,
9797
md: metadata.Pairs("key-abc", "val-abc"),
98-
wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"),
98+
wantStatus: status.New(codes.PermissionDenied, "authz: unauthorized RPC request rejected"),
9999
},
100100
"DeniesRPCMatchInDenyAndAllow": {
101101
authzPolicy: `{
@@ -125,7 +125,7 @@ var authzTests = map[string]struct {
125125
}
126126
]
127127
}`,
128-
wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"),
128+
wantStatus: status.New(codes.PermissionDenied, "authz: unauthorized RPC request rejected"),
129129
},
130130
"AllowsRPCNoMatchInDenyMatchInAllow": {
131131
authzPolicy: `{
@@ -192,7 +192,7 @@ var authzTests = map[string]struct {
192192
}
193193
]
194194
}`,
195-
wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"),
195+
wantStatus: status.New(codes.PermissionDenied, "authz: unauthorized RPC request rejected"),
196196
},
197197
"AllowsRPCEmptyDenyMatchInAllow": {
198198
authzPolicy: `{
@@ -240,7 +240,7 @@ var authzTests = map[string]struct {
240240
}
241241
]
242242
}`,
243-
wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"),
243+
wantStatus: status.New(codes.PermissionDenied, "authz: unauthorized RPC request rejected"),
244244
},
245245
"DeniesRPCRequestWithPrincipalsFieldOnUnauthenticatedConnection": {
246246
authzPolicy: `{
@@ -255,7 +255,7 @@ var authzTests = map[string]struct {
255255
}
256256
]
257257
}`,
258-
wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"),
258+
wantStatus: status.New(codes.PermissionDenied, "authz: unauthorized RPC request rejected"),
259259
},
260260
"DeniesRPCRequestNoMatchInAllowFailsPresenceMatch": {
261261
authzPolicy: `{
@@ -284,7 +284,7 @@ var authzTests = map[string]struct {
284284
]
285285
}`,
286286
md: metadata.Pairs("key-abc", ""),
287-
wantStatus: status.New(codes.PermissionDenied, "unauthorized RPC request rejected"),
287+
wantStatus: status.New(codes.PermissionDenied, "authz: unauthorized RPC request rejected"),
288288
},
289289
}
290290

authz/grpc_authz_server_interceptors.go

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (i *StaticInterceptor) UnaryInterceptor(ctx context.Context, req any, _ *gr
6565
if logger.V(2) {
6666
logger.Infof("unauthorized RPC request rejected: %v", err)
6767
}
68-
return nil, status.Errorf(codes.PermissionDenied, "unauthorized RPC request rejected")
68+
return nil, status.Errorf(codes.PermissionDenied, "authz: unauthorized RPC request rejected")
6969
}
7070
return nil, err
7171
}
@@ -82,7 +82,7 @@ func (i *StaticInterceptor) StreamInterceptor(srv any, ss grpc.ServerStream, _ *
8282
if logger.V(2) {
8383
logger.Infof("unauthorized RPC request rejected: %v", err)
8484
}
85-
return status.Errorf(codes.PermissionDenied, "unauthorized RPC request rejected")
85+
return status.Errorf(codes.PermissionDenied, "authz: unauthorized RPC request rejected")
8686
}
8787
return err
8888
}
@@ -92,24 +92,54 @@ func (i *StaticInterceptor) StreamInterceptor(srv any, ss grpc.ServerStream, _ *
9292
// FileWatcherInterceptor contains details used to make authorization decisions
9393
// by watching a file path that contains authorization policy in JSON format.
9494
type FileWatcherInterceptor struct {
95+
options FileWatcherOptions
9596
internalInterceptor unsafe.Pointer // *StaticInterceptor
96-
policyFile string
9797
policyContents []byte
98-
refreshDuration time.Duration
9998
cancel context.CancelFunc
10099
}
101100

101+
// FileWatcherOptions contains configuration options for the
102+
// FileWatcherInterceptor.
103+
//
104+
// # Experimental
105+
//
106+
// Notice: This API is EXPERIMENTAL and may be changed or removed in a
107+
// later release.
108+
type FileWatcherOptions struct {
109+
// PolicyFile contains a JSON string of the authorization policy.
110+
PolicyFile string
111+
// RefreshDuration is the delay between policy refreshes.
112+
RefreshDuration time.Duration
113+
// OnPolicyUpdate is a callback to be invoked when a policy is
114+
// loaded/updated. The loaded policy string is passed as an argument.
115+
//
116+
// The callback is executed synchronously, so should complete quickly or
117+
// risk blocking future updates.
118+
OnPolicyUpdate func(string)
119+
}
120+
102121
// NewFileWatcher returns a new FileWatcherInterceptor from a policy file
103122
// that contains JSON string of authorization policy and a refresh duration to
104123
// specify the amount of time between policy refreshes.
105124
func NewFileWatcher(file string, duration time.Duration) (*FileWatcherInterceptor, error) {
106-
if file == "" {
107-
return nil, fmt.Errorf("authorization policy file path is empty")
125+
return NewFileWatcherWithOptions(FileWatcherOptions{PolicyFile: file, RefreshDuration: duration, OnPolicyUpdate: nil})
126+
}
127+
128+
// NewFileWatcherWithOptions returns a new FileWatcherInterceptor from a set of
129+
// options.
130+
//
131+
// # Experimental
132+
//
133+
// Notice: This API is EXPERIMENTAL and may be changed or removed in a
134+
// later release.
135+
func NewFileWatcherWithOptions(options FileWatcherOptions) (*FileWatcherInterceptor, error) {
136+
if options.PolicyFile == "" {
137+
return nil, fmt.Errorf("authz: authorization policy file path is empty")
108138
}
109-
if duration <= time.Duration(0) {
110-
return nil, fmt.Errorf("requires refresh interval(%v) greater than 0s", duration)
139+
if options.RefreshDuration <= time.Duration(0) {
140+
return nil, fmt.Errorf("authz: requires refresh interval(%v) greater than 0s", options.RefreshDuration)
111141
}
112-
i := &FileWatcherInterceptor{policyFile: file, refreshDuration: duration}
142+
i := &FileWatcherInterceptor{options: options}
113143
if err := i.updateInternalInterceptor(); err != nil {
114144
return nil, err
115145
}
@@ -121,7 +151,7 @@ func NewFileWatcher(file string, duration time.Duration) (*FileWatcherIntercepto
121151
}
122152

123153
func (i *FileWatcherInterceptor) run(ctx context.Context) {
124-
ticker := time.NewTicker(i.refreshDuration)
154+
ticker := time.NewTicker(i.options.RefreshDuration)
125155
for {
126156
if err := i.updateInternalInterceptor(); err != nil {
127157
logger.Warningf("authorization policy reload status err: %v", err)
@@ -140,9 +170,9 @@ func (i *FileWatcherInterceptor) run(ctx context.Context) {
140170
// constructor, if there is an error in reading the file or parsing the policy, the
141171
// previous internalInterceptors will not be replaced.
142172
func (i *FileWatcherInterceptor) updateInternalInterceptor() error {
143-
policyContents, err := os.ReadFile(i.policyFile)
173+
policyContents, err := os.ReadFile(i.options.PolicyFile)
144174
if err != nil {
145-
return fmt.Errorf("policyFile(%s) read failed: %v", i.policyFile, err)
175+
return fmt.Errorf("policyFile(%s) read failed: %v", i.options.PolicyFile, err)
146176
}
147177
if bytes.Equal(i.policyContents, policyContents) {
148178
return nil
@@ -155,6 +185,9 @@ func (i *FileWatcherInterceptor) updateInternalInterceptor() error {
155185
}
156186
atomic.StorePointer(&i.internalInterceptor, unsafe.Pointer(interceptor))
157187
logger.Infof("authorization policy reload status: successfully loaded new policy %v", policyContentsString)
188+
if i.options.OnPolicyUpdate != nil {
189+
i.options.OnPolicyUpdate(policyContentsString)
190+
}
158191
return nil
159192
}
160193

authz/grpc_authz_server_interceptors_test.go

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package authz_test
2020

2121
import (
22+
"context"
2223
"fmt"
2324
"os"
2425
"path"
@@ -28,6 +29,8 @@ import (
2829
"google.golang.org/grpc/authz"
2930
)
3031

32+
const defaultTestTimeout = 10 * time.Second
33+
3134
func createTmpPolicyFile(t *testing.T, dirSuffix string, policy []byte) string {
3235
t.Helper()
3336

@@ -85,7 +88,7 @@ func (s) TestNewFileWatcher(t *testing.T) {
8588
}{
8689
"InvalidRefreshDurationFailsToCreateInterceptor": {
8790
refreshDuration: time.Duration(0),
88-
wantErr: fmt.Errorf("requires refresh interval(0s) greater than 0s"),
91+
wantErr: fmt.Errorf("authz: requires refresh interval(0s) greater than 0s"),
8992
},
9093
"InvalidPolicyFailsToCreateInterceptor": {
9194
authzPolicy: `{}`,
@@ -118,3 +121,54 @@ func (s) TestNewFileWatcher(t *testing.T) {
118121
})
119122
}
120123
}
124+
125+
func (s) TestOnPolicyUpdate(t *testing.T) {
126+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
127+
defer cancel()
128+
129+
updates := make(chan string, 1)
130+
onPolicyUpdate := func(s string) {
131+
updates <- s
132+
}
133+
134+
content := `{"name": "foo1", "allow_rules":[{"name":"bar"}]}`
135+
file := createTmpPolicyFile(t, "onpolicyupdate", []byte(content))
136+
opts := authz.FileWatcherOptions{PolicyFile: file, RefreshDuration: 50 * time.Millisecond, OnPolicyUpdate: onPolicyUpdate}
137+
i, err := authz.NewFileWatcherWithOptions(opts)
138+
if err != nil {
139+
t.Fatalf("NewFileWatcherWithOptions(%v) returned err: %v", opts, err)
140+
}
141+
defer i.Close()
142+
143+
select {
144+
case <-ctx.Done():
145+
t.Fatalf("Timeout waiting for policy update")
146+
case update := <-updates:
147+
if update != content {
148+
t.Fatalf("Unexpected contents on first load of policy file: got=%v, want=%v", update, content)
149+
}
150+
}
151+
152+
// Tweak the file, expect an update.
153+
content = `{"name": "foo2", "allow_rules":[{"name":"bar"}]}`
154+
if err := os.WriteFile(file, []byte(content), os.ModePerm); err != nil {
155+
t.Fatalf("os.WriteFile(%q) failed: %v", file, err)
156+
}
157+
158+
select {
159+
case <-ctx.Done():
160+
t.Fatalf("Timeout waiting for policy update")
161+
case update := <-updates:
162+
if update != content {
163+
t.Fatalf("Unexpected contents after policy file changed: got=%v, want=%v", update, content)
164+
}
165+
}
166+
167+
sCtx, sCancel := context.WithTimeout(ctx, 100*time.Millisecond)
168+
defer sCancel()
169+
select {
170+
case <-updates:
171+
t.Fatal("OnPolicyUpdate callback invoked more times than expected")
172+
case <-sCtx.Done():
173+
}
174+
}

0 commit comments

Comments
 (0)