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.

Sign up for GitHub

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

Jump to bottom

feat: Support more trust store types #1538

Merged
merged 22 commits into from
Jun 25, 2024
Merged

feat: Support more trust store types #1538

merged 22 commits into from
Jun 25, 2024

Conversation

junczhu
Copy link
Collaborator

@junczhu junczhu commented Jun 3, 2024 •
edited
Loading

Description

What this PR does / why we need it:

Differentiate between trust store types: ca, tsa and signingAuthority, with full backward compatibility.
Both previous CRDs and new CRDs are supported.

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):

Closes #1174

Type of change

  • scalability for TSA
  • backward compatibility
  • code quality
    • update variable naming
    • update logging for both error message and debugging
  • test cases fix & enhance

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Helm Chart Change (any edit/addition/update that is necessary for changes merged to the main branch)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

Checklist:

  • Does the affected code have corresponding tests?
  • Are the changes documented, not just with inline documentation, but also with conceptual documentation such as an overview of a new feature, or task-based documentation like a tutorial? Consider if this change should be announced on your project blog.
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have appropriate license header?

Post Merge Requirements

  • MAINTAINERS: manually trigger the "Publish Package" workflow after merging any PR that indicates Helm Chart Change

@codecov Codecov
Copy link

codecov bot commented Jun 3, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 86.88525% with 8 lines in your changes missing coverage. Please review.

Files Coverage Δ
pkg/verifier/notation/certstoresbytype.go 92.59% <92.59%> (ø)
pkg/verifier/notation/truststore.go 84.00% <85.71%> (-1.37%) ⬇️
pkg/verifier/notation/notation.go 87.87% <80.00%> (-2.37%) ⬇️

@junczhu junczhu marked this pull request as ready for review June 5, 2024 06:05
binbin-li
binbin-li reviewed Jun 11, 2024
View reviewed changes
Makefile Outdated Show resolved Hide resolved
pkg/verifier/notation/notation.go Outdated Show resolved Hide resolved
pkg/verifier/notation/notation.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certStoresByType.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certStoresByType.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certStoresByType.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certStoresByType.go Outdated Show resolved Hide resolved
akashsinghal
akashsinghal reviewed Jun 12, 2024
View reviewed changes
pkg/verifier/notation/certStoresByType.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certStoresByType.go Outdated Show resolved Hide resolved
pkg/verifier/notation/notation.go Outdated Show resolved Hide resolved
susanshi
susanshi reviewed Jun 19, 2024
View reviewed changes
Copy link
Collaborator

@susanshi susanshi left a comment

Choose a reason for hiding this comment

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

Left some comments. thanks!

Makefile Outdated Show resolved Hide resolved
pkg/verifier/notation/certstoresbytype.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certstoresbytype.go Outdated Show resolved Hide resolved
binbin-li
binbin-li reviewed Jun 19, 2024
View reviewed changes
pkg/verifier/notation/certstoresbytype.go Show resolved Hide resolved
pkg/verifier/notation/certstoresbytype.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certstoresbytype.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certstoresbytype.go Outdated Show resolved Hide resolved
@junczhu junczhu changed the title feat: Support more trust store types [WIP]feat: Support more trust store types Jun 20, 2024
@junczhu junczhu mentioned this pull request Jun 20, 2024
Merged
12 tasks
@junczhu junczhu changed the title [WIP]feat: Support more trust store types feat: Support more trust store types Jun 20, 2024
@junczhu junczhu enabled auto-merge (squash) June 21, 2024 10:04
binbin-li
binbin-li reviewed Jun 24, 2024
View reviewed changes
pkg/verifier/notation/certstoresbytype.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certStores.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certstoresbytype.go Outdated Show resolved Hide resolved
binbin-li
binbin-li previously approved these changes Jun 24, 2024
View reviewed changes
Copy link
Collaborator

@binbin-li binbin-li left a comment

Choose a reason for hiding this comment

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

lgtm, just a few minor comments, thanks for adding this feature!

pkg/verifier/notation/certstoresbytype.go Outdated Show resolved Hide resolved
pkg/verifier/notation/certstoresbytype.go Outdated Show resolved Hide resolved
@junczhu @binbin-li
Update pkg/verifier/notation/certstoresbytype.go
3e74a89 
Co-authored-by: Binbin Li <[email protected]>
Signed-off-by: Juncheng Zhu <[email protected]>
@junczhu junczhu changed the title feat: Support more trust store types [WIP]feat: Support more trust store types Jun 24, 2024
@junczhu junczhu marked this pull request as draft June 25, 2024 03:14
auto-merge was automatically disabled June 25, 2024 03:14

Pull request was converted to draft

@junczhu junczhu changed the title [WIP]feat: Support more trust store types feat: Support more trust store types Jun 25, 2024
@junczhu junczhu marked this pull request as ready for review June 25, 2024 03:59
@junczhu junczhu requested review from binbin-li and susanshi June 25, 2024 06:18
@junczhu junczhu enabled auto-merge (squash) June 25, 2024 11:59
binbin-li
binbin-li approved these changes Jun 25, 2024
View reviewed changes
Copy link
Collaborator

@binbin-li binbin-li left a comment

Choose a reason for hiding this comment

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

lgtm

@junczhu junczhu merged commit ae4385b into ratify-project:dev Jun 25, 2024
18 checks passed
@junczhu junczhu mentioned this pull request Jul 4, 2024
Merged
duffney pushed a commit to duffney/ratify that referenced this pull request Jul 10, 2024
@junczhu @binbin-li @duffney
feat: Support more trust store types (ratify-project#1538)
ec39df0 
Signed-off-by: Juncheng Zhu <[email protected]>
Co-authored-by: Binbin Li <[email protected]>
akashsinghal pushed a commit to akashsinghal/ratify that referenced this pull request Sep 13, 2024
@junczhu @binbin-li @akashsinghal
feat: Support more trust store types (ratify-project#1538)
b9722ce 
Signed-off-by: Juncheng Zhu <[email protected]>
Co-authored-by: Binbin Li <[email protected]>
Signed-off-by: akashsinghal <[email protected]>
binbin-li added a commit to binbin-li/ratify that referenced this pull request Sep 14, 2024
@junczhu @binbin-li
feat: Support more trust store types (ratify-project#1538)
90237f2 
Signed-off-by: Juncheng Zhu <[email protected]>
Co-authored-by: Binbin Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@binbin-li binbin-li binbin-li approved these changes

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

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

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

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

@susanshi susanshi Awaiting requested review from susanshi susanshi 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.

Support different types of trust store
4 participants
@junczhu @binbin-li @susanshi @akashsinghal