Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only process ADMX files when loading policies #56310

Merged
merged 4 commits into from
Mar 11, 2020

Conversation

twangboy
Copy link
Contributor

@twangboy twangboy commented Mar 5, 2020

What does this PR do?

Fixes an issue with processing non ADMX files in the Policy Definitions directory.

What issues does this PR fix or reference?

https://gitlab.com/saltstack/enterprise/lock/issues/3826

Previous Behavior

lgpo.get_policy_info would fail with the following stacktrace

  File "c:\salt\bin\site-packages\salt-3000-py3.5.egg\salt\utils\templates.py", line 392, in render_jinja_tmpl
    output = template.render(**decoded_context)
  File "c:\salt\bin\lib\site-packages\jinja2\environment.py",line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "c:\salt\bin\lib\site-packages\jinja2\environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "c:\salt\bin\lib\site-packages\jinja2\_compat.py", line 37, in reraise
    raise value.with_traceback(tb)
  File "<template>", line 16, in top-level template code
  File "c:\salt\bin\lib\site-packages\salt-3000-py3.5.egg\salt\modules\win_lgpo.py", line 7759, in get_policy_info
    adml_language=adml_language)
  File "c:\salt\bin\lib\site-packages\salt-3000-py3.5.egg\salt\modules\win_lgpo.py", line 7398, in _lookup_admin_template
    admx_policy_definitions = _get_policy_definitions(language=adml_language)
  Files "c:\salt\bin\lib\site-packages\salt-3000-py3.5.egg\salt\modules\win_lgpo.py", line 5105, in _get_policy_definitions
    _load_policy_definitions(path=path,language=language)
  File "c:\salt\bin\lib\site-packages\salt-3000-py3.5.egg\salt\modules\win_lgpo.py, line 5006, in _load_policy_definitions
    namespaces=namespaces)[0]

New Behavior

Non-ADMX files are skipped, no stacktrace

Tests written?

Oui

Commits signed with GPG?

Yes

@twangboy twangboy added the v3000.1 vulnerable version label Mar 5, 2020
@twangboy twangboy requested a review from a team as a code owner March 5, 2020 01:17
@ghost ghost requested a review from cmcmarrow March 5, 2020 01:17
Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just needs some test coverage.

@Ch3LL Ch3LL added the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Mar 11, 2020
@twangboy twangboy removed the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Mar 11, 2020
@twangboy
Copy link
Contributor Author

Added some tests

@codecov
Copy link

codecov bot commented Mar 11, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@2d78931). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #56310   +/-   ##
=========================================
  Coverage          ?   17.41%           
=========================================
  Files             ?     1234           
  Lines             ?   232648           
  Branches          ?    50745           
=========================================
  Hits              ?    40517           
  Misses            ?   189110           
  Partials          ?     3021
Flag Coverage Δ
#archlts 16.69% <0%> (?)
#centos7 24.42% <ø> (?)
#proxy 24.48% <ø> (?)
#py2 17.21% <0%> (?)
#py3 17.08% <0%> (?)
#runtests 17.41% <0%> (?)
#ubuntu1604 24.43% <ø> (?)
#zeromq 17.41% <0%> (?)
Impacted Files Coverage Δ
salt/modules/win_lgpo.py 6% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d78931...4bc5b05. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Mar 11, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@2d78931). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #56310   +/-   ##
=========================================
  Coverage          ?   17.41%           
=========================================
  Files             ?     1234           
  Lines             ?   232648           
  Branches          ?    50745           
=========================================
  Hits              ?    40517           
  Misses            ?   189110           
  Partials          ?     3021
Flag Coverage Δ
#archlts 16.69% <0%> (?)
#centos7 24.42% <ø> (?)
#proxy 24.48% <ø> (?)
#py2 17.21% <0%> (?)
#py3 17.08% <0%> (?)
#runtests 17.41% <0%> (?)
#ubuntu1604 24.43% <ø> (?)
#zeromq 17.41% <0%> (?)
Impacted Files Coverage Δ
salt/modules/win_lgpo.py 6% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d78931...4bc5b05. Read the comment docs.

@dwoz dwoz merged commit 19bb6aa into saltstack:master Mar 11, 2020
@twangboy twangboy deleted the fix_lgpo_admx branch May 26, 2020 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3000.1 vulnerable version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants