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

check the RPM signature of zypper pkg.download packages and report errors #33469

Merged

Conversation

meaksh
Copy link
Contributor

@meaksh meaksh commented May 24, 2016

What does this PR do?

  • Create new method in rpm module which allow you to check if the signature of a RPM file is correct or not:
# salt '*' lowpkg.checksum /var/cache/zypp/packages/SLE-12-SP1-x86_64-Pool/x86_64/nmap-6.46-1.72.x86_64.rpm
minionsles12sp1-suma3pg.vagrant.local:
    ----------
    /var/cache/zypp/packages/SLE-12-SP1-x86_64-Pool/x86_64/nmap-6.46-1.72.x86_64.rpm:
        True
  • Now zypper pkg.download checks if the packages are successfully downloaded and reports errors when some packages fail.

What issues does this PR fix or reference?

This PR fixes inconsistent behavior of zypper pkg.download as it currently doesn't report errors correctly and it doesn't check if the downloaded may be corrupted.

Before fix:

# salt '*' pkg.download nmap foo
minionsles12-suma3pg.vagrant.local:
    ----------
    nmap:
        ----------
        path:
            /var/cache/zypp/packages/SLE-12-x86_64-Pool/x86_64/nmap-6.46-1.72.x86_64.rpm
        repository-alias:
            SLE-12-x86_64-Pool
        repository-name:
            SLE-12-x86_64-Pool

After fix:

# salt '*' pkg.download nmap foo
minionsles12sp1-suma3pg.vagrant.local:
    ----------
    _error:
        The following package(s) failed to download: foo
    nmap:
        ----------
        path:
            /var/cache/zypp/packages/SLE-12-SP1-x86_64-Pool/x86_64/nmap-6.46-1.72.x86_64.rpm
        repository-alias:
            SLE-12-SP1-x86_64-Pool
        repository-name:
            SLE-12-SP1-x86_64-Pool

Tests written?

No

continue

if not __salt__['cmd.retcode'](["rpm", "-K", "--quiet", package_file], ignore_retcode=True, output_loglevel='trace', python_shell=False):
ret[package_file] = True
Copy link
Contributor

@isbm isbm May 24, 2016

Choose a reason for hiding this comment

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

I am not sure I like this part of re-assigning the same var. I would do this way:

for package_file in paths:
    ret[package_file] = (bool(__salt__['file.file_exists'](package_file)) and 
                        not __salt__['cmd.retcode'](["rpm", "-K", "--quiet", package_file], 
                                                    ignore_retcode=True, 
                                                    output_loglevel='trace', 
                                                    python_shell=False))

What you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I like your proposal, it doesn't break the flow as mine does.
I'm going to add it in a commit

@isbm
Copy link
Contributor

isbm commented May 24, 2016

@cachedout 🆗 in case you're wondering. 😉

@cachedout cachedout merged commit 9f56ab4 into saltstack:2015.8 May 24, 2016
@cachedout
Copy link
Contributor

@isbm I was going to ask! You beat me to it. ;]

Thanks for this @meaksh . Good work here.

@meaksh
Copy link
Contributor Author

meaksh commented May 25, 2016

Thank you @cachedout and @isbm for your help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants