Commit bf6b2ef3 authored by Kirill Smelkov's avatar Kirill Smelkov

X restore promises

After escaping strings with dumps (to avoid buildout breakage and code
injection on "tricky" references, e.g. with spaces), promises started to be
generated with !py! prefix in their filenames, e.g. as

    '!py!'\''RU1-sdr-busy.py'\'''

instead of just

    RU1-sdr-busy.py

The issue here is that our code set

    name = !py!'RU1-sdr-busy.py'

and then monitor-promise-base from stack/monitor does

    output = ${directory:plugins}/${:name}

which, I though, would be expanded to .../etc/plugin/RU1-sdr-busy.py but it did
not deserialized the :name upon expansion.

I feel like it is maybe a bug in nexedi/slapos.buildout@4e13dcb9 , but I'm not sure.

Anyway, workaround the problem by generating :output ourselve also via escaped way.
parent ad2199b1
...@@ -16,7 +16,7 @@ ...@@ -16,7 +16,7 @@
[template] [template]
filename = instance.cfg filename = instance.cfg
md5sum = 9ec3fde7ef51752fd393da51e9f4be51 md5sum = a3527093ffc01d8f8fbaf27696ad7b9b
[template-ors] [template-ors]
filename = instance-ors.cfg filename = instance-ors.cfg
...@@ -28,7 +28,7 @@ md5sum = 55d9182051ce13d07f1bc8e019173ae5 ...@@ -28,7 +28,7 @@ md5sum = 55d9182051ce13d07f1bc8e019173ae5
[ru_libinstance.jinja2.cfg] [ru_libinstance.jinja2.cfg]
_update_hash_filename_ = ru/libinstance.jinja2.cfg _update_hash_filename_ = ru/libinstance.jinja2.cfg
md5sum = 415d002d8f89e11194de24223edb3b02 md5sum = 21b7f5611c20dc6f29e4e1e456493234
[ru_sdr_libinstance.jinja2.cfg] [ru_sdr_libinstance.jinja2.cfg]
_update_hash_filename_ = ru/sdr/libinstance.jinja2.cfg _update_hash_filename_ = ru/sdr/libinstance.jinja2.cfg
......
...@@ -34,6 +34,7 @@ context = ...@@ -34,6 +34,7 @@ context =
key eggs_directory buildout:eggs-directory key eggs_directory buildout:eggs-directory
key develop_eggs_directory buildout:develop-eggs-directory key develop_eggs_directory buildout:develop-eggs-directory
raw buildout_directory ${buildout:directory} raw buildout_directory ${buildout:directory}
section directory directory
raw pythonwitheggs ${pythonwitheggs:exe} raw pythonwitheggs ${pythonwitheggs:exe}
section slap_connection slap-connection section slap_connection slap-connection
key slapparameter_dict slap-configuration:configuration key slapparameter_dict slap-configuration:configuration
......
...@@ -42,6 +42,7 @@ ...@@ -42,6 +42,7 @@
{{ part('promise-'+name) }} {{ part('promise-'+name) }}
<= monitor-promise-base <= monitor-promise-base
name = {{ dumps('%s.py' % pretty_name) }} name = {{ dumps('%s.py' % pretty_name) }}
output = {{ dumps('%s/plugin/%s.py' % (directory.etc, pretty_name)) }}
config-testing = {{ testing }} config-testing = {{ testing }}
config-stats-period = {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }} config-stats-period = {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }}
{%- endmacro %} {%- endmacro %}
......
  • The behaviour you describe is known to me: the magic prefix is only checked at the beginning of value.

    It has been long enough that I worked on that, and my memory of the details is nearly gone. I do not remember if it was intended as a feature (ex: "if you protect part of a value against injection, then why not protect it all ?"), or if it was only a limitation of how buildout files are interpreted: I think the variable expansion is handled differently (in standard python library's ConfigParser maybe ?) from where the deserialiser could be called (buildout's own code).

    In any case, the fix you propose here is exactly the intended way around this limitation: concatenate first, serialise second.

  • Vincent, thanks for feedback.

    I though that upon seeing name = !py!'RU1-...' buildout initializes options with name in internal option dict with deserialized value, and then when seeing output = ...${:name} it retrieves name from that internal dict, not in raw source form used in input. Then concatenation and other operations would work as expected. In other words in my idea using output = ...${:name} is different from output = ...!py!'RU1-...' because in the second version the magic is indeed coming not in the head and automatic deserialization is not activated.

    In any way I do not insist on my understanding and there is no urgency with fixing anything if at all. As we all see there is a way to workaround the problem.

  • I believe the reason the internal state does not contain the raw values is that it would require extra care when annotating and/or writing out the .installed.cfg result: serialisation would then be required at this point. But again, my memory of this is quite vague now, so I may be reinventing the whole thing rather than actually remembering :) .

  • mentioned in commit kirr/slapos@64092de9

    Toggle commit list
  • mentioned in commit kirr/slapos@9f33419d

    Toggle commit list
  • mentioned in commit kirr/slapos@c83fa3cb

    Toggle commit list
  • @vpelletier, thanks for all the feedback back then. I do not have much to add to the discussion and I did not investigate whether .installed.cfg is emitted one way or another. It is a bit of shame, but I'm really tired now to check. Just for the reference this topic, and related topic of generating "safe for buildout section name" is also covered in kirr/slapos@c83fa3cb that is part of !1533 (merged).

Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment