ini inventory now requires extension

[bcoca]

but has option to enable previous behaviour

related #83981

ISSUE TYPE

• Feature Pull Request

[flowerysong]

I think on balance this is probably a good idea, but it might need a deprecation cycle where it warns about non-matching filenames instead of ignoring them. Failing to parse an inventory source defaults to not being a fatal error so this could sneakily break things.

There's also the issue of DEFAULT_HOST_LIST defaulting to /etc/ansible/hosts, and the basic inventory example using that path.

And of course there's the question of INVENTORY_IGNORE_EXTS still containing .ini by default.

[bcoca]

ignore ext was my next stop, i was also considering a 'strict_ext' option (would be temporary) to do what you say and warn/advise about extensions or adding '' as part of the current default with a deprecation warning.

[nitzmahone]

Yeah, it should've been this way a long time ago, but IMO it should be a deprecation with a config knob, then we just change the default when the deprecation expires. (ie, if we parsed one successfully without an extension, fire the warning)

[bcoca]

@nitzmahone w/o extension is fine, the problem is 'all extensions', which keeps making ini plugin take toml/yaml/others lower in the precedence of the enable list.

I did have a 'no extension' version with deprecation, but then realized it didn't conflict with anything else.

[nitzmahone]

it didn't conflict with anything else

It does actually: script... But that would also be an easy fix (w/ or w/o deprecation): exclude executable files in verify.

[bcoca]

rarely, first, script has higher precedence by default and most scripts do have extensions. Another reason i don't exclude executable here is because 'samba' mounts normally make everything executable and are a favorite of .ini users. And scripts tend not to be 'ini compatible' like yaml or toml can be.

[nitzmahone]

yaml also support extensionless IIRC- I've definitely run into cases where the wrong plugin in the default list picked it up and actually "successfully" parsed it, took a long time to track down what was wrong 😆 .

That's of course not so much an issue with "real world" inventories- the most recent case I ran into was a new test for script in DT that was a "broken" script or something, but the details escape me.

[bcoca]

yes, why i was adding the restriction from any to 'ini' and w/o, yaml has higher precedence and has always had an extension list. The current settings should handle every case I've seen a ticket for.

[mattclay]

There are two previous behaviors:

1. 2.18 and earlier - parse all files
2. 2.19 and 2.20 - parse all files except .toml

The change in 2.19 was a bug fix (which was overlooked in the changelog). To keep that fix, the empty list should keep the second behavior instead of switching back to the first. In the unlikely event someone actually wants to parse .toml files with this plugin, they can specify that in their configuration by listing the extensions they want.

The empty list fallback behavior should also be deprecated, so we don't need to keep it around long-term.

[bcoca]

since it was an undocumented and backwards incompatible change and toml has since been decided to not be a first class citizen, i didn't make the toml exclusive change and I don't really think it is needed.

i was deprecating the fallback in previous iterations, i took it out because you said it was confusing ... now it should be back?

[mattclay]

since it was an undocumented and backwards incompatible change and toml has since been decided to not be a first class citizen, i didn't make the toml exclusive change and I don't really think it is needed.

What was backward incompatible?

[bcoca]

people using toml extensions, we had a couple of reports about that

[sivel]

I think I'd ignore people doing weird things, an deny toml extensions in the ini plugin.

[bcoca]

this denies all extensions, not just toml, since it, by default, only allows ini and 'no extension', i just dont see the use of adding a 'all extensions exctept toml' option.

[mattclay]

We only need to support two configurations:

1. A non-empty list of extensions. This should be the default and set to: ['ini', '']
2. An empty list of extensions. This should behave like 2.19/2.20, excluding only .toml.

The first configuration gives us the behavior we want, including allowing users to parse .toml files as INI. The second should be deprecated, and gives users who need the previous behavior time to update their inventory.