search

breadboard-ai / breadboard

A library for prototyping generative AI applications.
Apache License 2.0
183 stars 25 forks source link

[board-server] Move CORS configuration to an environment variable #2896

Closed timswanson-google closed 2 months ago

timswanson-google commented 2 months ago

The CORS config for Board Server is currently configured using a document called configuration/board-server-cors in Firestore

https://github.com/breadboard-ai/breadboard/blob/70445019c2da17e2deaabcee04d4bbb0cc2648b0/packages/board-server/src/server/store.ts#L132

Connection Server accomplishes this same thing using an ALLOWED_ORIGINS environment variable.

https://github.com/breadboard-ai/breadboard/blob/70445019c2da17e2deaabcee04d4bbb0cc2648b0/packages/connection-server/src/index.ts#L16

The Connection Server approach is better for a number of reasons.

The set of allowed origins is a core configuration value, and is unlikely to change frequently (if ever), so having it in storage offers no additional value over having it in config.

Also, using an environment variable allows explicitly setting the value when running locally.

https://github.com/breadboard-ai/breadboard/blob/70445019c2da17e2deaabcee04d4bbb0cc2648b0/packages/connection-server/package.json#L39

Currently, Board Server accomplishes this by explicitly checking for "localhost" in code.

https://github.com/breadboard-ai/breadboard/blob/70445019c2da17e2deaabcee04d4bbb0cc2648b0/packages/board-server/src/server/cors.ts#L52-L56

Finally, there is an effort to decouple Board Server from Firestore (e.g. #2889). This is one such dependency that will need to be broken for this to happen.

timswanson-google commented 2 months ago

Since this is a breaking change for existing Board Servers -- they will need to update their config to continue to work properly -- I think this will be a major version bump for Board Server.

timswanson-google commented 2 months ago

Reopening. Still need to remove (now unused) references to the table in code.