Skip to content

Commit

Permalink
Add name key to sources in Pipfiles (#7744)
Browse files Browse the repository at this point in the history
Newer versions of `pipenv` require any specified `sources` to be
explicitly `name`'d.

More context here: pypa/pipenv#5370 (comment)

This has been true since at least Sept 2022. Our version of `pipenv` is
from April of 2022, so it doesn't complain. But it will as soon as we upgrade.

Also, for sources that :dependabot: dynamically injects into the
`Pipfile`, they need a name. These sources are stripped from the final
`Pipfile` / `Pipfile.lock` during the `FileUpdater#post_process_lockfile`
method, so all we need is a placeholder `name` to placate `pipenv`.

Long term, we may want to add custom error handling to flag this
missing key as a `Dependabot::DependencyFileNotResolvable` error.

But I decided that was out of scope for now as this PR does not generate
the error... that will not happen until we upgrade to newer `pipenv`.
And even at that point, our first priority will be upgrading, and then
from there handling any new errors that start popping up.

And even then, most of the active users of `pipenv` are unlikely to see this
error because they're likely running a newer version of `pipenv`.
  • Loading branch information
jeffwidman authored Aug 10, 2023
1 parent 1ac5441 commit 918333f
Show file tree
Hide file tree
Showing 35 changed files with 78 additions and 14 deletions.
9 changes: 6 additions & 3 deletions python/lib/dependabot/python/file_updater/pipfile_preparer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,12 @@ def sub_auth_url(source, credentials)

def config_variable_sources(credentials)
@config_variable_sources ||=
credentials.
select { |cred| cred["type"] == "python_index" }.
map { |c| { "url" => AuthedUrlBuilder.authed_url(credential: c) } }
credentials.select { |cred| cred["type"] == "python_index" }.map.with_index do |c, i|
{
"name" => "dependabot-inserted-index-#{i}",
"url" => AuthedUrlBuilder.authed_url(credential: c)
}
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,34 @@
end
end

context "with a source not included in the original Pipfile" do
let(:credentials) do
[
{
"type" => "git_source",
"host" => "github.com",
"username" => "x-access-token",
"password" => "token"
},
{
"type" => "python_index",
"index-url" => "https://pypi.posrip.com/pypi/"
}
]
end

it "the source is not included in the final updated files" do
expect(updated_files.map(&:name)).to eq(%w(Pipfile.lock)) # because Pipfile shouldn't have changed

updated_lockfile = updated_files.find { |f| f.name == "Pipfile.lock" }
expect(updated_lockfile.content).not_to include("dependabot-inserted-index")
expect(updated_lockfile.content).not_to include("https://pypi.posrip.com/pypi/")

json_lockfile = JSON.parse(updated_lockfile.content)
expect(json_lockfile["_meta"]["sources"]).to eq(JSON.parse(lockfile.content)["_meta"]["sources"])
end
end

context "when the Pipfile included an environment variable source" do
let(:pipfile_fixture_name) { "environment_variable_source" }
let(:lockfile_fixture_name) { "environment_variable_source.lock" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,11 @@
let(:lockfile_fixture_name) { "version_not_specified.lock" }

it "adds the source" do
expect(updated_content).
to include("https://username:password@pypi.posrip.com/pypi/")
expect(updated_content).to include(
"[[source]]\n" \
"name = \"dependabot-inserted-index-0\"\n" \
"url = \"https://username:password@pypi.posrip.com/pypi/\"\n"
)
end

context "with auth details provided as a token" do
Expand All @@ -154,8 +157,11 @@
end

it "adds the source" do
expect(updated_content).
to include("https://username:password@pypi.posrip.com/pypi/")
expect(updated_content).to include(
"[[source]]\n" \
"name = \"dependabot-inserted-index-0\"\n" \
"url = \"https://username:password@pypi.posrip.com/pypi/\"\n"
)
end
end

Expand All @@ -178,7 +184,7 @@
it "keeps source config" do
expect(updated_content).to include(
"[[source]]\n" \
"name = \"pypi\"\n" \
"name = \"internal-pypi\"\n" \
"url = \"https://username:password@pypi.posrip.com/pypi/\"\n" \
"verify_ssl = true\n"
)
Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/arbitrary_equality
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/conflict_at_current
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/conflict_at_latest
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/${ENV_VAR}"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/exact_version
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
2 changes: 1 addition & 1 deletion python/spec/fixtures/pipfile_files/extra_subdependency
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
flask = "==1.0.*"
Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/git_source
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/git_source_bad_ref
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/git_source_no_ref
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/git_source_unreachable
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/hard_names
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/not_in_lockfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/only_dev
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/path_dependency
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/private_source
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "internal-pypi"
url = "https://some.internal.registry.com/pypi/"
verify_ssl = true

Expand Down
2 changes: 1 addition & 1 deletion python/spec/fixtures/pipfile_files/private_source_auth
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[[source]]
name = "internal-pypi"
url = "https://${ENV_USER}:${ENV_PASSWORD}@pypi.posrip.com/pypi/"
verify_ssl = true
name = "pypi"

[dev-packages]
pytest = "==3.4.0"
Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/prod_and_dev
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/prod_and_dev_different
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/required_python
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/required_python_invalid
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
3 changes: 1 addition & 2 deletions python/spec/fixtures/pipfile_files/unparseable
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
[[source]]

name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[dev-packages]

Expand Down
2 changes: 1 addition & 1 deletion python/spec/fixtures/pipfile_files/unsupported_dep
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
requests = "==2.18.0"
Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/version_hash
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/version_not_specified
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
{
"_meta": {
"hash": {
"sha256": "c402ea48092e9d467af51a483bb8dd8ad0620e11c94f009dcd433f97a99d45db"
"sha256": "e76ae491d793d659f05c7f7eab261fd6167dc062efcba08f17be68e73eb87665"
},
"pipfile-spec": 6,
"requires": {},
"sources": [
{
"name": "pypi",
"url": "https://pypi.org/simple",
"verify_ssl": true
}
Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/version_table
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/wildcard
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/with_quotes
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/yanked
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down

0 comments on commit 918333f

Please sign in to comment.