chore: add proxy support #67
Conversation
pgautier404
force-pushed
the
proxy-support
branch
from
5f6867b to
8839e9a
Compare
pgautier404
force-pushed
the
proxy-support
branch
from
8839e9a to
897ce93
Compare
There was a problem hiding this comment.
this is looking great, the way you got it working seems totally reasonable to me. Just need to finalize our API design.
As an artifact of this ticket I'd appreciate it if you could file a PR against standards and practices repo that includes the haproxy config that you included in your commit message, and a few (language-agnostic) words about how we support reverse proxying and when it might be useful. Then we can add a bit more detail to the README in this repo and include a link to the s&p doc
examples/proxy-example.php
Outdated
| "MOMENTO_AUTH_TOKEN", | ||
| "momento-control:4443", | ||
| "momento-cache:4444" | ||
| ); |
There was a problem hiding this comment.
There is so much other code in this example that this bit gets lost in the noise. tbh I think if we want to have a proxy example it should probably just have the auth provider instantiation, with more comments about what it's doing and why, and then like one call to "listCaches" or something. and we should remove the rest of the stuff that's duplicated from the other examples so that their attention is drawn to the important thing.
| public function getCacheProxyEndpoint(): string|null | ||
| { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Left you some input in slack about some other ideas for this interface.
pgautier404
force-pushed
the
proxy-support
branch
3 times, most recently
from
24101cf to
bccc429
Compare
pgautier404
force-pushed
the
proxy-support
branch
from
bccc429 to
68fecde
Compare
| mode tcp | ||
| default_backend cache-plane-fe | ||
|
|
||
| backend cache-plane-fe |
There was a problem hiding this comment.
Good catch, thanks.
| ```php | ||
| $authProvider = new EnvMomentoTokenProvider( | ||
| // name of the environment variable that contains our auth token | ||
| "MOMENTO_AUTH_TOKEN", |
There was a problem hiding this comment.
does php support named args?
There was a problem hiding this comment.
It does, I'll add them in, thanks.
| if (!$this->allAreDefined($endpointArgs)) { | ||
| throw new InvalidArgumentError( | ||
| "If any of controlEndpoint, cacheEndpoint, trustedCacheEndpointCertificateName, or " . | ||
| "trustedControlEndpointCertificateName are provided, they must all be."); |
There was a problem hiding this comment.
you can address this in a follow-up pr but why would we disallow the first two being passed by themselves?
|
I'll address all feedback in a followup PR coming up in just a minute. |
This contains initial R&D on adding proxy support in SimpleCacheClient. It adds a proxy-aware credential provider to route gRPC calls through a proxy (currently haproxy on layer 4) to the appropriate Momento endpoints. The proxy endpoints are passed in the constructor to the credential provider and the Momento endpoints are parsed out of the JWT token as before.
An haproxy instance configured to send requests on to the Momento backend is required to successfully run the
proxy-example.phpexample. You can add the below configuration to yourhaproxy.cfg(after replacing the placeholders with the actual hostnames from your JWT in theserverdirectives) to work with the configuration expected by that script.