diff --git a/Gemfile b/Gemfile index b92973f5e..df40b5160 100644 --- a/Gemfile +++ b/Gemfile @@ -59,6 +59,7 @@ gem 'verbose_migrations' gem 'temporary_tables' gem 'statement_timeout' gem 'union_of' +gem 'order_as_specified' # Pattern matching gem 'rails-pattern_matching' diff --git a/Gemfile.lock b/Gemfile.lock index b42476589..736a272f9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -328,6 +328,8 @@ GEM rails (>= 6.0) oj (3.13.23) openssl (3.1.0) + order_as_specified (1.7) + activerecord (>= 5.0.0) parallel (1.23.0) parallel_tests (4.2.1) parallel @@ -560,6 +562,7 @@ DEPENDENCIES null_association oj openssl (~> 3.1.0) + order_as_specified parallel_tests (~> 4.2.1) pg (~> 1.3.4) premailer (~> 1.23.0) diff --git a/app/controllers/api/v1/release_engines/oci/manifests_controller.rb b/app/controllers/api/v1/release_engines/oci/manifests_controller.rb index c3ccef2e1..ac8602f1a 100644 --- a/app/controllers/api/v1/release_engines/oci/manifests_controller.rb +++ b/app/controllers/api/v1/release_engines/oci/manifests_controller.rb @@ -10,7 +10,7 @@ class Oci::ManifestsController < Api::V1::BaseController def show authorize! package - manifest = package.manifests.find_by_reference!(params[:reference]) + manifest = package.manifests.find_by_reference!(params[:reference], content_type: request.accepts.collect(&:to_s)) authorize! manifest.artifact # for etag support diff --git a/app/models/release_artifact.rb b/app/models/release_artifact.rb index e68055c27..4cbe791dc 100644 --- a/app/models/release_artifact.rb +++ b/app/models/release_artifact.rb @@ -38,6 +38,12 @@ class ReleaseArtifact < ApplicationRecord inverse_of: :artifacts, autosave: true, optional: true + has_many :manifests, + class_name: 'ReleaseManifest', + foreign_key: :release_artifact_id, + inverse_of: :artifact, + dependent: :delete_all + # NOTE(ezekg) not a strict has-one but this is a convenience has_one :manifest, class_name: 'ReleaseManifest', foreign_key: :release_artifact_id, diff --git a/app/models/release_manifest.rb b/app/models/release_manifest.rb index 079e2fbaf..3274baed7 100644 --- a/app/models/release_manifest.rb +++ b/app/models/release_manifest.rb @@ -10,6 +10,8 @@ class ReleaseManifest < ApplicationRecord include Environmental include Accountable + extend OrderAsSpecified + belongs_to :artifact, class_name: 'ReleaseArtifact', foreign_key: :release_artifact_id, @@ -27,7 +29,6 @@ class ReleaseManifest < ApplicationRecord has_account default: -> { artifact&.account_id } validates :artifact, - uniqueness: { message: 'already exists', scope: %i[release_artifact_id] }, scope: { by: :account_id } validates :release, @@ -50,13 +51,38 @@ class ReleaseManifest < ApplicationRecord def as_gemspec = Gem::Specification.from_yaml(content) def as_package_json = JSON.parse(content) - def self.find_by_reference!(reference) + # find_by_reference is used for the oci engine to lookup a manifest by reference, + # which could be a digest, version, or tag, and also to request a specific media + # type in case of multiple manifests per-image. + def self.find_by_reference!(reference, content_type: nil) base = joins(:release) - base.where(content_digest: reference) - .or(base.where(release: { version: reference })) - .or(base.where(release: { tag: reference })) - .take! + manifests = base.where(content_digest: reference) + .or(base.where(release: { version: reference })) + .or(base.where(release: { tag: reference })) + + # oci clients may want a specific media type in case of multiple manifests + unless content_type.blank? + manifests = case content_type + in ['*/*', *] | '*/*' # has no preference i.e. accepts anything + manifests.where.not(content_type: nil) + in [*content_types, '*/*'] # has preference but accepts anything + manifests.where(content_type: content_types) + .or(manifests.where.not(content_type: nil)) + .order_as_specified( + content_type: content_types, + ) + in [*content_types] # has preferences + manifests.where(content_type: content_types) + .order_as_specified( + content_type: content_types, + ) + else # has preference + manifests.where(content_type:) + end + end + + manifests.take! end def self.find_by_reference(...) diff --git a/app/workers/process_npm_package_worker.rb b/app/workers/process_npm_package_worker.rb index 2cb351174..c8cebd591 100644 --- a/app/workers/process_npm_package_worker.rb +++ b/app/workers/process_npm_package_worker.rb @@ -64,6 +64,7 @@ def perform(artifact_id) content_digest: "sha512-#{Digest::SHA512.hexdigest(content)}", content_type: 'application/vnd.npm.install-v1+json', content_length: content.bytesize, + content_path: 'package.json', content:, ) end diff --git a/app/workers/process_oci_image_worker.rb b/app/workers/process_oci_image_worker.rb index 40d808b2c..df24ec91e 100644 --- a/app/workers/process_oci_image_worker.rb +++ b/app/workers/process_oci_image_worker.rb @@ -43,22 +43,29 @@ def perform(artifact_id) next unless item.exists? - case item - in OciImageLayout::Index => index + # store manifests as a manifest for easy reference without hitting the storage provider + if item in OciImageLayout::Index | OciImageLayout::Manifest => manifest raise ImageNotAcceptableError, 'manifest is too big' if - index.size > MAX_MANIFEST_SIZE + manifest.size > MAX_MANIFEST_SIZE ReleaseManifest.create!( account_id: artifact.account_id, environment_id: artifact.environment_id, release_id: artifact.release_id, release_artifact_id: artifact.id, - content_type: index.media_type, - content_digest: index.digest, - content_length: index.size, - content: index.read, + content_type: manifest.media_type, + content_digest: manifest.digest, + content_length: manifest.size, + content_path: manifest.path, + content: manifest.read, ) - in OciImageLayout::Blob => blob + + # still need to store it as a blob + manifest.rewind + end + + # store blobs as a descriptor pointing to the storage provider + if item in OciImageLayout::Blob => blob raise ImageNotAcceptableError, 'blob is too big' if blob.size > MAX_BLOB_SIZE diff --git a/app/workers/process_ruby_gem_worker.rb b/app/workers/process_ruby_gem_worker.rb index c852142a6..b9a700ea6 100644 --- a/app/workers/process_ruby_gem_worker.rb +++ b/app/workers/process_ruby_gem_worker.rb @@ -43,6 +43,7 @@ def perform(artifact_id) content_digest: "sha256-#{Digest::SHA256.hexdigest(yaml)}", content_type: 'application/x-yaml', content_length: yaml.bytesize, + content_path: gemspec.file_name, content: yaml, ) diff --git a/config/initializers/mime_types.rb b/config/initializers/mime_types.rb index d921a8eb0..89eccd865 100644 --- a/config/initializers/mime_types.rb +++ b/config/initializers/mime_types.rb @@ -5,11 +5,6 @@ application/vnd.keygen+json ] -Mime::Type.register 'application/vnd.oci+json', :oci, %W[ - application/vnd.oci.image.manifest.v1+json - application/vnd.oci.image.index.v1+json -] - # adds pattern matching class Mime::Type def deconstruct_keys(*) = { string:, symbol:, synonyms: } diff --git a/config/routes.rb b/config/routes.rb index 93fa0ec38..9d7839d6f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -108,7 +108,7 @@ concern :oci do # NOTE(ezekg) /v2 namespace is handled outside of this because docker wants it to always be at the root... - scope module: :oci, defaults: { format: :oci } do + scope module: :oci, defaults: { format: :binary } do # see: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pulling-manifests match ':package/manifests/:reference', via: %i[head get], to: 'manifests#show', as: :oci_manifest, constraints: { package: /[^\/]*/, diff --git a/db/migrate/20241114163421_add_content_attributes_to_release_manifests.rb b/db/migrate/20241114163421_add_content_attributes_to_release_manifests.rb index 9bb07ecfe..438fd4bfe 100644 --- a/db/migrate/20241114163421_add_content_attributes_to_release_manifests.rb +++ b/db/migrate/20241114163421_add_content_attributes_to_release_manifests.rb @@ -2,7 +2,7 @@ class AddContentAttributesToReleaseManifests < ActiveRecord::Migration[7.2] disable_ddl_transaction! def change - # FIXME(ezekg) eventually make these non-null? + add_column :release_manifests, :content_path, :string, null: true add_column :release_manifests, :content_type, :string, null: true add_column :release_manifests, :content_length, :bigint, null: true add_column :release_manifests, :content_digest, :string, null: true diff --git a/db/migrate/20241114214652_remove_unique_index_from_release_manifests.rb b/db/migrate/20241114214652_remove_unique_index_from_release_manifests.rb new file mode 100644 index 000000000..7bfc37375 --- /dev/null +++ b/db/migrate/20241114214652_remove_unique_index_from_release_manifests.rb @@ -0,0 +1,12 @@ +class RemoveUniqueIndexFromReleaseManifests < ActiveRecord::Migration[7.2] + disable_ddl_transaction! + + def change + remove_index :release_manifests, %i[release_artifact_id], + algorithm: :concurrently, + unique: true + + add_index :release_manifests, %i[release_artifact_id], + algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 8630c06b4..7ef5bcf5f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_11_14_163421) do +ActiveRecord::Schema[7.2].define(version: 2024_11_14_214652) do # These are extensions that must be enabled in order to support this database enable_extension "btree_gin" enable_extension "pg_stat_statements" @@ -610,13 +610,14 @@ t.jsonb "metadata" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "content_path" t.string "content_type" t.bigint "content_length" t.string "content_digest" t.index ["account_id", "created_at"], name: "index_release_manifests_on_account_id_and_created_at", order: { created_at: :desc } t.index ["environment_id"], name: "index_release_manifests_on_environment_id" t.index ["release_artifact_id", "content_digest"], name: "idx_on_release_artifact_id_content_digest_d7f016852f", unique: true - t.index ["release_artifact_id"], name: "index_release_manifests_on_release_artifact_id", unique: true + t.index ["release_artifact_id"], name: "index_release_manifests_on_release_artifact_id" t.index ["release_id"], name: "index_release_manifests_on_release_id" end diff --git a/lib/oci_image_layout.rb b/lib/oci_image_layout.rb index 4471f353b..9da1eca49 100644 --- a/lib/oci_image_layout.rb +++ b/lib/oci_image_layout.rb @@ -155,6 +155,7 @@ def initialize(io, media_type:, digest:, size:, path:, fs:) def exists? = io.present? def read(...) = (io.read(...) if exists?) + def seek(...) = (io.seek(...) if exists?) def rewind = (io.rewind if exists?) def close = (io.close if exists?) def closed? = (io.closed? if exists?) diff --git a/spec/workers/process_npm_package_worker_spec.rb b/spec/workers/process_npm_package_worker_spec.rb index b3edbb846..09d4c3011 100644 --- a/spec/workers/process_npm_package_worker_spec.rb +++ b/spec/workers/process_npm_package_worker_spec.rb @@ -52,6 +52,8 @@ it 'should process package' do expect { subject.perform_async(artifact.id) }.to change { artifact.reload.manifest } + expect(artifact.manifest.content_type).to eq 'application/vnd.npm.install-v1+json' + expect(artifact.manifest.content_path).to eq 'package.json' expect(artifact.manifest.content).to eq minified_package_json expect(artifact.status).to eq 'UPLOADED' end diff --git a/spec/workers/process_oci_image_worker_spec.rb b/spec/workers/process_oci_image_worker_spec.rb index c5e9176e1..cffe2ab81 100644 --- a/spec/workers/process_oci_image_worker_spec.rb +++ b/spec/workers/process_oci_image_worker_spec.rb @@ -17,14 +17,6 @@ context 'when artifact is a valid image' do let(:image_fixture) { 'alpine-3.20.3.tar' } let(:image_tarball) { file_fixture(image_fixture).open } - let(:image_index) { - tarball = file_fixture(image_fixture).open - - Minitar::Reader.open tarball do |archive| - archive.find { _1.file? && _1.name in 'index.json' } - .read - end - } before do Aws.config = { s3: { stub_responses: { get_object: [{ body: image_tarball }] } } } @@ -33,10 +25,12 @@ context 'when artifact is waiting' do let(:artifact) { create(:artifact, :oci_image, :waiting, account:) } + it 'should not change artifact status' do + expect { subject.perform_async(artifact.id) }.to_not change { artifact.reload.status } + end + it 'should not store manifest' do expect { subject.perform_async(artifact.id) }.to not_change { artifact.reload.manifest } - - expect(artifact.status).to eq 'WAITING' end it 'should not upload blobs' do @@ -47,14 +41,20 @@ context 'when artifact is processing' do let(:artifact) { create(:artifact, :oci_image, :processing, account:) } - it 'should store manifest' do - expect { subject.perform_async(artifact.id) }.to change { artifact.reload.manifest } + it 'should change artifact status' do + expect { subject.perform_async(artifact.id) }.to change { artifact.reload.status }.to('UPLOADED') + end - expect(artifact.manifest.content_digest).to eq 'sha256:355eee6af939abf5ba465c9be69c3b725f8d3f19516ca9644cf2a4fb112fd83b' - expect(artifact.manifest.content_type).to eq 'application/vnd.oci.image.index.v1+json' - expect(artifact.manifest.content_length).to eq 441 - expect(artifact.manifest.content).to eq image_index - expect(artifact.status).to eq 'UPLOADED' + it 'should store manifests' do + expect { subject.perform_async(artifact.id) }.to change { artifact.reload.manifests } + + expect(artifact.manifests).to satisfy { |manifests| + manifests in [ + ReleaseManifest(content_path: 'index.json', content_digest: 'sha256:355eee6af939abf5ba465c9be69c3b725f8d3f19516ca9644cf2a4fb112fd83b', content_type: 'application/vnd.oci.image.index.v1+json', content_length: 441, content: String), + ReleaseManifest(content_path: 'blobs/sha256/beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d', content_digest: 'sha256:beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d', content_type: 'application/vnd.docker.distribution.manifest.list.v2+json', content_length: 1853, content: String), + ReleaseManifest(content_path: 'blobs/sha256/33735bd63cf84d7e388d9f6d297d348c523c044410f553bd878c6d7829612735', content_digest: 'sha256:33735bd63cf84d7e388d9f6d297d348c523c044410f553bd878c6d7829612735', content_type: 'application/vnd.docker.distribution.manifest.v2+json', content_length: 528, content: String), + ] + } end context 'when blobs are not uploaded' do @@ -68,6 +68,7 @@ expect(artifact.descriptors).to satisfy { |descriptors| descriptors in [ ReleaseDescriptor(content_path: 'oci-layout', content_type: 'application/vnd.oci.layout.header.v1+json', content_digest: 'sha256:18f0797eab35a4597c1e9624aa4f15fd91f6254e5538c1e0d193b2a95dd4acc6', content_length: 30), + ReleaseDescriptor(content_path: 'index.json', content_type: 'application/vnd.oci.image.index.v1+json', content_digest: 'sha256:355eee6af939abf5ba465c9be69c3b725f8d3f19516ca9644cf2a4fb112fd83b', content_length: 441), ReleaseDescriptor(content_path: 'blobs/sha256/beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d', content_type: 'application/vnd.docker.distribution.manifest.list.v2+json', content_digest: 'sha256:beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d', content_length: 1853), ReleaseDescriptor(content_path: 'blobs/sha256/33735bd63cf84d7e388d9f6d297d348c523c044410f553bd878c6d7829612735', content_type: 'application/vnd.docker.distribution.manifest.v2+json', content_digest: 'sha256:33735bd63cf84d7e388d9f6d297d348c523c044410f553bd878c6d7829612735', content_length: 528), ReleaseDescriptor(content_path: 'blobs/sha256/43c4264eed91be63b206e17d93e75256a6097070ce643c5e8f0379998b44f170', content_type: 'application/vnd.docker.image.rootfs.diff.tar.gzip', content_digest: 'sha256:43c4264eed91be63b206e17d93e75256a6097070ce643c5e8f0379998b44f170', content_length: 3623807), @@ -79,6 +80,7 @@ it 'should upload blobs' do expect { subject.perform_async(artifact.id) }.to upload( { key: %r{oci-layout}, content_type: 'application/vnd.oci.layout.header.v1+json', content_length: 30 }, + { key: %r{index.json}, content_type: 'application/vnd.oci.image.index.v1+json', content_length: 441 }, { key: %r{blobs/sha256/beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d}, content_type: 'application/vnd.docker.distribution.manifest.list.v2+json', content_length: 1853 }, { key: %r{blobs/sha256/33735bd63cf84d7e388d9f6d297d348c523c044410f553bd878c6d7829612735}, content_type: 'application/vnd.docker.distribution.manifest.v2+json', content_length: 528 }, { key: %r{blobs/sha256/43c4264eed91be63b206e17d93e75256a6097070ce643c5e8f0379998b44f170}, content_type: 'application/vnd.docker.image.rootfs.diff.tar.gzip', content_length: 3623807 }, @@ -92,6 +94,21 @@ Aws.config[:s3][:stub_responses][:head_object] = [] end + it 'should store blobs' do + expect { subject.perform_async(artifact.id) }.to change { artifact.reload.descriptors } + + expect(artifact.descriptors).to satisfy { |descriptors| + descriptors in [ + ReleaseDescriptor(content_path: 'oci-layout', content_type: 'application/vnd.oci.layout.header.v1+json', content_digest: 'sha256:18f0797eab35a4597c1e9624aa4f15fd91f6254e5538c1e0d193b2a95dd4acc6', content_length: 30), + ReleaseDescriptor(content_path: 'index.json', content_type: 'application/vnd.oci.image.index.v1+json', content_digest: 'sha256:355eee6af939abf5ba465c9be69c3b725f8d3f19516ca9644cf2a4fb112fd83b', content_length: 441), + ReleaseDescriptor(content_path: 'blobs/sha256/beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d', content_type: 'application/vnd.docker.distribution.manifest.list.v2+json', content_digest: 'sha256:beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d', content_length: 1853), + ReleaseDescriptor(content_path: 'blobs/sha256/33735bd63cf84d7e388d9f6d297d348c523c044410f553bd878c6d7829612735', content_type: 'application/vnd.docker.distribution.manifest.v2+json', content_digest: 'sha256:33735bd63cf84d7e388d9f6d297d348c523c044410f553bd878c6d7829612735', content_length: 528), + ReleaseDescriptor(content_path: 'blobs/sha256/43c4264eed91be63b206e17d93e75256a6097070ce643c5e8f0379998b44f170', content_type: 'application/vnd.docker.image.rootfs.diff.tar.gzip', content_digest: 'sha256:43c4264eed91be63b206e17d93e75256a6097070ce643c5e8f0379998b44f170', content_length: 3623807), + ReleaseDescriptor(content_path: 'blobs/sha256/91ef0af61f39ece4d6710e465df5ed6ca12112358344fd51ae6a3b886634148b', content_type: 'application/vnd.docker.container.image.v1+json', content_digest: 'sha256:91ef0af61f39ece4d6710e465df5ed6ca12112358344fd51ae6a3b886634148b', content_length: 1471), + ] + } + end + it 'should not reupload blobs' do expect { subject.perform_async(artifact.id) }.to_not upload end @@ -101,10 +118,12 @@ context 'when artifact is uploaded' do let(:artifact) { create(:artifact, :oci_image, :uploaded, account:) } + it 'should not change artifact status' do + expect { subject.perform_async(artifact.id) }.to_not change { artifact.reload.status } + end + it 'should not store manifest' do expect { subject.perform_async(artifact.id) }.to not_change { artifact.reload.manifest } - - expect(artifact.status).to eq 'UPLOADED' end it 'should not upload blobs' do @@ -115,10 +134,12 @@ context 'when artifact is failed' do let(:artifact) { create(:artifact, :oci_image, :failed, account:) } + it 'should not change artifact status' do + expect { subject.perform_async(artifact.id) }.to_not change { artifact.reload.status } + end + it 'should not store manifest' do expect { subject.perform_async(artifact.id) }.to not_change { artifact.reload.manifest } - - expect(artifact.status).to eq 'FAILED' end it 'should not upload blobs' do diff --git a/spec/workers/process_ruby_gem_worker_spec.rb b/spec/workers/process_ruby_gem_worker_spec.rb index 0e82933f8..33306b737 100644 --- a/spec/workers/process_ruby_gem_worker_spec.rb +++ b/spec/workers/process_ruby_gem_worker_spec.rb @@ -37,6 +37,8 @@ it 'should process gem' do expect { subject.perform_async(artifact.id) }.to change { artifact.reload.manifest } + expect(artifact.manifest.content_type).to eq 'application/x-yaml' + expect(artifact.manifest.content_path).to eq gemspec.file_name expect(artifact.manifest.content).to eq gemspec.to_yaml expect(artifact.status).to eq 'UPLOADED' end