Reference Validator for Heat Templates (in progress) #84

Tlacenka · 2016-07-21T15:02:09Z

This version should handle stacks containing template file together with additional parameters and environments.

During development, the program is tested on tripleo templates and associated environments.

Code improvements and characteristics

the code was restructured into multiple files for better readability
remains compatibility for both Python 2.x and 3.x
some optimizations were made (but there are more to come)
the main parameters are called the same as when creating stack using heat client

Added functionality

the script is now able to resolve multiple nested references across multiple files and some of the AutoScaling/ResourceGroup specialties (not all)
the script should be able to resolve hierarchical mapping that can use wildcards
additional parameters and environment parameters and defaults are inserted into the templates and taken into account
the script also validates resource dependencies
a new parameter -t (--print-tree) enables the user to print the tree structure of the templates connected to the root template

…hecking references with access path leading to different files

…ing complex references README.md: added requirement - validating references moved after loading all additional information

…ter effect

…L_Prop_Par) - object YAML_Prop_Par applied for property x parameter pairs - adjusted validating properties against parameters - created method for merging Parameter and Property object attributes (at the beginning, they are not connected)

…r better access

…g fixes

- solve SnmpdReadonlyUserPassword problem - create states so that more options can continue with the same next state

- if referred resource(s) exist(s), sets usage flag - if not, new invalid reference type implemented

…L_Enums.py, YAML_Hotfile.py - code structured into files - reference_validator: parses arguments, runs validator, returns output - YAML_HotValidator: contains corresponding class without subclasses - YAML_Hotfile: contains corresponding class without subclasses - YAML_Enums: all enumeration types (YAML_Types, YAML_colours, YAML_tree_info) - YAML_HotClasses: other classes previously in YAML_HotValidator (YAML_Env, YAML_Prop_Par, YAML_Reference, YAML_Resource)

- improved formatting - optimized code - removed attribute "children" from YAML_Hotfile (self.resources.child used instead), left at YAML_Env

…s, formatting

…ping - resolves hierarchical mapping, wildcards (*) - params_default from environment added to parameters where needed

… functionality - -e and -f parameter longer versions changed according to heat - added -P parameter for adding parameters and/or values to root templates (or only values?) - adding parameters from environments and prompt to root template implemented

…t_ are now based on FSM - get_param FSM in final stages - in order to enable mapping to be used multiple times, a deep copy with some shared mutable objects that are not modified (such as structure) is created - applying mappings is adjusted accordingly

…r changes - parameter with value added via environment or prompt that has no match in root template has a new error type - parameter default added via environment without match in the stack has a new error type - environemnt file error output fixed, improved

- duplicate messages fixed - checking matching parameters fixed

- tests structure improved, log files and diff files used - test nr. 6 finished, test nr. 7 to be added

- mapping is done using DFS, it goes through nodes and applies mapping repeatedly for specific resources

…expansion)

tomassedovic · 2016-10-14T13:24:47Z

Okay, this is cool! I like the tests. I've looked through the code once, but there's a lot of it and I didn't try to understand everything yet. So my comments now are from a superficial reading of it and I hope to do a more substantive read later on.

One thing that's unclear to me:

The run_tests.sh file seems to require sudo, but I don't see any reason why it should? Can we remove that?

I'm happy to merge this PR once that's been cleared up.

Other general comments:

I've tested this on the Mitaka tripleo-heat-templates (tag 2.1.0) and it seems to be working fine. There are some complaints about unused parameters, etc. that might indicate I'm not passing all the environment files in, but we can figure that out later.
This won't work with the Newton templates, because they've added Jinja2 templating. In addition, in Newton and onwards, the generated template may never even appear on the disk. We'll have to address that, but again: later. This is useful to pre-Newton templates.
I'm wondering whether we can reduce the size of the code and some of the functions. Like pulling out duplicated bits into functions, etc. You know the codebase better than I do so you tell me: can we make this smaller? (if we can't, oh well)

I'll write up a few inline comments as well, but those are mostly about the code style, etc. so feel free to ignore them if you disagree. And anyway, that's something we can do in subsequent patches, I don't want to block this pull request with it.

tomassedovic · 2016-10-14T13:25:59Z

reference_validator/README.md

@@ -1,29 +1,58 @@
 Reference Validator
 ===================

-<h1>WARNING: Work in progress, may return invalid results </h1>
+<h2> Introduction </h2>


Markdown has syntax for header levels. For h2 it's:

Introduction --------------

or:

## Introduction

tomassedovic · 2016-10-14T13:26:33Z

reference_validator/README.md

-<h2> Introduction </h2>
-
-This script goes through all HOT files associated with root template, taking mapped resources from environment files into account. It validates references and detects unused variables/instances in YAML files. It accepts the same basic parameters as heat (root template and environments).
+ - nyanbar (for Python 2.x)


nyanbar is optional. Can we make it clear here?

tomassedovic · 2016-10-14T13:27:09Z

reference_validator/README.md


 <h2> Usage </h2>

-    $ python[3] reference_validator.py -f <path/to/yaml/root template> -e <path/to/yaml/environment file>[<another/path/to/env/files>] [-p/--pretty-format] [-u/--unused] [-h/--help]
+    $ python[3] reference_validator.py -f <path/to/yaml/root template> -e <path/to/yaml/environment file> [<another/path/to/env/files>] [-p/--pretty-format] [-u/--print-unused] [-n/--nyan] [-h/--help] [-t/--print-tree]

 <h3> Parameters </h3>


h3 in markdown is:

### Parameters

(h4 is four hash marks, h5 is five, etc.)

tomassedovic · 2016-10-14T13:30:06Z

reference_validator/hotclasses.py

+        Each parameter and its corresponding property share one.
+    '''
+
+    def __init__(self, structure, isPar):


Could you describe what isPar means/is used for? It's not immediately clear to me reading this function. As in, when is it True and when False?

Also, Python function parameters and local names are usually in snake_case, so something like is_par would be more idiomatic.

tomassedovic · 2016-10-14T13:30:34Z

reference_validator/hotclasses.py

+
+    def clone(self):
+        ''' Create a deep copy of the object,
+            mutable objects such as structure are shared.


It's not a deep copy if parts of it are shared.

tomassedovic · 2016-10-14T13:38:35Z

reference_validator/hotfile.py

+            elif cur_state == enum.GetParamStates.PARAM_NAME:
+                if type(hierarchy) == str:
+                    # Resolve pseudo parameters
+                    if hierarchy in ['OS::stack_name', 'OS::stack_id', 'OS::project_id']:


This should probably be a tuple, too.

tomassedovic · 2016-10-14T13:51:47Z

reference_validator/hotvalidator.py

+                    print('')
+
+                # Unused parameters
+                if False in [x.used for x in node.params]:


This can be written as:

if not all(x.used for x in node.params):

which I think is a bit clearer.

tomassedovic · 2016-10-14T15:02:54Z

reference_validator/hotvalidator.py

+                    print('')
+
+                # Print hidden parameters (optional)
+                if (self.print_unused) and [True for x in node.params if x.hidden]:


I think this can be simplified into:

if self.print_unused and any(x.hidden for x in node.params):

tomassedovic · 2016-10-14T15:03:46Z

reference_validator/run_tests.sh

+
+# Load tests
+TEST01=("Test 1 - Parsing valid YAML file:" \
+        "sudo python reference_validator.py -f tests/test_files/01_root.yaml -u")


The sudo shouldn't be necessary here.

tomassedovic · 2016-10-14T15:04:53Z

reference_validator/tests/test_files/01_root.yaml

@@ -0,0 +1,23 @@
+#!/bin/bash


This line doesn't belong here. Same in all the other yaml test files.

…ments, in progress

…ges in progress

Katka Pilatova added 30 commits March 30, 2016 16:48

reference_validator.py: added printing children, comments

e311f3b

reference_validator.py: changed, added YAML_Type, created basis for c…

774d59f

…hecking references with access path leading to different files

reference_validator.py: added nyanbar (-n), created basis for validat…

3029c61

…ing complex references README.md: added requirement - validating references moved after loading all additional information

reference_validator.py: Added duration to nyanbar if selected for bet…

fb31c30

…ter effect

reference_validator: added method for running validator

95a844f

reference_validator: added option for printing structure (in progress)

35f1325

README: adjustment

422162c

reference_validator.py: improved tree structure printing

828ad37

reference_validator: bug fixes

ff359fb

reference_validator: get_ validation split into separate functions fo…

1fee017

…r better access

reference_validator: path hierarchy in progress, typografic fixes, bu…

92167ed

…g fixes

reference_validator: bug fixes

53cb4a6

reference_validator: validating hierarchy in progress

