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.

Sign up for GitHub

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

Jump to bottom

[MODULES-9695] - Add support for signed-by in source entries #991

Merged
merged 1 commit into from
Jul 26, 2021
Merged

[MODULES-9695] - Add support for signed-by in source entries #991

merged 1 commit into from
Jul 26, 2021

Conversation

johanfleury
Copy link

This is a first step toward fixing MODULES-9695.

I refactored the option list as an hash, as per Antoine’s comment.

@johanfleury johanfleury requested a review from a team as a code owner July 16, 2021 16:00
@puppet-community-rangefinder
Copy link

apt::source is a type

Breaking changes to this file WILL impact these 327 modules (exact match):
Breaking changes to this file MAY impact these 87 modules (near match):

This module is declared in 234 of 576 indexed public Puppetfiles.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@johanfleury johanfleury force-pushed the feature/add-support-for-signed-by-in-source-entries branch 2 times, most recently from fbb6a25 to 0c35168 Compare July 16, 2021 16:26
@daianamezdrea
Copy link

Hi @johanfleury, is this still WIP ?

@johanfleury
Copy link
Author

Nope, I believe it’s ready to review.

Do you need me to test this in an actual code base (I didn’t have time this week-end, but I can do it today)?

@johanfleury
Copy link
Author

I tested on my own infra and it works as expected.

Just a few remarks regarding this PR:

  • I named the parameter $keyring and not $signed_by because I think it could later be used to manage the actual keyring file (see my comment on MODULES-9695).
  • I think, it could be a good idea instead of having every options as parameters, to have a single $options hash as there are a lot of options that this module doesn’t currently support.
  • Some options (signed-by included) allow multiple, comma separated, values (see the sources.list man page), it could be a good idea to allow passing $keyring as an Array instead of a single string.

What do you think?

@daianamezdrea
Copy link

Hi @johanfleury, it looks good for me, thank you for your contribution ! Cheers !

@daianamezdrea daianamezdrea changed the title Add support for signed-by in source entries [MODULES-9695] - Add support for signed-by in source entries Jul 26, 2021
@daianamezdrea
Copy link

I also looked at your comments from the PR and the ones from the ticket, I'm waiting for your new PR/PRs. Just let me know if you need help/review. Thank you again !

@daianamezdrea daianamezdrea merged commit 480d42b into puppetlabs:main Jul 26, 2021
@mpdude mpdude mentioned this pull request Jan 9, 2022
Merged
@verhoevenv verhoevenv mentioned this pull request Sep 20, 2022
Closed
@vegaaz vegaaz mentioned this pull request Jan 2, 2023
Open
@vegaaz
Copy link

vegaaz commented Jan 4, 2023 •
edited
Loading

This PR fixed the MODULES-9695 Issue.
Since the issue is still 'Open' some people have no idea this was fixxed.

Can someone with a tickets.puppetlabs.com-Account close the issue?
@daianamezdrea

@johanfleury
Copy link
Author

@vegaaz I don’t think this PR fixes MODULE-9695, it merely adds support for a new flag in sources.list files, but there’s still a lot of work to do to remove calls to apt-key. See my comment on the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kenyon kenyon kenyon approved these changes

@daianamezdrea daianamezdrea daianamezdrea approved these changes

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@johanfleury @daianamezdrea @vegaaz @kenyon