Skip to content
New issue

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

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

Already on GitHub? # to your account

Replaced Map forwarder with Yarp reverse Proxy for mobile BFF #218

Merged
merged 3 commits into from
Mar 8, 2024

Conversation

SrushtiPasari94
Copy link
Contributor

@SrushtiPasari94 SrushtiPasari94 commented Feb 28, 2024

Replaced Map forwarder with Yarp reverse Proxy for mobile BFF

@samsp-msft
Copy link
Member

Is there a reason you have so many paths in the config that all point to the same cluster - is this so that the proxy will only forward those specific requests - you could handle many of those with a wildcard.

@Eilon
Copy link
Member

Eilon commented Feb 29, 2024

Is there a reason you have so many paths in the config that all point to the same cluster - is this so that the proxy will only forward those specific requests - you could handle many of those with a wildcard.

I think we specifically want to avoid a wildcard because then it could accidentally map "too much". Instead, we want to map only exactly what we know we need so that we don't expose too much surface area of a back-end API.

@jamesmontemagno
Copy link
Member

Thanks @Eilon that helps. so if that is the case I think this is okay if testing goes well.

@adityamandaleeka
Copy link
Member

@Eilon do you plan to review this?

Copy link
Member

@jamesmontemagno jamesmontemagno left a comment

Choose a reason for hiding this comment

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

@SrushtiPasari94 testing going good?


foreach (var forwardedUrl in forwardedCatalogApis)
{
var mapFromPattern = "/catalog-api" + forwardedUrl;
Copy link
Member

Choose a reason for hiding this comment

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

The only thing I don't see in the proxy config now is the /catalog-api prefix that I think is used for the 'local name' of the API. That is, the Mobile App (e.g. .NET MAUI Blazor Hybrid app) talk to the Mobile BFF at:

http://mobile-bff/catalog-api/<some catalog path>
                  ^^^^^^^^^^^

But I don't see where the new proxy config has that? Or is it not needed?

Ultimately just running the .NET MAUI Blazor Hybrid app (the HybridApp folder in the repo) will either work or not work, but my inclination is that it won't work because this path doesn't exist. Unless the proxy config just figures that out somehow?

Copy link
Member

Choose a reason for hiding this comment

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

hmmmm let me take a look, I thought this would do it - "Address": "http://catalog-api/api/v1/catalog""

@jamesmontemagno
Copy link
Member

@Eilon this should address this :)

@Eilon
Copy link
Member

Eilon commented Mar 6, 2024

Thanks, that looks more like what I would have expected, but I'm not sure how exactly YARP maps things, so I wasn't sure.

Ultimately, does the HybridApp project run and is able to make API calls to load the catalog? If so, good. If not, back to work! 😁

@jamesmontemagno
Copy link
Member

@Eilon yup! working good here :) The port is different for some reason, but beyond that good to go.

image

@jamesmontemagno jamesmontemagno merged commit bafd891 into main Mar 8, 2024
3 checks passed
@jamesmontemagno jamesmontemagno deleted the v-sruspasari/yarpforMobileBFF branch March 8, 2024 16:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants