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

Use the timeout flag value in verify* commands. #3391

Merged
merged 2 commits into from Nov 28, 2023
Merged

Use the timeout flag value in verify* commands. #3391

merged 2 commits into from Nov 28, 2023

Conversation

zhaoyonghe
Copy link
Contributor

This PR closes #3390.

Summary

--timeout flag is not useful in verify* commands.

For example, when there is a network issue (not connecting to the company VPN but still try to access the internal service), even if I set --timeout 5s, the cosign verify will still hang for a long time (about one minute in my case) until erroring out.

$ ./cosign verify --timeout 5s internal.registry.com/image-name:tag
Error: Get "https://internal.registry.com/v2/": dial tcp <ip address>: i/o timeout
main.go:69: error during command execution: Get "https://internal.registry.com/v2/": dial tcp <ip address>: i/o timeout

After this pr, the command errors out when timeout.

$ ./cosign verify --timeout 5s internal.registry.com/image-name:tag
Error: Get "https://internal.registry.com/v2/": dial tcp <ip address>: context deadline exceeded
main.go:69: error during command execution: Get "https://internal.registry.com/v2/": context deadline exceeded

Release Note

Used the timeout flag value in verify* commands.

Documentation

No need to update documentation.

Signed-off-by: zhaoyonghe <yonghe.zhao@yahoo.com>
@zhaoyonghe zhaoyonghe marked this pull request as ready for review November 27, 2023 18:40
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (da53b08) 30.23% compared to head (18dc8c7) 30.22%.

Files Patch % Lines
cmd/cosign/cli/verify.go 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3391      +/-   ##
==========================================
- Coverage   30.23%   30.22%   -0.02%     
==========================================
  Files         155      155              
  Lines        9958     9962       +4     
==========================================
  Hits         3011     3011              
- Misses       6497     6501       +4     
  Partials      450      450              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…hTimeout should be called, not discarded, to avoid a context leak (govet)".

Signed-off-by: zhaoyonghe <yonghe.zhao@yahoo.com>
@zhaoyonghe
Copy link
Contributor Author

zhaoyonghe commented Nov 27, 2023

@haydentherapper Thank you! The lint check failed in the previous commit: https://github.com/sigstore/cosign/actions/runs/7009367023/job/19069346011?pr=3391. It is expected to pass now. Also, make lint did not catch this error on my laptop. I am unsure if it is just me.

$ make lint
rm -f /path/to/sigstore/cosign/bin/golangci-lint || :
set -e ;\
        GOBIN=/path/to/sigstore/cosign/bin go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55.2      ;\

# github.com/golangci/golangci-lint/cmd/golangci-lint
ld: warning: -bind_at_load is deprecated on macOS
/path/to/sigstore/cosign/bin/golangci-lint run -n

$ echo $?
0 

@haydentherapper haydentherapper merged commit 6b06819 into sigstore:main Nov 28, 2023
28 checks passed
@github-actions github-actions bot added this to the v2.3.0 milestone Nov 28, 2023
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.

--timeout flag is not useful in verify* commands.
2 participants