Merge pull request ManageIQ#12711 from enoodle/docker_pullable_contai…

…ner_images_ids

handling docker-pullable image ids

app/models/container_image.rb

@@ -6,6 +6,8 @@ class ContainerImage < ApplicationRecord
 
 
   DOCKER_IMAGE_PREFIX = "docker://"
+  DOCKER_PULLABLE_PREFIX = "docker-pullable://".freeze
+  DOCKER_PREFIXES = [DOCKER_IMAGE_PREFIX, DOCKER_PULLABLE_PREFIX].freeze
 
   belongs_to :container_image_registry
   belongs_to :ext_management_system, :foreign_key => "ems_id"
@@ -45,9 +47,10 @@ def operating_system=(value)
   end
 
   def docker_id
-    if image_ref.start_with?(DOCKER_IMAGE_PREFIX)
-      return image_ref[DOCKER_IMAGE_PREFIX.length..-1]
+    DOCKER_PREFIXES.each do |prefix|
+      return image_ref[prefix.length..-1] if image_ref.start_with?(prefix)
     end
+    nil
   end
 
   # The guid is required by the smart analysis infrastructure

app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb

@@ -728,31 +728,35 @@ def parse_base_item(item)
     def parse_image_name(image, image_ref)
       # parsing using same logic as in docker
       # https://github.com/docker/docker/blob/348f6529b71502b561aa493e250fd5be248da0d5/reference/reference.go#L174
-      parts = %r{
+      image_definition_re = %r{
         \A
+          (?<protocol>#{ContainerImage::DOCKER_PULLABLE_PREFIX})?
           (?:(?:
             (?<host>([^\.:/]+\.)+[^\.:/]+)|
             (?:(?<host2>[^:/]+)(?::(?<port>\d+)))|
             (?<localhost>localhost)
           )/)?
           (?<name>(?:[^:/@]+/)*[^/:@]+)
-          (?:(?::(?<tag>.+))|(?:\@(?<digest>.+)))?
+          (?::(?<tag>[^:/@]+))?
+          (?:\@(?<digest>.+))?
         \z
-      }x.match(image)
+      }x
+      image_parts = image_definition_re.match(image)
+      image_ref_parts = image_definition_re.match(image_ref)
 
-      hostname = parts[:host] || parts[:host2] || parts[:localhost]
+      hostname = image_parts[:host] || image_parts[:host2] || image_parts[:localhost]
       [
         {
-          :name          => parts[:name],
-          :tag           => parts[:tag],
-          :digest        => parts[:digest],
+          :name          => image_parts[:name],
+          :tag           => image_parts[:tag],
+          :digest        => image_parts[:digest] || (image_ref_parts[:digest] if image_ref_parts),
           :image_ref     => image_ref,
           :registered_on => Time.now.utc
         },
         hostname && {
           :name => hostname,
           :host => hostname,
-          :port => parts[:port],
+          :port => image_parts[:port],
         },
       ]
     end

app/models/manageiq/providers/kubernetes/container_manager/scanning/job.rb

@@ -113,6 +113,23 @@ def pod_wait
     end
   end
 
+  def verify_scanned_image_id
+    metadata = image_inspector_client.fetch_metadata
+    actual = metadata.Id
+    return nil if actual == options[:docker_image_id]
+    msg = "cannot analyze image %s with id %s: detected ids were %s" % [
+      options[:image_full_name], options[:docker_image_id][0..11], actual[0..11]]
+
+    if metadata.RepoDigests
+      metadata.RepoDigests.each do |repo_digest|
+        return nil if repo_digest == options[:docker_image_id]
+        msg << repo_digest.split('@')[0..11] + ", "
+      end
+    end
+
+    msg
+  end
+
   def analyze
     image = target_entity
     return queue_signal(:abort_job, "no image found", "error") unless image
@@ -126,13 +143,10 @@ def analyze
       :guest_os      => IMAGES_GUEST_OS
     }
 
-    actual = image_inspector_client.fetch_metadata.Id
-    if actual != options[:docker_image_id]
-      msg = "cannot analyze image %s with id %s: detected id was %s"
-      _log.error(msg % [options[:image_full_name], options[:docker_image_id], actual])
-      return queue_signal(:abort_job,
-                          msg % [options[:image_full_name], options[:docker_image_id][0..11], actual[0..11]],
-                          'error')
+    verify_error = verify_scanned_image_id
+    if verify_error
+      _log.error(verify_error)
+      return queue_signal(:abort_job, verify_error, 'error')
     end
 
     collect_compliance_data(image)
@@ -402,7 +416,7 @@ def add_secret_to_pod_def(pod_def, inspector_admin_secret_name)
   end
 
   def inspector_image
-    'docker.io/openshift/image-inspector:2.0'
+    'docker.io/openshift/image-inspector:2.1'
   end
 
   def inspector_proxy_env_variables

spec/models/container_image_spec.rb

@@ -24,6 +24,9 @@
     image = FactoryGirl.create(:container_image, :image_ref => "docker://id")
     expect(image.docker_id).to eq("id")
 
+    image = FactoryGirl.create(:container_image, :image_ref => "docker-pullable://repo/name@id")
+    expect(image.docker_id).to eq("repo/name@id")
+
     image = FactoryGirl.create(:container_image, :image_ref => "rocket://id")
     expect(image.docker_id).to eq(nil)
   end

spec/models/manageiq/providers/kubernetes/container_manager/refresh_parser_spec.rb

@@ -131,14 +131,26 @@
                                        :image_ref => example_ref},
                        :registry   => {:name => "localhost", :host => "localhost", :port => nil}},
 
