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

Resolve slow key retrieval when some GPG key servers are not reachable #1016

Merged
merged 7 commits into from
Jul 17, 2024

Conversation

h3xcat
Copy link
Contributor

@h3xcat h3xcat commented Jun 25, 2024

At the moment, if the host environment cannot reach first key server over port 11371, it takes 1 minute to retrieve individual keys from the other listed key servers. For the Python feature, the installation script retrieves 13 keys, so the total time to retrieve all the keys is 13 minutes. The delay could be further extended if subsequent servers also cannot be reached.

The reason it takes 1 minute rather than the 15 second default timeout in dirmngr is that I believe dirmngr internally tries the connection to the same keyserver 4 times before it moves on to a different server. Essentially, this multiplies the timeout delay by 4.

Instead of relying on the dirmngr timeout mechanism, the change implements a check for keyserver reachability using curl before the list is passed on to dirmngr.

If no keyservers are identified, the script will intentionally fail. If some keyservers are not reachable, the script will log this information, enabling users to more easily identify and resolve network issues in their environments.

Related issues:
#686
#337
#323

Affected features:

  • git-lfs
  • git
  • github-cli
  • kubectl-helm-minikube
  • python
  • ruby
  • terraform

I ran tests for all the features locally; most succeeded without issues. The only test that failed was autoPullDisabled from git-lfs, but it also fails with unmodified code, so I suspect something else in my environment is causing the tests to fail, not specific to my changes.

@h3xcat h3xcat requested a review from a team as a code owner June 25, 2024 16:42
@h3xcat h3xcat changed the title Reduce timeout to GPG key servers for Python feature Resolve slow key retrieval when first GPG key server is not reachable (Python feature) Jun 26, 2024
@h3xcat
Copy link
Contributor Author

h3xcat commented Jun 26, 2024

@microsoft-github-policy-service agree

@h3xcat h3xcat marked this pull request as draft June 26, 2024 00:52
@h3xcat h3xcat changed the title Resolve slow key retrieval when first GPG key server is not reachable (Python feature) Resolve slow key retrieval when some GPG key servers are not reachable Jun 26, 2024
@h3xcat h3xcat marked this pull request as ready for review June 26, 2024 05:28
Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

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

thanks for contributing, definitely a helpful improvement.

src/git-lfs/install.sh Show resolved Hide resolved
src/git-lfs/install.sh Outdated Show resolved Hide resolved
src/git-lfs/install.sh Outdated Show resolved Hide resolved
src/git-lfs/install.sh Show resolved Hide resolved
Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! Let's merge this on Monday to ensure we accidentally don't break anything.

@h3xcat
Copy link
Contributor Author

h3xcat commented Jul 17, 2024

@samruddhikhandale
Could you merge the pull request when you are able to? If there are any concerns, please let me know.

@samruddhikhandale samruddhikhandale merged commit f455189 into devcontainers:main Jul 17, 2024
60 checks passed
trxcllnt added a commit to trxcllnt/devcontainers that referenced this pull request Jul 23, 2024
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

Successfully merging this pull request may close these issues.

None yet

2 participants