Extend Applier's Apply() method with an optional options parameter by stefanberger · Pull Request #3123 · containerd/containerd

stefanberger · 2019-03-22T20:07:38Z

Extend the Applier interface's Apply method with an optional
options map[string]string.

Signed-off-by: Stefan Berger [email protected]

stefanberger · 2019-03-22T20:17:19Z

As for the approach for adding encryption parameters, after some thought, it would be best to add key id and other public parameters on this descriptor, that will become part of the image store. For the private key, however, that should come through as an option which is dumped after use. We don't want to risk storing an encryption key in the containerd database or logs.

@stevvooe I took you by the word and this patch here adds an optional options field to the Apply() method.

codecov-io · 2019-03-22T21:02:32Z

Codecov Report

Merging #3123 into master will decrease coverage by 3.1%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3123      +/-   ##
==========================================
- Coverage   43.58%   40.48%   -3.11%     
==========================================
  Files         104       74      -30     
  Lines       11154     9893    -1261     
==========================================
- Hits         4862     4005     -857     
+ Misses       5553     5322     -231     
+ Partials      739      566     -173

Flag	Coverage Δ
#linux	`?`
#windows	`40.48% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
snapshots/native/native.go	`1.79% <0%> (-41.26%)`	⬇️
archive/tar.go	`16.99% <0%> (-26.8%)`	⬇️
metadata/snapshot.go	`21.53% <0%> (-24.28%)`	⬇️
cio/io.go	`1.52% <0%> (-21.38%)`	⬇️
content/local/writer.go	`56.86% <0%> (-0.99%)`	⬇️
gc/scheduler/scheduler.go	`66.34% <0%> (-0.97%)`	⬇️
oci/spec_opts.go	`30.07% <0%> (-0.26%)`	⬇️
mount/temp_unix.go
sys/reaper_linux.go
services/server/server_linux.go
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b882c4...488f2bd. Read the comment docs.

codecov-io · 2019-03-22T21:02:33Z

Codecov Report

Merging #3123 into master will decrease coverage by 1.89%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #3123     +/-   ##
=========================================
- Coverage   42.38%   40.48%   -1.9%     
=========================================
  Files         115       74     -41     
  Lines       12751     9893   -2858     
=========================================
- Hits         5404     4005   -1399     
+ Misses       6513     5322   -1191     
+ Partials      834      566    -268

Flag	Coverage Δ
#linux	`?`
#windows	`40.48% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
snapshots/native/native.go	`1.79% <0%> (-41.26%)`	⬇️
archive/tar.go	`16.99% <0%> (-26.8%)`	⬇️
metadata/snapshot.go	`21.53% <0%> (-24.28%)`	⬇️
cio/io.go	`1.52% <0%> (-21.38%)`	⬇️
content/local/writer.go	`56.86% <0%> (-0.99%)`	⬇️
gc/scheduler/scheduler.go	`66.34% <0%> (-0.97%)`	⬇️
oci/spec_opts.go	`30.07% <0%> (-0.52%)`	⬇️
mount/temp_unix.go
sys/reaper_linux.go
services/server/server_linux.go
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f60e38...baf3403. Read the comment docs.

stefanberger · 2019-03-27T14:32:10Z

@dmcgowan This is our latest proposal for passing the decryption parameters to the applier's Apply() method.

Extend the Applier interface's Apply method with an optional options parameter. For the container image encryption we intend to use the options parameter to pass image decryption parameters ('dcparameters'), which are primarily (privatte) keys, in form of a JSON document under the map key '_dcparameters', and pass them to the Applier's Apply() method. This helps us to access decryption keys and start the pipeline with the layer decryption before the layer data are unzipped and untarred. Signed-off-by: Stefan Berger <[email protected]> Signed-off-by: Harshal Patil <[email protected]>

stefanberger · 2019-04-11T12:44:34Z

@dmcgowan Is this ok now?

crosbymichael · 2019-04-16T19:42:40Z

 }

