Is changing the default setting of TRANSFORM_INVALID_GROUP_CHARS a good idea?

[Jörg Kastning]

Hello everybody,

After updating to ansible 2.8.2 I faced the following deprecation warning when running a playbook:

[DEPRECATION WARNING]: The TRANSFORM_INVALID_GROUP_CHARS settings is set to allow bad characters in
 group names by default, this will change, but still be user configurable on deprecation. This
feature will be removed in version 2.10. Deprecation warnings can be disabled by setting
deprecation_warnings=False in ansible.cfg.
 [WARNING]: Invalid characters were found in group names but not replaced, use -vvvv to see details

Searching for additional information on the web I found the two open issues on GitHub:

1. Provide means of flipping Ansible config TRANSFORM_INVALID_GROUP_CHARS #3513
2. TRANSFORM_INVALID_GROUP_CHARS doesn't document valid group patterns #56930

In #56930 a discussion took place about the reasons for this change in default behaviour and whether it would be a wise choice or not. The user bcoca pointed out that GitHub is the wrong place for these discussions to take place and summarized the reasons that have lead to the current decision. He closed his comment with the statement:

I hope this addresses all the major issues, as always we are open for discussion, feel free to drop by ML or IRC, we just avoid using github since it is not a good place for such things.

So here we are. I hope this is a good place for this discussion to take place. If there is any other mailing list that would be a better fit for this discussion, please point me there.

I understand that there is some confusion about variable names and group names leading to a certain amount of support cases. Is that right? So the default setting of TRANSFORM_INVALID_GROUP_CHARS is changed because some people are trying to access a group name that they have mistaken for a variable?

IMHO changing the default setting is not a good idea, though. The hyphen or the dash are common chars for group and host names in most environments. To restrict them without a very good reason seems not to be wise.

What I have not understand yet, do hyphens and dash causes confusion and support cases or only the use of dots and colones etc.? Would it be possible to only ban the chars that are causing the most of the trouble?

In my environmen we are using dash/hyphen in almost every group name and we won't change it. So I have to keep track and change the default every time I setup a new environment. For me that's unfortunate. But if I have to do it I would expect to have information on how to do so in documentation, configuration files, porting guides, etc.

Best regads,

Joerg

[Brian Coca]

>What I have not understand yet, do hyphens and dash causes confusion and support cases or only the use of dots and colones etc.? Would it be possible to only ban the chars that are causing the most of the trouble?

Most of the issues have to do with the hyphen, there are incidents
with the dots also (I've even seen `@groupname` once) but if we are
only going to ban the characters after people hit the issue i still
think it is a bad experience for the new user.

>In my environmen we are using dash/hyphen in almost every group name and we won't change it. So I have to keep track and change the default every time I setup a new environment. For me that's unfortunate. But if I have to do it I would expect to >have information on how to do so in documentation, configuration files, porting guides, etc.

I know this is an inconvenient, I wish I had a better solution, but
keeping the things as they were before is IMHO a worse inconvenience
to many users, one I cannot really fix as it is a feature from a
project we depend on (Jinja) and a very popular one at that. My first
suggestion was to 'not promote dot notation in our documentation', but
that got very strong resistance and was rejected. My second suggestion
on making it a user configurable expression to sanitize the names was
also rejected . In the end we voted to force all inventory to go
through the same standard sanitation, at least I did insist on keeping
a toggle to bypass it, but the current default (backwards compatible)
had to be deprecated in favor of the 'safe default'.

We also looked (and are still looking) at other solutions, mostly
internal name replacement, but this is not something I expect to work,
even if it does it might have other major consequences.
If anyone has a solution that fixes all the underlying issues w/o
creating a burden on any of the users, we are more than happy to
listen, 'revert this cause it affects me' is not a solution as it
keeps the original problem.



----------
Brian Coca

[Jörg Kastning]

Hello Brian,

And thanks for taking the time of answering.

Am Montag, 5. August 2019 16:41:36 UTC+2 schrieb Brian Coca:

If anyone has a solution that fixes all the underlying issues w/o
creating a burden on any of the users, we are more than happy to
listen, 'revert this cause it affects me' is not a solution as it
keeps the original problem.

Well, I don't understand enough of the development process to provide an alternative solution. And I understand for sure that the change won't be reverted because it affects me. From the discussion on GitHub I got the impression that a lot of users have issues with this change. And I'm wondering how broad the user base was in the discussion that has lead to the decision.

Maybe anyone else has a good idea on how to solve this. Let's see.

I'm really sad that a change with the potential to affact a very large user group was made without proper documentation. I know that developers have the curse of knowledge but when the behaviour changes the documentation has to be changed, too. Well, an issue is open for that. Let's see when it could be solved.

I hope you could keep your promise:

Just to say this once, clearly , YOU WILL ALWAYS BE ABLE TO USE DASHES IN GROUP NAMES also dots and other characters that are now considered 'invalid', just not by default. This 'default' is what is being deprecated, the default will be 'safe' in 2.11, but you will always have an option to 'opt in' to the old behaviour.

Should that change my colleagues would force me to change the config managment. :-(

Best regards,

Joerg

[Brian Coca]

> I'm really sad that a change with the potential to affact a very large user group was made without proper documentation. I know that developers have the curse of knowledge but when the behaviour changes the documentation has to be changed, too. Well, an issue is open for that. Let's see when it could be solved.

We knew it would affect a good chunk of users, its always a balancing
act, what is the right thing to do? how many people will this
inconvenience? is there a less intrusive solution?
Again, I think aside from the 'future' inconvenience of having to 'opt
in' to this feature, most of the problem was a misunderstanding on the
behaviour going away permanently.

> I hope you could keep your promise:

me too, just to confirm, no one has made the case to do otherwise,
most of the problem was confusion due to a bug (deprecation always
showed, now fixed) and my poor wording of warnings and docs (which is
still being worked on).




--
----------
Brian Coca

[Jeff Geerling]

I think part of the problem is the confusion as to whether or not dashes would be completely unsupported after 2.11, and it's good to know that is resolved. However, having to set a variable with "invalid" in the name makes it sound like "you are doing something wrong"... and afaict, dashes are perfectly valid for dict keys in Python.

One of the earliest things I remember learning when dealing with dicts in my Python journey was that dot notation is great in a limited set of circumstances, otherwise you use bracket notation for anything besides very_simple_keys.

I strongly disagree with the core team's direction on this, because:

1. It feels like the war on dashes has spilled over from roles (then collections) to now group naming conventions.
2. Dashes are used (and preferred) for labels and pathing purposes almost universally (if you'd like, I can cite examples), so a great number of users will now need to add this default override in every ansible.cfg file they maintain.
3. Ansible end users should not have the mental burden of having to deal with (in this case, seemingly arbitrary) variable naming character decisions.

I think the 'principle of least surprise' should apply here: I use dashes almost universally with naming things. I use dashes in hostnames in my inventory groups. I use dashes for file naming, project naming, etc. No other software blares warnings or defaults to usage which would restrict my use of dashes.

When I saw this in the Ansible output after upgrading to 2.8.x, I was at first confused (because of the wording of the message), then bewildered by the fact that dashes are in a sense made 'second class citizens' in inventory group names.

I mean, I would definitely go for dropping the dot object key access syntax in documentation entirely (and spend a week or so cleaning up all references) if that's truly the only reason this change was made. That doesn't make sense to me because most people would learn once that they can just switch to bracket (something['group-with-dashes']) notation, and live the rest of their lives happy... but anything would be better than continuing to burden long-time Ansible users with deprecation after deprecation of heavily used syntax/conventions, on every Ansible release.

-Jeff

[Brian Coca]

On Friday, August 23, 2019 at 9:23:45 AM UTC-4, Jeff Geerling wrote:


> It feels like the war on dashes has spilled over from roles (then collections) to now group naming conventions.

I can empathize with the 'feeling' but the reality is that this has more to do with underlying tools used (jinja2, python imports) than what core devs want.

> Ansible end users should not have the mental burden of having to deal with (in this case, seemingly arbitrary) variable naming character decisions.

The choice is between that and the mental burden of 'knowing how to avoid errors in templates' when using 2 'default' features, dashes in names and Jinja2 dot notation. We took the option we thought would be most helpful to users (not have a exception trap waiting to happen) and calculated that having a setting would be minor inconvenience to those that use dashes and a way for them to be informed of said trap.

> When I saw this in the Ansible output after upgrading to 2.8.x, I was at first confused (because of the wording of the message), 

yes, this was my mistake and has since been corrected and clarified.

> then bewildered by the fact that dashes are in a sense made 'second class citizens' in inventory group names.

This was always the case, it was just not uniform across inventory sources, the change was 'making it known' to all users and deprecating the default 'wild west' that depended exclusively of inventory source used, some even did sanitization partially and inconsistently within the source itself.

> I mean, I would definitely go for dropping the dot object key access syntax in documentation entirely (and spend a week or so cleaning up all references)

This was the first solution proposed and it was overwhelmingly shot down.

>  but anything would be better than continuing to burden long-time Ansible users with deprecation after deprecation of heavily used syntax/conventions, on every Ansible release.

This is something we have been struggling with as if we don't deprecate it we keep a broken/incomplete functionality and the 'new way' that covers and fixes most of the previous issues is not adopted unless there is something pushing users. Then there is the maintenance aspect, some of the 'old way' ends up crippling the new features or fixes we want due to the constraint of keeping backwards compatibility. 

This is why feedback is important and we continue to listen, learn and adapt, we would also benefit from more people attending the IRC meetings where these things get proposed, voted on and decided. https://github.com/ansible/community/tree/master/meetings

--
----------
Brian Coca

[Steve Kieu]

There are many comments already so I just want to be short. Please correct if I am wrong in understanding that based on your comment, you think - 

allowing hyphen in group name is broken. 

And I can not find a good rationale for that yet still. 

Except for the wrong way of using it by dot notation - which is wrong in my opinion as it is not an complex object type (an instance of a class) but simply a dict, there is no reason to use dot notation to access a dict value. 

Python does not allow it, why ansible allows it in the first place. 

IMHO the accessing value method using dot notation *is* the broken way of doing rather than group name having hyphen.

However for some reason ansible developers opted to fix the non-broken to allow the broken feature to be used.

Does it make sense? No, completely not to me. Group name and hostname now is a mess and having different class of specifications, thanks god hostname is still conform with standard to allow hyphen. 

Because using hyphen is recommended way for hostname (also DNS)

As by this nature many dynamic inventory tool will return group and host names using hyphen. They are now classified as second class citizen and being threatened to be deprecated at least by the sound of the deprecation warning - Until I saw the announcement somewhere that it might not ever been really deprecated - and this is the first time I feel happy, secured my mind enough to continue normal daily work without seeking another configuration tools to replace our more than 5 years investment in ansible automation stack.

My simple recommendation is:

1. Identify what is really a broken and fixes it if possible, not fixing something that is not broken by design.

2. And accessing value using dot notation for a dict like data structure is a broken way of doing things

For 2. it *might* be convenient to do so to avoid the big impact - the fix might simply be a good training of users (documentation) to understand.

Thank you for listening. 

SteveK

[Kai Stian Olstad]

On 23.08.2019 23:12, Steve Kieu wrote:
> Except for the wrong way of using it by dot notation - which is wrong
> in my
> opinion as it is not an complex object type (an instance of a class)
> but
> simply a dict, there is no reason to use dot notation to access a dict
> value.
>
> Python does not allow it, why ansible allows it in the first place.

Ansible uses Jinja, and Jinja allow it
https://jinja.palletsprojects.com/en/2.10.x/templates/#variables

> Does it make sense? No, completely not to me. Group name and hostname
> now
> is a mess and having different class of specifications, thanks god
> hostname
> is still conform with standard to allow hyphen.
>
> Because using hyphen is recommended way for hostname (also DNS)

Hyphen in hostname is not affected by this, just group names since they
are variables.


--
Kai Stian Olstad

[Jörg Kastning]

Am Freitag, 23. August 2019 18:32:18 UTC+2 schrieb Brian Coca:

This is why feedback is important and we continue to listen, learn and adapt, we would also benefit from more people attending the IRC meetings where these things get proposed, voted on and decided. https://github.com/ansible/community/tree/master/meetings

Well, maybe for decisions which would affect almost all and every ansible user it would be awesome to start a poll on a place where you could reach more users like here in the google groups or at github. Or at least announce that there would be a poll and how to participate.

I'm not a developer but a user of ansible. I subscribed to the announcements, read the changelog before version updates and report and try to triage issues if I encounter them. But that's it. Unfortunately there is no more time to attend to irc meetings and so on. And I'm for sure one out of many who act in a very similar way. But this way I recognize the change when the decision is made and it's to late to spend my two cent on it. :-/

Regards,

Joerg

[Magnus Lubeck]

Den lördag 24 augusti 2019 kl. 00:37:41 UTC+2 skrev Kai Stian Olstad:

>
> Because using hyphen is recommended way for hostname (also DNS)

Hyphen in hostname is not affected by this, just group names since they
are variables.

Groups are not at all a variable, to be honest.  It is a group, just like a group in LDAP or a band playing on a stage. In Jinja, the most common way to access a group, is through a hash, where the group name is the key.

//magnus

 

[Mark Goddard]

I am posting here so that at least I can say I tried. I'm sure I'm missing some of the context available to the Ansible developers, but as a user this really is not good. There have been a number of changes similar to this over Ansible's history, but in general with a single (sometimes painful) migration of code they can be accommodated. In this case I feel that disallowing hyphens in group names is fundamentally wrong.

Many dynamic inventory sources generate group names based on real entities that have names with hyphens in.

Hostnames cannot have underscores in (https://en.wikipedia.org/wiki/Hostname). So we now have the requirement that our inventory file must contain only hyphens for hostnames, and only underscores for group names. That is very unfriendly for users. (of course I realise that your inventory_hostname does not need to match your real hostname, but...)

I realise that this behaviour can be configured, but broken by default is not good - it should work out of the box.

[Will Thames]

On Tuesday, August 6, 2019 at 12:41:36 AM UTC+10, Brian Coca wrote:

>What I have not understand yet, do hyphens and dash causes confusion and support cases or only the use of dots and colones etc.? Would it be possible to only ban the chars that are causing the most of the trouble?

Most of the issues have to do with the hyphen, there are incidents
with the dots also (I've even seen `@groupname` once)  but if we are
only going to ban the characters after people hit the issue i still
think it is a bad experience for the new user.

Hi Brian,

Can you provide an example of something where using groups with dashes leads to issues?

I understand the general principle that `a.b-c.d` is not equivalent to `a['b-c']['d]`

Is it people trying to access `groups.group-a.vars` instead of `groups['group-a'].vars`? Is this a particularly widespread pattern? (You have to be pretty advanced to want to access vars from other groups, by which point hopefully you understand Jinja syntax)

Will

[Jörg Kastning]

Hello Brian,

This is why feedback is important and we continue to listen, learn and adapt, we would also benefit from more people attending the IRC meetings where these things get proposed, voted on and decided. https://github.com/ansible/community/tree/master/meetings

Not all Ansible users are on IRC nor willing to join this communication channel unless they use it already. I would appreciate if hearbreaking or at least codebreaking changes would be discussed and surveyed on the mailling lists, too.

[Andrew Feller]

I have to agree with Jörg as most of Ansible's customers are using products like Slack these days to participate on multiple workspaces already.  But even if Ansible discussions were hosted there, there is the reality that most Ansible customers work the same long hours that Ansible employees and contributors do with little leeway to actively participate in every discussion that affects the tools we use, so discussions for deciding potentially breaking changes should be clearly and proactively communicated in multiple channels barring alternative customer interactions. 

[Andrew Feller]

I'm all for dashes and want to see them preserved as a valid character; I also want to explicitly point out that dots / periods (.) are also a valid character as they have been long in things like hostnames and packages to delineate strata as much as accessing things.  Outside of special cases like the k8s and openshift inventory plugins where groups are created for things, which don't allow for periods in their own domains, there shouldn't be a reason it is denied.