Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenAPI spec as input #2792

Closed
steve-chavez opened this issue May 20, 2023 · 3 comments
Closed

OpenAPI spec as input #2792

steve-chavez opened this issue May 20, 2023 · 3 comments
Labels
idea Needs of discussion to become an enhancement, not ready for implementation

Comments

@steve-chavez
Copy link
Member

Problem

Database as Single Source of Truth has its limitations for an API.

  1. We cannot restrict which operators can be used for columns (how to restrict which operators can be used and which props can be used for ordering #2442)
  2. We cannot restrict which columns can be used for ORDER BY (how to restrict which operators can be used and which props can be used for ordering #2442).
  3. We cannot customize the path of a resource, be it table, view or function(Customize URL structure - Confusion about /rpc/ prefix, move stored procedures to root level #1086).
  4. We cannot customize the resource representation, meaning we can't map an aggregation to a media type(Feature request: allow user-defined function to handle application-specific Accept headers #1548)

We have tried hard to encode these concerns in the database itself:

  1. Non-viable solution(too complicated): Support for PGroonga operators #2028 (comment)
  2. No solution
  3. Viable solution: Customize URL structure - Confusion about /rpc/ prefix, move stored procedures to root level #1086 (comment)
  4. Viable solution: Add pgrst.accept setting to RPC #1582 (comment).

While 3 and 4 have viable solutions(4 being specially clever and sound), these still feel like concerns that shouldn't be in the database. We just need a way to add metadata.

Solution

Accept an OpenAPI spec as input. It can be a file or also live in the database as a function(db-openapi-input). The user can take our generated OpenAPI as a blueprint and modify it to its needs.

Let's say we have an employees table, our generated OpenAPI can be:

(using YAML as it's simpler)

paths:
  /employees:
    x-pg-table: employees
    get:
     parameters:
        - in: query
          name: eq
        - in: query
          name: order
      # ...
      responses:
          # ...
          content:  
            application/json:  
              x-pgrst-rep:
                $ref: '#/definitions/jsonRep'                
         
definitions:
  JsonRep:
    x-pg-function: "json_agg"

(Note: using openapi extensions)

To solve each case:

  1. Remove the unwanted parameters from the spec.
  2. Insert an enum into order. e.g. enum: [id, name]
  3. Edit the path in the spec. PostgREST will know it belongs to the employees table thanks to x-pg-table.
  4. Edit the x-pgrst-rep in the spec. A specific x-pg-function can be used per resource and per media type. It can also be DRYed up by using $ref.

For cases where there's a big OpenAPI, we can optionally store the spec contents in a table so it's easier to edit. This table can finally produce the openapi spec using a json_agg.

@steve-chavez steve-chavez added the idea Needs of discussion to become an enhancement, not ready for implementation label May 20, 2023
@steve-chavez
Copy link
Member Author

steve-chavez commented May 22, 2023

Some issues with the openAPI spec:

  • The status code is a subkey of the media type, when it should be the other way around. Really the status code is an implementation detail. I believe API clients should only handle media types. For example, on failure we can reply with application/problem+json and success with application/json. A client then can process the json accordingly.
  • It can be used to remove a path and hide a table. This weakens the database security as users could do this instead of hiding the table on a private schema or revoking all privileges on the table. We should try to add security mechanisms not replace them.

@steve-chavez
Copy link
Member Author

Disregarding problem 3 and 4 for now. Also the openapi spec. I think we can solve 1 and 2 using the pre-config hook on #2703.

Say I want to only allow eq operators and only on primary keys:

-- pgrst.query_grammar is the config prefix 
-- '*' is the default "allow all"
-- ' ' empty is "disallow all"
select set_config('pgrst.query_grammar.op', ' ', true);

-- op.eq is the allowlist for columns
select set_config('pgrst.query_grammar.op.eq', array_agg(c.table_name || '.' || c.column_name)::text, true) 
from information_schema.table_constraints tc
join information_schema.constraint_column_usage as ccu using (constraint_schema, constraint_name)
join information_schema.columns as c
  on c.table_schema = tc.constraint_schema and tc.table_name = c.table_name and ccu.column_name = c.column_name
where constraint_type = 'PRIMARY KEY' and c.table_schema = 'test';

select current_setting('pgrst.query_grammar.op.eq') as eq_allowed;

eq_allowed | {items.id,items2.id,items3.id,clients.id,compound_pk.k1,compound_pk.k2,..

A similar config can be used for order too:

select set_config('pgrst.query_grammar.order', 'table.col ', true);

I think that's so far the best interface for solving 1 and 2. It provides a lot of flexibility and removes smartness from the config format. It can be wrapped in a function too.

create or replace function pgrst_query_grammar()
returns void as $$
  select set_config('pgrst.query_grammar.op', '', true);
  select set_config('pgrst.query_grammar.op.eq', array_agg(c.table_name || '.' || c.column_name)::text, true)
  --...
$$ language sql;

@steve-chavez
Copy link
Member Author

Closing and following up on #2805

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea Needs of discussion to become an enhancement, not ready for implementation
Development

No branches or pull requests

1 participant