Skip to content

fix(repositories): force aptkey if signed-by and allow aptkey #79

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

Conversation

didiermfb
Copy link
Contributor

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

#76

Describe the changes you're proposing

If key_url is defined and aptkey is not defined, force aptkey to false when signed-by is present in opts.
Use aptkey if defined in any case.
Allow to define keyring directory and clean this directory (cleaning is set to false by default)

Pillar / config required to test the proposed changes

Added to pillar.example

Debug log showing how the proposed changes work

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

keyring directory is managed automatically by salt not by this formula (didn't see any possible configuration in pkgrepo to change this) The directory is just set to be able to clean this directory.
It should be better to define all the signed-by in the same directory (except for keyrings pushed by the keyring_package)

Closing previous PR to split modifications and base this PR on specific branch

@didiermfb didiermfb marked this pull request as ready for review April 11, 2025 07:50
Copy link
Member

@javierbertoli javierbertoli left a comment

Choose a reason for hiding this comment

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

It LGTM, just a simple change, for consistency with other vars

@didiermfb didiermfb force-pushed the fix/repository_aptkey_signedby branch from 6120921 to 3e11c59 Compare April 11, 2025 11:56
@didiermfb didiermfb requested a review from javierbertoli April 11, 2025 12:02
@didiermfb
Copy link
Contributor Author

Var renamed.

Copy link
Member

@javierbertoli javierbertoli left a comment

Choose a reason for hiding this comment

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

Great work, thanks!

@didiermfb
Copy link
Contributor Author

The review is completed. Can someone merge this PR ? (I would like to rebase #80 because there will be conflicts)

@javierbertoli javierbertoli merged commit 928b962 into saltstack-formulas:master Apr 14, 2025
4 checks passed
@saltstack-formulas-github

🎉 This PR is included in version 0.11.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@didiermfb didiermfb deleted the fix/repository_aptkey_signedby branch April 14, 2025 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants