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

sync: Add a new flag to enforce syncing only signed images, closes #455 #456

Merged
merged 3 commits into from
Mar 24, 2022

Conversation

peusebiu
Copy link
Collaborator

sync: When checking if a image is already synced also check for changes in upstream signatures.

Signed-off-by: Petu Eusebiu peusebiu@cisco.com

What type of PR is this?

Which issue does this PR fix:

What does this PR do / Why do we need it:

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades. Has updating a running cluster been tested?:

Does this change require updates to the CNI daemonset config files to work?:

Does this PR introduce any user-facing change?:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Mar 11, 2022

Codecov Report

Merging #456 (a27c42c) into main (dd6cedc) will increase coverage by 0.30%.
The diff coverage is 85.16%.

@@            Coverage Diff             @@
##             main     #456      +/-   ##
==========================================
+ Coverage   84.10%   84.41%   +0.30%     
==========================================
  Files          50       51       +1     
  Lines       10375    10548     +173     
==========================================
+ Hits         8726     8904     +178     
+ Misses       1290     1287       -3     
+ Partials      359      357       -2     
Impacted Files Coverage Δ
pkg/storage/s3/storage.go 85.43% <ø> (ø)
pkg/extensions/sync/sync.go 85.51% <71.71%> (-5.72%) ⬇️
pkg/cli/root.go 92.98% <75.00%> (+0.02%) ⬆️
pkg/extensions/sync/signatures.go 83.33% <83.33%> (ø)
pkg/api/routes.go 74.79% <86.66%> (+0.51%) ⬆️
pkg/extensions/sync/on_demand.go 92.36% <91.66%> (+4.16%) ⬆️
pkg/api/controller.go 91.56% <100.00%> (-0.27%) ⬇️
pkg/cli/config_reloader.go 81.25% <100.00%> (+3.20%) ⬆️
pkg/compliance/v1_0_0/check.go 99.23% <100.00%> (ø)
pkg/extensions/sync/utils.go 92.92% <100.00%> (+10.01%) ⬆️
... and 6 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@peusebiu peusebiu force-pushed the sync_signature_flag branch 2 times, most recently from e88e902 to e211598 Compare March 14, 2022 16:38
@peusebiu peusebiu requested a review from rchincha March 14, 2022 17:11
@peusebiu peusebiu force-pushed the sync_signature_flag branch 3 times, most recently from 0be1a20 to 5178320 Compare March 16, 2022 12:01
examples/config-sync.json Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
@peusebiu peusebiu force-pushed the sync_signature_flag branch 3 times, most recently from 32b5d67 to 969b814 Compare March 18, 2022 16:28
@peusebiu peusebiu force-pushed the sync_signature_flag branch 5 times, most recently from 969a1fc to 8b342b3 Compare March 22, 2022 16:32
…oject-zot#455

sync: When checking if a image is already synced also check for changes in upstream signatures.

Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
@andaaron andaaron added this to In progress in zot-core via automation Mar 23, 2022
@andaaron andaaron moved this from In progress to Review pending in zot-core Mar 23, 2022
@rchincha
Copy link
Contributor

Too many things happening in this PR?

  1. Linter changes
  2. constants (so currently as you have it, we will have pkg/api/constants/, maybe pkg/storage/constants/?)
    Why not have pkg/api/constants.go itself?
  3. The actual flag

Let's split it like above in 3 separate commits in this PR

@peusebiu
Copy link
Collaborator Author

peusebiu commented Mar 24, 2022

Too many things happening in this PR?

  1. Linter changes
  2. constants (so currently as you have it, we will have pkg/api/constants/, maybe pkg/storage/constants/?)
    Why not have pkg/api/constants.go itself?
  3. The actual flag

Let's split it like above in 3 separate commits in this PR

Ok, I will split it into 3 commits
2. I can not put it into pkg/api/constants.go (package api) because it will result in circular import when importing pkg/api in sync.

…imports

Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>
Copy link
Contributor

@rchincha rchincha left a comment

Choose a reason for hiding this comment

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

lgtm

zot-core automation moved this from Review pending to Reviewer approved Mar 24, 2022
@rchincha rchincha merged commit be910cf into project-zot:main Mar 24, 2022
zot-core automation moved this from Reviewer approved to Done Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
zot-core
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants