Enable analysis for OCI images #712

mukultaneja · 2020-06-01T18:39:16Z

This commit proposes changes to analyze
OCI images using tern.

Work towards: #678

Signed-off-by: mukultaneja [email protected]

mukultaneja · 2020-06-01T18:45:05Z

@nishakm @rnjudge I have been working on issue #678 which is to analyze OCI images. I would like to propose an approach here. This PR as of now is in scratch form and mostly to take suggestions over my approach. Currently, I am targetting OCI images without umoci tool version. Please take a look once and share your feedback.
I am using the below command to analyze OCI debain image,

skopeo copy docker://debain:latest oci:///root/debian:latest
tern report -c oci:///root/debian:latest -o output.txt -f default

rnjudge · 2020-06-01T19:21:53Z

tern/__main__.py

+            # Check if the image is of oci://<image-location>:<image-tag>
+            if not check_oci_image_string(args.oci_image):
+                sys.stderr.write('Error running Tern\n'
+                                 'Please provide docker image '


is the --oci-image command line option designed to handle both a docker image string in image:tag and oci://<image-location>:<image-tag> format? If so, we should modify the help string on the parser add_argument lines below, as that help message only directs users to use oci://<image-location>:<image-tag> and doesn't mention the ability to use docker image:tag. If not, this string should be modified to direct users to only provide the oci image location/tag format.

I think we should have separate arguments to deal with both types of images and yes I need to change the string message to direct users to only provide the oci image location/tag format.

Would it make sense to add a new command line option called --type, -t were you can enter docker or oci?

@nishakm I added -t option as you mentioned and I think it would make more sense having one option -i to define container image and -t is type of container image to call respective operations on to the image.

rnjudge · 2020-06-01T19:23:35Z

tern/analyze/docker/run.py

@@ -69,6 +69,35 @@ def analyze(image_obj, args, dfile_lock=False, dfobj=None):
        analyze_docker_image(image_obj, args.redo, dfile_lock, dfobj)


+def execute_oci_image(args):


We should create a directory called tern/analyze/oci and put this function in a run.py file there.

rnjudge · 2020-06-01T19:28:37Z

tern/utils/oci_image.py

@@ -0,0 +1,32 @@
+


I think this file should be under: tern/analyze/oci/helpers.py

I updated the code to add modularity similar to the docker.

nishakm · 2020-06-03T23:16:58Z

Currently, I am targetting OCI images without umoci tool version. Please take a look once and share your feedback.

I'd say this is the expected way that folks would give an OCI image to tern, i.e., they've used skopeo to copy a docker image into the oci format into a local directory and provide that directory to tern.

I am using the below command to analyze OCI debain image,
skopeo copy docker://debain:latest oci:///root/debian:latest
tern report -c oci:///root/debian:latest -o output.txt -f default

I wonder if podman would be able to natively build an OCI type image. Maybe that's something I can poke at.

tern/analyze/oci/__init__.py

nishakm · 2020-06-05T22:08:24Z

tern/__main__.py

+        if general.check_tar(args.image):
+            logger.error("%s", errors.incorrect_raw_option)


I think this option should be different from the -i option now. Or else this is going to get more complicated. @rnjudge What do you think?

Not sure I understand your question. The -w option is already separate from the -i option, right?

Shouldn't our check here then be if args.raw?

I believe this check here is to make sure that the user didn't provide a raw option tarball when they are trying to provide a docker or OCI image.

@nishakm, this check is part of the execute_image method and this method is only for docker or OCI image execution. The args.raw_image still I am keeping the part do_main method.

Confirmed that this is just a validation check to make sure the user didn't accidentally choose the -i option but provide a raw image tarball. Seems OK to me.

This commit proposes changes to analyze OCI images using tern. Work towards: #678 Signed-off-by: mukultaneja <[email protected]>

rnjudge · 2020-06-08T19:49:19Z

tern/__main__.py

                               " The option can be used to pull docker"
                               " images by digest as well -"
                               " <repo>@<digest-type>:<digest>")
