Skip to content

Commit 98f1b32

Browse files
authored
Merge pull request #121 from josephschorr/config-improvements
Small improvements and fixes to configuration of the proxy
2 parents aa1d072 + 305a193 commit 98f1b32

File tree

6 files changed

+78
-37
lines changed

6 files changed

+78
-37
lines changed

cmd/spicedb-kubeapi-proxy/main.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,15 @@ func NewProxyCommand(ctx context.Context) *cobra.Command {
3535
requests with SpiceDB and keeps relationship data up to date. The proxy handles
3636
TLS termination and authentication with a backend kube apiserver.`,
3737
RunE: func(cmd *cobra.Command, args []string) error {
38-
if err := options.Complete(ctx); err != nil {
38+
completed, err := options.Complete(ctx)
39+
if err != nil {
3940
return err
4041
}
4142
if errs := options.Validate(); errs != nil {
4243
return errors.NewAggregate(errs)
4344
}
4445

45-
server, err := proxy.NewServer(ctx, *options)
46+
server, err := proxy.NewServer(ctx, completed)
4647
if err != nil {
4748
return err
4849
}

e2e/e2e_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,11 @@ var _ = SynchronizedBeforeSuite(func() []byte {
133133
opts.SpiceDBOptions.SpiceDBEndpoint = proxy.EmbeddedSpiceDBEndpoint
134134
opts.SecureServing.BindAddress = net.ParseIP("127.0.0.1")
135135
opts.Authentication.BuiltInOptions.ClientCert.ClientCA = clientCA.Path()
136-
Expect(opts.Complete(ctx)).To(Succeed())
137-
proxySrv, err = proxy.NewServer(ctx, *opts)
136+
137+
c, err := opts.Complete(ctx)
138+
Expect(err).To(Succeed())
139+
140+
proxySrv, err = proxy.NewServer(ctx, c)
138141
Expect(err).To(Succeed())
139142

140143
// speed up backoff for tests

pkg/proxy/authn_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,11 @@ func runProxyRequest(t testing.TB, ctx context.Context, headers map[string][]str
185185
return rules.NewResolveInputFromHttp(req)
186186
})
187187

188-
require.NoError(t, opts.Complete(ctx))
189-
proxySrv, err := NewServer(ctx, *opts)
188+
c, err := opts.Complete(context.Background())
189+
require.NoError(t, err)
190+
require.NotNil(t, c)
191+
192+
proxySrv, err := NewServer(ctx, c)
190193
require.NoError(t, err)
191194

192195
// Start the server in a separate context that won't be cancelled until we're done

pkg/proxy/options.go

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,11 @@ func (o *Options) FromConfigFlags(configFlags *genericclioptions.ConfigFlags) *O
126126
return nil, nil, fmt.Errorf("unable to load kube REST config: %w", err)
127127
}
128128

129-
return restConfig, restConfig.WrapTransport(http.DefaultTransport), nil
129+
transport, err := rest.TransportFor(restConfig)
130+
if err != nil {
131+
return nil, nil, fmt.Errorf("unable to create transport: %w", err)
132+
}
133+
return restConfig, transport, nil
130134
}
131135
return o
132136
}
@@ -144,9 +148,13 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) {
144148
fs.StringVar(&o.RuleConfigFile, "rule-config", "", "The path to a file containing proxy rule configuration")
145149
}
146150

147-
func (o *Options) Complete(ctx context.Context) error {
151+
type CompletedConfig struct {
152+
config *Options
153+
}
154+
155+
func (o *Options) Complete(ctx context.Context) (*CompletedConfig, error) {
148156
if err := logsv1.ValidateAndApply(o.Logs, utilfeature.DefaultFeatureGate); err != nil {
149-
return err
157+
return nil, err
150158
}
151159

152160
var err error
@@ -171,7 +179,7 @@ func (o *Options) Complete(ctx context.Context) error {
171179
default:
172180
backendConfig, err := o.configFromPath()
173181
if err != nil {
174-
return fmt.Errorf("couldn't load kubeconfig from path: %w", err)
182+
return nil, fmt.Errorf("couldn't load kubeconfig from path: %w", err)
175183
}
176184

177185
o.RestConfigFunc = func() (*rest.Config, http.RoundTripper, error) {
@@ -195,15 +203,15 @@ func (o *Options) Complete(ctx context.Context) error {
195203
if o.Matcher == nil {
196204
ruleFile, err := os.Open(o.RuleConfigFile)
197205
if err != nil {
198-
return fmt.Errorf("couldn't open rule config file: %w", err)
206+
return nil, fmt.Errorf("couldn't open rule config file: %w", err)
199207
}
200208
ruleConfigs, err := proxyrule.Parse(ruleFile)
201209
if err != nil {
202-
return fmt.Errorf("couldn't parse rule config file: %w", err)
210+
return nil, fmt.Errorf("couldn't parse rule config file: %w", err)
203211
}
204212
o.Matcher, err = rules.NewMapMatcher(ruleConfigs)
205213
if err != nil {
206-
return fmt.Errorf("couldn't compile rule configs: %w", err)
214+
return nil, fmt.Errorf("couldn't compile rule configs: %w", err)
207215
}
208216
}
209217
if o.InputExtractor == nil {
@@ -215,35 +223,35 @@ func (o *Options) Complete(ctx context.Context) error {
215223
}
216224

217225
if err := o.SecureServing.MaybeDefaultWithSelfSignedCerts("localhost", []string{"kubernetes.default.svc", "kubernetes.default", "kubernetes"}, nil); err != nil {
218-
return err
226+
return nil, err
219227
}
220228

221229
var loopbackClientConfig *rest.Config
222230
if err := o.SecureServing.ApplyTo(&o.ServingInfo, &loopbackClientConfig); err != nil {
223-
return err
231+
return nil, err
224232
}
225233
if err := o.Authentication.ApplyTo(ctx, &o.AuthenticationInfo, o.ServingInfo); err != nil {
226-
return err
234+
return nil, err
227235
}
228236

229237
o.AdditionalAuthEnabled = o.Authentication.AdditionalAuthEnabled()
230238

231239
spicedbURl, err := url.Parse(o.SpiceDBOptions.SpiceDBEndpoint)
232240
if err != nil {
233-
return fmt.Errorf("unable to parse SpiceDB endpoint URL: %w", err)
241+
return nil, fmt.Errorf("unable to parse SpiceDB endpoint URL: %w", err)
234242
}
235243

236244
var conn *grpc.ClientConn
237245
if spicedbURl.Scheme == "embedded" {
238246
klog.FromContext(ctx).WithValues("spicedb-endpoint", spicedbURl).Info("using embedded SpiceDB")
239247
o.SpiceDBOptions.EmbeddedSpiceDB, err = spicedb.NewServer(ctx, spicedbURl.Path)
240248
if err != nil {
241-
return fmt.Errorf("unable to stand up embedded SpiceDB: %w", err)
249+
return nil, fmt.Errorf("unable to stand up embedded SpiceDB: %w", err)
242250
}
243251

244252
conn, err = o.SpiceDBOptions.EmbeddedSpiceDB.GRPCDialContext(ctx, grpc.WithTransportCredentials(insecure.NewCredentials()))
245253
if err != nil {
246-
return fmt.Errorf("unable to open gRPC connection with embedded SpiceDB: %w", err)
254+
return nil, fmt.Errorf("unable to open gRPC connection with embedded SpiceDB: %w", err)
247255
}
248256
} else {
249257
klog.FromContext(ctx).WithValues("spicedb-endpoint", o.SpiceDBOptions.SpiceDBEndpoint).
@@ -255,7 +263,7 @@ func (o *Options) Complete(ctx context.Context) error {
255263

256264
tokens := strings.Split(o.SpiceDBOptions.SecureSpiceDBTokensBySpace, ",")
257265
if len(tokens) == 0 {
258-
return fmt.Errorf("no SpiceDB token defined")
266+
return nil, fmt.Errorf("no SpiceDB token defined")
259267
}
260268

261269
token := strings.TrimSpace(tokens[0])
@@ -272,12 +280,12 @@ func (o *Options) Complete(ctx context.Context) error {
272280
if len(o.SpiceDBOptions.SpicedbCAPath) > 0 {
273281
certs, err = grpcutil.WithCustomCerts(verification, o.SpiceDBOptions.SpicedbCAPath)
274282
if err != nil {
275-
return fmt.Errorf("unable to load custom certificates: %w", err)
283+
return nil, fmt.Errorf("unable to load custom certificates: %w", err)
276284
}
277285
} else {
278286
certs, err = grpcutil.WithSystemCerts(verification)
279287
if err != nil {
280-
return fmt.Errorf("unable to load system certificates: %w", err)
288+
return nil, fmt.Errorf("unable to load system certificates: %w", err)
281289
}
282290
}
283291

@@ -289,7 +297,7 @@ func (o *Options) Complete(ctx context.Context) error {
289297
defer cancel()
290298
conn, err = grpc.DialContext(timeoutCtx, o.SpiceDBOptions.SpiceDBEndpoint, opts...)
291299
if err != nil {
292-
return fmt.Errorf("unable to open gRPC connection to remote SpiceDB at %s: %w", o.SpiceDBOptions.SpiceDBEndpoint, err)
300+
return nil, fmt.Errorf("unable to open gRPC connection to remote SpiceDB at %s: %w", o.SpiceDBOptions.SpiceDBEndpoint, err)
293301
}
294302
}
295303

@@ -301,7 +309,7 @@ func (o *Options) Complete(ctx context.Context) error {
301309
o.WatchClient = v1.NewWatchServiceClient(conn)
302310
}
303311

304-
return nil
312+
return &CompletedConfig{o}, nil
305313
}
306314

307315
func (o *Options) configFromPath() (*clientcmdapi.Config, error) {

pkg/proxy/options_test.go

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,19 @@ func TestKubeConfig(t *testing.T) {
2828
opts := optionsForTesting(t)
2929
opts.SpiceDBOptions.SpiceDBEndpoint = EmbeddedSpiceDBEndpoint
3030
require.Empty(t, opts.Validate())
31-
require.NoError(t, opts.Complete(context.Background()))
31+
32+
c, err := opts.Complete(context.Background())
33+
require.NoError(t, err)
34+
require.NotNil(t, c)
3235

3336
require.NoError(t, logsv1.ResetForTest(utilfeature.DefaultFeatureGate))
3437
opts = optionsForTesting(t)
3538
opts.BackendKubeconfigPath = uuid.NewString()
36-
err := opts.Complete(context.Background())
39+
40+
c, err = opts.Complete(context.Background())
3741
require.ErrorContains(t, err, "couldn't load kubeconfig")
3842
require.ErrorContains(t, err, opts.BackendKubeconfigPath)
43+
require.Nil(t, c, "expected nil config on error")
3944
}
4045

4146
func TestInClusterConfig(t *testing.T) {
@@ -46,9 +51,12 @@ func TestInClusterConfig(t *testing.T) {
4651
opts.BackendKubeconfigPath = ""
4752
opts.UseInClusterConfig = true
4853
require.Empty(t, opts.Validate())
49-
err := opts.Complete(context.Background())
54+
55+
c, err := opts.Complete(context.Background())
5056
require.NoError(t, err)
57+
require.NotNil(t, c)
5158
require.NotNil(t, opts.RestConfigFunc, "missing kube client REST config")
59+
5260
_, _, err = opts.RestConfigFunc()
5361
require.ErrorContains(t, err, "unable to load in-cluster configuration")
5462
}
@@ -57,7 +65,11 @@ func TestEmbeddedSpiceDB(t *testing.T) {
5765
opts := optionsForTesting(t)
5866
opts.SpiceDBOptions.SpiceDBEndpoint = EmbeddedSpiceDBEndpoint
5967
require.Empty(t, opts.Validate())
60-
require.NoError(t, opts.Complete(context.Background()))
68+
69+
c, err := opts.Complete(context.Background())
70+
require.NoError(t, err)
71+
require.NotNil(t, c)
72+
6173
require.NotNil(t, opts.SpiceDBOptions.EmbeddedSpiceDB)
6274
require.NotNil(t, opts.PermissionsClient)
6375
require.NotNil(t, opts.WatchClient)
@@ -79,13 +91,16 @@ func TestRemoteSpiceDB(t *testing.T) {
7991
opts.SpiceDBOptions.Insecure = true
8092
opts.SpiceDBOptions.SecureSpiceDBTokensBySpace = "foobar"
8193
require.Empty(t, opts.Validate())
82-
require.NoError(t, opts.Complete(context.Background()))
94+
95+
c, err := opts.Complete(context.Background())
96+
require.NoError(t, err)
97+
require.NotNil(t, c)
8398

8499
require.Nil(t, opts.SpiceDBOptions.EmbeddedSpiceDB)
85100
require.NotNil(t, opts.PermissionsClient)
86101
require.NotNil(t, opts.WatchClient)
87102

88-
_, err := opts.PermissionsClient.CheckPermission(ctx, &v1.CheckPermissionRequest{})
103+
_, err = opts.PermissionsClient.CheckPermission(ctx, &v1.CheckPermissionRequest{})
89104
grpcutil.RequireStatus(t, codes.InvalidArgument, err)
90105
}
91106

@@ -95,14 +110,19 @@ func TestRemoteSpiceDBCerts(t *testing.T) {
95110
opts.SpiceDBOptions.SecureSpiceDBTokensBySpace = "foobar"
96111
opts.SpiceDBOptions.SpicedbCAPath = "test"
97112
require.Empty(t, opts.Validate())
98-
require.ErrorContains(t, opts.Complete(context.Background()), "unable to load custom certificates")
113+
114+
_, err := opts.Complete(context.Background())
115+
require.ErrorContains(t, err, "unable to load custom certificates")
99116
}
100117

101118
func TestRuleConfig(t *testing.T) {
102119
opts := optionsForTesting(t)
103120
opts.SpiceDBOptions.SpiceDBEndpoint = EmbeddedSpiceDBEndpoint
104121
require.Empty(t, opts.Validate())
105-
require.NoError(t, opts.Complete(context.Background()))
122+
123+
c, err := opts.Complete(context.Background())
124+
require.NoError(t, err)
125+
require.NotNil(t, c)
106126

107127
rules := opts.Matcher.Match(&request.RequestInfo{
108128
APIGroup: "authzed.com",
@@ -135,7 +155,9 @@ prefilter:
135155
opts.SpiceDBOptions.SpiceDBEndpoint = EmbeddedSpiceDBEndpoint
136156
opts.RuleConfigFile = errConfigFile
137157
require.Empty(t, opts.Validate())
138-
require.ErrorContains(t, opts.Complete(context.Background()), "SyntaxError")
158+
159+
_, err = opts.Complete(context.Background())
160+
require.ErrorContains(t, err, "SyntaxError")
139161
}
140162

141163
func optionsForTesting(t *testing.T) *Options {

pkg/proxy/server.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,13 @@ type Server struct {
4444
Matcher *rules.Matcher
4545
}
4646

47-
func NewServer(ctx context.Context, o Options) (*Server, error) {
47+
func NewServer(ctx context.Context, c *CompletedConfig) (*Server, error) {
48+
if c == nil {
49+
return nil, fmt.Errorf("nil completed config")
50+
}
51+
4852
s := &Server{
49-
opts: o,
53+
opts: *c.config,
5054
}
5155

5256
var err error
@@ -126,15 +130,15 @@ func NewServer(ctx context.Context, o Options) (*Server, error) {
126130
workflowClient, worker, err := distributedtx.SetupWithSQLiteBackend(ctx,
127131
s.opts.PermissionsClient,
128132
s.KubeClient.RESTClient(),
129-
o.WorkflowDatabasePath)
133+
c.config.WorkflowDatabasePath)
130134
if err != nil {
131135
return nil, fmt.Errorf("failed to initialize distributed transaction handling: %w", err)
132136
}
133137
s.WorkflowWorker = worker
134138

135139
// Matcher is a pointer to an interface to make it easy to swap at runtime in tests
136140
s.Matcher = &s.opts.Matcher
137-
handler, err := authz.WithAuthorization(clusterProxy, failHandler, restMapper, o.PermissionsClient, o.WatchClient, workflowClient, s.Matcher, s.opts.InputExtractor)
141+
handler, err := authz.WithAuthorization(clusterProxy, failHandler, restMapper, c.config.PermissionsClient, c.config.WatchClient, workflowClient, s.Matcher, s.opts.InputExtractor)
138142
if err != nil {
139143
return nil, fmt.Errorf("unable to create authorization handler: %w", err)
140144
}

0 commit comments

Comments
 (0)