Bring Saltnado up to feature parity with rest_cherrypy

[whiteinge]

As has been discussed now that Tornado is a dep for Salt core it makes sense to move development effort on a REST API to Saltnado. We do not yet have a timeline for this work but we cannot deprecate rest_cherrypy until Saltnado has feature parity and identical interfaces so this issue will serve as a place to track that work.

Missing features:

• [ ] Implement support for all *client interfaces.
  • [x] local
  • [x] local_async
  • [ ] local_subset
  • [x] runner
  • [x] runner_async
  • [ ] ssh
  • [ ] wheel
  • [ ] wheel_async
• [ ] Logout.
• [ ] Passing the session token via cookies or the X-Auth-Header.
• [ ] Generating minion keys.
• [ ] Serving static files.
• [ ] Bootstrapping a rich-client HTML app (and support for the history API -- no 404s on browser refresh).
• [x] CORS. #27142
• [ ] Vet API compatibility for all endpoints. Client libraries should be able to switch between them seamlessly.
• [ ] Login accepts urlencoded and JSON payloads, JSON payload can be with or without a containing array. #37803
• [ ] Session tokens should hide Salt eauth tokens, though users should be able to bypass that on some endpoints. #38139
• [ ] xsrf support.

[Lothiraldan]

I'm working on a proof-of-concept of saltpad in react and hit the non-support of preflight request in either rest_cherrypy or rest_tornado, could I help?

[whiteinge]

The rest_cherrypy module has undocumented (:disappointed:) support for CORS. There's no tests in place for that so I don't know off the top of my head if it's missing anything.

If you're interested in diving into the Saltnado code we would gladly welcome the effort! Pick whatever you're interested in and have at it (with an eye toward interface consistency with rest_cherrypy). That said, since you're a client author it's worth underscoring the mention in OP that we don't yet have a definitive timeline for a concerted effort toward this migration. I'd hate to see Saltpad stuck in a place where it needs bleeding-edge features in Saltnado as well as the yet-to-be-migrated features from rest_cherrypy.

In any case, please feel free to ping me if I can assist with anything as you dive in.

[jacksontj]

From a quick look at these, a few features are unclear to me:

    Serving static files.

? We just need to serve a directory of static files? Seems odd for salt-api :/

    Bootstrapping a rich-client HTML app (and support for the history API -- no 404s on browser refresh).

What history API is this referring to? I did a couple quick searches and was unable to find anything

    CORS.

Setting headers to be callable from other domains? Or is this the whole options thing to make sure its valid?

    Vet API compatibility for all endpoints. Client libraries should be able to switch between them seamlessly.

I think this had a ticket before-- where we want to make a shared "http api" test that ran against both :)

[Lothiraldan]

I've made some quick fix on saltnado to make it compatible with browser-js usage. I will made severals PRs in the next day for CORS support and some fixes for better support of real 'Accept' header and 'Content-Type' values.

[whiteinge]

We just need to serve a directory of static files? Seems odd for salt-api :/

rest_cherrypy was written in-part to be able to drive a rich-client web app.

https://github.com/saltstack/salt/blob/b676e32/salt/netapi/rest_cherrypy/app.py#L102-L106

What history API is this referring to? I did a couple quick searches and was unable to find anything

This one:

http://diveintohtml5.info/history.html

Implemented here:

https://github.com/saltstack/salt/blob/b676e32/salt/netapi/rest_cherrypy/app.py#L300-L332

Setting headers to be callable from other domains? Or is this the whole options thing to make sure its valid?

The former.

https://github.com/saltstack/salt/blob/b676e32/salt/netapi/rest_cherrypy/app.py#L453-L493

I think this had a ticket before-- where we want to make a shared "http api" test that ran against both :)

I closed that issue since we're no longer talking about maintaining both modules. You're right that work would definitely be of help here and we can certainly reopen it to track that work. Since the end-goal is deprecation, I don't think we need to spend much time building this out -- maybe some light integration tests or maybe we can just ask the various client authors to vet against Saltnado during the deprecation phase.

[jacksontj]

We just need to serve a directory of static files? Seems odd for salt-api :/

rest_cherrypy was written in-part to be able to drive a rich-client web app.

IMO that sort of functionality doesn't really make sense to keep in the API. Especially since we are adding COORS headers (so the app can be served from another domain all together) I can't really see why we'd need/want this in the API? Just seems like another moving part that no one really needs :/

https://github.com/saltstack/salt/blob/b676e32/salt/netapi/rest_cherrypy/app.py#L102-L106

What history API is this referring to? I did a couple quick searches and was unable to find anything

This one:

http://diveintohtml5.info/history.html

