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

ansible galaxy signature verification #76961

Conversation

s-hertel
Copy link
Contributor

@s-hertel s-hertel commented Feb 7, 2022

SUMMARY

New PR for #76681 to see if that fixes the files view. See the original PR for reviews/comments/history.

Allow verifying the authenticity of the MANIFEST.json before installing and manually at any time.

The Galaxy server will be queried for signatures and a list of supplemental URLs can be provided in the requirements file:

collections:
  - name: ns.coll
    signatures:
      - https://.../test.asc
      - file:///.../test.asc

Related PRs: pulp/pulp_ansible#754, ansible/galaxy_ng#1076

Besides completion (now feature complete) + bikeshedding, needs:

  • Changelog
  • Documentation
  • Tests
ISSUE TYPE
  • Feature Pull Request

s-hertel and others added 30 commits January 7, 2022 16:05
…laxy collection install|verify'

Co-authored-by: Matt Martz <[email protected]>
… it around

Ensure keyring is defined for 'ansible-galaxy install -r ...'
Use frozen=True optimization for GPG error dataclasses

Fix using the same display as the caller

clean up some code

clean up the diff

figure out the sanity errors ...
Remove more unused imports

Remove unused variable

consolidate exception handling

iterate over a tuple of attributes instead
rename variable from rc to gpg_verification_rc

use global variable for the manifest filename

prepend _ for private attrs

move global to class attr
… to satisfy all requirements

Fix displaying errors for all provided signatures

Fix typo

Add type annotations

Use class docstrings for gpg error messages

Make ConcreteArtifactsManager.keyring a property

Move get_signature_from_source from gpg file to ConcreteArtifactsManager

Fix pep8

Update MultiGalaxyAPIProxy.get_signatures() to take a Candidate argument
Make the flag singular (--signature) and additive (store='append')
Fix checking if generator has items - no walrus operator for me

Add integration tests for user-provided signature sources

- Test installing collections with valid/invalid signature sources
- Test disabling GPG verification when installing collections
- Test verifying collections with valid/invalid signature sources

Add unit tests for galaxy server signatures
Remove side effect from running gpg command
- indicate which collection failed signature verification

- include gpg return code in message about failed signature verification

- give a general warning if the keyring doesn't exist and a collection has signatures and signature verification is not disabled

Don't write offline Galaxy info if there are no signatures
- test using ansible-galaxy role with collection-only option

- test installing roles + collections from a requirements file that contains collection signatures

- test error message when installing from requirements but providing signatures on the cli

- test skipping signature verification errors with --ignore-errors

- test general warning when keyring does not exist
Fixes 'ansible-galaxy collection install ... --keyring dir_doesnt_exist/file_doesnt_exist.kbx'
…vided by the Galaxy server

- Make the default keyring None
- Warn if the keyring is None but the Galaxy server provided signatures
- Error if the keyring is None but the user supplied signatures
- Error if the keyring is not None but is invalid
…ails and handle display info in the caller

Use with contextlib.suppress() instead of try-except; pass

Move get_signature_from_source from ConcreteArtifactsManager back to gpg file as standalone function

Fix MultiGalaxyAPIProxy to fetch signatures from the Candidate's Galaxy server

Use ArgumentSpecValidator to validate the offline Galaxy info schema instead of reinventing the wheel

Only validate the offline Galaxy info once (by adding an __init__ method to _ComputedReqKindsMixin)

Add the method construct_galaxy_info_path to _ComputedReqKindsMixin rather than duplicating logic

Write offline metadata for provenance regardless of signatures

Refactor
fix up docs
@ansibot ansibot added affects_2.13 core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels Feb 7, 2022
@s-hertel
Copy link
Contributor Author

s-hertel commented Feb 7, 2022

Closing since Github has resolved the issue with the original PR.

@s-hertel s-hertel closed this Feb 7, 2022
@s-hertel s-hertel deleted the moar_ansible_galaxy_signature_verification branch February 7, 2022 18:03
@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Feb 7, 2022
@ansible ansible locked and limited conversation to collaborators Feb 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.13 core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. feature This issue/PR relates to a feature request. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants