Values expected to be bools fetched from DataSource.ds_cfg should use the util methods to support common string inputs

[Dan Watkins]

At the moment, there are some places where we do something along the lines of `bool_thing = self.ds_cfg.get("cfg_option", False)`. This is incorrect, because specifying anything other than the empty string in configuration (such as "no" or "false") will result in a True-ish value.

A rough audit suggests the following places have this problem:

cloudinit/sources/DataSourceDigitalOcean.py:43: self.use_ip4LL = self.ds_cfg.get('use_ip4LL', MD_USE_IPV4LL)
cloudinit/sources/DataSourceAzure.py:503: self.ds_cfg.get('apply_network_config')):
cloudinit/sources/DataSourceAzure.py:682: preserve_ntfs=self.ds_cfg.get(
cloudinit/sources/DataSourceAzure.py:702: if self.ds_cfg.get('apply_network_config'):

(This audit was just done with grep, so will have missed any places where ds_cfg is referred to as anything but self.ds_cfg.)

[Scott Moser (smoser)]

Dan,

I know I've talked to Chad and Ryan before about this, but I kind of
see the plethora of values that are accepted as bad design (my bad
design).

Ultimately, I'd rather deprecate and remove that functionality than
extend it to new places. Its just not really helpful and makes
checking more difficult.

I realize we're in a bad place and you're trying to make it better. I
think the long path to making it better is to take yaml bool for
boolean values and complain on non-boolean values.

On Thu, Aug 8, 2019 at 3:55 PM Dan Watkins <email address hidden> wrote:
>
> Public bug reported:
>
> At the moment, there are some places where we do something along the
> lines of `bool_thing = self.ds_cfg.get("cfg_option", False)`. This is
> incorrect, because specifying anything other than the empty string in
> configuration (such as "no" or "false") will result in a True-ish value.
>
> A rough audit suggests the following places have this problem:
>
> cloudinit/sources/DataSourceDigitalOcean.py:43: self.use_ip4LL = self.ds_cfg.get('use_ip4LL', MD_USE_IPV4LL)
> cloudinit/sources/DataSourceAzure.py:503: self.ds_cfg.get('apply_network_config')):
> cloudinit/sources/DataSourceAzure.py:682: preserve_ntfs=self.ds_cfg.get(
> cloudinit/sources/DataSourceAzure.py:702: if self.ds_cfg.get('apply_network_config'):
>
> (This audit was just done with grep, so will have missed any places
> where ds_cfg is referred to as anything but self.ds_cfg.)
>
> ** Affects: cloud-init
> Importance: Undecided
> Status: New
>
> --
> You received this bug notification because you are subscribed to cloud-
> init.
> https://bugs.launchpad.net/bugs/1839538
>
> Title:
> Values expected to be bools fetched from DataSource.ds_cfg should use
> the util methods to support common string inputs
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/cloud-init/+bug/1839538/+subscriptions

[Dan Watkins (oddbloke)]

On Fri, Aug 09, 2019 at 02:14:32PM -0000, Scott Moser wrote:
> I know I've talked to Chad and Ryan before about this, but I kind of
> see the plethora of values that are accepted as bad design (my bad
> design).
> Ultimately, I'd rather deprecate and remove that functionality than
> extend it to new places. Its just not really helpful and makes
> checking more difficult.
> I realize we're in a bad place and you're trying to make it better. I
> think the long path to making it better is to take yaml bool for
> boolean values and complain on non-boolean values.

OK, great, I'm not a huge fan of the multiple values thing either!
(That said, I do think that relying on people to know what YAML bools
are in 2019 is a lot more reasonable than it would have been back in
2012, so I don't know that this was bad design at the time!)

So I'm going to mark this as Invalid. A brief search around didn't find
a bug for the removal/migration of those values; do you know if we
already have one, or shall I file one?

(I cannot resist this pun: I'm a strong {1, "on", "true", "yes"} on this
as the long-term plan.)

[Scott Moser (smoser)]

I dont' think there is a migration bug anywhere.

On Fri, Aug 9, 2019 at 12:46 PM Dan Watkins
<email address hidden> wrote:
>
> On Fri, Aug 09, 2019 at 02:14:32PM -0000, Scott Moser wrote:
> > I know I've talked to Chad and Ryan before about this, but I kind of
> > see the plethora of values that are accepted as bad design (my bad
> > design).
> > Ultimately, I'd rather deprecate and remove that functionality than
> > extend it to new places. Its just not really helpful and makes
> > checking more difficult.
> > I realize we're in a bad place and you're trying to make it better. I
> > think the long path to making it better is to take yaml bool for
> > boolean values and complain on non-boolean values.
>
> OK, great, I'm not a huge fan of the multiple values thing either!
> (That said, I do think that relying on people to know what YAML bools
> are in 2019 is a lot more reasonable than it would have been back in
> 2012, so I don't know that this was bad design at the time!)
>
> So I'm going to mark this as Invalid. A brief search around didn't find
> a bug for the removal/migration of those values; do you know if we
> already have one, or shall I file one?
>
> (I cannot resist this pun: I'm a strong {1, "on", "true", "yes"} on this
> as the long-term plan.)
>
>
> ** Changed in: cloud-init
> Status: New => Invalid
>
> ** Changed in: cloud-init
> Status: Invalid => Won't Fix
>
> --
> You received this bug notification because you are subscribed to cloud-
> init.
> https://bugs.launchpad.net/bugs/1839538
>
> Title:
> Values expected to be bools fetched from DataSource.ds_cfg should use
> the util methods to support common string inputs
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/cloud-init/+bug/1839538/+subscriptions

[Dan Watkins (oddbloke)]

I've filed https://bugs.launchpad.net/cloud-init/+bug/1839659 for it; thoughts and comments appreciated and encouraged!

[James Falcon (falcojr)]

Tracked in Github Issues as https://github.com/canonical/cloud-init/issues/3424