Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

fix: ocsp revocation check with context #235

Merged
merged 8 commits into from
Oct 11, 2024
Merged

fix: ocsp revocation check with context #235

merged 8 commits into from
Oct 11, 2024

Conversation

Two-Hearts
Copy link
Contributor

@Two-Hearts Two-Hearts commented Oct 10, 2024 •
edited
Loading

This PR adds context to OCSP revocation check.

Resolves #223

@Two-Hearts
ocsp context
7f1bc3c 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@Two-Hearts
ocsp context
185798b 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@codecov Codecov
Copy link

codecov bot commented Oct 10, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 91.30435% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.89%. Comparing base (e90546b) to head (dd19068).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
revocation/internal/ocsp/ocsp.go 90.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #235      +/-   ##
==========================================
+ Coverage   91.76%   91.89%   +0.13%     
==========================================
  Files          30       30              
  Lines        2065     2074       +9     
==========================================
+ Hits         1895     1906      +11     
+ Misses        115      114       -1     
+ Partials       55       54       -1

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

@Two-Hearts
ocsp context
c244102 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@Two-Hearts
ocsp context
136725f 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@Two-Hearts
update
2ceed71 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@Two-Hearts
update
3728d48 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@Two-Hearts Two-Hearts changed the title fix: ocsp revocation check with log fix: ocsp revocation check with context Oct 10, 2024
JeyJeyGao
JeyJeyGao reviewed Oct 10, 2024
View reviewed changes
revocation/internal/ocsp/ocsp.go Outdated Show resolved Hide resolved
revocation/ocsp/ocsp.go Outdated Show resolved Hide resolved
revocation/internal/ocsp/ocsp.go Outdated Show resolved Hide resolved
@Two-Hearts
update
4638a96 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
shizhMSFT
shizhMSFT previously approved these changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@Two-Hearts
update
dd19068 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
shizhMSFT
shizhMSFT approved these changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@Two-Hearts Two-Hearts requested a review from JeyJeyGao October 10, 2024 07:54
JeyJeyGao
JeyJeyGao approved these changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@JeyJeyGao JeyJeyGao left a comment

Choose a reason for hiding this comment

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

LGTM

priteshbandi
priteshbandi approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

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

LGTM

@priteshbandi priteshbandi merged commit 3067ab1 into notaryproject:main Oct 11, 2024
9 checks passed
@Two-Hearts Two-Hearts deleted the ocsp branch October 12, 2024 00:54
@Two-Hearts Two-Hearts mentioned this pull request Dec 6, 2024
Closed
6 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@JeyJeyGao JeyJeyGao JeyJeyGao approved these changes

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@priteshbandi priteshbandi priteshbandi approved these changes

@gokarnm gokarnm Awaiting requested review from gokarnm gokarnm is a code owner

@NiazFK NiazFK Awaiting requested review from NiazFK NiazFK is a code owner

@rgnote rgnote Awaiting requested review from rgnote rgnote is a code owner

@toddysm toddysm Awaiting requested review from toddysm toddysm is a code owner

@vaninrao10 vaninrao10 Awaiting requested review from vaninrao10 vaninrao10 is a code owner

@yizha1 yizha1 Awaiting requested review from yizha1 yizha1 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OCSP request missing logs
4 participants
@Two-Hearts @priteshbandi @shizhMSFT @JeyJeyGao