search

warpnet / salt-lint

A command-line utility that checks for best practices in SaltStack.
https://salt-lint.readthedocs.io/en/latest/
MIT License
152 stars 39 forks source link

207, 208 and 210 are useless, as salt removes leading zeroes when parsing integers with yaml #322

Open sylvainfaivre opened 7 months ago

sylvainfaivre commented 7 months ago

See https://github.com/saltstack/salt/issues/60773 and https://github.com/saltstack/salt/blob/2b364c92e6319ec3a9884afff10e6e4e1e1642db/salt/utils/yamlloader.py#L89

When loading a yaml scalar, salt takes special care of removing the leading zero unless it's starting with 0b or 0x.

This has been the case for more than 10 years, see https://github.com/saltstack/salt/blame/bbdcd5d4b496845eb6a6811b7c04898a99b4e11e/salt/utils/yamlloader.py#L91C53-L91C55

So I would argue that these rules are useless and could be removed. Or at the least, if people want to use them to ensure coding style consistency, add a warning in the docs explaining they are not real issues.

I hope this report gets the conversation going. Should we reach a consensus on this, I'd be willing to propose a PR.

ohartl commented 5 months ago

Correct, it seems like this was useful when we still used python2, but nowadays this is not useful for rulesets / best practices anymore.

NoRePercussions commented 2 months ago

Since Salt no longer complies to any particular YAML version (the custom loader does not recognize old-style octal 0123 nor new-style octal 0o123, I'd suggest we stay with the least common denominator of YAML parsers and keep these rules.

For another example of a major library that has done this, https://github.com/go-yaml/yaml continues to support old-style, citing "most parsers still use the old format".

I'd also pose that a file mode is an octal number, and treating it as decimal to begin with is a hack for convenience. I don't think it helps to allow any ambiguity as to whether a file mode will be parsed as octal or decimal, and whether that parsing will be supported, so I believe that enforcing a string format for the mode is sensible regardless upstream support, if salt-lint is aiming to be opinionated.