bbfb63e

reference_validator.py: resolving get_attr implemented

1241a1d

reference_validator.py: resolving get_param in progress

99b7b7c

reference_validator.py: resolving get_param, get_attr in progress

59d358f

reference_validator.py: get_attr and get_param in progress, bug fixes

d0d9cba

- solve SnmpdReadonlyUserPassword problem - create states so that more options can continue with the same next state

reference_validator.py: added checking dependencies

f178a1d

- if referred resource(s) exist(s), sets usage flag - if not, new invalid reference type implemented

YAML_Hotfile.py: compatibility fix

8b1a106

README, YAML_Hotfile, reference_validator: bug fixes, updated README

b4ec05e

README: minor fix

a099a21

README: improved formatting

745c73f

YAML_Enums, HotClasses, HotValidator, Hotfile: improvements, bug fixes

b2dbfda

- improved formatting - optimized code - removed attribute "children" from YAML_Hotfile (self.resources.child used instead), left at YAML_Env

YAML_HotValidator.py, YAML_Hotfile.py: removed comments

b67cde4

reference validator: --unused changed to --print-unused, optimization…

ac2760e

…s, formatting

YAML_HotValidator: fixed parameter default, added regexp resource map…

b4a3c2c

…ping - resolves hierarchical mapping, wildcards (*) - params_default from environment added to parameters where needed

Merge remote-tracking branch 'origin' into reference_validator

72922f2

Katka Pilatova added 23 commits September 30, 2016 14:55

hotfile, hotvalidator: minor changes

27173dd

hotclasses, hotfile, hotvalidator: added checks for missing structure

b58d3dd

test_files, run_tests: base for testing added, in progress

4169a7e

hotvalidator, test nr. 6: minor fixes

dc1129d

- duplicate messages fixed - checking matching parameters fixed

hotfile, test nr. 6: minor changes, expanded test case

6b659ad

test nr. 6 files: in progress

913aa10

hotvalidator, tests: improved test script, validator input

e47f98b

- tests structure improved, log files and diff files used - test nr. 6 finished, test nr. 7 to be added

test_logs: empty folder for logs

bbbe36a

enum, README: updated README and enum header content

748fde0

README: update

088dcd7

README: fix (hopefully)

25a72dd

test nr. 7, hotclasses, run_tests: test nr. 7 in progress, bug fix

29976ec

reference validator: stripped typographical changes

b9d2421

hotvalidator, test nr. 7: new mapping algorithm, bug fix in test files

adf9857

- mapping is done using DFS, it goes through nodes and applies mapping repeatedly for specific resources

run_tests: verbose option added (prints differences)

6020347

run_tests: coloured verbose output

6d685fd

test nr. 7: in progress, HOT structure fixed

2b0918c

hotfile, test nr. 7: bug fix

37d6abe

README, test nr. 7: README expanded, test expanded

e33cca6

enum, hotfile, hotvalidator: added new state, bug fixes

43daca8

tests: typographical fixes, test nr. 7 is complete (open for further …

7dda445

…expansion)

tomassedovic reviewed Oct 14, 2016

View reviewed changes

Katka Pilatova added 5 commits October 14, 2016 17:41

README, hotclasses, hotfile, hotvalidator: adjustments based on comments

23685de

testfiles, test diffs, run_tests.sh: added tests for Python 3, adjust…

222c03d

…ments, in progress

test_diffs: deleted original diff files

0e07b31

hotfile, test nr. 6: sorting error messages, removing duplicate messa…

d04c062

…ges in progress

hotfile, tests: output order in progress

ec5b8e2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reference Validator for Heat Templates (in progress) #84

Reference Validator for Heat Templates (in progress) #84

Tlacenka commented Jul 21, 2016

tomassedovic commented Oct 14, 2016

tomassedovic Oct 14, 2016

tomassedovic Oct 14, 2016

tomassedovic Oct 14, 2016

tomassedovic Oct 14, 2016

tomassedovic Oct 14, 2016

tomassedovic Oct 14, 2016

tomassedovic Oct 14, 2016

tomassedovic Oct 14, 2016

tomassedovic Oct 14, 2016

tomassedovic Oct 14, 2016

Reference Validator for Heat Templates (in progress) #84

Are you sure you want to change the base?

Reference Validator for Heat Templates (in progress) #84

Conversation

Tlacenka commented Jul 21, 2016

tomassedovic commented Oct 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment