[openstack-dev] [all][oslo] Disable option value interpolation in oslo.config
Hi ALL, I have been working a bug [1] about option value interpolation in oslo.config[2], in short, option value interpolation can't handle password containing special characters from environment variable and I proposed a fix of provide way to forbid option value interpolation explicitly[3]. copy of Doug Hellmann's coments: "The problem is that the end user who is setting the value of the option cannot control whether the option will do interpolation or not. So the programmer who defines the option has to make that choice, and then we can't change it because that would break existing deployments. The result is that end users won't know for any given option whether interpolation works or not, and if not why (did they do something wrong, or is it not supported). I've set -2 on this patch because I think it's a bad approach. I see 2 other ways we could solve the problem you describe (and I agree that it's an issue we should help with). 1. We could have an option that turns off interpolation globally, and let the user control that by setting the flag in their configuration file. I'm not sure I like this, but it does give you what you're looking for at the risk of breaking applications that are relying on interpolation, like the nova example. 2. We could disable interpolation when we get values from environment variables. That would be a big behavioral change, so we would need to think about how to roll it out carefully. For example, do we provide a helper function to give to application developers who are setting default values to environment variables so the variable value can be escaped to avoid interpolation? Or do we build it into the Opt class somehow? I think I like the helper function approach but we should give it more thought." I would like to collect more suggestions to decide the direction to fix similar bug. Any thoughts ? [1]https://bugs.launchpad.net/oslo.config/+bug/1577731 [2] http://docs.openstack.org/developer/oslo.config/cfg.html#option-value-interpolation [3]https://review.openstack.org/#/c/338668/ -- ChangBo Guo(gcb) __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all][oslo] Disable option value interpolation in oslo.config
On Fri, Jul 08, 2016 at 10:14:20PM +0800, ChangBo Guo wrote: > Hi ALL, > > I have been working a bug [1] about option value interpolation in > oslo.config[2], in short, option value interpolation can't handle password > containing special characters from environment variable and I proposed a > fix of provide way to forbid option value interpolation explicitly[3]. > > copy of Doug Hellmann's coments: > > "The problem is that the end user who is setting the value of the option > cannot control whether the option will do interpolation or not. So the > programmer who defines the option has to make that choice, and then we > can't change it because that would break existing deployments. The result > is that end users won't know for any given option whether interpolation > works or not, and if not why (did they do something wrong, or is it not > supported). > > I've set -2 on this patch because I think it's a bad approach. I see 2 > other ways we could solve the problem you describe (and I agree that it's > an issue we should help with). > > 1. We could have an option that turns off interpolation globally, and let > the user control that by setting the flag in their configuration file. I'm > not sure I like this, but it does give you what you're looking for at the > risk of breaking applications that are relying on interpolation, like the > nova example. > > 2. We could disable interpolation when we get values from environment > variables. That would be a big behavioral change, so we would need to think > about how to roll it out carefully. For example, do we provide a helper > function to give to application developers who are setting default values > to environment variables so the variable value can be escaped to avoid > interpolation? Or do we build it into the Opt class somehow? I think I like > the helper function approach but we should give it more thought." > I would like to collect more suggestions to decide the direction to fix > similar bug. Any thoughts ? I don't see a compelling need to change oslo behaviour wrt interpolation at all, given all the option suggested here break copatibility in some manner or another. The curernt behaviour should just have its error reporting fixed, so that it explicitly tells the user that the env var it tried to interpolate contains invalid characters, instead of printing the incomprehensible stack trace. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all][oslo] Disable option value interpolation in oslo.config
On 07/08/2016 09:14 AM, ChangBo Guo wrote:
> Hi ALL,
>
> I have been working a bug [1] about option value interpolation in
> oslo.config[2], in short, option value interpolation can't handle
> password containing special characters from environment variable and I
> proposed a fix of provide way to forbid option value interpolation
> explicitly[3].
>
> copy of Doug Hellmann's coments:
>
> "The problem is that the end user who is setting the value of the option
> cannot control whether the option will do interpolation or not. So the
> programmer who defines the option has to make that choice, and then we
> can't change it because that would break existing deployments. The
> result is that end users won't know for any given option whether
> interpolation works or not, and if not why (did they do something wrong,
> or is it not supported).
>
> I've set -2 on this patch because I think it's a bad approach. I see 2
> other ways we could solve the problem you describe (and I agree that
> it's an issue we should help with).
>
> 1. We could have an option that turns off interpolation globally, and
> let the user control that by setting the flag in their configuration
> file. I'm not sure I like this, but it does give you what you're looking
> for at the risk of breaking applications that are relying on
> interpolation, like the nova example.
This feels like a big hammer for a relatively small edge case, and as
you note it doesn't handle the case where you want to use interpolation,
but have an environment variable default that breaks when interpolated.
>
> 2. We could disable interpolation when we get values from environment
> variables. That would be a big behavioral change, so we would need to
> think about how to roll it out carefully. For example, do we provide a
> helper function to give to application developers who are setting
> default values to environment variables so the variable value can be
> escaped to avoid interpolation? Or do we build it into the Opt class
> somehow? I think I like the helper function approach but we should give
> it more thought."
One possibility would be an "env_default" (or something) parameter to
Opt that would read the env var, escape it as necessary, and set the opt
default to the escaped value. Then we could recommend that people stop
using os.environ for setting defaults this way. Using default and
env_default at the same time would raise an exception.
So the shaker example would look like:
cfg.StrOpt('os-password', metavar='',
env_default='OS_PASSWORD',
sample_default='',
help='Authentication password, defaults to env[OS_PASSWORD].')
It does require that developers DTRT when pulling defaults from the env,
but IMHO that's better than a backwards incompatible change or requiring
that users DTRT, which seem to be the alternatives here.
I suppose it would break anyone who happened to be using interpolation
of a default from the env, but I feel like that's unlikely and a much
less common case than "my password has a $ character in it".
>
> I would like to collect more suggestions to decide the direction to fix
> similar bug. Any thoughts ?
>
> [1]https://bugs.launchpad.net/oslo.config/+bug/1577731
> [2]http://docs.openstack.org/developer/oslo.config/cfg.html#option-value-interpolation
> [3]https://review.openstack.org/#/c/338668/
>
>
> --
> ChangBo Guo(gcb)
>
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all][oslo] Disable option value interpolation in oslo.config
Excerpts from Daniel P. Berrange's message of 2016-07-08 15:25:16 +0100: > On Fri, Jul 08, 2016 at 10:14:20PM +0800, ChangBo Guo wrote: > > Hi ALL, > > > > I have been working a bug [1] about option value interpolation in > > oslo.config[2], in short, option value interpolation can't handle password > > containing special characters from environment variable and I proposed a > > fix of provide way to forbid option value interpolation explicitly[3]. > > > > copy of Doug Hellmann's coments: > > > > "The problem is that the end user who is setting the value of the option > > cannot control whether the option will do interpolation or not. So the > > programmer who defines the option has to make that choice, and then we > > can't change it because that would break existing deployments. The result > > is that end users won't know for any given option whether interpolation > > works or not, and if not why (did they do something wrong, or is it not > > supported). > > > > I've set -2 on this patch because I think it's a bad approach. I see 2 > > other ways we could solve the problem you describe (and I agree that it's > > an issue we should help with). > > > > 1. We could have an option that turns off interpolation globally, and let > > the user control that by setting the flag in their configuration file. I'm > > not sure I like this, but it does give you what you're looking for at the > > risk of breaking applications that are relying on interpolation, like the > > nova example. > > > > 2. We could disable interpolation when we get values from environment > > variables. That would be a big behavioral change, so we would need to think > > about how to roll it out carefully. For example, do we provide a helper > > function to give to application developers who are setting default values > > to environment variables so the variable value can be escaped to avoid > > interpolation? Or do we build it into the Opt class somehow? I think I like > > the helper function approach but we should give it more thought." > > I would like to collect more suggestions to decide the direction to fix > > similar bug. Any thoughts ? > > I don't see a compelling need to change oslo behaviour wrt interpolation > at all, given all the option suggested here break copatibility in some > manner or another. > > The curernt behaviour should just have its error reporting fixed, so that > it explicitly tells the user that the env var it tried to interpolate > contains invalid characters, instead of printing the incomprehensible > stack trace. > > > Regards, > Daniel Sure, the bug should be fixed. I was also trying to find a way to address the usability issue ChangBo pointed out in the commit message, which is that defaults being pulled from environment variables need to be treated carefully. In some cases the values should be escaped but in others they shouldn't, and the same environment variable is being used in all cases. Doug __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all][oslo] Disable option value interpolation in oslo.config
Excerpts from Ben Nemec's message of 2016-07-08 10:24:54 -0500:
> On 07/08/2016 09:14 AM, ChangBo Guo wrote:
> > Hi ALL,
> >
> > I have been working a bug [1] about option value interpolation in
> > oslo.config[2], in short, option value interpolation can't handle
> > password containing special characters from environment variable and I
> > proposed a fix of provide way to forbid option value interpolation
> > explicitly[3].
> >
> > copy of Doug Hellmann's coments:
> >
> > "The problem is that the end user who is setting the value of the option
> > cannot control whether the option will do interpolation or not. So the
> > programmer who defines the option has to make that choice, and then we
> > can't change it because that would break existing deployments. The
> > result is that end users won't know for any given option whether
> > interpolation works or not, and if not why (did they do something wrong,
> > or is it not supported).
> >
> > I've set -2 on this patch because I think it's a bad approach. I see 2
> > other ways we could solve the problem you describe (and I agree that
> > it's an issue we should help with).
> >
> > 1. We could have an option that turns off interpolation globally, and
> > let the user control that by setting the flag in their configuration
> > file. I'm not sure I like this, but it does give you what you're looking
> > for at the risk of breaking applications that are relying on
> > interpolation, like the nova example.
>
> This feels like a big hammer for a relatively small edge case, and as
> you note it doesn't handle the case where you want to use interpolation,
> but have an environment variable default that breaks when interpolated.
>
> >
> > 2. We could disable interpolation when we get values from environment
> > variables. That would be a big behavioral change, so we would need to
> > think about how to roll it out carefully. For example, do we provide a
> > helper function to give to application developers who are setting
> > default values to environment variables so the variable value can be
> > escaped to avoid interpolation? Or do we build it into the Opt class
> > somehow? I think I like the helper function approach but we should give
> > it more thought."
>
> One possibility would be an "env_default" (or something) parameter to
> Opt that would read the env var, escape it as necessary, and set the opt
> default to the escaped value. Then we could recommend that people stop
> using os.environ for setting defaults this way. Using default and
> env_default at the same time would raise an exception.
That sounds a lot like my helper function example, but builds the
concept of taking a default from an environment variable into Opt.
>
> So the shaker example would look like:
>
> cfg.StrOpt('os-password', metavar='',
>env_default='OS_PASSWORD',
>sample_default='',
>help='Authentication password, defaults to env[OS_PASSWORD].')
>
> It does require that developers DTRT when pulling defaults from the env,
> but IMHO that's better than a backwards incompatible change or requiring
> that users DTRT, which seem to be the alternatives here.
>
> I suppose it would break anyone who happened to be using interpolation
> of a default from the env, but I feel like that's unlikely and a much
> less common case than "my password has a $ character in it".
That's also my assumption. I think this has come up before, especially
since a lot of the password generators out there like to use characters
the parser considers "special."
Doug
>
> >
> > I would like to collect more suggestions to decide the direction to fix
> > similar bug. Any thoughts ?
> >
> > [1]https://bugs.launchpad.net/oslo.config/+bug/1577731
> > [2]http://docs.openstack.org/developer/oslo.config/cfg.html#option-value-interpolation
> > [3]https://review.openstack.org/#/c/338668/
> >
> >
> > --
> > ChangBo Guo(gcb)
> >
> >
> > __
> > OpenStack Development Mailing List (not for usage questions)
> > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >
>
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all][oslo] Disable option value interpolation in oslo.config
On 07/08/2016 10:58 AM, Doug Hellmann wrote:
> Excerpts from Ben Nemec's message of 2016-07-08 10:24:54 -0500:
>> On 07/08/2016 09:14 AM, ChangBo Guo wrote:
>>> Hi ALL,
>>>
>>> I have been working a bug [1] about option value interpolation in
>>> oslo.config[2], in short, option value interpolation can't handle
>>> password containing special characters from environment variable and I
>>> proposed a fix of provide way to forbid option value interpolation
>>> explicitly[3].
>>>
>>> copy of Doug Hellmann's coments:
>>>
>>> "The problem is that the end user who is setting the value of the option
>>> cannot control whether the option will do interpolation or not. So the
>>> programmer who defines the option has to make that choice, and then we
>>> can't change it because that would break existing deployments. The
>>> result is that end users won't know for any given option whether
>>> interpolation works or not, and if not why (did they do something wrong,
>>> or is it not supported).
>>>
>>> I've set -2 on this patch because I think it's a bad approach. I see 2
>>> other ways we could solve the problem you describe (and I agree that
>>> it's an issue we should help with).
>>>
>>> 1. We could have an option that turns off interpolation globally, and
>>> let the user control that by setting the flag in their configuration
>>> file. I'm not sure I like this, but it does give you what you're looking
>>> for at the risk of breaking applications that are relying on
>>> interpolation, like the nova example.
>>
>> This feels like a big hammer for a relatively small edge case, and as
>> you note it doesn't handle the case where you want to use interpolation,
>> but have an environment variable default that breaks when interpolated.
>>
>>>
>>> 2. We could disable interpolation when we get values from environment
>>> variables. That would be a big behavioral change, so we would need to
>>> think about how to roll it out carefully. For example, do we provide a
>>> helper function to give to application developers who are setting
>>> default values to environment variables so the variable value can be
>>> escaped to avoid interpolation? Or do we build it into the Opt class
>>> somehow? I think I like the helper function approach but we should give
>>> it more thought."
>>
>> One possibility would be an "env_default" (or something) parameter to
>> Opt that would read the env var, escape it as necessary, and set the opt
>> default to the escaped value. Then we could recommend that people stop
>> using os.environ for setting defaults this way. Using default and
>> env_default at the same time would raise an exception.
>
> That sounds a lot like my helper function example, but builds the
> concept of taking a default from an environment variable into Opt.
Yeah, I was mostly +1'ing that suggestion and then brainstorming how to
integrate it into the opt definition. This seemed like the simplest way
to do it, but the benefit of a separate helper function would be if
there are other places we might want to do the same thing besides just
the Opt constructor. I could see someone wanting to set a None default
and then conditionally set the default to an environment variable in
their code, so maybe standalone would be better.
>
>>
>> So the shaker example would look like:
>>
>> cfg.StrOpt('os-password', metavar='',
>>env_default='OS_PASSWORD',
>>sample_default='',
>>help='Authentication password, defaults to env[OS_PASSWORD].')
>>
>> It does require that developers DTRT when pulling defaults from the env,
>> but IMHO that's better than a backwards incompatible change or requiring
>> that users DTRT, which seem to be the alternatives here.
>>
>> I suppose it would break anyone who happened to be using interpolation
>> of a default from the env, but I feel like that's unlikely and a much
>> less common case than "my password has a $ character in it".
>
> That's also my assumption. I think this has come up before, especially
> since a lot of the password generators out there like to use characters
> the parser considers "special."
>
> Doug
>
>>
>>>
>>> I would like to collect more suggestions to decide the direction to fix
>>> similar bug. Any thoughts ?
>>>
>>> [1]https://bugs.launchpad.net/oslo.config/+bug/1577731
>>> [2]http://docs.openstack.org/developer/oslo.config/cfg.html#option-value-interpolation
>>> [3]https://review.openstack.org/#/c/338668/
>>>
>>>
>>> --
>>> ChangBo Guo(gcb)
>>>
>>>
>>> __
>>> OpenStack Development Mailing List (not for usage questions)
>>> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?s
Re: [openstack-dev] [all][oslo] Disable option value interpolation in oslo.config
Excerpts from Ben Nemec's message of 2016-07-08 13:27:04 -0500: > On 07/08/2016 10:58 AM, Doug Hellmann wrote: > > Excerpts from Ben Nemec's message of 2016-07-08 10:24:54 -0500: > >> On 07/08/2016 09:14 AM, ChangBo Guo wrote: > >>> Hi ALL, > >>> > >>> I have been working a bug [1] about option value interpolation in > >>> oslo.config[2], in short, option value interpolation can't handle > >>> password containing special characters from environment variable and I > >>> proposed a fix of provide way to forbid option value interpolation > >>> explicitly[3]. > >>> > >>> copy of Doug Hellmann's coments: > >>> > >>> "The problem is that the end user who is setting the value of the option > >>> cannot control whether the option will do interpolation or not. So the > >>> programmer who defines the option has to make that choice, and then we > >>> can't change it because that would break existing deployments. The > >>> result is that end users won't know for any given option whether > >>> interpolation works or not, and if not why (did they do something wrong, > >>> or is it not supported). > >>> > >>> I've set -2 on this patch because I think it's a bad approach. I see 2 > >>> other ways we could solve the problem you describe (and I agree that > >>> it's an issue we should help with). > >>> > >>> 1. We could have an option that turns off interpolation globally, and > >>> let the user control that by setting the flag in their configuration > >>> file. I'm not sure I like this, but it does give you what you're looking > >>> for at the risk of breaking applications that are relying on > >>> interpolation, like the nova example. > >> > >> This feels like a big hammer for a relatively small edge case, and as > >> you note it doesn't handle the case where you want to use interpolation, > >> but have an environment variable default that breaks when interpolated. > >> > >>> > >>> 2. We could disable interpolation when we get values from environment > >>> variables. That would be a big behavioral change, so we would need to > >>> think about how to roll it out carefully. For example, do we provide a > >>> helper function to give to application developers who are setting > >>> default values to environment variables so the variable value can be > >>> escaped to avoid interpolation? Or do we build it into the Opt class > >>> somehow? I think I like the helper function approach but we should give > >>> it more thought." > >> > >> One possibility would be an "env_default" (or something) parameter to > >> Opt that would read the env var, escape it as necessary, and set the opt > >> default to the escaped value. Then we could recommend that people stop > >> using os.environ for setting defaults this way. Using default and > >> env_default at the same time would raise an exception. > > > > That sounds a lot like my helper function example, but builds the > > concept of taking a default from an environment variable into Opt. > > Yeah, I was mostly +1'ing that suggestion and then brainstorming how to > integrate it into the opt definition. This seemed like the simplest way > to do it, but the benefit of a separate helper function would be if > there are other places we might want to do the same thing besides just > the Opt constructor. I could see someone wanting to set a None default > and then conditionally set the default to an environment variable in > their code, so maybe standalone would be better. On the other hand, if we build "get a default from an environment variable" into Opt, we can include that information in the help and sample config output. If we make the escaping function public, and then also use it to implement the "environment_variable" behavior in Opt, that would give us the best of both solutions. Doug __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
