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

[RFC] 0003 Proxy capabilities of ui5-server #41

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

RandomByte
Copy link
Member

@RandomByte RandomByte commented Jul 16, 2018

Add proxy capabilities to the ui5-server module.

Read: RFC 0003 Proxy capabilities of ui5-server

@RandomByte RandomByte added the RFC Request for Comment (pull request) label Jul 16, 2018
@RandomByte
Copy link
Member Author

RandomByte commented Jul 16, 2018

Question:

When looking at the Possible use cases, is use case A (only proxying single paths) something commonly required and possibly preferred over something like use case C (proxying everything that can't be found locally)?

@MichaelSp
Copy link
Member

My use-case is pretty much covered by A (only proxy single path)
I haven't seen B or C in the wild, so I would avoid (IMHO) any additional complexity if not necessary.
I also don't need URL-Rewriting, although I can imagine that it may come in handy later as projects grow.
For the other features:

  • Proxy auth not required
  • Basic Authentication would be nice
  • TLS is required (insecure option would be nice)
  • Websocket is required
  • HTTP/2 would be nice
  • Prefer "single project"-setup over "separate project"-setup

That would fit my use-case

@MichaelSp
Copy link
Member

any progress/decision yet?

@matz3
Copy link
Member

matz3 commented Aug 3, 2018

Unfortunately not

@mlenkeit
Copy link
Member

I agree with Michael: for application (and lib) development, I've only seen option A. For explicit URL rewriting, I haven't seen any need yet.

Regarding other features:

  • Basic auth with credentials in either plain text (for dev system), from env vars, or from some kind of secure vault (we haven't found a good solution for that yet)
  • SAML2 auth to authenticate against e.g. SCP
  • SSO certificate support (we can provide details here)

@mcanalesmayo
Copy link

My use case would be covered by C. Please, keep in mind that we should keep our local environments as similar as possible to the remote one (for both OData calls and UI5 resources). Take into account that there can be a lot of developers working on apps, extensions or libraries for the same remote system. It would be great if developers don't have to maintain a local copy of the framework. Same thing with the framework copy used by the CI/CD environment.

Authentication capabilities would be also nice to have, specially SAML2 for SSO.

@ghost
Copy link

ghost commented Jan 18, 2019

@RandomByte Has this feature been abandoned?

@RandomByte
Copy link
Member Author

Definitely not! It's on our roadmap: #101

@hschaefer123
Copy link

Hi,
we are currently using @uniorg/localneo developed from Mark (The Masch).

We use it as a replacement for the cloud web ide and therefore we need SCP support to be compatible.
It is reading the neo-app.json configuration for the paths and the missing destinations are mapped
with a small file like this

{
"server": {
"port": "12345",
"hostname": "localhost",
"open": true
},
"service": {
"sapui5": {
"useSAPUI5": true,
"version": "1.48.5"
}
},
"applications": {
"mylib": {
"path": "../mylib"
},
"images": {
"path": "/var/www/images"
}
},

"destinations": {
"SAP_BACKEND": {
"url": "https://our_secret_sap_system.com"
}
}
}

Currently it support BasicAuth on destinations ( we do not found an OData Client in NPM to support SAML with SCP).

It even handles the resources passthrouwed to CDN.

Something like this would be very handy to switch over to the new tooling.

We need Option A, because it should work similar to neo.
I think our application proxy it replaced by npm link or somethin similar and you server component,
so everything for the odata/rest destinations would be valueable.

Regards Holger

@uxkjaer
Copy link
Member

uxkjaer commented Mar 6, 2019

Hi,

Just going through and thinking how we can provide the neoapp files. As the RFC states we need to be able to proxy different requests. But we need to do it in such a way we don't have to change the manifest files. This is where the neoapp files comes in. However because we would need to pass in credentials and potentially change default clients, this is why i thought the neo-dest.json file made sense. It follows a similar pattern to WebIDE and SCP.

We could put the file paths into the yml file as neoapp and neodest. This way we could make it slightly more flexible and have it served from the tree in the index.

Would this suffice?

@hschaefer123
Copy link

Hi,
this sounds similar to our needs.

Our neo-dest.json file looks like this

{
    "server": {
        "port": "8080",
        "path": "/webapp/index.html",
        "open": true
    },
    "service": {
        "sapui5": {
            "useSAPUI5": true,
            "version": "1.56.5"
        }
    },
    "destinations": {
        "ODATA_SRV": {
            "url": "https://my.domain.de/odata_srv",
            "auth": "user@domain.de:12345678"
        }
    }
}

but you maybe need only a portion of this.

The service section is for easy customization off the always used /resources and /test-resources services inside neo-app.json. We auto link to CDN while using SAPUI5 by default or OPENUI5 if useSAPUI5 is false. By default returning the latest UI5 version or an explicit one (the same way like this can be configured in the Web IDE on project level).

I think currently the ui5/tooling does not support preload libs and always fetches all source classes, but hopefully this will also be an option in the future to load libs!

Using it this way, we are currently completely web-ide/neo compatible.

Regards Holger

@uxkjaer
Copy link
Member

uxkjaer commented Mar 7, 2019

I've followed Petr Plenkov's example in my edition of the neo-app.json and neo-dest.json. However these would need to be added to the yaml file for proper config instead of being hardcoded in the js file. Just want to make sure it makes sense before i try to do it and these changes would fulfill the RFC.

@gregorwolf
Copy link
Contributor

I'm still wondering why this issue isn't solved using the existing @sap/approuter module? @vobu just described in cf for your pocket how to use it locally.

@RandomByte
Copy link
Member Author

I'm still wondering why this issue isn't solved using the existing @sap/approuter module? @vobu just described in cf for your pocket how to use it locally.

Main issue for us is that it is not open source. Also it does not easily integrate into existing server solutions like ours. It's rather the other way around, as the approuter project provides extension hooks itself. We'd therefore rather leave an integration of the UI5 Tooling into the approuter up to the community for now.

@mlenkeit
Copy link
Member

@RandomByte @matz3 is there any update on this? After all, this topic has been in discussion for over a year now, although it's probably the most important feature that's missing in the new UI5 tooling when it comes to adopting it for app development.

Maybe it's worth considering a change in strategy from building a handle-all-cases solution that's convenient to use to building the minimum functionality (MVP) for the 80% case. You can still add sugar coating, such as getting the information from neo-app.json or xs-app.json, taking SCP destinations into account, etc. at a later point in time.

@RandomByte
Copy link
Member Author

RandomByte commented Aug 19, 2019

fyi, we have published a possible implementation of a somewhat NetWeaver ABAP / Gateway focused "proxy mode" as part of the standard UI5 Tooling: SAP/ui5-server#223

This is a PoC that was initially developed quite some time ago and now finally got published. However, I think its scope is limited to a certain set of UI5 developers working with NetWeaver ABAP systems.

Any feedback is highly appreciated!

@preetamkajalrout
Copy link

If anyone is still looking for a proxy plugin that supports neo-app.json. You can give it a try to plugin I created for my use-case:
ui5-middleware-destination

Feedback are welcome 👍

@RandomByte
Copy link
Member Author

It's been a while and I would like to do a quick pulse check:

Does anybody here still have proxy-related requirements that are not, or can not be covered by custom server middleware?

Today, there are multiple proxy extensions available in the ui5-community organization. Others can be found on GitHub and npm, like the one @preetamkajalrout mentioned.


In UI5 Tooling we always struggled with the different requirements for different proxy scenarios. Instead of implementing a "standard" proxy functionality, we are now rather thinking of enhancing the extensibility mechanisms to offer for example helper-functions to custom proxy implementations. But also here, we are not sure what is missing or required. We therefore highly appreciate your feedback.

@LukasHeimann
Copy link

LukasHeimann commented Mar 31, 2021

Hi @RandomByte,

I'm trying to build a proxy that can cover websockets as well. This may be just lacking in the documentation, but I have a feeling like this isn't possible with middlewares today.

My middleware looks like this:

const url = require('url');
const httpProxy = require('http-proxy');

module.exports = function({resources, options}) {
    const proxy = httpProxy.createProxyServer({
        changeOrigin: true, // change the origin to the target host
        ignorePath: true, // we combine the full target ourselves
        secure: false // don't try to valide (internal) certificates
    });

    proxy.on('error', (err, req, res) => {
        console.error(err);
        res.end(err.message);
    });

    return function backendProxy(req, res, next) {
        const proxyTarget = url.resolve(options.configuration.url, req.path);
        proxy.web(req, res, { target: proxyTarget });
        return;
    }
};

This doesn't cover websockets. As per http-proxy's documentation, you'd need to attach to the httpserver to do that, i.e. (disclaimer: don't know if that'd work):

ui5InternalHttpServer.on('upgrade', function (req, socket, head) {
  // TODO: Check if the middleware's `mountPath` matches -- unknown where to read that from
  const proxyTarget = url.resolve(options.configuration.url, req.path);
  proxy.ws(req, socket, head, { target: proxyTarget });
});

As I'd still like to proxy normal requests as well, I'd still have a "proper" middleware to return and would just add that snippet, given that I'd have a reference to the http server and the plugin's mount path.

If there is a way to do this right now with middlewares, I'd like to hear about it, but it seems to not fit their design.

Maybe, what's needed would be a different thing to return:

const url = require('url');
const httpProxy = require('http-proxy');

module.exports = function({resources, options}) {
    const proxy = httpProxy.createProxyServer({
        changeOrigin: true, // change the origin to the target host
        ignorePath: true, // we combine the full target ourselves
        secure: false // don't try to valide (internal) certificates
    });

    proxy.on('error', (err, req, res) => {
        console.error(err);
        res.end(err.message);
    });

    return {
        middleware: function (req, res, next) {
            const proxyTarget = url.resolve(options.configuration.url, req.path);
            proxy.web(req, res, { target: proxyTarget });
            return;
        },
        // ui5 cli would register on upgrade for me, already filtering so I only receive events for my mountPath
        upgrade: function (req, socket, head) {
            const proxyTarget = url.resolve(options.configuration.url, req.path);
            proxy.ws(req, socket, head, { target: proxyTarget });
        })
    };
};

Thanks for helping me out here!
Kind regards
Lukas

@flovogt flovogt changed the base branch from master to main November 3, 2022 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for Comment (pull request)
Projects
Development

Successfully merging this pull request may close these issues.

None yet

10 participants