From 55d062e15f67f50f5bb15eb4a233851295b875da Mon Sep 17 00:00:00 2001 From: Jude N Date: Tue, 4 Jun 2019 08:15:23 -0400 Subject: [PATCH] Checkpoint commit / importlib-inspect approach --- .gitignore | 1 + README | 6 +- bin/salty-linter | 22 +--- example_sls/requires.sls | 31 ++++- example_sls/two-items.sls | 2 +- requirements.txt | 4 +- salty_linter/__init__.py | 151 ++++++++++++----------- salty_linter/data/salty_linter.yaml | 172 --------------------------- salty_linter/data/salty_linter.yaml~ | 66 ---------- 9 files changed, 115 insertions(+), 340 deletions(-) delete mode 100644 salty_linter/data/salty_linter.yaml delete mode 100644 salty_linter/data/salty_linter.yaml~ diff --git a/.gitignore b/.gitignore index ce0ad42..ca5e2a8 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ env MANIFEST *.egg-info *~ +__pycache__ diff --git a/README b/README index 6958949..c58f2f5 100644 --- a/README +++ b/README @@ -5,7 +5,7 @@ TODO: - some parameters take either a string or a list blah: - ,: + .: - : value - : @@ -49,8 +49,10 @@ blah: ===================================== Development Notes -> virtualenv --python=python3.5 env +> virtualenv --python=python3.6 env > source ./env/bin/activate +> pip3 install -r requirements.txt +> python3 setup.py develop Build the tar.gz > python setup.py sdist diff --git a/bin/salty-linter b/bin/salty-linter index c06aa1f..0784311 100755 --- a/bin/salty-linter +++ b/bin/salty-linter @@ -6,7 +6,6 @@ import os.path import pkg_resources import pprint import sys -import yaml from ruamel.yaml import YAML import salty_linter @@ -20,24 +19,12 @@ argparser = argparse.ArgumentParser(description="salty-linter. Let's lint some s epilog='Now let\'s get some salt states in ship shop shape !') argparser.add_argument('--verbose', dest='verbose', action='store_true', help="Let's be super chatty and generate a lot of output.") argparser.add_argument('--no-werror', dest='werror', action='store_false', help="Don't treat warnings as errors") -argparser.add_argument('--typedb', nargs='?', default=default_types, type=argparse.FileType('r'), help="alternative type database.") argparser.add_argument('sls', nargs='+', help="list of salt state files") argparser.set_defaults(verbose=False, werror=True) args = argparser.parse_args() -pp.pprint(args) - if args.verbose: - print("HI I'M SALTY_LINTER. LETS GET CHATTY.") - -# Open the type database file and read in the type database -# Error out of that file doesn't parse / doesn't exist / is unreadab -typedb = yaml.load(args.typedb) - -if args.verbose: - print("HERE'S YOUR TYPE DATABSE. WALLOW IN IT.") - pp.pprint(typedb) - print("WRAP UP YOUR TYPE DATABASE WALLOWING.") + print("HELLO EVERYONE I'M SALTY-LINTER AND I'M VERBOSE.") exit_status = 0 for an_sls_filename in args.sls: @@ -55,9 +42,9 @@ for an_sls_filename in args.sls: continue with open(fullpath_sls_filename) as fp: - yaml = YAML(typ='rt') + yaml = YAML(typ='jinja2') sls_yaml = yaml.load(fp) - (lint_errors, lint_warnings) = salty_linter.lint(sls_yaml, typedb) + (lint_errors, lint_warnings) = salty_linter.lint(sls_yaml) if lint_errors or (lint_warnings and args.werror): exit_status = 1 for (line_number, an_error) in lint_errors: @@ -67,7 +54,6 @@ for an_sls_filename in args.sls: if exit_status == 0: print("NO OBVIOUS FAILURES.") -else: - print("MORE SHIP-SHOPPING FOR YOU !") + sys.exit(exit_status) diff --git a/example_sls/requires.sls b/example_sls/requires.sls index 5c69090..a2bf454 100644 --- a/example_sls/requires.sls +++ b/example_sls/requires.sls @@ -1,14 +1,43 @@ # I WROTE THIS LINTER AFTER RUNNING INTO THIS PROBLEM MULTIPLE TIME - require_not_requires: file.managed: - contents: "it's require not requires" - requires: - cmd: "/bin/echo OOPS I DID IT AGAIN" +# Will this work for some state I never played with ? +some_state_i_never_used: + zpool.absent: + - name: ok + - export: true + - force: false + +some_state_i_never_used_with_a_bad_function_name: + zpool.shivermetimbers: + - name: me timbers rrr so shivery + +some_state_i_never_used_with_a_bad_parameter: + zpool.present: + - nope: thats nopesir to you multiball: pkg: - installed service: - running + +# skip over the jinja2 content +{% set ignore_me_please = "PLEASE" %} + +duplicates: + pkg.installed: + - name: duplicate + - name: duplicate + +multiline: + file.managed: + - name: multiline + - contents: | + This is a + multiline file + - mode: 640 diff --git a/example_sls/two-items.sls b/example_sls/two-items.sls index 5584dd1..ebb15da 100644 --- a/example_sls/two-items.sls +++ b/example_sls/two-items.sls @@ -1,4 +1,4 @@ -# This file has two states +# This file has three states /etc/motd: file.managed: diff --git a/requirements.txt b/requirements.txt index 8bfbd06..7729865 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ -pyyaml -ruamel.yaml ruamel.yaml[jinja2] +ruamel.yaml +salt diff --git a/salty_linter/__init__.py b/salty_linter/__init__.py index a5cf233..11e70ca 100644 --- a/salty_linter/__init__.py +++ b/salty_linter/__init__.py @@ -1,10 +1,12 @@ -import pprint +import importlib +import inspect -def lint(sls_yaml, types): +from salt.state import STATE_INTERNAL_KEYWORDS + +def lint(sls_yaml): """ Check the sls_yaml for Salt errors / warnings - typedb : Nested dictionary of Salt states/functions/parameters (partial) sls_yaml: ruamel.yaml[jinja2].load()'d output for some Salt state file. Returns: (errors, warnings) @@ -12,67 +14,50 @@ def lint(sls_yaml, types): warnings: List of (line#, warnings) tuples """ - pp = pprint.PrettyPrinter(indent=4) - #pp.pprint(dir(sls_yaml)) - #pp.pprint(sls_yaml.__dict__) + module_cache = {} - - def lint_parameters(function_name, parameters, parameter_types): + def lint_parameters(state_name, function_name, actual_parameters, state_module): """ - Input: - function_name: String of the form "." currently being linted - parameters: Map of {paramter_name: parameter_data} items currently being linted - parameter_types: Map of {parameter_name: parameter_type} items - Output: - errors: List of error string - warnings: List of warning strings """ - errors = [] - warnings = [] - for parameter in parameters: - parameter_name = next(iter(parameter.keys())) - - ## parameter is either just a string or a {'parameter' : data} map, or always a map ? - - if parameter_name in types['globals']: - continue - elif parameter_name not in parameter_types: - s_warning = "Unexpected parameter name {} for {}" - warnings.append((parameter.lc.line + 1, s_warning.format(parameter_name, function_name))) - else: - ## TODO: Add that parameter.values()[0] matches the against parameter_types[parameter] type - pass - return (errors, warnings) + perrors = [] + pwarnings = [] + seen_parameters = [] - - def lint_function(state_name, function_calls, function_types): - """ - Input: - state_name: String of the state_name currently being linted - function_calls: Map of {function_name: parameters} items currently being linteds - function_types; Map of {function_name: parameter_types} items - Output: - errors: List of error string - warnings: List of warning strings - """ - errors = [] - warnings = [] - for function_name, parameters in function_calls.items(): - if function_name not in function_types: - warnings.append((parameters.lc.line+1, "Unexpected function name {}.{}.".format(state_name, function_name))) + try: + allowable_parameters = inspect.signature(vars(state_module)[function_name]).parameters + except Exception as e: + s_error = "Unexpected error looking up {}.{} parameters: {}." + perrors.append((1, s_error.format(state_name, function_name, e))) + return (perrors, pwarnings) + + for a_parameter in actual_parameters: + + # TODO: A comment explaining this line... + parameter_name = next(iter(a_parameter.keys())) + + # Accept any of the global state arguments + # (maybe ignore 'state' and 'fun' ??) + if parameter_name in STATE_INTERNAL_KEYWORDS: continue + elif parameter_name not in allowable_parameters: + s_warning = "Unexpected parameter '{}' for function '{}.{}'." + pwarnings.append((a_parameter.lc.line + 1, s_warning.format(parameter_name, state_name, function_name))) + + # If there are duplicate parameters in the state, it looks like salt will use the last seen parameter, + # but it looks like that's undefined behavior, but its more likely some copypasta error on your part. + if parameter_name in seen_parameters: + s_error = "Duplicate parameter '{}' for function '{}.{}'." + perrors.append((a_parameter.lc.line + 1, s_error.format(parameter_name, state_name, function_name))) else: - (parameter_errors, parameter_warnings) = lint_parameters(state_name + "." + function_name, parameters, function_types[function_name]) - errors += parameter_errors - warnings += parameter_warnings - return (errors, warnings) + seen_parameters.append(parameter_name) - ## ------------------------------ + return (perrors, pwarnings) - state_types = types['states'] + + ## ------------------------------ errors = [] - warnings = [] + warnings = [] for label, label_value in sls_yaml.items(): # It's possible the label_value is just a 'state.function'. See example_sls/two-items.sls @@ -80,60 +65,70 @@ def lint(sls_yaml, types): value = {label_value: []} else: value = label_value - + for state_function_name, function_calls_or_parameters in value.items(): # state_function_name should be 'state_name' or 'state_name.function_name' # sosf => 'state or state.function' - # = [state_name] or [state_name, function_name] + # = [state_name] or [state_name, function_name] sosf = state_function_name.split('.') - # Verify that sosf[0] is an expected state: + # Load the state module if needed. + # If it's already been loaded, use the cached version state_name = '' - if sosf[0] not in state_types: - warnings.append((value.lc.line + 1, "Unexpected state name {}.".format(sosf[0]))) - continue - else: + if sosf[0] in module_cache: state_name = sosf[0] + state_mod = module_cache[state_name] + else: + try: + state_mod = importlib.import_module('salt.states.' + sosf[0]) + except ModuleNotFoundError: + warnings.append((value.lc.line + 1, "Unexpected state name {}.".format(sosf[0]))) + continue + else: + state_name = sosf[0] + module_cache[state_name] = state_mod # "state" case # function_calls_or_parameters[0] will be the function name - # function_call_or_parameters[1:] will be the parameters + # function_calls_or_parameters[1:] will be the parameters if len(sosf) == 1: if not function_calls_or_parameters: errors.append(label_value.lc.line + 1, "Missing function name in parameter list for state {}.".format(state_name)) continue - + function_name = function_calls_or_parameters[0] - if function_name not in state_types[state_name]: - warnings.append((state_function_name.lc.line + 1, "Unexpected function name {} for {}.".format(function_name, state_name))) - continue - parameters = function_call_or_parameters[1:] - (function_errors, function_warnings) = lint_function(state_name + "." + function_name, parameters, state_types[state_name][function_name]) - errors += function_errors - warnings += function_warnings + parameters = function_calls_or_parameters[1:] + + if function_name not in vars(state_mod): + warnings.append((state_function_name.lc.line + 1, "Unexpected function name '{}' for state {}.".format(function_name, state_name))) + continue + + (perrors, pwarnings) = lint_parameters(state_name, function_name, parameters, state_mod) + errors += perrors + warnings += pwarnings # "state.function" case # function_calls_or_parameters will be a list of parameters elif len(sosf) == 2: function_name = sosf[1] - if function_name not in state_types[state_name]: + if function_name not in vars(state_mod): # TODO: This gives the line number at the top of the label block which isn't great. # It would be better to give the exact like number of the invalid function - warnings.append((sls_yaml.lc.key(label)[0] + 1, "Unexpected function name {} for {}.".format(function_name, state_name))) + warnings.append((sls_yaml.lc.key(label)[0] + 1, "Unexpected function name '{}' for state {}.".format(function_name, state_name))) continue - # Children should be parameters of "state.function" - (parameter_errors, parameter_warnings) = lint_parameters(state_name + "." + function_name, function_calls_or_parameters, state_types[state_name][function_name]) - errors += parameter_errors - warnings += parameter_warnings + parameters = function_calls_or_parameters + (perrors, pwarnings) = lint_parameters(state_name, function_name, parameters, state_mod) + errors += perrors + warnings += pwarnings # Anything other than "state" or "state.function" is an error. else: - errors.append((value.lc.line + 1, "Expected or .: Too many '.'s in {}".format(state_function_name))) + errors.append((value.lc.line + 1, "Expected or .: Too many '.'s in {}.".format(state_function_name))) continue return(errors, warnings) - + diff --git a/salty_linter/data/salty_linter.yaml b/salty_linter/data/salty_linter.yaml deleted file mode 100644 index d5a51f3..0000000 --- a/salty_linter/data/salty_linter.yaml +++ /dev/null @@ -1,172 +0,0 @@ ---- - -# Based on the 2018.3.2 salt release -# This is a partial state/function/parameter database based on the states I use -# Please submit merge requests for additional coverage - -type: - basic: - - boolean - - char - - dict - - list - - string - - solos # string_or_list_of_strings: String | [String] - template: - - cheetah - - genshi - - jinja - - mako - - py - - wempy - -# global state arguments -globals: - - check_cmd - - fire_event - - listen - - listen_in - - mod_run_check - - mod_run_check_cmd - - onchanges - - onchanges_any - - onchanges_in - - onfail - - onfail_any - - onfail_in - - onlyif - - prereq - - prereq_in - - require - - require_in - - require_any - - reload_grains - - reload_modules - - reload_pillar - - retry # additional parameter checking available here (attempts, until, interval, splay) - - runas - - unas_password - - unless - - use - - use_in - - watch - - watch_any - - watch_in - -# states: -# state-name: -# function-name: -# parameter: type -# ... -# ... -# ... -states: - file: - directory: - allow_symlink: boolean - backupname: string - children_only: boolean - clean: integer - dir_mode: string - exclude_pat: boolean - file_mode: string - follow_symlinks; boolean - force: boolean - group: string - makedirs: boolean - max_depth: integer - mode: string - name: string - recurse: list - user: string - win_owner: string - win_perms: dict - win_deny_perms: dict - win_inheritence: boolean - win_perms_reset: boolean - managed: - allow_empty: boolean - attrs: string - backup: string - check_cmd: string - contents: solos - contents_delimiter: char - contents_grains: string - contents_newline: boolean - contents_pillar: string - context: dict - create: boolean - defaults: dict - dir_mode: string - encoding: stirng - encoding_errors: string - follow_symlinks: boolean - group: string - keep_source: boolean - makedirs: boolean - mode: string - name: string - replace: boolean - skip_verify: boolean - show_changes: boolean - source: solos - source_hash: string - source_hash_name: string - template: template - tmp_ext: string - user: string - win_deny_perms: dict - win_inheritance: boolean - win_owner: string - win_perms: dict - win_perms_reset: boolean - missing: - name: string - touch: - atime: string - mtime: string - makedirs: boolean - name: string - - pkg: - installed: - allow_updates: boolean - cache_valid_time: string - fromrepo: string - hold: boolean - ignore_epoch: boolean - ignore_types: list - install_recommends: boolean - name: string - names: list - normalize: bool - only_upgrade: boolean - pkg_verify: bool - pkgs: list - refresh: boolean - report_reboot_exit_codes: boolean - resolve_capabilities: boolean - skip_suggestions: boolean - skip_verify: boolean - sources: list - update_holds: boolean - verify_options: list - version: string - - selinux: - mode: - name: string - - service: - enabled: - name: string - running: - enable: boolean - init_delay: integer - name: string - no_block: boolean - sig: string - unmask: boolean - unmask_runtime: boolean - - diff --git a/salty_linter/data/salty_linter.yaml~ b/salty_linter/data/salty_linter.yaml~ deleted file mode 100644 index 368dab8..0000000 --- a/salty_linter/data/salty_linter.yaml~ +++ /dev/null @@ -1,66 +0,0 @@ ---- - -# Based on the 2018.3.2 salt release -# This is a partial state/function/parameter database based on the states I use -# Please submit merge requests for additional coverage - -type: - basic: - - boolean - - char - - mos # map of strings - - string - - solos # string_or_list_of_strings: String | [String] - template: - - cheetah - - genshi - - jinja - - mako - - py - - wempy - -# states: -# state-name: -# function-name: -# parameter: type -# ... -# ... -# ... -states: - file: - managed: - allow_empty: boolean - attrs: string - backup: string - check_cmd: string - contents: solos - contents_delimiter: char - contents_grains: string - contents_newline: boolean - contents_pillar: string - context: mos - create: boolean - defaults: mos - dir_mode: string - encoding: stirng - encoding_errors: string - follow_symlinks: boolean - group: string - keep_source: boolean - makedirs: boolean - mode: string - name: string - replace: boolean - skip_verify: boolean - show_changes: boolean - source: solos - source_hash: string - source_hash_name: string - template: template - tmp_ext: string - user: string - win_deny_perms: mos - win_inheritance: boolean - win_owner: string - win_perms: mos - win_perms_reset: boolean -- 2.39.5