[EUWE] Identifying container images by registry, name and digest

app/models/container_image.rb

@@ -36,6 +36,7 @@ class ContainerImage < ApplicationRecord
   after_create :raise_creation_event
 
   def full_name
+    return docker_id if image_ref && image_ref.start_with?(DOCKER_PULLABLE_PREFIX)
     result = ""
     result << "#{container_image_registry.full_name}/" unless container_image_registry.nil?
     result << name

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

@@ -25,16 +25,10 @@ def ems_inv_to_hashes(inventory)
       get_endpoints(inventory)
       get_services(inventory)
       get_component_statuses(inventory)
-      get_images
       EmsRefresh.log_inv_debug_trace(@data, "data:")
       @data
     end
 
-    def get_images
-      images = @data_index.fetch_path(:container_image, :by_ref_and_registry_host_port).try(:values) || []
-      process_collection(images, :container_images) { |n| n }
-    end
-
     def get_nodes(inventory)
       process_collection(inventory["node"], :container_nodes) { |n| parse_node(n) }
       @data[:container_nodes].each do |cn|
@@ -663,13 +657,17 @@ def parse_container_image(image, imageID)
         end
       end
 
+      # old docker references won't yield a digest and will always be distinct
+      container_image_identity = container_image[:digest] || container_image[:image_ref]
       stored_container_image = @data_index.fetch_path(
-        :container_image, :by_ref_and_registry_host_port,  "#{host_port}:#{container_image[:image_ref]}")
+        :container_image, :by_digest, container_image_identity)
 
       if stored_container_image.nil?
         @data_index.store_path(
-          :container_image, :by_ref_and_registry_host_port,
-          "#{host_port}:#{container_image[:image_ref]}", container_image)
+          :container_image, :by_digest,
+          container_image_identity, container_image
+        )
+        process_collection_item(container_image, :container_images) { |img| img }
         stored_container_image = container_image
       end
 
@@ -740,6 +738,15 @@ def parse_image_name(image, image_ref)
       image_ref_parts = image_definition_re.match(image_ref)
 
       hostname = image_parts[:host] || image_parts[:host2] || image_parts[:localhost]
