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

ignore_epoch needs to be passed through to version_cmp functions #34397

Closed
jaredhanson11 opened this issue Jun 30, 2016 · 7 comments
Closed

ignore_epoch needs to be passed through to version_cmp functions #34397

jaredhanson11 opened this issue Jun 30, 2016 · 7 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Execution-Module fixed-pls-verify fix is linked, bug author to confirm fix Platform Relates to OS, containers, platform-based utilities like FS, system based apps
Milestone

Comments

@jaredhanson11
Copy link

jaredhanson11 commented Jun 30, 2016

Description of Issue/Question

Not necessarily an issue or bug, but just as something I've noticed while working with pkg.version_cmp that you guys may or may not want to address. I've been working on a cve scan for HubbleStack, and sometimes the vulnerable package version number I grab from an outside source will not correspond exactly to the format given by pkg.list_pkgs. It can happen that local version number is 1:6.6p1-2ubuntu2.4 while the source of the cve's returns the vulnerable version number as 6.6p1-2ubuntu2.7, omitting the prefix 1:. This can cause unwanted behavior by the pkg.version_cmp command. I work around it by getting rid of all x: prefixes, but it would be nice to have pkg.version_cmp handle the discrepancy, or at least return None representing an error comparing the versions.

Steps to Reproduce Issue

ubuntu@:~$ sudo salt '*' pkg.version_cmp 1:6.6p1-2ubuntu2.4 6.6p1-2ubuntu2.7
myminion:
    1
ubuntu@:~$ sudo salt '*' pkg.version_cmp 1:6.6p1-2ubuntu2.4 1:6.6p1-2ubuntu2.7
myminion:
    -1

Versions Report

Salt Version:
           Salt: 2016.3.1
@Ch3LL
Copy link
Contributor

Ch3LL commented Jul 1, 2016

@jaredhanson11 thanks for the heads up.

ping @terminalmage looks like there is some problems here when using what i believe is an epoch number when comparing packages. Do you think this is a bug/issue?

@Ch3LL Ch3LL added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jul 1, 2016
@Ch3LL Ch3LL added this to the Blocked milestone Jul 1, 2016
@terminalmage
Copy link
Contributor

terminalmage commented Jul 1, 2016

@jaredhanson11 As of 2015.8.9 there is an ignore_epoch parameter that was added to some of the pkg states. I'm not really sure if this would suit your needs however, because you didn't provide an actual use case. The ignore_epoch param is only used in the state module, it isn't (and probably should be made) available to the version_cmp functions. So if you rely on pkg.version_cmp, this param won't help you right now, but it would for pkg states.

@jaredhanson11
Copy link
Author

jaredhanson11 commented Jul 2, 2016

@terminalmage I loop through all the local packages that are gathered with local_pkgs = __salt__['pkg.list_pkgs'](versions_as_list=True), and use __salt__['pkg.version_cmp'](local, vulnerable) to compare the local version with a vulnerable version from an external source, gathered from the api at vulners.com. The problem is that sometimes the version returned from vulners.com has the epoch, other times it doesn't, so it would be nice if pkg.version_cmp could handle the case of comparing versions where one version number has an epoch, while the other doesn't. Again, it's not necessarily a bug or issue, more just a heads up, because if you didn't know any better and just compared the versions without checking whether they match in terms of both having epoch's or not, you could introduce a bug that might not be so obvious to spot.

@terminalmage terminalmage changed the title pkg.version_cmp edge case ignore_epoch needs to be passed through to version_cmp functions Jul 5, 2016
@terminalmage terminalmage added Execution-Module Bug broken, incorrect, or confusing behavior Platform Relates to OS, containers, platform-based utilities like FS, system based apps TEAM Core and removed Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels Jul 5, 2016
@terminalmage terminalmage modified the milestones: C 7, Blocked Jul 5, 2016
@terminalmage
Copy link
Contributor

@jaredhanson11 OK, then I just need to adjust the code to pass through the ignore_epoch param to the cmp functions. I'll get that taken care of, and update this comment thread when done.

@terminalmage
Copy link
Contributor

@jaredhanson11 This is implemented in #34531.

@terminalmage terminalmage added the fixed-pls-verify fix is linked, bug author to confirm fix label Jul 7, 2016
@terminalmage
Copy link
Contributor

#34531 has been merged, closing.

@aabognah
Copy link
Contributor

just a heads up. If you update to 2016.3.2 and do not restart the minions this merge will cause and error message:

          ID: auth_packages
    Function: pkg.installed
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python2.7/site-packages/salt/state.py", line 1723, in call
                  else:
                File "/usr/lib/python2.7/site-packages/salt/loader.py", line 1650, in wrapper
                  with salt.utils.context.func_globals_inject(f, **inject_globals):
                File "/usr/lib/python2.7/site-packages/salt/states/pkg.py", line 1094, in installed
                  **kwargs)
                File "/usr/lib/python2.7/site-packages/salt/states/pkg.py", line 487, in _find_install_targets
                  ignore_epoch=ignore_epoch):
                File "/usr/lib/python2.7/site-packages/salt/states/pkg.py", line 160, in _fulfills_version_spec
                  ignore_epoch=ignore_epoch):
              TypeError: compare_versions() got an unexpected keyword argument 'ignore_epoch'
     Started: 18:04:15.821507
    Duration: 17.952 ms
     Changes:   

It goes away once you restart the minion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Execution-Module fixed-pls-verify fix is linked, bug author to confirm fix Platform Relates to OS, containers, platform-based utilities like FS, system based apps
Projects
None yet
Development

No branches or pull requests

4 participants