Add support for Docker engine #914

ezekg · 2024-11-08T17:06:59Z

Closes #911. In the end, it should be as simple as docker save keygen/api:1.0.0 > keygen-1.0.0.tar and keygen upload keygen-1.0.0.tar — the rest should be automatically handled by the Docker engine.

Like with npm and friends, docker push will not be supported right away.

Prereqs

Implement the Docker/OCI manifest and blob API endpoints.
Test parsing of popular images, e.g. Alpine, Ubuntu, etc.
Write tests for OCI spec conformance.
Write tests for manifests API.
Write tests for blobs API.
Helm compatibility.
Make EE only?

Subreqs

spec/workers/process_docker_image_worker_spec.rb

app/workers/process_docker_image_worker.rb

spec/support/matchers/upload_matcher.rb

ezekg · 2024-11-10T00:34:11Z

app/workers/process_docker_image_worker.rb

+require 'minitar'
+require 'zlib'
+
+class ProcessDockerImageWorker < BaseWorker


This worker should be in a high_memory queue.

app/workers/process_docker_image_worker.rb

reverts commit 04dabba.

app/controllers/api/v1/release_engines/oci/blobs_controller.rb

app/controllers/api/v1/release_engines/oci/manifests_controller.rb

ezekg · 2024-11-15T00:36:07Z

app/controllers/application_controller.rb

@@ -86,6 +86,8 @@ def render_forbidden(**kwargs)

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


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.

ezekg · 2024-11-15T00:36:55Z

app/controllers/concerns/rendering.rb

          response.content_type = Mime::Type.lookup_by_extension(:jsonapi)
-        in json:
+        in json: _


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

- oci expects any manifest to be referenceable not just the index

ezekg · 2024-11-15T02:45:23Z

app/controllers/api/v1/release_engines/oci/blobs_controller.rb

+      descriptor = package.descriptors.find_by!(content_digest: params[:digest])
+      authorize! descriptor.artifact
+
+      redirect_to vanity_v1_account_release_artifact_url(current_account, descriptor.artifact, filename: descriptor.content_path, host: request.host),


Maybe rename this from vanity to descriptor or smth? It's more than a vanity URL now.

app/controllers/api/v1/release_engines/oci/manifests_controller.rb

app/workers/process_npm_package_worker.rb

ezekg · 2024-11-15T02:56:17Z

app/workers/process_oci_image_worker.rb

+  MIN_TARBALL_SIZE  = 512.bytes     # to avoid processing empty or invalid tarballs
+  MAX_TARBALL_SIZE  = 512.megabytes # to avoid downloading excessive tarballs
+  MAX_MANIFEST_SIZE = 1.megabyte    # to avoid storing large manifests
+  MAX_BLOB_SIZE     = 512.megabytes # to avoid storing large layers


Probably too big?

ezekg · 2024-11-15T03:00:06Z

app/workers/wait_for_artifact_upload_worker.rb

@@ -54,6 +54,14 @@ def perform(artifact_id, enqueued_at = Time.current.iso8601)
      )

      ProcessNpmPackageWorker.perform_async(artifact.id)
+    in filetype: ReleaseFiletype(:tar), engine: ReleaseEngine(:oci)


Support other filetypes?

For example tar.gz and tgz.

spec/factories/release_manifest.rb

spec/support/matchers/upload_matcher.rb

spec/workers/process_oci_image_worker_spec.rb

ezekg · 2024-11-15T03:16:19Z

spec/workers/process_oci_image_worker_spec.rb

+
+        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),


Add content asserts

ezekg · 2024-11-15T03:17:48Z

spec/workers/process_oci_image_worker_spec.rb

+    end
+
+    it 'should be efficient' do
+      expect { subject.perform_async(artifact.id) }.to allocate_less_than(5.megabytes)


Refactor this to sample memory usage and assert against peak

ezekg · 2024-11-15T17:37:28Z