Implemented here:

https://github.com/saltstack/salt/blob/b676e32/salt/netapi/rest_cherrypy/app.py#L300-L332

Similar questions-- not sure why this needs to be implemented on an API endpoint? From the looking I did on the UIs all of them require standing up some webserver-- so all they should be doing to the API is sending API calls. I say that of course realizing that hasn't always been the case-- but I think we'll want to encourage better service oriented architecture here-- and IMO that includes keeping the API just that-- an API.

Setting headers to be callable from other domains? Or is this the whole options thing to make sure its valid?

The former.

Cool, definitely easy to add :)

https://github.com/saltstack/salt/blob/b676e32/salt/netapi/rest_cherrypy/app.py#L453-L493

I think this had a ticket before-- where we want to make a shared "http api" test that ran against both :)

I closed that issue since we're no longer talking about maintaining both modules. You're right that work would definitely be of help here and we can certainly reopen it to track that work. Since the end-goal is deprecation, I don't think we need to spend much time building this out -- maybe some light integration tests or maybe we can just ask the various client authors to vet against Saltnado during the deprecation phase.

Fair enough :)

[whiteinge]

A few clients have made JS apps on top of the API that I know of. Serving static files is a tiny addition. I'm not a huge fan of the server-side requirements for the HTML5 history API but it's also an easy addition -- just a header inspection and a routing bypass.

Setting up a second web server and trying to get CORS working correctly is a bunch of trouble for small deployments. I do agree that encouraging a good SOA is worthwhile -- but only for apps of significant users or significant complexity. It's way overkill for just slapping a small UI on top of Salt, especially when you already have a running web server right there.

[jacksontj]

Fair enough, if we are going to implement the "small shop" features I think we'll want to make the handlers configurable to load (similar to how the websockets stuff is today).

On Thu, Sep 3, 2015 at 10:01 AM, Seth House notifications@github.com wrote:

A few clients have made JS apps on top of the API that I know of. Serving static files is a tiny addition. I'm not a huge fan of the server-side requirements for the HTML5 history API but it's also an easy addition -- just a header inspection and a routing bypass.

Setting up a second web server and trying to get CORS working correctly is a bunch of trouble for small deployments. I do agree that encouraging a good SOA is worthwhile -- but only for apps of significant users or significant complexity. It's way overkill for just slapping a small UI on top of Salt, especially when you already have a running web server right there.

— Reply to this email directly or view it on GitHub https://github.com/saltstack/salt/issues/26505#issuecomment-137511740.

[whiteinge]

Works for me.

I'm digging that pattern in general and wouldn't mind seeing all endpoints have that "lazy-loaded" aspect as a first-class API. I.e., the Salt loader applied to REST endpoints. Not necessary to complete the TODOs in this issue, of course, but food for thought for the future.

[jacksontj]

Yea, I'm thinking we break it into pieces. And (at least today) those would probably be something like "core", "experimental" (ws), "extra" (fileserver, etc.)

On Thu, Sep 3, 2015 at 7:30 PM, Seth House notifications@github.com wrote:

Works for me.

I'm digging that pattern in general and wouldn't mind seeing all endpoints have that "lazy-loaded" aspect as a first-class API. I.e., the Salt loader applied to REST endpoints. Not necessary to complete the TODOs in this issue, of course, but food for thought for the future.

— Reply to this email directly or view it on GitHub https://github.com/saltstack/salt/issues/26505#issuecomment-137628322.

[Lothiraldan]

@whiteinge I've a PR ready for CORS support in saltnado https://github.com/saltstack/salt/pull/27142

[whiteinge]

Hawt!

[Lothiraldan]

Do we have an idea when CORS support could lands in a saltstack release, it's currently available only on develop branch and has not been yet merge in 2015.8 branch, did I need to create PR for CORS support for the 2015.8 branch?

[Lothiraldan]

@whiteinge I tested right now the unofficial CORS support of rest_cherrypy and it doesn't works at least with authentication by header, I will try the cookie authentication but it's not very standard for API to use cookies.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[mchugh19]

This is still an issue and could use a new champion

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[mattp-]

tagging https://github.com/saltstack/salt/issues/35798 as another issue to resolve for cherrypy parity

[untra]

whats the status of this?

[fechnert]

I'm also very interested in using Tornado and the websocket features rather than rest_cherrypy.

[mattp-]

I have a fix/rewrote the event handling for #35798 at least. just haven't got it sent upstream.

[fechnert]

@mattp- i would love to see and use these changes. The race condition is quite annoying, which prevents me currently from using the websocket as a nice event stream for multiple clients ...

[baby-gnu]

Hello, is there any progress on this issue?