fix(build): check for env vars in all dirs

[agilgur5]

Motivation

• previously, some of the cmd/ subdirs were missed
• and server/ was missed in the check-documented func
  • instead use a variable to have consistent directories for both
• env.Get*( calls were missed as well

I was responding to this Slack thread in #argo-sig-scalability and reviewing argoproj/argo-cd#17068 when I re-checked the Server's pprof code and found an undocumented flag. So began the whole process of fixing the reason why it and other env vars were undocumented etc

Modifications

• add missing env vars to doc

  • ARGO_DEBUG_PAUSE_*, ALLOWED_LINK_PROTOCOL, ARGO_ARTIFACT_SERVER, ARGO_PPROF, ARGO_SERVER_METRICS_AUTH, BASE_HREF
  • link to pprof docs when referencing it
  • also consistently alphabetize the Server env vars -- GRPC_MESSAGE_SIZE is not last alphabetically

• remove some no longer used env vars

  • ARGO_SERVER_PPROF was no longer used, meanwhile ARGO_PPROF was undocumented for the Server (was only documented for the Controller)
    • it was added in fix(server): Only hydrate nodes if they are needed. Fixes #6000 #6004 and then replaced in fix: Fix gosec warnings, disable pprof by default. Fixes #6594 #6596 but this code was leftover
• refactor check-env-doc.sh

  • consistent 2 space indentation, not mixed 2 or 4 space
  • UX: list out all missed variables before exiting
    • previously it would exit after the first miss, meaning you'd have to fix -> run -> fix -> run until you got everything - and it's not the fastest script given how many files there are to parse through, so it's a productivity increase
  • fix some shellcheck issues with loops / arrays: https://www.shellcheck.net/wiki/SC2031 and https://www.shellcheck.net/wiki/SC2207
  • something something "portable shell code" something something "maybe use Python instead next time"

Verification

Script

1. In my devcontainer (for a somewhat standardized version of bash), ran: bash hack/check-env-doc.sh
   • that's how I got this list
2. Also tested removing a few env vars and confirmed it would list all of them in one go now.

Docs

1. mkdocs build
2. open site/environment-variables/index.html
3. Screenshot with new env vars in the tables:
   

Notes to Reviewers

1. Can skip over all the docs/cli changes as it's all generated one-liner changes due to the conn.go description change
2. Can "hide whitespace" for the hack/check-env-doc.sh refactor to (mostly) ignore the indentation fixes

Future Work

1. Still not quite all env vars are found by this script; in particular, dynamically named env vars are missing
   • such as those in common.go, although there are others too
2. Would be good to distinguish the section of the docs as well, since there have been some env vars misplaced in the docs before (e.g. GRPC_MESSAGE_SIZE I moved in docs: full copy-edit of environment-variables.md #12148) and some env vars that are used by multiple Argo components/binaries.
3. Remove some no longer needed env vars
   • would be a breaking change, but BASE_HREF and ALLOWED_LINK_PROTOCOL are no longer needed since all flags can be env vars since feat: Add env vars config for argo-server and workflow-controller #6767 (ARGO_BASE_HREF and ARGO_ALLOWED_LINK_PROTOCOL can be used instead). might be some others.
     • EDIT: See chore!: remove duplicate Server env vars, plus --basehref -> --base-href #12653 as this breaking change

[isubasinghe]

nit: I'd personally prefer if we kept PRs atomic. I think documentation correction like this can be fixed in it's own separate PR even if it's minor.

[isubasinghe]

It just makes it a bit harder to review for the issue in question itself, which in this case is checking for missing env vars.

[agilgur5]

Generally, I agree, which is why the vast majority of my PRs are small and split into independent chunks. (there are some that carry a different opinion though).

I changed this line because I had to create the docs for the BASE_HREF env var for this PR and so referenced these and found the mistakes. So it is related to the purpose, and this PR does already change/add docs for BASE_HREF specifically.

Unfortunately this one-liner causes a fairly large diff due to the codegen.

Also I'm not sure if "atomicity" is the right term here. As technically all commits and PRs are atomic. "Small, independent, singular purpose" are all terms I've used to describe this.

[isubasinghe]

Approved although I have a minor nit

[isubasinghe]

@agilgur5 we seem to just generally be doing a lot of programming as a part of the build process via the hack directory, wonder if we should maybe just use something like this: https://magefile.org/ and get rid of the hacks alltogether?

I guess my reasoning here is that I find the sed/awk/grep combos a bit hard to parse and understand. I'd rather write more easy to understand code instead.

[agilgur5]

I guess my reasoning here is that I find the sed/awk/grep combos a bit hard to parse and understand. I'd rather write more easy to understand code instead.

Oh I agree 100%, and I say this having had to write a ton of bash at my last two jobs for fleet management... and also having written proposals for two of those teams to "use a programming language instead of hard-to-maintain shell hackery" (there's a lot more issues with shell than it being hard to read, including directory awareness, arg parsing, (lack of) dependency management, portability, etc etc etc etc). In the PR description I did also literally write: "something something 'portable shell code' something something 'maybe use Python instead next time'"

@agilgur5 we seem to just generally be doing a lot of programming as a part of the build process via the hack directory, wonder if we should maybe just use something like this: https://magefile.org/ and get rid of the hacks alltogether? [sic]

A few things:

1. This wouldn't get rid of the hacks, it might just move some of the code to a different place. We also already have several hacks written in Go. Magefile isn't necessary for that piece, it's more a make substitute (and I think there are better substitutes).
   • hacks/ is also a pretty conventional Go directory, have seen it many times
   • The larger a codebase gets and the more it does, the more necessary build programming becomes (part of the reason why I've worked in DevOps for many years). This codebase in particular has a ton of codegen hacked together.
2. I wouldn't call Go the most readable or ergonomic scripting language either, would probably prefer Python
   • And Go has no real way of separating build deps from prod deps 😕 one of the reason why there are some build / compilation hacks and why it takes a bit to run. We do already have Python and JS build deps.