+      if image_ref.start_with?(ContainerImage::DOCKER_IMAGE_PREFIX) && image_parts[:digest]
+        image_ref = "%{prefix}%{registry}%{name}@%{digest}" % {
+          :prefix   => ContainerImage::DOCKER_PULLABLE_PREFIX,
+          :registry => ("#{hostname}:#{image_parts[:port]}/" if hostname && image_parts[:port]),
+          :name     => image_parts[:name],
+          :digest   => image_parts[:digest],
+        }
+      end
+
       [
         {
           :name          => image_parts[:name],

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

@@ -14,13 +14,7 @@ def ems_inv_to_hashes(inventory)
       end
 
       def get_openshift_images(inventory)
-        process_collection(inventory["image"], :container_images) { |n| parse_openshift_image(n) }
-        @data[:container_images].uniq! { |n| n[:image_ref] }
-
-        @data[:container_images].each do |ns|
-          @data_index.store_path(:container_images, :by_name, ns[:name], ns)
-          @data_index.store_path(:container_images, :by_image_ref, ns[:image_ref], ns)
-        end
+        inventory["image"].each { |img| parse_openshift_image(img) }
       end
 
       def get_builds(inventory)

spec/models/container_image_spec.rb

@@ -9,6 +9,9 @@
     reg = ContainerImageRegistry.new(:name => "docker.io", :host => "docker.io", :port => "1234")
     image.container_image_registry = reg
     expect(image.full_name).to eq("docker.io:1234/fedora:v1")
+
+    image.image_ref = "docker-pullable://registry/repo/name@id"
+    expect(image.full_name).to eq("registry/repo/name@id")
   end
 
   it "#display_registry" do

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

@@ -39,118 +39,139 @@
   describe "parse_image_name" do
     example_ref = "docker://abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"
     example_images = [{:image_name => "example",
+                       :image_ref  => example_ref,
                        :image      => {:name => "example", :tag => nil, :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => nil},
 
                       {:image_name => "example:tag",
+                       :image_ref  => example_ref,
                        :image      => {:name => "example", :tag => "tag", :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => nil},
 
                       {:image_name => "user/example",
+                       :image_ref  => example_ref,
                        :image      => {:name => "user/example", :tag => nil, :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => nil},
 
                       {:image_name => "user/example:tag",
+                       :image_ref  => example_ref,
                        :image      => {:name => "user/example", :tag => "tag", :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => nil},
 
                       {:image_name => "example/subname/example",
+                       :image_ref  => example_ref,
                        :image      => {:name => "example/subname/example", :tag => nil, :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => nil},
 
                       {:image_name => "example/subname/example:tag",
+                       :image_ref  => example_ref,
                        :image      => {:name => "example/subname/example", :tag => "tag", :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => nil},
 
                       {:image_name => "host:1234/subname/example",
+                       :image_ref  => example_ref,
                        :image      => {:name => "subname/example", :tag => nil, :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => {:name => "host", :host => "host", :port => "1234"}},
 
                       {:image_name => "host:1234/subname/example:tag",
+                       :image_ref  => example_ref,
                        :image      => {:name => "subname/example", :tag => "tag", :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => {:name => "host", :host => "host", :port => "1234"}},
 
                       {:image_name => "host.com:1234/subname/example",
+                       :image_ref  => example_ref,
                        :image      => {:name => "subname/example", :tag => nil, :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => {:name => "host.com", :host => "host.com", :port => "1234"}},
 
                       {:image_name => "host.com:1234/subname/example:tag",
+                       :image_ref  => example_ref,
                        :image      => {:name => "subname/example", :tag => "tag", :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => {:name => "host.com", :host => "host.com", :port => "1234"}},
 
                       {:image_name => "host.com/subname/example",
+                       :image_ref  => example_ref,
                        :image      => {:name => "subname/example", :tag => nil, :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => {:name => "host.com", :host => "host.com", :port => nil}},
 
                       {:image_name => "host.com/example",
+                       :image_ref  => example_ref,
                        :image      => {:name => "example", :tag => nil, :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => {:name => "host.com", :host => "host.com", :port => nil}},
 
                       {:image_name => "host.com:1234/subname/more/names/example:tag",
+                       :image_ref  => example_ref,
                        :image      => {:name => "subname/more/names/example", :tag => "tag", :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => {:name => "host.com", :host => "host.com", :port => "1234"}},
 
                       {:image_name => "localhost:1234/name",
+                       :image_ref  => example_ref,
                        :image      => {:name => "name", :tag => nil, :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => {:name => "localhost", :host => "localhost", :port => "1234"}},
 
                       {:image_name => "localhost:1234/name@sha256:1234567abcdefg",
+                       :image_ref  => example_ref,
                        :image      => {:name => "name", :tag => nil, :digest => "sha256:1234567abcdefg",
-                                       :image_ref => example_ref},
+                                       :image_ref => "docker-pullable://localhost:1234/name@sha256:1234567abcdefg"},
                        :registry   => {:name => "localhost", :host => "localhost", :port => "1234"}},
 
                       # host with no port. more than one subdomain (a.b.c.com)
                       {:image_name => "reg.access.rh.com/openshift3/image-inspector",
+                       :image_ref  => example_ref,
                        :image      => {:name => "openshift3/image-inspector", :tag => nil, :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => {:name => "reg.access.rh.com", :host => "reg.access.rh.com", :port => nil}},
 
                       # host with port. more than one subdomain (a.b.c.com:1234)
                       {:image_name => "host.access.com:1234/subname/more/names/example:tag",
+                       :image_ref  => example_ref,
                        :image      => {:name => "subname/more/names/example", :tag => "tag", :digest => nil,
                                        :image_ref => example_ref},
                        :registry   => {:name => "host.access.com", :host => "host.access.com", :port => "1234"}},
 
                       # localhost no port
                       {:image_name => "localhost/name",
+                       :image_ref  => example_ref,
                        :image      => {:name => "name", :tag => nil, :digest => nil,
                                        :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_ref  => example_ref,
                        :image      => {:name => "name1", :tag => "tagos", :digest => "sha256:123abcdef",
-                                       :image_ref => example_ref},
+                                       :image_ref => "docker-pullable://reg.example.com:1234/name1@sha256:123abcdef"},
                        :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_ref  => "docker-pullable://reg.example.com:1234/name1@sha256:321bcd",
                        :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_ref  => example_ref,
                        :image      => {:name => "example", :tag => nil, :digest => "sha256:1234567abcdefg",
-                                       :image_ref => example_ref},
+                                       :image_ref => "docker-pullable://example@sha256:1234567abcdefg"},
                        :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], ex[:image][:image_ref])
+        result_image, result_registry = parser.send(:parse_image_name, ex[:image_name], ex[:image_ref])
 
         expect(result_image.except(:registered_on)).to eq(ex[:image])
         expect(result_registry).to eq(ex[:registry])
@@ -595,10 +616,11 @@
   describe "parse_container_image" do
     shared_image_without_host = "shared/image"
     shared_image_with_host = "host:1234/shared/image"
-    shared_ref = "shared:ref"
-    unique_ref = "unique:ref"
+    shared_ref = "docker-pullable://host:1234/repo/image@sha256:123456"
+    other_registry_ref = "docker-pullable://other-host:4321/repo/image@sha256:123456"
+    unique_ref = "docker-pullable://host:1234/repo/image@sha256:abcdef"
 
-    it "returns unique object *identity* for same image but different ref/id" do
+    it "returns unique object *identity* for same image but different digest" do
       [shared_image_with_host, shared_image_without_host].each do |shared_image|
         first_obj  = parser.parse_container_image(shared_image, shared_ref)
         second_obj = parser.parse_container_image(shared_image, unique_ref)
@@ -607,7 +629,7 @@
       end
     end
 
-    it "returns unique object *content* for same image but different ref/id" do
+    it "returns unique object *content* for same image but different digest" do
       [shared_image_with_host, shared_image_without_host].each do |shared_image|
         first_obj  = parser.parse_container_image(shared_image, shared_ref)
         second_obj = parser.parse_container_image(shared_image, unique_ref)
@@ -616,14 +638,23 @@
       end
     end
 
-    it "returns same object *identity* for same image and ref/id" do
+    it "returns same object *identity* for same digest" do
       [shared_image_with_host, shared_image_without_host].each do |shared_image|
         first_obj  = parser.parse_container_image(shared_image, shared_ref)
         second_obj = parser.parse_container_image(shared_image, shared_ref)
 
         expect(first_obj).to be(second_obj)
       end
     end
+
+    it "returns same object *identity* for same digest and different repo" do
+      [shared_image_with_host, shared_image_without_host].each do |shared_image|
+        first_obj  = parser.parse_container_image(shared_image, other_registry_ref)
+        second_obj = parser.parse_container_image(shared_image, shared_ref)
+
+        expect(first_obj).to be(second_obj)
+      end
+    end
   end
 
   describe "cross_link_node" do

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

@@ -154,19 +154,48 @@
       },]
       parser.instance_variable_get('@data_index').store_path(
         :container_image,
-        :by_ref_and_registry_host_port,
-        "#{image_registry}:#{image_registry_port}:#{image_ref}",
+        :by_digest,
+        image_digest,
         parser.instance_variable_get('@data')[:container_images][0])
 
       inventory = {"image" => [image_from_openshift,]}
 
       parser.get_openshift_images(inventory)
       expect(parser.instance_variable_get('@data')[:container_images].size).to eq(1)
       expect(parser.instance_variable_get('@data')[:container_images][0]).to eq(
-        parser.instance_variable_get('@data_index')[:container_image][:by_ref_and_registry_host_port].values[0])
+        parser.instance_variable_get('@data_index')[:container_image][:by_digest].values[0])
       expect(parser.instance_variable_get('@data')[:container_images][0][:architecture]).to eq('amd64')
     end
 
+    it "matches images by digest" do
+      FIRST_NAME = "first_name".freeze
+      FIRST_TAG = "first_tag".freeze
+      FIRST_REF = "first_ref".freeze
+      parser.instance_variable_get('@data')[:container_images] = [{
+        :name          => FIRST_NAME,
+        :tag           => FIRST_TAG,
+        :digest        => image_digest,
+        :image_ref     => FIRST_REF,
+        :registered_on => Time.now.utc - 2.minutes
+      },]
+      parser.instance_variable_get('@data_index').store_path(
+        :container_image,
+        :by_digest,
+        image_digest,
+        parser.instance_variable_get('@data')[:container_images][0]
+      )
+
+      inventory = {"image" => [image_from_openshift,]}
+
+      parser.get_openshift_images(inventory)
+      expect(parser.instance_variable_get('@data')[:container_images].size).to eq(1)
+      expect(parser.instance_variable_get('@data')[:container_images][0]).to eq(
+        parser.instance_variable_get('@data_index')[:container_image][:by_digest].values[0]
+      )
+      expect(parser.instance_variable_get('@data')[:container_images][0][:architecture]).to eq('amd64')
+      expect(parser.instance_variable_get('@data')[:container_images][0][:name]).to eq(FIRST_NAME)
+    end
+
     context "image registries from openshift images" do
       def parse_single_openshift_image_with_registry
         inventory = {"image" => [image_from_openshift]}