chore: adjust env vars so that check-env-var-annotations passes

[phil-davis]

Description

1. chore: adjust documentation of check-env-var-annotations.
2. display line numbers in the output of check-env-var-annotatrions. This makes it easy to find the exact line that has the problem.
3. adjust logic for annotation checks so that it really passes when there are no problems.
4. add introductionVersion to all environment variable documentation.
5. add missing descriptions to environment variable documentation.

I added some "vacuous" description text in the cases where I didn't really know what to write. Suggestions welcome.

Related Issue

• Fixes Add the introductionVersion field to all existing environment variables #8434

How Has This Been Tested?

Local run of:
make check-env-var-annotations gives:

.make/check-env-var-annotations.sh
All introductionVersion annotations are valid
All description annotations are valid

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

[phil-davis]

@kulmann ready for review.

I can add the check to CI once we get these changes merged. It passes locally.

[phil-davis]

ToDo:

I put pre5.0 for all the env vars. But some of them might be introduced in 5.0.
How can I find out which are introduced in 5.0?

(Then I can adjust the introductionVersion for those env vars)

[dragonchaser]

@phil-davis they are listed in that table: https://github.com/owncloud/ocis/blob/cc18f5c372b7d3795167addb3c50574a95717941/docs/services/general-info/env-var-deltas/4.0.0-5.0.0-added.md

[phil-davis]

@phil-davis they are listed in that table: https://github.com/owncloud/ocis/blob/cc18f5c372b7d3795167addb3c50574a95717941/docs/services/general-info/env-var-deltas/4.0.0-5.0.0-added.md

done - actually most of the env vars are added in 5.0. Hopefully that is correct.

[phil-davis]

Now I just need some suggestions for the env vars that needed descriptions added.
See my questions above.

[dragonchaser]

Now I just need some suggestions for the env vars that needed descriptions added. See my questions above.

Added patch suggestions above.

[phil-davis]

I rebased, resolved the conflicts with recent changes in master, and added introductionVersion 5.0 to some new env vars.

make check-env-var-annotations passes locally for me.

Please review. Then maybe we merge this now (to reduce the ongoing conflicts happening). And I can work on a separate PR to add make check-env-var-annotations to CI.

[micbar]

@phil-davis Thanks so far! This is hard to review. I need to check, but a lot of env variables got a 5.0 introduction version which have already been there from the beginning.

[phil-davis]

@phil-davis Thanks so far! This is hard to review. I need to check, but a lot of env variables got a 5.0 introduction version which have already been there from the beginning.

I used the list in https://github.com/owncloud/ocis/blob/cc18f5c372b7d3795167addb3c50574a95717941/docs/services/general-info/env-var-deltas/4.0.0-5.0.0-added.md

That list is "long", and claims that a lot of env vars have been added in 5.0. If that list is not correct, then where is the next place to look to try and find out what was added in 5.0?

[dragonchaser]

@phil-davis Thanks so far! This is hard to review. I need to check, but a lot of env variables got a 5.0 introduction version which have already been there from the beginning.

I used the list in https://github.com/owncloud/ocis/blob/cc18f5c372b7d3795167addb3c50574a95717941/docs/services/general-info/env-var-deltas/4.0.0-5.0.0-added.md

That list is "long", and claims that a lot of env vars have been added in 5.0. If that list is not correct, then where is the next place to look to try and find out what was added in 5.0?

Oh there might be some confusion here, sorry I forgot to mention that. The List always states the full List of variables if one of them has been added. e.g. there is a global variable starting with OCIS_ and a service related one starting with <SERVICENAME>_ the global has most likely been there before. But since both can configure the same thing, both are mentioned.

[phil-davis]

But since both can configure the same thing, both are mentioned.

For lines that have both variables, what is required. For example:

Level  string `mapstructure:"level" env:"OCIS_LOG_LEVEL;ANTIVIRUS_LOG_LEVEL" desc:"The log level. Valid values are: 'panic', 'fatal', 'error', 'warn', 'info', 'debug', 'trace'." introductionVersion:"5.0"`

If OCIS_LOG_LEVEL was introduced pre5.0 and ANTIVIRUS_LOG_LEVEL is introduced in 5.0 then what do we set introductionVersion to?

And the same question if it happened the other way around?

(this is just an example to get the principle of this, I haven't looked in detail to see exactly when these were introduced)

[dragonchaser]

@phil-davis I think this is an agreement thing. Introduction version is only valid for the non-global part OR the global it is the only var in the line.

[phil-davis]

An example might help me:
env-var-deltas/4.0.0-5.0.0-added.md has an entry like:

AUTH_SERVICE_DEBUG_ADDR

easy - only one env var, it was added in 5.0

but:

OCIS_LOG_LEVEL;AUTH_SERVICE_LOG_LEVEL

and

OCIS_LOG_LEVEL;CLIENTLOG_USERLOG_LOG_LEVEL

and lots of the "log level" env vars.

I need to know what was introduced in 5.0 - was it OCIS_LOG_LEVEL (that applies as a default to lots of services), or AUTH_SERVICE_LOG_LEVEL CLIENTLOG_USERLOG_LOG_LEVEL ... that are the env var names that let you set a different log level for each service, or were all of these introduced in 5.0.

Apart from manually digging through git history, is there a way for me to find out exactly what was "really" introduced in 5.0?

[dragonchaser]

You are right... We might have to rethink the annotation.... maybe structured text like OCIS_LOG_LEVEL:pre5.0;AUTH_SERVICE_LOG_LEVEL:5.0 but I am not sure about this....

[dragonchaser]

@phil-davis I just have talked to @kobergj, in the foreseeable future we will only have one variable per line (see #8259). So for now it is fine to just use the version of the latest introduced variable. We do NOT need to mess around with structured text 😌

[phil-davis]

Rebased, resolved conflict, added introductionVersion:"5.0" to new env var lines added in recent PRs.

[phil-davis]

I have done what I think is "correct" according to the documented list of env vars changed/added in 5.0
It might be true that most env var declaration did change, if various common env vars have been added and apply in many places.

Please try to review. I think that "review" is going to mean just seeing that "something reasonable" has happened to all the modified lines. To check each env var against the documented list will take you just as long as it took me to do it!

[phil-davis]

Rebased again. Please let me know if/what you want me to do with this.

[phil-davis]

I will adjust this tomorrow. There are less "introduced in 5.0" env vars than I thought!

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[phil-davis]

See #8574