+// ApplyConfig is used to hold parameters needed for a apply operation
+type ApplyConfig struct {


What's going to go in this? Right now this is all a no-op

We want to use the options for encrypted container image support patches for passing decryption parameters (keys) to the Applier.

Just trying to imagine how flexible this will be

This is here is our work-in-progress tree: https://github.com/stefanberger/containerd/commits/image-encryption.v5

This patch appears there under the name "Extend Applier's Apply() method with an optional options parameter". On top of it are two more patches where the 1st one is called 'Prepare Apply call to carry dcparameters using the ApplyOpts' and the 2nd one converts the existing code to use this way of passing the parameters ('Pass the dcparameters via the ApplyOpts now and remove old code').

dmcgowan

LGTM

crosbymichael · 2019-06-03T15:13:36Z

LGTM

stefanberger force-pushed the extend_apply_call_with_options_parameter branch from ffd8e3d to 488f2bd Compare March 22, 2019 20:43

stefanberger force-pushed the extend_apply_call_with_options_parameter branch 2 times, most recently from 73297b6 to 895d6e1 Compare March 25, 2019 13:20

dmcgowan reviewed Mar 27, 2019

View reviewed changes

Comment thread diff/diff.go Outdated

stefanberger force-pushed the extend_apply_call_with_options_parameter branch from 895d6e1 to 8b7d8a9 Compare April 2, 2019 22:14

stefanberger force-pushed the extend_apply_call_with_options_parameter branch from 8b7d8a9 to baf3403 Compare April 2, 2019 22:20

dmcgowan added this to the 1.3 milestone Apr 11, 2019

crosbymichael reviewed Apr 16, 2019

View reviewed changes

crosbymichael assigned dmcgowan May 16, 2019

dmcgowan approved these changes May 28, 2019

View reviewed changes

crosbymichael merged commit 2ec2089 into containerd:master Jun 3, 2019

stefanberger deleted the extend_apply_call_with_options_parameter branch June 3, 2019 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend Applier's Apply() method with an optional options parameter#3123

Extend Applier's Apply() method with an optional options parameter#3123
crosbymichael merged 1 commit intocontainerd:masterfrom
stefanberger:extend_apply_call_with_options_parameter

stefanberger commented Mar 22, 2019 •

edited

Loading

Uh oh!

stefanberger commented Mar 22, 2019

Uh oh!

codecov-io commented Mar 22, 2019

Uh oh!

codecov-io commented Mar 22, 2019 •

edited

Loading

Uh oh!

stefanberger commented Mar 27, 2019

Uh oh!

Uh oh!

stefanberger commented Apr 11, 2019

Uh oh!

crosbymichael Apr 16, 2019

Uh oh!

stefanberger Apr 16, 2019

Uh oh!

crosbymichael Apr 16, 2019

Uh oh!

stefanberger Apr 17, 2019

Uh oh!

dmcgowan left a comment

Uh oh!

crosbymichael commented Jun 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

stefanberger commented Mar 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stefanberger commented Mar 22, 2019

Uh oh!

codecov-io commented Mar 22, 2019

Codecov Report

Uh oh!

codecov-io commented Mar 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

stefanberger commented Mar 27, 2019

Uh oh!

Uh oh!

stefanberger commented Apr 11, 2019

Uh oh!

crosbymichael Apr 16, 2019

Choose a reason for hiding this comment

Uh oh!

stefanberger Apr 16, 2019

Choose a reason for hiding this comment

Uh oh!

crosbymichael Apr 16, 2019

Choose a reason for hiding this comment

Uh oh!

stefanberger Apr 17, 2019

Choose a reason for hiding this comment

Uh oh!

dmcgowan left a comment

Choose a reason for hiding this comment

Uh oh!

crosbymichael commented Jun 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stefanberger commented Mar 22, 2019 •

edited

Loading

codecov-io commented Mar 22, 2019 •

edited

Loading