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

feat(compute): adds pg_session_jwt extension to compute image #8888

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

davidgomes
Copy link
Contributor

@davidgomes davidgomes commented Sep 2, 2024

Problem

We need the pg_session_jwt extension in the compute image. This PR adds it.

Summary of changes

I added the pg_session_jwt extension in a very similar way to how the pggraphql and pgtiktoken extensions were added (since they're all written with pgrx). Then I tested this.

$ cd docker-compose/
$ PG_VERSION=16 TAG=10667533475 docker-compose up --build -d
$ psql postgresql://cloud_admin:cloud_admin@localhost:55433/postgres

cloud_admin@postgres=# create extension pg_session_jwt;
CREATE EXTENSION
Time: 43.048 ms

cloud_admin@postgres=# \df auth.*;
                              List of functions
┌────────┬──────────────────┬──────────────────┬─────────────────────┬──────┐
│ Schema │       Name       │ Result data type │ Argument data types │ Type │
├────────┼──────────────────┼──────────────────┼─────────────────────┼──────┤
│ auth   │ get              │ jsonb            │ s text              │ func │
│ auth   │ init             │ void             │ kid bigint, s jsonb │ func │
│ auth   │ jwt_session_init │ void             │ s text              │ func │
│ auth   │ user_id          │ text             │                     │ func │
└────────┴──────────────────┴──────────────────┴─────────────────────┴──────┘
(4 rows)

cloud_admin@postgres=# select auth.init(cast('1' as bigint), to_jsonb(TEXT '{ "kty": "EC", "kid": "571683be-33cf-4e67-bccc-8905c0ebb862", "crv": "P-521", "alg": "ES512", "x": "AM_GsnQvKML2yXdn_OsN8PdgO1Sf9XMXih5vQMKLmJkp-Iz_FFWJUt6uyR_qp4brr8Ji2kjGJgN4cQJpg2kskH7V", "y": "AZg-salw24lCmsBP-BCBa5jT6INkTwLtCOC7o0BIxDVvmIEH1-PQAJVYVJPTFvPMi_PLa0QlOm-ufJYkynwa2Mau" }'));
ERROR:  called `Result::unwrap()` on an `Err` value: Error("invalid type: string \"{ \\\"kty\\\": \\\"EC\\\", \\\"kid\\\": \\\"571683be-33cf-4e67-bccc-8905c0ebb862\\\", \\\"crv\\\": \\\"P-521\\\", \\\"alg\\\": \\\"ES512\\\", \\\"x\\\": \\\"AM_GsnQvKML2yXdn_OsN8PdgO1Sf9XMXih5vQMKLmJkp-Iz_FFWJUt6uyR_qp4brr8Ji2kjGJgN4cQJpg2kskH7V\\\", \\\"y\\\": \\\"AZg-salw24lCmsBP-BCBa5jT6INkTwLtCOC7o0BIxDVvmIEH1-PQAJVYVJPTFvPMi_PLa0QlOm-ufJYkynwa2Mau\\\" }\", expected struct JwkEcKey", line: 0, column: 0)
Time: 6.991 ms

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.
  • Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
  • If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.

Checklist before merging

  • Move the download location to a proper URL

@davidgomes davidgomes marked this pull request as draft September 2, 2024 12:26
Copy link

github-actions bot commented Sep 2, 2024

5013 tests run: 4855 passed, 0 failed, 158 skipped (full report)


Flaky tests (3)

Postgres 17

Postgres 16

Code coverage* (full report)

  • functions: 31.4% (7489 of 23886 functions)
  • lines: 49.6% (60114 of 121201 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
a257f8b at 2024-10-01T08:31:07.440Z :recycle:

@davidgomes
Copy link
Contributor Author

CC @devjv @conradludgate @stradig @vadim2404 Going to keep this as a Draft for now, but it works and you can all take a look.

Copy link
Member

@skyzh skyzh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as the compute node image can be built successfully I'm confident that this can be merged.

README.md Outdated Show resolved Hide resolved
Dockerfile.compute-node Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@davidgomes davidgomes force-pushed the pg-neon-jwt-extension branch 2 times, most recently from 6ffb508 to c0c2320 Compare September 6, 2024 05:05
vendor/postgres-v14 Outdated Show resolved Hide resolved
@davidgomes davidgomes force-pushed the pg-neon-jwt-extension branch 8 times, most recently from 2951ea6 to 3a6576e Compare September 20, 2024 07:42
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@conradludgate conradludgate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, although maybe we shouldn't merge just yet. I don't think we have a way to feature gate extensions so it's best if we manually configure the compute images with this one built in CI for now

@davidgomes davidgomes force-pushed the pg-neon-jwt-extension branch 8 times, most recently from 2b1fbca to 1b3eddf Compare September 20, 2024 11:48
@davidgomes davidgomes marked this pull request as ready for review September 20, 2024 17:58
@davidgomes davidgomes changed the title feat(compute): adds pg neon jwt extension to compute image feat(compute): adds pg_session_jwt extension to compute image Sep 20, 2024
Dockerfile.compute-node Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
compute/Dockerfile.compute-node Show resolved Hide resolved
@davidgomes davidgomes enabled auto-merge (squash) October 1, 2024 07:39
@davidgomes davidgomes merged commit d6c6b0a into main Oct 1, 2024
81 checks passed
@davidgomes davidgomes deleted the pg-neon-jwt-extension branch October 1, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants