Add option to disable clients in netapi

[barneysowood]

What does this PR do?

Adds a new config option "netapi_disable_clients" that takes a list of clients to disable in the netapi.

Checks the list early in handling the the request before authentication occurs. Should be useful where certain clients (eg ssh or wheel) aren't required and should be disabled to reduce attack surface in the salt-api.

replaces #58872

What issues does this PR fix or reference?

New Behavior

Adds "netapi_disable_clients" list option to the config which is checked when a request is passed to NetApiClient.run() and an exception raised if the requested client is in the list.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltstack.com/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

No

[barneysowood]

re-run ci/py3/macosxmojave/pytest

[barneysowood]

re-run pr-macosxmojave-py3-pytest

[sagetherage]

@barneysowood I updated the branch with master as I think we have fixed these tests that are failing, here. That reruns the full test suite so I will check back, as well. I hope that that helps!

[barneysowood]

@barneysowood I updated the branch with master as I think we have fixed these tests that are failing, here. That reruns the full test suite so I will check back, as well. I hope that that helps!

Thanks @sagetherage. Looks like the failing tests are fixed now - thanks! Unfortunately seems to have times out on ci/py3/centos7/pycryptodome/pytest - I'll try that again now.

[barneysowood]

re-run pr-centos7-py3-pycryptodome-pytest

[barneysowood]

OK, tests now all green.

@garethgreenaway - if you get a chance to review that would be great, thanks!

[Ch3LL]

I approved but look like there is a merge conflict. Would also like to get @dwoz review here as well.

[barneysowood]

Hmm, have totally messed up the rebase trying to sort the conflict. Will fix that and re-push

[barneysowood]

re-run pr-freebsd-122-amd64-py3-pytest

[barneysowood]

re-run pr-ubuntu-2004-arm64-py3-pytest

[barneysowood]

re-run pr-macosx-mojave-x86_64-py3-pytest

[barneysowood]

@Ch3LL and @dwoz - merge conflict resolved and all tests now passing. If you could review that would be great, thanks.

[barneysowood]

re-run pr-fedora-34-x86_64-py3-pytest

[barneysowood]

re-run pr-fedora-34-x86_64-py3-pytest

[barneysowood]

re-run pr-macosx-catalina-x86_64-py3-pytest

[barneysowood]

re-run pr-macosx-mojave-x86_64-py3-pytest

[dwoz]

Honestly, due to the amount of problems we've had with the netapi I think this should be an opt in scenario rather than opt out. Meaning it'd be netapi_enable_clients and they are all disabled by default.

[barneysowood]

Honestly, due to the amount of problems we've had with the netapi I think this should be an opt in scenario rather than opt out. Meaning it'd be netapi_enable_clients and they are all disabled by default.

@dwoz - I would tend to agree, but I was trying to avoid a breaking change. If everyone is OK with that I'm happy rework this to be netapi_enable_clients and also add some documentation for that.

The other option would be to set the default (either using the netapi_enable_clients or netapi_disable_clients form) to disable the ssh client by default as that has tended to be the most problematic.

[barneysowood]

re-run pr-amazon-2-x86_64-py3-pytest

[Ch3LL]

@dwoz I really like the idea of the opt in as well, but that seems like a big change for users of salt-api. Shouldn't we add a deprecation notice somehow or give users notice before we make this change?

[barneysowood]

Closing in favour of #63050