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

Premature key removal in cleanup due to concurrency #144

Open
bennieswart opened this issue Jun 21, 2022 · 3 comments
Open

Premature key removal in cleanup due to concurrency #144

bennieswart opened this issue Jun 21, 2022 · 3 comments

Comments

@bennieswart
Copy link

Behaviour

When multiple runners on the same machine simultaneously run a job using this action it often happens that this action's cleanup in one runner will clean the key out from under the others, causing them to fail.

Steps to reproduce this issue

Consider a job with the following steps:
A: load keys (uses: uses: crazy-max/ghaction-import-gpg@v5)
B: use keys in various ways
C: Post A (cleanup)
Now consider the case where this job simultaneously runs in runners 1 and 2 in the following order: A1 B1 A2 C1 B2 C2.
Once C1 completes, the key is removed and B2 fails.

The output of C2 is as follows, confirming that the key it is expecting to remove no longer exists:

Post job cleanup.
Removing key 47CF7092419B6B580DE41EC020876FE7C6051B
Warning: gpg: key "47CF70292419B6B580DE41EC020876F3E7C6051B" not found: Not found
gpg: 7CF70292419B6B580DE41EC020876F3E7C6051B: delete key failed: Not found

Expected behaviour

Cleanup in one job should not affect another, so B2 should not fail.

Actual behaviour

Runner 1 removes the key that runner 2 is still using, causing the job in runner 2 to fail.

Configuration

The relevant config is shown below, but the problem as stated should be simple enough without needing this.

      - name: Import GPG key
        uses: crazy-max/ghaction-import-gpg@v5
        with:
          gpg_private_key: ${{ secrets.GPG_PRIVATE_KEY }}
          passphrase: ${{ secrets.GPG_PASSPHRASE }}

      - name: Commit changes
        uses: stefanzweifel/git-auto-commit-action@v4
        with:
          commit_message: '<removed>'
          commit_user_name: '<removed>'
          commit_user_email: '<removed>'
          commit_author: '<removed>'
          commit_options: '--no-verify -S'

Logs

Work-related so I'd rather not make it available, but it should not be necessary.

@bennieswart
Copy link
Author

@crazy-max any ideas on how this could be solved? I'll give the implementation a shot once there's a plan.

@imRentable
Copy link

imRentable commented May 9, 2023

We struggle with this as well. Error logs from attempting to encrypt deployment secrets via helm secrets:

Could not generate data key: [failed to encrypt new data key with master key "*************": could not encrypt data key with PGP key: golang.org/x/crypto/openpgp error: key with fingerprint ************* is not available in keyring and could not be retrieved from keyserver; GPG binary error: exit status 2]
Error: plugin "secrets" exited with error
Error: Process completed with exit code 1.

The actual fingerprint has been replaced by *************. The aforementioned logs from the Post step are identical for us. We are currently executing this action with a concurrency of 7. Most of these steps succeed but 1-3 will always fail. If we re-run those, they will be successful as well.

@imRentable
Copy link

There is another error message that we sometimes see when invoking our helm secrets step concurrently:

Group 0: FAILED
  *************: FAILED
    - | could not decrypt data key with PGP key:
      | github.com/ProtonMail/go-crypto/openpgp error: Could not
      | load secring: open /home/github/.gnupg/secring.gpg: no such
      | file or directory; GPG binary error: exit status 2

Recovery failed because no master key was able to decrypt the file. In
order for SOPS to recover the file, at least one key has to be successful,
but none were.
Error: plugin "secrets" exited with error
Error: Process completed with exit code 1.

Here, the fingerprint has been replaced by ************* and the Post step logs complain about not finding the key as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants