diff --git a/app/models/container_image.rb b/app/models/container_image.rb index 9b2e908af87..940db2a280f 100644 --- a/app/models/container_image.rb +++ b/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 diff --git a/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb b/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb index f574cac2b65..737f1e8ea79 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb +++ b/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 + (?#{ContainerImage::DOCKER_PULLABLE_PREFIX})? (?:(?: (?([^\.:/]+\.)+[^\.:/]+)| (?:(?[^:/]+)(?::(?\d+)))| (?localhost) )/)? (?(?:[^:/@]+/)*[^/:@]+) - (?:(?::(?.+))|(?:\@(?.+)))? + (?::(?[^:/@]+))? + (?:\@(?.+))? \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 diff --git a/app/models/manageiq/providers/kubernetes/container_manager/scanning/job.rb b/app/models/manageiq/providers/kubernetes/container_manager/scanning/job.rb index fa29cc8d986..d79be9514db 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/scanning/job.rb +++ b/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 diff --git a/spec/models/container_image_spec.rb b/spec/models/container_image_spec.rb index 2282d3a5c5a..91fbb52f5bd 100644 --- a/spec/models/container_image_spec.rb +++ b/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 diff --git a/spec/models/manageiq/providers/kubernetes/container_manager/refresh_parser_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager/refresh_parser_spec.rb index 07bbcb4e3b7..7b3c96d394b 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager/refresh_parser_spec.rb +++ b/spec/models/manageiq/providers/kubernetes/container_manager/refresh_parser_spec.rb @@ -131,6 +131,18 @@ :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}, @@ -138,7 +150,7 @@ 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]) diff --git a/spec/models/manageiq/providers/kubernetes/container_manager/scanning/job_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager/scanning/job_spec.rb index 4a3c463ac70..2377bd754ad 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager/scanning/job_spec.rb +++ b/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