+    parser_report.add_argument('-t', '--type',


I would like to propose that we call this optionclient instead of type (using -c option). This makes more sense to me as we would like to support the podman client in the future which represents image blobs differently than docker or skopeo might once they are pulled down locally, even if they all are stored on dockerhub technically as OCI images.

Agree - this makes better sense than the generic "type" in the container image context.

nishakm · 2020-06-09T14:14:01Z

I am using the below command to analyze OCI debain image,
skopeo copy docker://debain:latest oci:///root/debian:latest
tern report -c oci:///root/debian:latest -o output.txt -f default

From my experiments, it looks like the typical way that an oci image is represented is as a directory with this layout:

index.json
oci-layout
blobs/
        |_ <hash>
                       |_ artifact1
                       |_ artifact2

You can get this layout using the "docker-ish" way using podman.

podman pull docker://debian:latest
podman save --format oci-archive -o debian.tar debian:latest
tar xvf debian.tar -C debian-oci

Podman also has an oci-dir format for saving but this looks like a cross between the docker image layout and the oci image layout. I will ask about this further. I think it's a way to make podman compatible with images built with docker but not sure.

You can also get this layout using skopeo:

skopeo copy docker://debian:latest oci:debian:latest

This will create a folder called debian which will have the above OCI layout.

So we have a number of ways to get to the common directory with the OCI layout. So I think we should expect that users provide a path to the OCI directory as input for Tern and perform tests on the layout accordingly. We can inform users in the documentation how to generate the OCI directory. Note that we can't do this with Docker as that directory is only accessible via docker save which won't work unless you do a docker pull, so we can keep that machinery.

nishakm · 2020-06-09T15:25:39Z

@mukultaneja This is a pretty big contribution. Would it be possible to split it up into several smaller commits that we can merge in little by little? How about starting off with the OCIImage image class and some tests for that?

mukultaneja · 2020-06-09T16:35:04Z

@mukultaneja This is a pretty big contribution. Would it be possible to split it up into several smaller commits that we can merge in little by little? How about starting off with the OCIImage image class and some tests for that?

do you want me to close this PR and send separate PRs with several commits? I was thinking if we could use this PR as a module and make modifications in it as we feel. Say the next commit would be adding tests for OCI images. Anyway so far changes, would allow us to generate a default report for an OCI image.

nishakm · 2020-06-09T16:53:04Z

@mukultaneja This is a pretty big contribution. Would it be possible to split it up into several smaller commits that we can merge in little by little? How about starting off with the OCIImage image class and some tests for that?

do you want me to close this PR and send separate PRs with several commits? I was thinking if we could use this PR as a module and make modifications in it as we feel. Say the next commit would be adding tests for OCI images. Anyway so far changes, would allow us to generate a default report for an OCI image.

I would rather the commits be self describing and independent. You can either do one PR with many commits or several PRs each being independent. The idea is that the changes are small and logical so they are easy to follow.

nishakm · 2020-06-09T20:08:01Z

Reg: podman save --format oci-dir: containers/podman#6544 (comment)

rnjudge reviewed Jun 1, 2020

View reviewed changes

mukultaneja requested review from nishakm and rnjudge June 4, 2020 21:48

rnjudge reviewed Jun 5, 2020

View reviewed changes

tern/analyze/oci/__init__.py Outdated Show resolved Hide resolved

nishakm reviewed Jun 5, 2020

View reviewed changes

Enable analysis for OCI images

4295628

This commit proposes changes to analyze OCI images using tern. Work towards: #678 Signed-off-by: mukultaneja <[email protected]>

rnjudge reviewed Jun 8, 2020

View reviewed changes

mukultaneja closed this Jun 17, 2020

rnjudge mentioned this pull request Apr 28, 2021

Use skopeo to pull container images #948

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable analysis for OCI images #712

Enable analysis for OCI images #712

mukultaneja commented Jun 1, 2020

mukultaneja commented Jun 1, 2020 •

edited

Loading

rnjudge Jun 1, 2020 •

edited

Loading

mukultaneja Jun 2, 2020

nishakm Jun 3, 2020

mukultaneja Jun 4, 2020

rnjudge Jun 1, 2020 •

edited

Loading

rnjudge Jun 1, 2020

mukultaneja Jun 4, 2020

nishakm commented Jun 3, 2020 •

edited

Loading

nishakm Jun 5, 2020

rnjudge Jun 5, 2020

nishakm Jun 5, 2020

rnjudge Jun 5, 2020

mukultaneja Jun 6, 2020

rnjudge Jun 8, 2020

rnjudge Jun 8, 2020 •

edited

Loading

nishakm Jun 8, 2020

nishakm commented Jun 9, 2020

nishakm commented Jun 9, 2020

mukultaneja commented Jun 9, 2020

nishakm commented Jun 9, 2020

nishakm commented Jun 9, 2020

		@@ -69,6 +69,35 @@ def analyze(image_obj, args, dfile_lock=False, dfobj=None):
		analyze_docker_image(image_obj, args.redo, dfile_lock, dfobj)


		def execute_oci_image(args):

		if general.check_tar(args.image):
		logger.error("%s", errors.incorrect_raw_option)

Enable analysis for OCI images #712

Enable analysis for OCI images #712

Conversation

mukultaneja commented Jun 1, 2020

mukultaneja commented Jun 1, 2020 • edited Loading

rnjudge Jun 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rnjudge Jun 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nishakm commented Jun 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rnjudge Jun 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nishakm commented Jun 9, 2020

nishakm commented Jun 9, 2020

mukultaneja commented Jun 9, 2020

nishakm commented Jun 9, 2020

nishakm commented Jun 9, 2020

mukultaneja commented Jun 1, 2020 •

edited

Loading

rnjudge Jun 1, 2020 •

edited

Loading

rnjudge Jun 1, 2020 •

edited

Loading

nishakm commented Jun 3, 2020 •

edited

Loading

rnjudge Jun 8, 2020 •

edited

Loading