Passthrough-enable for nova.conf mustache template is not correct

Bug #1324598 reported by Andrea Rosa
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
tripleo
Fix Released
High
Andrea Rosa

Bug Description

This change https://review.openstack.org/88036 added the definition of a section in the nova.conf mustache template file to enable the passthrough settings.

The template as-is has two problems:
- the comment are not rendered at all. If there is comment in the passthrough configuration the resulting nova-conf file instead of reporting the comment it reports "repeats for each section "
- as all variables are HTML-escaped by default in a mustache template we want to use the triple braces to rendere unscaped HTML

here an example that shows what I described above:
JSON
{
  "nova": {
    "config": [
      {"section": "DEFAULT",
       "values": [
          {
            "comment": "this is a comment",
            "option": "snapshots_directory",
            "value": "/mnt/state/var/lib/nova/tmp"
          }
        ]
      }
    ]
  }
}

Output
 repeats for each section
[DEFAULT]
snapshots_directory=/mnt/state/var/lib/nova/tmp

Changed in tripleo:
assignee: nobody → Andrea Rosa (andrea-rosa-m)
tags: added: low-hanging-fruit
Ben Nemec (bnemec)
Changed in tripleo:
status: New → Triaged
importance: Undecided → High
description: updated
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to tripleo-image-elements (master)

Fix proposed to branch: master
Review: https://review.openstack.org/96711

Changed in tripleo:
status: Triaged → In Progress
Revision history for this message
Clint Byrum (clint-fewbar) wrote :

I cannot reproduce the bug stated.. I get this when running the given json through os-apply-config:

[DEFAULT]
snapshots_directory=/mnt/state/var/lib/nova/tmp

Changed in tripleo:
status: In Progress → Incomplete
Revision history for this message
Andrea Rosa (andrea-rosa-m) wrote :

Clint, thanks for the review.
You are right that the escape problem is solved by os-apply-config but the problem about comments which are not rendered is still there.
As you can see from your test the comment is not rendered at all.
I moved, in the fix, the comment before each option so as we can specify a comment at key level, that is what is done in other templates for other images, like for example for the keystone template (tripleo-image-elements/elements/keystone/os-apply-config/etc/keystone/keystone.conf)

If you agree I'd leave in the patch the triple braces as that doesn't harm and it seems to me more clear and more close to the mustache syntax.
Do you agree with me?

If so I'll amend the description of the bug.

Thanks
--
Andrea

Changed in tripleo:
status: Incomplete → In Progress
Revision history for this message
Clint Byrum (clint-fewbar) wrote :

OH! I didn't realize the comment was intended to be rendered! :-P

And agree, the triple braces seem to be the norm, so we'll leave that. Thanks for clarifying.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tripleo-image-elements (master)

Reviewed: https://review.openstack.org/96711
Committed: https://git.openstack.org/cgit/openstack/tripleo-image-elements/commit/?id=bc5f5cec5fec1686d0e031ef60c97267b0fe9139
Submitter: Jenkins
Branch: master

commit bc5f5cec5fec1686d0e031ef60c97267b0fe9139
Author: Andrea Rosa <email address hidden>
Date: Fri May 30 11:12:48 2014 +0100

    Fix the nova.conf mustache template

    The nova.conf mustache template is not correct in the section that enables the
    passthrough settings.
    The template has two problems:
    - comments are not rendered
    - as all variables are HTML-escaped by default in a mustache template we want
      to use triple braces to return unescaped HTML.

    This matches https://review.openstack.org/#/c/91625/3
    Closes-bug: #1324598

    Change-Id: Ib5831d1fd26e0f5b2d023d7f892420b776892ca1

Changed in tripleo:
status: In Progress → Fix Committed
Jay Dobies (jdob)
Changed in tripleo:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.