chore!: remove duplicate Server env vars, plus `--basehref` -> `--base-href`

[agilgur5]

Follow-up to #12652 and https://github.com/argoproj/argo-workflows/issues/6923#issuecomment-1937357156 on findings of duplicate env vars

Motivation

• every Server CLI flag has had an equivalent env var since #6767
• ALLOWED_LINK_PROTOCOL and BASE_HREF each had a duplicate env var as the odd ones out
• The Server flag --basehref was also inconsistent with the env var, other flags, and the client, change it to --base-href for consistency

Modifications

• remove the two env vars in a breaking change, and refer users to their ARGO_* equivalents in the upgrade notes
• replace --basehref with --base-href as a breaking change and mention it in the upgrade notes
• modify the docs and descriptions that referred to the two env vars
  • plus some small grammar and consistency fixes therein

Verification

Docs build & lint pass.

Notes to Reviewers

Can skip docs/cli/* as it's all auto-generated from the CLI changes

[agilgur5]

I think we also need to change https://github.com/argoproj/argo-workflows/blob/main/cmd/argo/commands/root.go#L87C102-L87C111 to ARGO_BASE_HREF

~See my in-line comment, I gave context on this in the PR description.~ EDIT: per below, I misread, this is a separate Long description/comment of the CLI, my bad!

[tczhao]

Thanks for the links. They make a lot of sense! I still think we need to change https://github.com/argoproj/argo-workflows/blob/main/cmd/argo/commands/root.go#L87C102-L87C111 from If your server is behind an ingress with a path (you'll be running "argo server --basehref /...) or "BASE_HREF=/... argo server"): to If your server is behind an ingress with a path (you'll be running "argo server --basehref /...) or "ARGO_BASE_HREF=/... argo server"): or in any style that make sense, the BASE_HREF=/... argo server seems wrong since the env var now has a ARGO_ preifx

[agilgur5]

Ohhh, good catch! I totally missed that in the root.go, especially since it says ARGO_BASE_HREF on the very next line. Sorry I misread your first comment! Thanks for clarifying and quoting it directly, that made it more obvious 🙂

Yea let me modify that too. I could've sworn I searched for all references to BASE_HREF so let me double check that too if I missed this one

[agilgur5]

https://github.com/argoproj/argo-workflows/blob/main/cmd/argo/commands/root.go#L87C102-L87C111

Ah I totally missed this column number in the link -- I was on mobile where it's far off screen to the right so I didn't see the column highlight on first glance

(Screenshot for reference: )

[agilgur5]

https://github.com/argoproj/argo-workflows/blob/main/cmd/argo/commands/root.go#L87C102-L87C111

Yea let me modify that too. I could've sworn I searched for all references to BASE_HREF so let me double check that too if I missed this one

Fixed this one plus a few others. That should be all of them now

[tczhao]

https://github.com/argoproj/argo-workflows/blob/main/cmd/argo/commands/root.go#L87C102-L87C111

Ah I totally missed this column number in the link -- I was on mobile where it's far off screen to the right so I didn't see the column highlight on first glance

(Screenshot for reference: )

No worries. It happens

[rijos]

@agilgur5, seems this change will break the helm chart, it still refers to BASE_HREF. See: https://github.com/argoproj/argo-helm/blob/2f82fb5992fe1e390d1ebdbc4be6d5d6c6549a37/charts/argo-workflows/templates/server/server-deployment.yaml#L98

[agilgur5]

Yes, all downstream consumers that use this, like the Helm Charts, will need to be updated. That is the nature of breaking changes.

[agilgur5]

Merged main for new PR checks per #13027

[agilgur5]

Going to merge this as Tianchu approved and Bala LGTM'd and there have been no reviews from others in the past ~4 months

[agilgur5]

@agilgur5, seems this change will break the helm chart, it still refers to BASE_HREF. See: https://github.com/argoproj/argo-helm/blob/2f82fb5992fe1e390d1ebdbc4be6d5d6c6549a37/charts/argo-workflows/templates/server/server-deployment.yaml#L98

Since ARGO_BASE_HREF already exists, I addressed this in the chart for forward compat: https://github.com/argoproj/argo-helm/pull/2756

[agilgur5]

Since ARGO_BASE_HREF already exists, I addressed this in the chart for forward compat: argoproj/argo-helm#2756

Annnd I didn't re-read my own release notes or this PR (it's 4 months old, so quite a time delay) and caused a regression downstream in Argo Helm. Reverted that change in https://github.com/argoproj/argo-helm/pull/2770 Sorry I screwed that up, I meant to be proactive there, but the time delay meant I forgot how my own change worked