Skip to content

Add OCI header support to Docker #6969

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.

Sign up for GitHub

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

Merged
deivid-rodriguez merged 2 commits into from
Mar 31, 2023
Merged

Add OCI header support to Docker #6969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd5ee8c
Cleanup no longer needed monkeypatch
deivid-rodriguez Mar 31, 2023
05c5224
Add OCI header support
deivid-rodriguez Mar 31, 2023
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
2 changes: 1 addition & 1 deletion common/dependabot-common.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Gem::Specification.new do |spec|
spec.add_dependency "aws-sdk-ecr", "~> 1.5"
spec.add_dependency "bundler", ">= 1.16", "< 3.0.0"
spec.add_dependency "commonmarker", ">= 0.20.1", "< 0.24.0"
spec.add_dependency "docker_registry2", "~> 1.13"
spec.add_dependency "docker_registry2", "~> 1.14"
Copy link
Copy Markdown
Member

@jeffwidman jeffwidman Mar 31, 2023

Choose a reason for hiding this comment

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

I just bumped this dep manually in:

Ideally :dependabot: would open PR's notifying us when this dep bumps so that we don't have to manually track when they have releases... I don't remember enough about the bundler ecosystem to remember why that isn't happening here...

Copy link
Copy Markdown
Contributor Author

@deivid-rodriguez deivid-rodriguez Mar 31, 2023

Choose a reason for hiding this comment

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

Yeah, I think it's because of our "weird" setup (library in common, application in updater, acting somewhat as a monorepo).

Copy link
Copy Markdown
Contributor

@mctofu mctofu Mar 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We'd need to run dependabot against the common gemspec? https://github.com/dependabot/dependabot-core/blob/main/common/dependabot-common.gemspec

That wouldn't update the updater's gemfile.lock though.

You could also add it as a direct dep in https://github.com/dependabot/dependabot-core/blob/main/updater/Gemfile.

Copy link
Copy Markdown
Contributor Author

@deivid-rodriguez deivid-rodriguez Mar 31, 2023

Choose a reason for hiding this comment

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

I think I changed it from common to updater a while ago when I "unified dependencies", because otherwise we were getting duplicated dependencies for RuboCop and others.

Maybe if we added configuration to also bump indirect dependencies to updater/ maybe that could work? 🤔

Copy link
Copy Markdown
Contributor Author

@deivid-rodriguez deivid-rodriguez Mar 31, 2023

Choose a reason for hiding this comment

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

In any case, this release happened 10h ago, so I think we were faster than the bot this time :)

spec.add_dependency "excon", "~> 0.96", "< 0.100"
spec.add_dependency "faraday", "2.7.4"
spec.add_dependency "faraday-retry", "2.0.0"
Expand Down
30 changes: 0 additions & 30 deletions docker/lib/dependabot/docker/update_checker.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,6 @@
require "dependabot/docker/requirement"
require "dependabot/docker/utils/credentials_finder"

module DockerRegistry2
class Registry
private

# By default the Docker Registry client sets the Accept header to
# `application/vnd.docker.distribution.manifest.v2+json`. This is fine for
# most images, but for multi-architecture images, it fetches the digest of a
# specific architecture instead of the digest for the multi-architecture
# image. We override the header to tell the Docker API to vary its behavior
# depending on whether the image is a uses a traditional (non-list) manifest
# or a manifest list. If the image uses a traditional manifest, the API will
# return the manifest digest. If the image uses a manifest list, the API
# will return the manifest list digest.
def headers(payload: nil, bearer_token: nil)
headers = {}
headers["Authorization"] = "Bearer #{bearer_token}" unless bearer_token.nil?
if payload.nil?
headers["Accept"] = %w(
application/vnd.docker.distribution.manifest.v2+json
application/vnd.docker.distribution.manifest.list.v2+json
application/json
).join(",")
end
headers["Content-Type"] = "application/vnd.docker.distribution.manifest.v2+json" unless payload.nil?

headers
end
end
end

module Dependabot
module Docker
class UpdateChecker < Dependabot::UpdateCheckers::Base
Expand Down
4 changes: 2 additions & 2 deletions updater/Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ PATH
aws-sdk-ecr (~> 1.5)
bundler (>= 1.16, < 3.0.0)
commonmarker (>= 0.20.1, < 0.24.0)
docker_registry2 (~> 1.13)
docker_registry2 (~> 1.14)
excon (~> 0.96, < 0.100)
faraday (= 2.7.4)
faraday-retry (= 2.0.0)
Expand Down Expand Up @@ -143,7 +143,7 @@ GEM
irb (>= 1.5.0)
reline (>= 0.3.1)
diff-lcs (1.5.0)
docker_registry2 (1.13.0)
docker_registry2 (1.14.0)
rest-client (>= 1.8.0)
domain_name (0.5.20190701)
unf (>= 0.0.5, < 1.0.0)
Expand Down