Skip to content

[BUG] clean_keyrings_d: true does not keep wanted/configured keyrings #81

@realbodo

Description

@realbodo

Your setup

Formula commit hash / release tag

latest master 0202854 / v0.12.0

Versions reports (master & minion)

          Salt: 3007.8
 
Python Version:
        Python: 3.10.18 (main, Sep  5 2025, 22:48:51) [GCC 11.2.0]
 
Dependency Versions:
          cffi: 1.16.0
      cherrypy: unknown
  cryptography: 42.0.5
      dateutil: 2.8.2
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.6
       libgit2: Not Installed
  looseversion: 1.3.0
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.7
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 24.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: Not Installed
  python-gnupg: 0.5.2
        PyYAML: 6.0.1
         PyZMQ: 25.1.2
        relenv: 0.20.6
         smmap: Not Installed
       timelib: 0.3.0
       Tornado: 6.4.2
           ZMQ: 4.3.4
 
Salt Package Information:
  Package Type: onedir
 
System Versions:
          dist: debian 13.1 trixie
        locale: utf-8
       machine: x86_64
       release: 6.12.48+deb13-amd64
        system: Linux
       version: Debian GNU/Linux 13.1 trixie

master/minion run the same OS/python/salt-version

Pillar / config used

local:
  apt:
    clean_keyrings_d: true
    clean_sources_list_d: false
    keyrings_dir: /etc/apt/keyrings
    remove_sources_list: false
    repositories:
      saltstack:
        comps:
        - main
        distro: stable
        key_url: https://packages.broadcom.com/artifactory/api/security/keypair/SaltProjectKey/public
        opts: signed-by=/etc/apt/keyrings/salt-archive-keyring.pgp
        type:
        - binary
        url: https://packages.broadcom.com/artifactory/saltproject-deb

Bug details

Describe the bug

If pillar apt:clean_keyrings_d is set true, the keyrings-directory will be completely cleaned at later state.applys. even the keyrings that have been configured in pillars will be removed from keyring_directory

Steps to reproduce the bug

output of first state.apply apt.repositories, that verifies the correctness of directory /etc/apt/keyrings/ and the configuration of the new package repository (though the creation of /etc/apt/keyrings/salt-archive-keyring.pgp does not appear mentioned in state.apply’s output):

          ID: /etc/apt/keyrings
    Function: file.directory
      Result: True
     Comment: The directory /etc/apt/keyrings is in the correct state
     Started: 22:39:21.020816
    Duration: 1.567 ms
     Changes:   
----------
          ID: deb saltstack
    Function: pkgrepo.managed
        Name: deb [ signed-by=/etc/apt/keyrings/salt-archive-keyring.pgp ] https://packages.broadcom.com/artifactory/saltproject-deb stable main
      Result: True
     Comment: Configured package repo 'deb [ signed-by=/etc/apt/keyrings/salt-archive-keyring.pgp ] https://packages.broadcom.com/artifactory/saltproject-deb stable main'
     Started: 22:39:21.023166
    Duration: 845.71 ms
     Changes:   
              ----------
              repo:
                  deb [ signed-by=/etc/apt/keyrings/salt-archive-keyring.pgp ] https://packages.broadcom.com/artifactory/saltproject-deb stable main

2nd state.apply apt.repositories cleans the directory again, configured pkgrepo has not changed:

    Function: file.directory
      Result: True
     Comment: Files cleaned from directory /etc/apt/keyrings
     Started: 22:39:58.686503
    Duration: 2.058 ms
     Changes:   
              ----------
              /etc/apt/keyrings/salt-archive-keyring.pgp:
                  ----------
                  removed:
                      Removed due to clean
              removed:
                  - /etc/apt/keyrings/salt-archive-keyring.pgp
----------
          ID: deb saltstack
    Function: pkgrepo.managed
        Name: deb [ signed-by=/etc/apt/keyrings/salt-archive-keyring.pgp ] https://packages.broadcom.com/artifactory/saltproject-deb stable main
      Result: True
     Comment: Package repo 'deb [ signed-by=/etc/apt/keyrings/salt-archive-keyring.pgp ] https://packages.broadcom.com/artifactory/saltproject-deb stable main' already configured
     Started: 22:39:58.689417
    Duration: 6.279 ms
     Changes:   

3rd state.apply does not re-create the keyring, most likely because the package repo has already been configured, and the necessary keyring does not need to be downloaded again.

          ID: /etc/apt/keyrings
    Function: file.directory
      Result: True
     Comment: The directory /etc/apt/keyrings is in the correct state
     Started: 22:41:41.562330
    Duration: 1.414 ms
     Changes:   
----------
          ID: deb saltstack
    Function: pkgrepo.managed
        Name: deb [ signed-by=/etc/apt/keyrings/salt-archive-keyring.pgp ] https://packages.broadcom.com/artifactory/saltproject-deb stable main
      Result: True
     Comment: Package repo 'deb [ signed-by=/etc/apt/keyrings/salt-archive-keyring.pgp ] https://packages.broadcom.com/artifactory/saltproject-deb stable main' already configured
     Started: 22:41:41.564477
    Duration: 6.089 ms
     Changes:   

Expected behaviour

/etc/apt/keyrings/ should not be cleaned from keys/keyrings, that appear in key_url/opts-pillars

Attempts to fix the bug

Checked source code, haven’t found anything about how the keyring-file will be created. Otherwise I would have added a pull-request with a change similar to this:

% git diff           
diff --git a/apt/repositories.sls b/apt/repositories.sls
index 6f61a1d..b41220d 100644
--- a/apt/repositories.sls
+++ b/apt/repositories.sls
@@ -85,6 +85,11 @@
   {%- for type in args.type|d(['binary']) %}
     {%- set r_type = 'deb-src' if type == 'source' else 'deb' %}
     {%- set r_file = args.filename if args.filename is defined else repo ~ '-' ~ type ~ '.list' %}
+    {%- if keyring_file_necessary %}
+      {%- set r_keyring_file = extract_file_name_from_opts_pillar_or_other_aptpkgs_magic %}
+    {%- else %}
+      {%- set r_keyring_file = None %}
+    {%- endif %}
 
 {{ r_type }} {{ repo }}:
   pkgrepo.managed:
@@ -118,6 +123,13 @@
       - file: {{ sources_list_dir }}
       # require_in the directory clean state
       # This way, we don't remove all the files, just to add them again.
+    {% if r_keyring_file is defined %}
+  file.managed:
+    - name: {{ keyrings_dir }}/{{ r_keyring_file }}
+    - replace: false
+    - require_in:
+      - file: {{ keyrings_dir }}
+    {% endif %}
   {%- endfor %}
 {% endfor %}
 

Additional context

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions