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

Separate https-proxy plugin #982

Merged
merged 5 commits into from
Nov 26, 2024
Merged

Separate https-proxy plugin #982

merged 5 commits into from
Nov 26, 2024

Conversation

agerard-godaddy
Copy link
Contributor

Summary

We have more use cases for an https proxy besides local development with Next.js built-in server. In particular, having an https proxy sidecar to deploy with Next.js apps for on-container HTTPS.

This introduces a new plugin, action and lifecycle with the https proxy features for a better separation of concerns for these scenarios.

Changelog

@gasket/plugin-https-proxy

  • Initial implementation

Test Plan

Tested locally in app.

@agerard-godaddy agerard-godaddy marked this pull request as ready for review November 25, 2024 23:12
@agerard-godaddy agerard-godaddy requested a review from a team as a code owner November 25, 2024 23:12

The above example forwards https requests on port `443` to `localhost:80`.

The `protocol` and `hostname` are only used for logging about the proxy server
Copy link
Contributor

@jpage-godaddy jpage-godaddy Nov 26, 2024

Choose a reason for hiding this comment

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

I wonder if nesting things under a source property to complement target would make that more intuitive.

Update:

I guess if this is a pass-through configuration to another library it'd make sense to keep the config in the same format.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, one thing that's a bit strange is that this is called an https proxy specifically but you have to specify the protocol as https as well. Should this be a general purpose proxy plugin instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is mostly pass-through configuration. I could default the protocol to https - I'll make that change. So, technically yea, the underlying http-proxy package could be used to proxy to other services or something, but I don't see it being useful that way in a Gasket context. I went back and forth on the name, but an HTTPS proxy is the specific gap intended to be filled with this plugin.

logger.info(`Proxy server started: ${protocol}://${hostname}:${port}`);
}

export default {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a default export here? The importer can do an import * if they want to get all the exports.

}

export interface GasketActions {
startProxyServer: () => Promise<void>;
Copy link
Contributor

Choose a reason for hiding this comment

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

There may be a use case for the caller to receive the proxy server object and have the action return that instead of this being a Promise<void>.

@@ -0,0 +1,81 @@
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
Copy link
Contributor

Choose a reason for hiding this comment

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

Sweet. I'm also a big fan of vitest; it finally made modules just work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 becoming a fan also

Copy link
Contributor

@jpage-godaddy jpage-godaddy left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor suggestions.

@agerard-godaddy agerard-godaddy merged commit af5470f into main Nov 26, 2024
2 checks passed
@agerard-godaddy agerard-godaddy deleted the https-proxy branch November 26, 2024 17:58
agerard-godaddy added a commit that referenced this pull request Dec 11, 2024
* feat: separate https-proxy plugin

* feat: use https-proxy with prompts

* fix: typos and grammar

* fix: avoid default export

* fix: return proxy server
agerard-godaddy added a commit that referenced this pull request Dec 11, 2024
* feat: separate https-proxy plugin

* feat: use https-proxy with prompts

* fix: typos and grammar

* fix: avoid default export

* fix: return proxy server
mmason2-godaddy added a commit that referenced this pull request Dec 11, 2024
* [PFX-813] ESM: Port plugin-metadata (#978)

* chore: convert to esm

* tests: added vitest

* chore: update changelog; remove jest deps

* fix: test

* chore: remove unused space

* [PFX-793] Dynamic plugins (#970)

* feat: dynamic plugins

* chore: add cjs tranpilation

* tests: default-plugins

* tests: core

* tests: dynamic index

* tests: prepare hook

* fix: lint

* chore: add types for dynamic plugins

* docs: dynamic plugins

* chore: update package.json

* docs: update changelog

* chore: switch to utilize sub-environments

* chore: add metadata

* fix: lint

* fix: test

* chore: remove cjs transpilation

* docs: update dynamic and core

* fix: test

* Update packages/gasket-plugin-dynamic-plugins/README.md

Co-authored-by: Andrew Gerard <[email protected]>

---------

Co-authored-by: Andrew Gerard <[email protected]>

* chore: remove depreacted http2 dep (#981)

* Normalized gasket request (#973)

* feat: normalized gasket request

* fix: attempt types adjustments

* fix: types adjustments

* test: the things

* docs: package docs

* fix: docs

* fix: one way to make

* fix: ensure expected defaults

* fix: better types

* fix: handle parallel executions

* fix: next request helper

* test: adjustments

* fix: unused import

* feat: add WeakPromiseKeeper for consistent promise to value caching

* fix: types

* fix: docs

* feat: handle IncomingMessage url

* fix: tighten types

* fix: test cruft

* docs: fix

* fix: only parse url when needed

* test: using gasket.symbol as weakmap key (#964)

* Separate https-proxy plugin (#982)

* feat: separate https-proxy plugin

* feat: use https-proxy with prompts

* fix: typos and grammar

* fix: avoid default export

* fix: return proxy server

* chore: upgrade lerna

* fix: publish issues for command plugin

* docs: next.config.js (#987)

* [PFX-507] Add DocSearch to Gasket Site (#984)

* change dynamic require to string interpolation (#985)

* pin react-intl version to 6.6.X (#988)

* feat: initial fixup for getting command name early and using prepare lifecycle as an async config

* Fix typo

* Tune types

* Opt for sync configure hook

* Clean up old references to command.id

* Add isReady, update command property

* Update readme

* Tune command plugin functionality

* Add temp logging for debug

* Cleanup debugs

* Persist commands config when commandId is undefined

* Tune tests

* lockfile

* remove duplicate tests from merge

* lockfile

* lockfile

* feat: initial fixup for getting command name early and using prepare lifecycle as an async config

* Fix typo

* Tune types

* Opt for sync configure hook

* Clean up old references to command.id

* Add isReady, update command property

* Update readme

* Tune command plugin functionality

* Add temp logging for debug

* Cleanup debugs

* Persist commands config when commandId is undefined

* Tune tests

* Import plugin command types

* Relax the regex for gasket file in argv

* Remove timing from ready hook

* fix test

---------

Co-authored-by: Jordan Pina <[email protected]>
Co-authored-by: Andrew Gerard <[email protected]>
Co-authored-by: Andrew Gerard <[email protected]>
Co-authored-by: Kawika Bader <[email protected]>
Co-authored-by: bbetts-godaddy <[email protected]>
agerard-godaddy added a commit that referenced this pull request Dec 18, 2024
* feat: separate https-proxy plugin

* feat: use https-proxy with prompts

* fix: typos and grammar

* fix: avoid default export

* fix: return proxy server
agerard-godaddy added a commit that referenced this pull request Dec 18, 2024
* [PFX-813] ESM: Port plugin-metadata (#978)

* chore: convert to esm

* tests: added vitest

* chore: update changelog; remove jest deps

* fix: test

* chore: remove unused space

* [PFX-793] Dynamic plugins (#970)

* feat: dynamic plugins

* chore: add cjs tranpilation

* tests: default-plugins

* tests: core

* tests: dynamic index

* tests: prepare hook

* fix: lint

* chore: add types for dynamic plugins

* docs: dynamic plugins

* chore: update package.json

* docs: update changelog

* chore: switch to utilize sub-environments

* chore: add metadata

* fix: lint

* fix: test

* chore: remove cjs transpilation

* docs: update dynamic and core

* fix: test

* Update packages/gasket-plugin-dynamic-plugins/README.md

Co-authored-by: Andrew Gerard <[email protected]>

---------

Co-authored-by: Andrew Gerard <[email protected]>

* chore: remove depreacted http2 dep (#981)

* Normalized gasket request (#973)

* feat: normalized gasket request

* fix: attempt types adjustments

* fix: types adjustments

* test: the things

* docs: package docs

* fix: docs

* fix: one way to make

* fix: ensure expected defaults

* fix: better types

* fix: handle parallel executions

* fix: next request helper

* test: adjustments

* fix: unused import

* feat: add WeakPromiseKeeper for consistent promise to value caching

* fix: types

* fix: docs

* feat: handle IncomingMessage url

* fix: tighten types

* fix: test cruft

* docs: fix

* fix: only parse url when needed

* test: using gasket.symbol as weakmap key (#964)

* Separate https-proxy plugin (#982)

* feat: separate https-proxy plugin

* feat: use https-proxy with prompts

* fix: typos and grammar

* fix: avoid default export

* fix: return proxy server

* chore: upgrade lerna

* fix: publish issues for command plugin

* docs: next.config.js (#987)

* [PFX-507] Add DocSearch to Gasket Site (#984)

* change dynamic require to string interpolation (#985)

* pin react-intl version to 6.6.X (#988)

* feat: initial fixup for getting command name early and using prepare lifecycle as an async config

* Fix typo

* Tune types

* Opt for sync configure hook

* Clean up old references to command.id

* Add isReady, update command property

* Update readme

* Tune command plugin functionality

* Add temp logging for debug

* Cleanup debugs

* Persist commands config when commandId is undefined

* Tune tests

* lockfile

* remove duplicate tests from merge

* lockfile

* lockfile

* feat: initial fixup for getting command name early and using prepare lifecycle as an async config

* Fix typo

* Tune types

* Opt for sync configure hook

* Clean up old references to command.id

* Add isReady, update command property

* Update readme

* Tune command plugin functionality

* Add temp logging for debug

* Cleanup debugs

* Persist commands config when commandId is undefined

* Tune tests

* Import plugin command types

* Relax the regex for gasket file in argv

* Remove timing from ready hook

* fix test

---------

Co-authored-by: Jordan Pina <[email protected]>
Co-authored-by: Andrew Gerard <[email protected]>
Co-authored-by: Andrew Gerard <[email protected]>
Co-authored-by: Kawika Bader <[email protected]>
Co-authored-by: bbetts-godaddy <[email protected]>
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.

3 participants