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

[BUG] gpg.present succeeds when the keyserver is unreachable #65169

Closed
lkubb opened this issue Sep 11, 2023 · 2 comments · Fixed by #63162
Closed

[BUG] gpg.present succeeds when the keyserver is unreachable #65169

lkubb opened this issue Sep 11, 2023 · 2 comments · Fixed by #63162
Assignees
Labels
Bug broken, incorrect, or confusing behavior

Comments

@lkubb
Copy link
Contributor

lkubb commented Sep 11, 2023

Description
When a keyserver is unreachable, gpg.present will report success even though the import failed.

This is in addition to (not already covered by) several other reporting issues: #63153, #63144, #63159, #62129 (nevermind the state module not respecting test mode, for example).

Setup

  • Have a non-responding keyserver (pgp.mit.edu currently) or make up a funny one yourself (wut.rofl.lol)

Steps to Reproduce the behavior

CAFEBABED000F005:
  gpg.present:
    - keyserver: wut.rofl.lol

Current master says nothing is wrong and nothing changed:

[WARNING ] gpg returned a non-zero error code: 2
local:
----------
          ID: CAFEBABED000F005
    Function: gpg.present
      Result: True
     Comment: Adding CAFEBABED000F005 to GPG keychain
     Started: 18:15:05.420191
    Duration: 34.502 ms
     Changes:

Summary for local
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time:  34.502 ms

With the fixes for the other issues (#63162), it reports changes and still thinks everything is rosy (I remember noticing this case in theory, but was not sure what or even if anything triggered it):

[WARNING ] gpg returned a non-zero error code: 2
local:
----------
          ID: CAFEBABED000F005
    Function: gpg.present
      Result: True
     Comment: Added CAFEBABED000F005 to GPG keychain
     Started: 18:13:57.543630
    Duration: 228.049 ms
     Changes:
              ----------
              CAFEBABED000F005:
                  ----------
                  added:
                      True

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time: 228.049 ms

Expected behavior
Report failure

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
          Salt: 3006.3

Python Version:
        Python: 3.10.13 (main, Sep  6 2023, 02:11:27) [GCC 11.2.0]

Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.2
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 6.0.1
         PyZMQ: 23.2.0
        relenv: 0.13.10
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4

System Versions:
          dist: rocky 9.2 Blue Onyx
        locale: utf-8
       machine: x86_64
       release: 5.14.0-284.18.1.el9_2.x86_64
        system: Linux
       version: Rocky Linux 9.2 Blue Onyx

Additional context
The current official GPG modules (especially the key operations + state module) are in a bad shape and fixes have been submitted and left without actionable input for a long time (there were other attempts years before mine, which I submitted in November 2022 with ample additional test coverage for previously untested code).

Please, somehow find the resources to at least surface and advance (1) well-tested and (2) well-documented fixes for (3) essential code* in a somewhat reasonable timeframe - even some kind of unprompted meh/not interested/sorry busy reaction in weeks instead of months or years would go a long way in avoiding to discourage contributions. At times, it feels like gambling whether an extra effort vs fixing locally will have some impact or be left to rot in PR saltitude.
I know you are a small, hard-working team and probably each one of you has much more work than time on their plate. Let the community help you more smoothly, I'm sure even a minor regular effort would be well spent and would allow you to focus your expertise where it is badly needed.

Sorry for the rant btw and thanks for everything you're doing, really appreciate it, hence my overly frustrated wording and the effort I try to put into my PRs.

* Not saying mine is any of that, just trying to come up with a tangible score. I do think working with GPG reliably is essential for software that manages executables and configuration though.

@lkubb lkubb added Bug broken, incorrect, or confusing behavior needs-triage labels Sep 11, 2023
@lkubb lkubb changed the title [BUG] gpg.present succeeds when no keyserver can be reached [BUG] gpg.present succeeds when the keyserver is unreachable Sep 11, 2023
@anilsil anilsil added this to the Sulfur v3006.4 milestone Sep 11, 2023
@s0undt3ch
Copy link
Collaborator

First of all, we get the frustration you verbosed.
We are indeed a small team with more work than time but we're always modifying/improving our processes to improve the responsiveness.

I'd also like to Thank You for the work put on your PR which I'll be reviewing next. If it's all bugfixes, I'll add the backport label to get your work included in the 3006.x branch too.

@lkubb
Copy link
Contributor Author

lkubb commented Sep 12, 2023

@s0undt3ch I appreciate the acknowledgement and thanks for prioritizing the PR. It should indeed be just bugfixes.

Like I said, even receiving a brief reaction by a core member after sending a reminder several weeks/months post submission to confirm it's on any kind of queue would relieve a lot of frustration and should not require much effort. In many cases this works, but I have witnessed or experienced some outliers (PR processes, not people).

Also thanks for improving the processes, the new automation/test suite is awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants