docker: FileParser consider image prefix/suffixes as unique

[thepwagner]

This modifies the Dependabot::Docker::FileParser to treat image variants like image:1.0.0 and image:1.0.0-alpine as unique dependencies, even within a single multistage Dockerfile.

Consider a multistage Dockerfile like:

FROM nginx:1.19.9 AS debian
FROM nginx:1.19.9-alpine AS alpine

Previously, Dependabot would parse this as multiple requirements for an image called nginx, and update all FROM statements to a single image if available.

#<Dependabot::Dependency:0x000055591e6cac10 @name="nginx", @version="1.19.9", @requirements=[{:requirement=>nil, :groups=>[], :file=>"Dockerfile", :source=>{:tag=>"1.19.9"}}, {:requirement=>nil, :groups=>[], :file=>"Dockerfile", :source=>{:tag=>"1.19.9-alpine"}}], @previous_version=nil, @previous_requirements=nil, @package_manager="docker">

By switching the parsing to allow multiple versions, the existing suffix handling code "just works" 🎉 .

Pre

[dependabot-core-dev] ~/dependabot-core $ bin/dry-run.rb docker nginxinc/kubernetes-ingress --commit=54e0b07503414ab0c28472646a681b0674d1f3b9 --dir="/build" --dep="nginx"
warning: parser/current is loading parser/ruby26, which recognizes
warning: 2.6.7-compliant syntax, but you are running 2.6.6.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
=> fetching dependency files
=> dumping fetched dependency files: ./dry-run/nginxinc/kubernetes-ingress/build
=> parsing dependency files
=> updating 1 dependencies: nginx

=== nginx (1.19.9)
 => checking for updates 1/1
 => latest available version is 1.19.10
 => latest allowed version is 1.19.10
 => requirements to unlock: own
 => requirements update strategy:
 => updating nginx from 1.19.9 to 1.19.10

    ± Dockerfile
    ~~~
    5c5
    < FROM nginx:1.19.9 AS debian
    ---
    > FROM nginx:1.19.10 AS debian
    17c17
    < FROM nginx:1.19.9-alpine AS alpine
    ---
    > FROM nginx:1.19.10 AS alpine
    ~~~

Post

[dependabot-core-dev] ~/dependabot-core $ bin/dry-run.rb docker nginxinc/kubernetes-ingress --commit=54e0b07503414ab0c28472646a681b0674d1f3b9 --dir="/build" --dep="nginx"
warning: parser/current is loading parser/ruby26, which recognizes
warning: 2.6.7-compliant syntax, but you are running 2.6.6.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
=> fetching dependency files
=> dumping fetched dependency files: ./dry-run/nginxinc/kubernetes-ingress/build
=> parsing dependency files
=> updating 2 dependencies: nginx, nginx

=== nginx (1.19.9)
 => checking for updates 1/2
 => latest available version is 1.19.10
 => latest allowed version is 1.19.10
 => requirements to unlock: own
 => requirements update strategy:
 => updating nginx from 1.19.9 to 1.19.10

    ± Dockerfile
    ~~~
    5c5
    < FROM nginx:1.19.9 AS debian
    ---
    > FROM nginx:1.19.10 AS debian
    ~~~

=== nginx (1.19.9-alpine)
 => checking for updates 2/2
 => latest available version is 1.19.10-alpine
 => latest allowed version is 1.19.10-alpine
 => requirements to unlock: own
 => requirements update strategy:
 => updating nginx from 1.19.9-alpine to 1.19.10-alpine

    ± Dockerfile
    ~~~
    17c17
    < FROM nginx:1.19.9-alpine AS alpine
    ---
    > FROM nginx:1.19.10-alpine AS alpine
    ~~~

Related

• Closes Docker image tag suffix changes with multi stage Dockerfile #3261

[thepwagner]

@dependabot/reviewers I'd dig an early peek at this thinking: it seems promising (e.g. dry-runs in the OP) but I may be missing something.

I intend to follow up with tests before RFR.

[mctofu]

I had a prior look at this as a day of learning exercise when looking into #3173.

I started with a similar approach and there were some concerns about increasing the # of PRs we'd open: #3251

That led to #3277 which was able to make all the changes in the same PR but didn't yet support this multi stage docker case.

That said, I think it's worth revisiting if this is the better change. It's simpler and avoids some awkwardness when the PR says it's updating php 8.0.1-apache but is also updating php 8.0.1-cli. I could also see the multiple PR concern being more of an issue when they both target the same file though.

[thepwagner]

@mctofu noice, that's exactly the context I was looking for! I pinged the reporters in #3261 for their expectations about single/multiple PRs.

I'm pondering a middle ground where instead of using (name, version) as the key for unique dependences in the FileParser (which is where we both started, neato!), we use (name, prefix, suffix, format) - like you were in comparable_tags?.
The goal would be to keep php:8.0.1 and php:8.0.3 as grouped dependencies, but treat php:8.0.1 as a distinct from php:8.0.1-apache and php:8.0.1-cli.

[feelepxyz]

The goal would be to keep php:8.0.1 and php:8.0.3 as grouped dependencies, but treat php:8.0.1 as a distinct from php:8.0.1-apache and php:8.0.1-cli.

This sounds like a good approach and I like that the change is pretty minimal 👍

Thinking about it, there's probably not a huge number of projects with the same image with different prefixes/suffixes (assuming we'd hear a lot more noise if it was the case?) so probably won't start creating a lot more PRs and fixes currently broken PRs which is a big plus 💯