app/controllers/api/v1/release_artifacts_controller.rb

@@ -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 },


app/controllers/api/v1/release_engines/oci/blobs_controller.rb

app/controllers/api/v1/release_engines/oci/manifests_controller.rb

app/models/release_artifact.rb

app/models/release_manifest.rb

db/migrate/20241113134529_create_release_descriptors.rb

db/migrate/20241114163421_add_content_attributes_to_release_manifests.rb

ezekg · 2024-11-15T23:59:05Z

features/api/v1/engines/oci/conformance.feature

+    Then the response status should be "405"
+
+  @sp
+  Scenario: Endpoint should fail delete


s/delete/yank/

ezekg · 2024-11-16T00:00:53Z

lib/enumerator_io.rb

+      @buffer.clear
+    end
+
+    buffer.empty? ? nil : buffer


buffer.presence should suffice?

ezekg · 2024-11-16T00:01:20Z

lib/oci_image_layout.rb

+
+  def self.parse(io) = Parser.new(io).parse
+
+  class DigestIO < SimpleDelegator


Move to separate lib since it's duplicated in keygen/ee too.

ezekg · 2024-11-16T05:05:22Z

lib/oci_image_layout.rb

+    end
+  end
+
+  class Index < Blob


Store manifest version?

ezekg · 2024-11-17T05:26:19Z

lib/oci_image_layout.rb

+      # a Minitar::Reader, and that closes an entry's reader io after
+      # reading the entry. This lets us avoid buffering entries into
+      # memory, since some of the blobs could be large layers.
+      io.each do |entry|


Move all this outside of initializer?

ezekg · 2024-11-17T05:31:00Z

lib/oci_image_layout.rb

+      io.rewind
+    end
+
+    def each(&)


Should we add return to_enum(:each) unless block_given??

ezekg · 2024-11-21T23:21:50Z

db/migrate/20241114163421_add_content_attributes_to_release_manifests.rb

+      algorithm: :concurrently,
+      unique: true
+
+    add_index :release_manifests, %i[content_type release_artifact_id],


Doesn't need artifact in the index.

ezekg · 2024-11-21T23:22:02Z

db/migrate/20241113134529_create_release_descriptors.rb

+      t.index %i[release_artifact_id]
+      t.index %i[content_path release_artifact_id],   unique: true
+      t.index %i[content_digest release_artifact_id], unique: true
+      t.index %i[content_type release_artifact_id]


Drop artifact from index.

ezekg · 2024-11-21T23:23:39Z

db/migrate/20241114163421_add_content_attributes_to_release_manifests.rb

+  disable_ddl_transaction!
+
+  def change
+    add_column :release_manifests, :content_path,   :string, null: true


Eventually make these not-null.

- blocker: keygen-sh/keygen-api#914

ezekg force-pushed the feature/docker-engine branch 3 times, most recently from 49c2e66 to ba9255f Compare November 8, 2024 23:08

ezekg commented Nov 9, 2024

View reviewed changes

spec/workers/process_docker_image_worker_spec.rb Outdated Show resolved Hide resolved

ezekg commented Nov 10, 2024

View reviewed changes

app/workers/process_docker_image_worker.rb Outdated Show resolved Hide resolved

app/workers/process_docker_image_worker.rb Outdated Show resolved Hide resolved

spec/support/matchers/upload_matcher.rb Outdated Show resolved Hide resolved

ezekg commented Nov 10, 2024

View reviewed changes

app/workers/process_docker_image_worker.rb Outdated Show resolved Hide resolved

ezekg commented Nov 10, 2024

View reviewed changes

app/workers/process_docker_image_worker.rb Outdated Show resolved Hide resolved

ezekg force-pushed the feature/docker-engine branch 4 times, most recently from 576478c to 39740c4 Compare November 11, 2024 22:21

ezekg added 11 commits November 13, 2024 07:31

revert "temp: remove docker image processor"

ebc1c00

reverts commit 04dabba.

add tests for docker image worker

3b80495

add enumeratorio to improve memory efficiency

573e1db

refactor npm worker to use enumeratorio

9de0639

update naming

1d8014d

fix typo

afcbf80

update image processor to use index.json

49cabe8

adjust tarball min/max sizes

7548ded

s/docker/oci/

91207ec

add regex support to upload matcher

82c343b

add base for oci routes

19c6359

ezekg force-pushed the feature/docker-engine branch 2 times, most recently from 7758cb0 to a4c8292 Compare November 15, 2024 00:10

ezekg commented Nov 15, 2024

View reviewed changes

ezekg force-pushed the feature/docker-engine branch from a4c8292 to 306a26c Compare November 15, 2024 01:13

ezekg added 3 commits November 14, 2024 20:19

refactor oci image processing to store media types

8775471

remove unsupported chunked writes

c1051ab

add support for multiple manifests per artifact

da5fa6b

- oci expects any manifest to be referenceable not just the index

ezekg force-pushed the feature/docker-engine branch from 306a26c to da5fa6b Compare November 15, 2024 02:19

ezekg commented Nov 15, 2024

View reviewed changes

ezekg added 2 commits November 15, 2024 08:45

add unique index on manifest content path

144c0d6

s/fixme/todo/

c4cfaa5

ezekg commented Nov 15, 2024

View reviewed changes

ezekg added 10 commits November 15, 2024 12:35

add tests for oci spec conformance

803b4e7

fix manifest and descriptor authz

8094f2d

refactor manifest and descriptor authz into policies

1053303

add tests for oci manifest api

3094047

s/docker/oci spec/

56ec1e7

add note on octal encoding

f754f90

remove json minification

1b4a67c

fix format

ad8808f

fix typo

ce3ad8c

remove superfluous requires

f106a2c

ezekg force-pushed the feature/docker-engine branch from ff1fc3e to f106a2c Compare November 15, 2024 22:06

fix max size mismatch between factory and worker

83a2fb3

ezekg commented Nov 16, 2024

View reviewed changes

lib/oci_image_layout.rb

end

end

class Index < Blob

Copy link

Member Author

ezekg Nov 16, 2024

Choose a reason for hiding this comment

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

Store manifest version?

ezekg commented Nov 18, 2024

View reviewed changes

ezekg added 2 commits November 21, 2024 15:06

rename filename to path

c3bc0be

fix indexes

bbac712

ezekg commented Nov 21, 2024

View reviewed changes

ezekg added a commit to keygen-sh/keygen-relay that referenced this pull request Nov 26, 2024

temp: comment image recipes

9425ebd

- blocker: keygen-sh/keygen-api#914

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Docker engine #914

Add support for Docker engine #914

ezekg commented Nov 8, 2024 •

edited

Loading

ezekg Nov 10, 2024

ezekg Nov 15, 2024

ezekg Nov 15, 2024

ezekg Nov 15, 2024

ezekg Nov 15, 2024

ezekg Nov 15, 2024

ezekg Nov 20, 2024

ezekg Nov 15, 2024

ezekg Nov 15, 2024

ezekg Nov 15, 2024

ezekg Nov 15, 2024

ezekg Nov 16, 2024

ezekg Nov 16, 2024 •

edited

Loading

ezekg Nov 16, 2024

ezekg Nov 17, 2024

ezekg Nov 17, 2024

ezekg Nov 21, 2024

ezekg Nov 21, 2024

ezekg Nov 21, 2024


		def self.parse(io) = Parser.new(io).parse

		class DigestIO < SimpleDelegator

Add support for Docker engine #914

Are you sure you want to change the base?

Add support for Docker engine #914

Conversation

ezekg commented Nov 8, 2024 • edited Loading

Prereqs

Subreqs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezekg Nov 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezekg commented Nov 8, 2024 •

edited

Loading

ezekg Nov 16, 2024 •

edited

Loading