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.

Sign up for GitHub

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

Jump to bottom

feat!: rewrite the API gateway in go #709

Merged
merged 62 commits into from
Sep 2, 2024
Merged

feat!: rewrite the API gateway in go #709

merged 62 commits into from
Sep 2, 2024

Conversation

leafty
Copy link
Member

@leafty leafty commented Jul 30, 2024 •
edited
Loading

Rewrites the API Gateway in Go. All the functionality of the API Gateway is now located inside a single component.

Details:

  • Deprecate the token injector server (Python)
  • Add login server to handle authentication for web-based clients (web UI)
  • Handle all routes for backend services with credential injection

This refactor requires changes in ui-server (removal of login handling) and in renku (changes in ingress and deployment of renku-gateway).

See also:

/deploy renku=leafty/gateway-refactor renku-ui=leafty/refactor-gateway

olevski and others added 30 commits October 1, 2023 11:39
@olevski
chore: update to go 1.21
3a43dbd
@wesjdj @olevski
feat: add redis adapter and unit tests (#1)
3ac7197
@wesjdj @olevski
feat: add token refresher (#3)
1d57975
@olevski
squashme: update models
7f87ab5
@olevski
squashme: update redis adapters
51ef69c
@olevski
squashme: move redis adapters
6a3c850
@olevski
squashme: cleanup token refresher
4e22ea3
@olevski
squashme: move token refresher
b5b37ad
@olevski
fix: sticky sessions
f93bd7a
@olevski
squashme: move redisadapters
77d1a29
@olevski
squashme: add id generators
eb82ad2
@olevski
squashme: remove unused stuff
45e279f
@olevski
squashme: add the oidc handler
020ad6f
@olevski
squashme: remove unneeded code
dc5bcd4
@olevski
chore: remove python code
534d2d6
@olevski
chore: use zitadel oidc library for login
db53830
@olevski
chore: refactor config and code for zitadel oidc
b91ca0f
@olevski
chore: remove unused config
9be7a11
@olevski
chore: move revproxy code to internal
46ffdc5
@olevski
chore: update packages and cleanup code
40ce59c
@olevski
squashme: enable proper parsing of env vars into config
56dce0b
@olevski
squashme: helm chart cleanup
04d8fa9
@olevski
Merge branch 'master' into new-login-flow
f8938b1
@olevski
squashme: cleanup CI tests
c602a72
@olevski
squashme: minor fixes
26e6b09
@olevski
squashme: fix config parsing
56880c9
@olevski
squashme: handle uiserver
ce6a69f
@olevski
squashme: minor fixes
a49a27b
@olevski
feat: inject tokens in requests directly from Redis
ea4170b
@olevski
squashme: minor fixes
af7cd33
@leafty leafty temporarily deployed to renku-ci-gw-709 August 20, 2024 11:12 — with GitHub Actions Inactive
@leafty leafty temporarily deployed to renku-ci-gw-709 August 20, 2024 11:25 — with GitHub Actions Inactive
@leafty leafty force-pushed the leafty/refactor-gateway branch from a9b375a to 9d9f293 Compare August 20, 2024 11:33
@leafty leafty temporarily deployed to renku-ci-gw-709 August 20, 2024 11:34 — with GitHub Actions Inactive
@leafty leafty marked this pull request as ready for review August 20, 2024 11:52
@leafty leafty temporarily deployed to renku-ci-gw-709 August 20, 2024 11:57 — with GitHub Actions Inactive
@leafty leafty temporarily deployed to renku-ci-gw-709 August 20, 2024 12:04 — with GitHub Actions Inactive
@leafty leafty temporarily deployed to renku-ci-gw-709 August 20, 2024 12:10 — with GitHub Actions Inactive
@leafty leafty force-pushed the leafty/refactor-gateway branch from faceab5 to c4640a1 Compare August 20, 2024 12:16
@leafty leafty temporarily deployed to renku-ci-gw-709 August 20, 2024 12:17 — with GitHub Actions Inactive
@leafty leafty mentioned this pull request Aug 22, 2024
Merged
olevski
olevski requested changes Sep 1, 2024
View reviewed changes
Copy link
Member

@olevski olevski left a comment

Choose a reason for hiding this comment

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

Thank you for doing this Flora!

I apologize if I added comments on code that you did not touch at all. It has been a while so I cannot even remember what I wrote and what you have modified/added.

cmd/gateway/main.go Outdated Show resolved Hide resolved
cmd/gateway/middlewares.go Outdated Show resolved Hide resolved
apispec.yaml Outdated Show resolved Hide resolved
internal/config/login.go Outdated Show resolved Hide resolved
internal/db/token_repo_redis_test.go Outdated Show resolved Hide resolved
internal/sessions/token_handling.go Outdated Show resolved Hide resolved
internal/sessions/session_store.go Outdated
utils.GetRequestID(c),
)
}
c.Set(SessionCtxKey, session)
Copy link
Member

Choose a reason for hiding this comment

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

suggestion/question: Why set the session if there could have been an error when you are trying to get it? My understanding here is that you expect a valid new session to be returned from Get even if it errors out, is that right? If yes, I think we should not expect session.Get to give you a valid empty session when it errors out. And even it if did using the result from a method that errored out is just weird. I think it would be nicer here to just create a new session either from scratch or with a dedicated function (i.e. smth like NewSession()) and then use that when session.Get errors out.

Copy link
Member Author

@leafty leafty Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No, the middleware will not create a new session if none is found. Only the endpoints which need a session will do so with Create().

internal/sessions/session_store.go Show resolved Hide resolved
internal/revproxy/middlewares.go Show resolved Hide resolved
internal/login/login_server_routes.go Outdated Show resolved Hide resolved
@leafty leafty mentioned this pull request Sep 2, 2024
Open
@leafty leafty temporarily deployed to renku-ci-gw-709 September 2, 2024 08:14 — with GitHub Actions Inactive
@leafty leafty temporarily deployed to renku-ci-gw-709 September 2, 2024 08:44 — with GitHub Actions Inactive
olevski
olevski approved these changes Sep 2, 2024
View reviewed changes
Copy link
Member

@olevski olevski left a comment

Choose a reason for hiding this comment

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

🚀

@leafty leafty temporarily deployed to renku-ci-gw-709 September 2, 2024 09:07 — with GitHub Actions Inactive
@leafty leafty temporarily deployed to renku-ci-gw-709 September 2, 2024 09:21 — with GitHub Actions Inactive
@leafty leafty temporarily deployed to renku-ci-gw-709 September 2, 2024 09:36 — with GitHub Actions Inactive
@leafty leafty deployed to renku-ci-gw-709 September 2, 2024 09:45 — with GitHub Actions Active
@leafty leafty merged commit 7c9f3c1 into master Sep 2, 2024
19 checks passed
@leafty leafty deleted the leafty/refactor-gateway branch September 2, 2024 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olevski olevski olevski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@leafty @RenkuBot @olevski @wesjdj