search

saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.12k stars 5.47k forks source link

Update docs for 3005 (i.e. phosphorus) and after to mention modules to install in onedir environment #64160

Open zbukhari-apex opened 1 year ago

zbukhari-apex commented 1 year ago

Description Your documentation is very good but having recently gone from classic to onedir, I've been running into issues here and there. I also am not sure if there was a "pre" check script so that users will know what they'll have to pip install in their onedir environment to keep humming along but it's an issue as I keep going back to the docs and everything seems fine ... but then I go into the trenches and find out I need to make some slight changes due to onedir.

Suggested Fix

I'm not entirely sure but honestly I feel if just a mention (i.e. right by "Required Python modules") was made that I need to install psycopg2-binary into my onedir installation ... and perhaps a link to that, it would be good. But not just for that instance, but for all things not shipped with onedir that are part of your official documentation.

For example here:

https://docs.saltproject.io/en/3005/ref/returners/all/salt.returners.pgjsonb.html#module-salt.returners.pgjsonb

Where it says Required Python modules: Psycopg2 it would say Required Python modules: Psycopg2 (NOTE: If using onedir, this needs to be installed into your onedir path) and then that last part could just be a link to https://docs.saltproject.io/salt/install-guide/en/latest/topics/upgrade-to-onedir.html#how-to-upgrade-to-onedir

Type of documentation

I don't know but I think it's mostly for all things Salt on your site but maybe you mean modules. Not sure.

Location or format of documentation https://docs.saltproject.io/en/3005/ref/returners/all/salt.returners.pgjsonb.html#module-salt.returners.pgjsonb

Additional context

Thanks for your time!

welcome[bot] commented 1 year ago

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

zbukhari-apex commented 1 year ago

One other mention capital letters and all that may be risk, I don't mess with pip often, I use OS packages, but if someone has a PsYcOpG2 if it's not the same as Psycopg2 then that's a bad situation waiting to happen. Perhaps you can specify the package name and or see if there's pip pub keys that users can save themselves from by verifying against them?

Basically I'd hate if script-kiddie-12 puts up PsycoPg2 and it's pretty much meant to hack/crack/ransomware/etc.

Thanks again!

barbaricyawps commented 1 year ago

@zbukhari-apex , thanks for opening this issue. Can I get some more clarity about what the suggested documentation fix is? Is it that:

zbukhari-apex commented 1 year ago

Hello @barbaricyawps,

My response is in your reply.

@zbukhari-apex , thanks for opening this issue. Can I get some more clarity about what the suggested documentation fix is? Is it that:

  • You need information in the install guide that explains that Salt module dependencies need to be installed in the onedir directory specifically?

Yes. I was thinking in the official documentation. But not exactly this. I like your suggestion more. For example, the official docs have steps to build salt onedir, but ya'll make the packages and AFAIK/AFAICT, they, packages and docs, are good. So nice to know but not needed for most users of Salt.

However steps on how to install Salt python module dependencies on the normal documentation would be great.

Originally I was of thinking of making mention and linking people to step 4 here for every page where a python module is required that is not part of onedir.

However your suggestion plays better. I and many others have developed a familiarity with how your normal documentation is laid out. So even though the "steps to install onedir" is "docs.saltproject.io", I go for the normal docs because it feels, plays, familiar.

Currently there's a section in the ToC, "Onedir Packaging". I was thinking somewhere around there a section, "Installing Python module dependencies for Onedir", would be nice. Even though it may be a tiny page, it would be very nice.

  • You want the individual module documentation to list which dependencies they require?

Yes. For example here we have at the top:

...
depends:
  python-psycopg2
...

I believe it should say this instead:

...
depends:
  python-psycopg2 (Note: Not included with onedir, required [installation])
...

Also python-psycopg2 reads like an OS package and that would probably be something useful to get change. Maybe just linking to the pypi link. I have noticed some docs also have the python2 variant of the OS package, such as here.

Super duper preferably:

depends: psycopg2 (Note: Not included with onedir, requires installation) <-- or just have it expand it because it is pretty simple to do

That way at least users can sift through their own OS packages and or know exactly what to go for in pypi. It reduces the risk of installing an outdated package or something.

  • Both of these?

Yes. But only because your ask/mention makes me feel it actually would be better :-)

s0undt3ch commented 1 year ago

All modules which are not hard dependencies, must be salt-pip installed.

What could be clarified here, IE, make it more explicit, is, what are the hard dependencies of Salt, because to list those, there's a few requirements files, depending on OS.

Linux

As for module python dependencies not respecting case in the package name, that does not open the door for attackers and dependency chain attacks.

Follow these two URLs

Of course, respecting the package name as shown in PyPi wouldn't hurt anyone 😁

zbukhari-apex commented 1 year ago

Hello @s0undt3ch,

Thanks for the response. There is still the python2-MODULE package names but good to know about the case. However I still feel linking to the package is best as that documentation will still be used for people on older versions. As the OS package name can vary, going to PyPi and directly to the package is better IMHO. Not a big deal, just a "very nice to have" suggestion.

As to the requirements, the issue is more jumping between two sets of documents, technically three - more on that later. I looked at this requirements linked to from the upgrade to onedir. Then after digging around, realized that I do need to install it within the onedir python libs as outlined in onedir. I installed it and was good. My thoughts were that people go to documentation before source.

For example, say a new Salt admin wants to set up a returner or even me at another place or environment. They go, they browse around, they feel that a postgresql json returner fits the bill. They're going to go to that documentation before they go to the github requirements file. It would be helpful for later docs to mention what is needed especially with onedir as for the most part, basic salt requirements are taken care of. So a lot of depends are mentioned but, it's already done for us. For ones that are not, it's helpful.

Now as to the third docs, when I was running into issues I was just searching google and came across some documents from vmware for salt that said the pgjsonb returner was no longer available here - Under Upgrade the master plugin - item 7 Check that the master_job_cache and event_return entries are set to sseapi. The pgjsonb returner is no longer available. So it gets confusing and I was saying that for me, your base documentation is what I and many others rely on.

Either way, thanks!