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

handle semodule version >=2.4 (#37732) and fix typo #37736

Merged
merged 4 commits into from Nov 24, 2016
Merged

handle semodule version >=2.4 (#37732) and fix typo #37736

merged 4 commits into from Nov 24, 2016

Conversation

dhaines
Copy link

@dhaines dhaines commented Nov 16, 2016

What does this PR do?

Checks for more recent versions of semodule (v2.4+) and runs semodule differently depending on what side of the version line it's on, storing different results in the dictionary

What issues does this PR fix or reference?

#37732

Previous Behavior

errors on any state.apply of a selinux.module state

New Behavior

no more errors, but no more selinux module versioning for newer versions of the selinux userspace (but that's upstream's choice)

Tests written?

No

Please review Salt's Contributing Guide for best practices.

David Haines and others added 3 commits November 16, 2016 14:47
missed a pair of parens
both the existing code and my addition erroneously strip the first result from the semodule call
'Version': comps[1]}
semodule_versioncheck = __salt__['cmd.run']('semodule -h').splitlines()
semodule_version = ''
for line in versioncheck[1:]:
Copy link
Contributor

Choose a reason for hiding this comment

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

semodule_versioncheck? This looks like it's undefined.

if semodule_version == 'new':
mdata = __salt__['cmd.run']('semodule -lfull').splitlines()
ret = {}
for line in mdata[0:]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reasoning for splicing this as you've done? for line in mdata: should be fine.

Copy link
Author

Choose a reason for hiding this comment

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

I was merely parroting what was there before (well, it was mdata[1:] for some reason, but I digress).

else:
mdata = __salt__['cmd.run']('semodule -l').splitlines()
ret = {}
for line in mdata[0:]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question about the splice here.

Copy link
Author

Choose a reason for hiding this comment

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

Same as above.

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Nov 17, 2016
Fixed (hopefully) a couple of issues with the commit. That's what I get for parroting (and for not really knowing Python syntax yet).
@dhaines
Copy link
Author

dhaines commented Nov 23, 2016

Do my fixes address your concerns?

@cachedout
Copy link
Contributor

@dhaines Looks great. Thanks so much!

@cachedout cachedout merged commit 371b0a8 into saltstack:2016.3 Nov 24, 2016
@dhaines dhaines deleted the issue-37732 branch November 24, 2016 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants