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

do a bunch of stuff #185

Merged
merged 28 commits into from
Mar 27, 2019
Merged

do a bunch of stuff #185

merged 28 commits into from
Mar 27, 2019

Conversation

mattp-
Copy link
Contributor

@mattp- mattp- commented Mar 12, 2019

went down some rabbitholes in trying to get things behaving with tornado. there will be a yet-to-be-pushed PR to salt for fixes to salt-tornado to make it more backwards compatible. Until we can test against that most of the xfails are unfortunately going to remain for the rest_tornado backend.

some notes:

  • I was not able to get pytest-salt daemons working with parameterized fixtures, so moved it to a pytest --salt-api-backend arg instead. if we can get that working inside pytest thats ideal
  • --run-uri / /login are now tested on all integration tests
  • I've made pepper output with salt outputters more identical to salt cli, but this is technically a breaking change
  • travis is assuredly broken with the matrix, I had no way to test this beforehand to see if it would work

rather than attempting to deconstruct args locally to fully reproduce
the proper low state to pass to the api, we instead leave the kwargs as
is. The reason for this is the RunnerClient will deconstruct and
deserialize nested yaml args. We could equivalently do that locally but
it seems pepper tries to be a no-dependency dist so we just hand off the
work instead.

This PR partially depends on release of saltstack/salt#50124
to determine if a release is neon or newer; however, we can safely assume that if
the salt_version header is not provided in a request it is a salt-api
older than that.

Fixes saltstack#57
I originally tried making this work as a pytest parameterized fixture,
but couldn't get things working properly with pytest-salt daemon mgmt.
If someone figures that out feel free to revert this tox level knob.
was being applied pepper.cli before which is useless for
pepper.libpepper etc.
minions.connected doesnt take an arg
failed output isnt returned if nothing failed anymore. additionally,
returned as valid json
make this act more reasonable. we drop the x-auth-token header when
using /run as the idea of that endpoint is to use whats in the lowstate.
we get rid of the /token route handling as it seemingly does the same
thing as /login, and doesnt actually exist in the rest_tornado impl so
I'm considering it deprecated for simplicity.
this seems to do the right thing in more scenarios.
This reverts commit 59c013a.

this fixes tornado but breaks cherrypy - I mistakenly assumed the
payload token returned on login was the real eauth token not the
session.id. I'll add a route to /token in tornado that is equivalent to
/login on tornado to mimick backcompat behavior even though they are
equivalent there due to not having a proxy session store.
auth should be handled only by token provided in low
runner output should be identical now
both codepaths are valid in both cherrypy and tornado. bunch of stuff
xfail'd for now in tornado as there are pending develop/ PR's to salt to
make things fully cross-compatible.
this will be used by tornado to know how long to persist sync calls for
instead of the app_default_timeout. Also seems like a reasonable thing
to do anyway.
@mattp-
Copy link
Contributor Author

mattp- commented Mar 12, 2019

saltstack/salt#51979 for salt side tornado fixes

@mattp-
Copy link
Contributor Author

mattp- commented Mar 13, 2019

this PR still needs some TLC, but bookkeeping: fairly sure fixes #180, #170, #141, #135, #104, #31

It is too old for pytest-salt :(
reverting to what this was previously because I dropped 2017.7
for some reason travis-ci still has it listed as 3.7-dev
@gtmanfred
Copy link
Contributor

Thanks for this @mattp- looks great

@gtmanfred gtmanfred merged commit 7413e54 into saltstack:develop Mar 27, 2019
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.

2 participants