+                      # tag and digest together
+                      {:image_name => "reg.example.com:1234/name1:tagos@sha256:123abcdef",
+                       :image      => {:name => "name1", :tag => "tagos", :digest => "sha256:123abcdef",
+                                       :image_ref => example_ref},
+                       :registry   => {:name => "reg.example.com", :host => "reg.example.com", :port => "1234"}},
+
+                      # digest from new docker-pullable
+                      {:image_name => "reg.example.com:1234/name1:tagos",
+                       :image      => {:name => "name1", :tag => "tagos", :digest => "sha256:321bcd",
+                                       :image_ref => "docker-pullable://reg.example.com:1234/name1@sha256:321bcd"},
+                       :registry   => {:name => "reg.example.com", :host => "reg.example.com", :port => "1234"}},
+
                       {:image_name => "example@sha256:1234567abcdefg",
                        :image      => {:name => "example", :tag => nil, :digest => "sha256:1234567abcdefg",
                                        :image_ref => example_ref},
                        :registry   => nil}]
 
     example_images.each do |ex|
       it "tests '#{ex[:image_name]}'" do
-        result_image, result_registry = parser.send(:parse_image_name, ex[:image_name], example_ref)
+        result_image, result_registry = parser.send(:parse_image_name, ex[:image_name], ex[:image][:image_ref])
 
         expect(result_image.except(:registered_on)).to eq(ex[:image])
         expect(result_registry).to eq(ex[:registry])

spec/models/manageiq/providers/kubernetes/container_manager/scanning/job_spec.rb

@@ -44,12 +44,23 @@ def auth_options(*_args)
 end
 
 class MockImageInspectorClient
-  def initialize(for_id)
+  def initialize(for_id, repo_digest = nil)
     @for_id = for_id
+    @repo_digest = repo_digest
   end
 
   def fetch_metadata(*_args)
-    OpenStruct.new('Id' => @for_id)
+    if @repo_digest
+      OpenStruct.new('Id' => @for_id, 'RepoDigests' => [@repo_digest])
+    else
+      OpenStruct.new('Id' => @for_id)
+    end
+  end
+
+  def fetch_oscap_arf
+    File.read(
+      File.expand_path(File.join(File.dirname(__FILE__), "ssg-fedora-ds-arf.xml"))
+    ).encode("UTF-8")
   end
 end
 
@@ -109,15 +120,7 @@ def fetch_metadata(*_args)
 
     context "completes successfully" do
       before(:each) do
-        if OpenscapResult.openscap_available?
-          allow_any_instance_of(MockImageInspectorClient).to receive(:fetch_oscap_arf) do
-            File.read(
-              File.expand_path(File.join(File.dirname(__FILE__), "ssg-fedora-ds-arf.xml"))
-            ).encode("UTF-8")
-          end
-        else
-          allow_any_instance_of(described_class).to receive_messages(:collect_compliance_data)
-        end
+        allow_any_instance_of(described_class).to receive_messages(:collect_compliance_data) unless OpenscapResult.openscap_available?
 
         VCR.use_cassette(described_class.name.underscore, :record => :none) do # needed for health check
           expect(@job.state).to eq 'waiting_to_start'
@@ -132,11 +135,6 @@ def fetch_metadata(*_args)
 
       it 'should persist openscap data' do
         skip unless OpenscapResult.openscap_available?
-        allow_any_instance_of(MockImageInspectorClient).to receive(:fetch_oscap_arf) do
-          File.read(
-            File.expand_path(File.join(File.dirname(__FILE__), "ssg-fedora-ds-arf.xml"))
-          ).encode("UTF-8")
-        end
 
         expect(@image.openscap_result).to be
         expect(@image.openscap_result.binary_blob.md5).to eq('d1f1857281573cd777b31d76e8529dc9')
@@ -235,19 +233,30 @@ def fetch_metadata(*_args)
     end
 
     context 'when the image tag points to a different image' do
+      MODIFIED_IMAGE_ID = '0d071bb732e1e3eb1e01629600c9b6c23f2b26b863b5321335f564c8f018c452'.freeze
       before(:each) do
-        MODIFIED_IMAGE_ID = '0d071bb732e1e3eb1e01629600c9b6c23f2b26b863b5321335f564c8f018c452'.freeze
         allow_any_instance_of(described_class).to receive_messages(
           :image_inspector_client => MockImageInspectorClient.new(MODIFIED_IMAGE_ID))
       end
 
+      it 'should check for repo_digests' do
+        allow_any_instance_of(described_class).to receive_messages(:collect_compliance_data) unless OpenscapResult.openscap_available?
+        allow_any_instance_of(described_class).to receive_messages(
+          :image_inspector_client => MockImageInspectorClient.new(MODIFIED_IMAGE_ID, IMAGE_ID))
+        VCR.use_cassette(described_class.name.underscore, :record => :none) do # needed for health check
+          @job.signal(:start)
+          expect(@job.state).to eq 'finished'
+          expect(@job.status).to eq 'ok'
+        end
+      end
+
       it 'should report the error' do
         VCR.use_cassette(described_class.name.underscore, :record => :none) do # needed for health check
           @job.signal(:start)
           expect(@job.state).to eq 'finished'
           expect(@job.status).to eq 'error'
           expect(@job.message).to eq "cannot analyze image #{IMAGE_NAME} with id #{IMAGE_ID[0..11]}:"\
-                                     " detected id was #{MODIFIED_IMAGE_ID[0..11]}"
+                                     " detected ids were #{MODIFIED_IMAGE_ID[0..11]}"
         end
       end
     end