fog / fog-openstack

Fog for OpenStack Platform
MIT License
68 stars 130 forks source link

Fix potential issue with frozen strings with openstack_endpoint_type #494

Closed ramereth closed 5 years ago

ramereth commented 5 years ago

It seems when using fog-openstack-1.x with Chef [1] , @openstack_endpoint_type becomes a frozen string and fails with FrozenError. Unfortunately, I haven't been able to reproduce this issue in unit tests but I know for a fact that this fixes the issue. Looking through the code base, this is the only place where a gsub! is used in this manner.

Further debugging shows this stack trace which lead me to this:

DEBUG: FrozenError: openstack_role[service] (openstack-identity::registration line 70) had an error: FrozenError: can't modify frozen String
/opt/chef/embedded/lib/ruby/gems/2.5.0/gems/fog-openstack-1.0.8/lib/fog/openstack/core.rb:200:in `gsub!'
/opt/chef/embedded/lib/ruby/gems/2.5.0/gems/fog-openstack-1.0.8/lib/fog/openstack/core.rb:200:in `setup'
/opt/chef/embedded/lib/ruby/gems/2.5.0/gems/fog-openstack-1.0.8/lib/fog/openstack/core.rb:43:in `initialize'
/opt/chef/embedded/lib/ruby/gems/2.5.0/gems/fog-core-2.1.2/lib/fog/core/service.rb:115:in `new'
/opt/chef/embedded/lib/ruby/gems/2.5.0/gems/fog-core-2.1.2/lib/fog/core/service.rb:115:in `new'
/tmp/kitchen/cache/cookbooks/openstackclient/libraries/openstack_base.rb:22:in `connection'
/tmp/kitchen/cache/cookbooks/openstackclient/libraries/openstack_role.rb:28:in `block in <class:OpenstackRole>' (eval):2:in `block in action_create'

[1] https://review.opendev.org/665827

theopenlab-ci[bot] commented 5 years ago

Build succeeded.

ramereth commented 5 years ago

Please review @dhague, @Ladas, @seanhandley, @mdarby and @jjasghar

seanhandley commented 5 years ago

Hi @ramereth

Thanks for the PR! I'd like to understand a little more about this issue.

First of all, how is the string becoming frozen? Is something in chef freezing it?

Second, I don't quite see how your code change would fix the offending behaviour. Can you help me understand why this change helps?

ramereth commented 5 years ago

@seanhandley I'm not entirely sure what's causing the string to become frozen but I did some debugging and did confirm that when I run it within chef as a library, it returns as a frozen string. I could consult with some other Chef folks if you would like.

Regarding how this code changes the offending behavior, Using gsub! means the string variable gets changed in place which is a problem if the variable is frozen. To work around that, I use gsub instead which returns the changed string and I simply just copy the result back into the same variable.