Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Docker engine #914

Draft
wants to merge 29 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ebc1c00
revert "temp: remove docker image processor"
ezekg Nov 7, 2024
3b80495
add tests for docker image worker
ezekg Nov 8, 2024
573e1db
add enumeratorio to improve memory efficiency
ezekg Nov 8, 2024
9de0639
refactor npm worker to use enumeratorio
ezekg Nov 8, 2024
1d8014d
update naming
ezekg Nov 11, 2024
afcbf80
fix typo
ezekg Nov 11, 2024
49cabe8
update image processor to use index.json
ezekg Nov 11, 2024
7548ded
adjust tarball min/max sizes
ezekg Nov 11, 2024
91207ec
s/docker/oci/
ezekg Nov 11, 2024
82c343b
add regex support to upload matcher
ezekg Nov 11, 2024
19c6359
add base for oci routes
ezekg Nov 11, 2024
8775471
refactor oci image processing to store media types
ezekg Nov 13, 2024
c1051ab
remove unsupported chunked writes
ezekg Nov 14, 2024
da5fa6b
add support for multiple manifests per artifact
ezekg Nov 14, 2024
144c0d6
add unique index on manifest content path
ezekg Nov 15, 2024
c4cfaa5
s/fixme/todo/
ezekg Nov 15, 2024
803b4e7
add tests for oci spec conformance
ezekg Nov 15, 2024
8094f2d
fix manifest and descriptor authz
ezekg Nov 15, 2024
1053303
refactor manifest and descriptor authz into policies
ezekg Nov 15, 2024
3094047
add tests for oci manifest api
ezekg Nov 15, 2024
56ec1e7
s/docker/oci spec/
ezekg Nov 15, 2024
f754f90
add note on octal encoding
ezekg Nov 15, 2024
1b4a67c
remove json minification
ezekg Nov 15, 2024
ad8808f
fix format
ezekg Nov 15, 2024
ce3ad8c
fix typo
ezekg Nov 15, 2024
f106a2c
remove superfluous requires
ezekg Nov 15, 2024
83a2fb3
fix max size mismatch between factory and worker
ezekg Nov 15, 2024
c3bc0be
rename filename to path
ezekg Nov 21, 2024
bbac712
fix indexes
ezekg Nov 21, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
3 changes: 3 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion app/controllers/api/v1/release_artifacts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ def show
return render jsonapi: artifact if
!artifact.downloadable? || prefers?('no-download')

download = artifact.download!(ttl: release_artifact_query[:ttl])
download = artifact.download!(
filename: params.fetch(:filename) { artifact.filename },
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tests.

ttl: release_artifact_query[:ttl],
)

BroadcastEventService.call(
# NOTE(ezekg) The `release.downloaded` event is for backwards compat
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ class Npm::PackageMetadataController < Api::V1::BaseController
before_action :set_package

def show
authorize! package,
to: :show?
authorize! package

artifacts = authorized_scope(package.artifacts.tarballs).order_by_version
.where_assoc_exists(:manifest) # must exist
Expand Down
44 changes: 44 additions & 0 deletions app/controllers/api/v1/release_engines/oci/blobs_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# frozen_string_literal: true

module Api::V1::ReleaseEngines
class Oci::BlobsController < Api::V1::BaseController
before_action :scope_to_current_account!
before_action :require_active_subscription!
before_action :authenticate_with_token
before_action :set_package

def show
authorize! package

descriptor = authorized_scope(package.descriptors).find_by!(
content_digest: params[:digest],
ezekg marked this conversation as resolved.
Show resolved Hide resolved
)
authorize! descriptor

if request.head?
# see: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#checking-if-content-exists-in-the-registry
head :ok
else
redirect_to vanity_v1_account_release_artifact_url(current_account, descriptor.artifact, filename: descriptor.content_path, host: request.host),
status: :see_other
end
end

private

attr_reader :package

def set_package
scoped_packages = authorized_scope(current_account.release_packages.oci)
.where_assoc_exists(
:descriptors, # must exist
)

@package = Current.resource = FindByAliasService.call(
scoped_packages,
id: params[:package],
aliases: :key,
)
end
end
end
51 changes: 51 additions & 0 deletions app/controllers/api/v1/release_engines/oci/manifests_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# frozen_string_literal: true

module Api::V1::ReleaseEngines
class Oci::ManifestsController < Api::V1::BaseController
before_action :scope_to_current_account!
before_action :require_active_subscription!
before_action :authenticate_with_token
before_action :set_package

def show
authorize! package

manifest = authorized_scope(package.manifests).find_by_reference!(
params[:reference],
content_type: request.accepts.collect(&:to_s),
ezekg marked this conversation as resolved.
Show resolved Hide resolved
)
authorize! manifest

# for etag support
return unless
stale?(manifest, cache_control: { max_age: 1.day, private: true })

# oci spec is very particular about content/media types
response.content_type = manifest.content_type

if request.head?
# see: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#checking-if-content-exists-in-the-registry
head :ok
else
render body: manifest.content
end
end

private

attr_reader :package

def set_package
scoped_packages = authorized_scope(current_account.release_packages.oci)
.where_assoc_exists(
:manifests, # must exist
)

@package = Current.resource = FindByAliasService.call(
scoped_packages,
id: params[:package],
aliases: :key,
)
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ def show

def set_artifact
scoped_artifacts = authorized_scope(current_account.release_artifacts)
.for_product(params[:product_id])
.for_package(params[:package_id])
.for_release(params[:release_id])
.for_product(params[:product])
.for_package(params[:package])
.for_release(params[:release])

@artifact = Current.resource = FindByAliasService.call(
scoped_artifacts.order_by_version,
id: params[:id],
id: params[:artifact],
aliases: :filename,
reorder: false,
)
Expand Down
18 changes: 18 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ def render_forbidden(**kwargs)

respond_to do |format|
format.any {
response.content_type = Mime::Type.lookup_by_extension(:jsonapi)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should respond with a JSON content type if asked (or if the route uses the :json format), instead of always responding with JSONAPI.


render status: :forbidden, json: {
meta: { id: request.request_id },
errors: [{
Expand All @@ -111,6 +113,8 @@ def render_unauthorized(**kwargs)

respond_to do |format|
format.any {
response.content_type = Mime::Type.lookup_by_extension(:jsonapi)

render status: :unauthorized, json: {
meta: { id: request.request_id },
errors: [{
Expand All @@ -134,6 +138,8 @@ def render_unprocessable_entity(**kwargs)

respond_to do |format|
format.any {
response.content_type = Mime::Type.lookup_by_extension(:jsonapi)

render status: :unprocessable_entity, json: {
meta: { id: request.request_id },
errors: [{
Expand All @@ -157,6 +163,8 @@ def render_not_found(**kwargs)

respond_to do |format|
format.any {
response.content_type = Mime::Type.lookup_by_extension(:jsonapi)

render status: :not_found, json: {
meta: { id: request.request_id },
errors: [{
Expand All @@ -181,6 +189,8 @@ def render_bad_request(**kwargs)

respond_to do |format|
format.any {
response.content_type = Mime::Type.lookup_by_extension(:jsonapi)

render status: :bad_request, json: {
meta: { id: request.request_id },
errors: [{
Expand All @@ -204,6 +214,8 @@ def render_conflict(**kwargs)

respond_to do |format|
format.any {
response.content_type = Mime::Type.lookup_by_extension(:jsonapi)

render status: :conflict, json: {
meta: { id: request.request_id },
errors: [{
Expand All @@ -227,6 +239,8 @@ def render_payment_required(**kwargs)

respond_to do |format|
format.any {
response.content_type = Mime::Type.lookup_by_extension(:jsonapi)

render status: :payment_required, json: {
meta: { id: request.request_id },
errors: [{
Expand All @@ -250,6 +264,8 @@ def render_internal_server_error(**kwargs)

respond_to do |format|
format.any {
response.content_type = Mime::Type.lookup_by_extension(:jsonapi)

render status: :internal_server_error, json: {
meta: { id: request.request_id },
errors: [{
Expand All @@ -273,6 +289,8 @@ def render_service_unavailable(**kwargs)

respond_to do |format|
format.any {
response.content_type = Mime::Type.lookup_by_extension(:jsonapi)

render status: :service_unavailable, json: {
meta: { id: request.request_id },
errors: [{
Expand Down
21 changes: 14 additions & 7 deletions app/controllers/concerns/rendering.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,24 @@ module Base
include ActionController::MimeResponds

# overload render method to automatically set content type
def render(args, ...)
case args
in jsonapi:
def render(options, ...)
mime_type, * = Mime::Type.parse(response.content_type.to_s) rescue nil

# skip if we've already set content type
unless mime_type.nil?
return super
end

case options
in jsonapi: _
response.content_type = Mime::Type.lookup_by_extension(:jsonapi)
in json:
in json: _
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can respond with JSONAPI depending on route format and accept header, since JSON and JSONAPI are often synonymous.

response.content_type = Mime::Type.lookup_by_extension(:json)
in body:
in body: _
response.content_type = Mime::Type.lookup_by_extension(:binary)
in html:
in html: _
response.content_type = Mime::Type.lookup_by_extension(:html)
in gz:
in gz: _
response.content_type = Mime::Type.lookup_by_extension(:gzip)
else
# leave as-is
Expand Down
1 change: 1 addition & 0 deletions app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class Account < ApplicationRecord
has_many :release_engines, dependent: :destroy_async
has_many :release_packages, dependent: :destroy_async
has_many :release_manifests, dependent: :destroy_async
has_many :release_descriptors, dependent: :destroy_async
has_many :release_platforms, dependent: :destroy_async
has_many :release_arches, dependent: :destroy_async
has_many :release_filetypes, dependent: :destroy_async
Expand Down
33 changes: 17 additions & 16 deletions app/models/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,23 @@ def owned = where(bearer: proxy_association.owner)
end

# TODO(ezekg) Should deleting queue up a cancelable background job?
has_many :webhook_endpoints, dependent: :destroy_async
has_many :webhook_events, dependent: :destroy_async
has_many :entitlements, dependent: :destroy_async
has_many :groups, dependent: :destroy_async
has_many :products, dependent: :destroy_async
has_many :policies, dependent: :destroy_async
has_many :licenses, dependent: :destroy_async
has_many :license_users, dependent: :destroy_async
has_many :machines, dependent: :destroy_async
has_many :machine_processes, dependent: :destroy_async
has_many :machine_components, dependent: :destroy_async
has_many :users, dependent: :destroy_async
has_many :releases, dependent: :destroy_async
has_many :release_artifacts, dependent: :destroy_async
has_many :release_manifests, dependent: :destroy_async
has_many :release_packages, dependent: :destroy_async
has_many :webhook_endpoints, dependent: :destroy_async
has_many :webhook_events, dependent: :destroy_async
has_many :entitlements, dependent: :destroy_async
has_many :groups, dependent: :destroy_async
has_many :products, dependent: :destroy_async
has_many :policies, dependent: :destroy_async
has_many :licenses, dependent: :destroy_async
has_many :license_users, dependent: :destroy_async
has_many :machines, dependent: :destroy_async
has_many :machine_processes, dependent: :destroy_async
has_many :machine_components, dependent: :destroy_async
has_many :users, dependent: :destroy_async
has_many :releases, dependent: :destroy_async
has_many :release_artifacts, dependent: :destroy_async
has_many :release_manifests, dependent: :destroy_async
has_many :release_packages, dependent: :destroy_async
has_many :release_descriptors, dependent: :destroy_async

has_account
has_role :environment
Expand Down
8 changes: 6 additions & 2 deletions app/models/release.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,16 @@ class Release < ApplicationRecord
class_name: 'ReleaseUploadLink',
inverse_of: :release,
dependent: :delete_all
has_many :artifacts,
class_name: 'ReleaseArtifact',
inverse_of: :release,
dependent: :delete_all
has_many :manifests,
class_name: 'ReleaseManifest',
inverse_of: :release,
dependent: :delete_all
has_many :artifacts,
class_name: 'ReleaseArtifact',
has_many :descriptors,
class_name: 'ReleaseDescriptor',
inverse_of: :release,
dependent: :delete_all
has_many :filetypes,
Expand Down
16 changes: 13 additions & 3 deletions app/models/release_artifact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,22 @@ 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,
inverse_of: :artifact,
dependent: :delete
has_many :descriptors,
class_name: 'ReleaseDescriptor',
foreign_key: :release_artifact_id,
inverse_of: :artifact,
dependent: :delete_all
has_one :channel,
through: :release
has_one :product,
Expand Down Expand Up @@ -462,7 +473,6 @@ def key_for(path) = "artifacts/#{account_id}/#{release_id}/#{path}"
def key = key_for(filename)

def presigner = Aws::S3::Presigner.new(client:)

def client
case backend
when 'S3'
Expand Down Expand Up @@ -513,8 +523,8 @@ def redirect_url?
redirect_url.present?
end

def download!(ttl: 1.hour)
self.redirect_url = presigner.presigned_url(:get_object, bucket:, key:, expires_in: ttl&.to_i)
def download!(filename: self.filename, ttl: 1.hour)
ezekg marked this conversation as resolved.
Show resolved Hide resolved
self.redirect_url = presigner.presigned_url(:get_object, bucket:, key: key_for(filename), expires_in: ttl&.to_i)

release.download_links.create!(url: redirect_url, ttl:, account:)
end
Expand Down
Loading