Pin markdownlint-fix-docker to the released version

[jmclong]

Thank you for maintaining this project!

My pre-commit hook using markdownlint-fix-docker always pulls from the latest version of the docker image. If there are new markdown lint rules introduced, this can break the pre-commit hook without any version updates in .pre-commit-config.yaml. A solution would be to pin the hook to the released version on each release.

markdownlint-fix-docker..................................................Failed
- hook id: markdownlint-fix-docker
- duration: 6.01s
- exit code: 1

Unable to find image 'ghcr.io/igorshubovych/markdownlint-cli:latest' locally
latest: Pulling from igorshubovych/markdownlint-cli

As a workaround, I swapped to use the non-docker version.

[jmclong]

I saw a dupe here, but I wonder if this would still be something that could be addressed in the future? Thank you so much!

#538

[DavidAnson]

Yes, this issue is a duplicate. I would have made the change for yesterday's release, but was pressured by people worried about the (irrelevant) glob vulnerability to publish earlier than intended. This is still on my list to do, likely in a patch release.

[dncrews]

Hey. I just wanted to add my team to the list of large orgs this broke because it's pulling ghcr.io/igorshubovych/markdownlint-cli:latest (even though we're pinned to 0.39.0 of the hook) Is there anything I can do to help fix and prevent this @DavidAnson? I'm happy to get elbows-deep if this makes it easy and more stable for everybody else.

[DavidAnson]

@dncrews, if this is urgent, you can PR the equivalent of the below change directly off the current tag and I can update the tag to your commit.

https://github.com/DavidAnson/markdownlint-cli2/blob/main/.pre-commit-hooks.yaml

More generally, there's nothing to be done with the current release because this binding change will only take effect for the NEXT tag.

[DavidAnson]

To clarify, that PR needs to be made for the tag you want to bind to which is probably not the latest tag.

[DavidAnson]

@dncrews, if you want to really make a difference, see if you can get the pre-commit team to handle this automatically. Even a one-time change to opt in would be better than what seems necessary today which is updating this file for every new release and tag.

[dncrews]

@DavidAnson I wrote this PR before I saw your message, sorry**. You were faster at responding than I expected you to be. If you'd prefer a different approach, I'd love the feedback, too.

**or "sorry, not sorry", I guess? 🤣

[DavidAnson]

I appreciate the effort very much!

However, because my interactions with pre-commit almost exclusively involve working around decisions they've made that I disagree with, I am not eager to add all of the complexity your PR seems to introduce.

Bulk updating hardcoded version references when I bump the package version is something I already do, so the incremental cost of including two new references is practically zero. But I think it shouldn't be needed at all.

This seems like a very common request and something that pre-commit could implement themselves for the ecosystem rather than having every individual project solve in their own way.

[dncrews]

@DavidAnson thank you for taking the time to review it, and I apologize in advance for the following Ted Talk™.

I am not eager to add all of the complexity your PR seems to introduce.

Bulk updating hard-coded version references when I bump the package version is something I already do, so the incremental cost of including two new references is practically zero. But I think it shouldn't be needed at all.

I totally understand that. All of this could even be replaced by a single test that just makes sure your contributors don't accidentally forget to update it, just like the --version tests already do:

test('documentation has been properly updated with version', async t => {
  const readmeContent = await fsPromises.readFile(path.resolve(__dirname, '../README.md'), 'utf8');
  t.true(readmeContent.includes(`rev: v${packageJson.version}`));

  const preCommitHooksContent = await fsPromises.readFile(path.resolve(__dirname, '../.pre-commit-hooks.yaml'), 'utf8');
  t.true(preCommitHooksContent.includes(`ghcr.io/igorshubovych/markdownlint-cli:v${packageJson.version}`));
  t.true(preCommitHooksContent.includes(`ghcr.io/igorshubovych/markdownlint-cli:v${packageJson.version} --fix`));
});

✏️ If you wanted that, I'd be happy to submit a PR or update my other one.

---

This seems like a very common request and something that pre-commit could implement themselves for the ecosystem rather than having every individual project solve in their own way.

Honestly I'd even be willing to have a go at the pre-commit side if they'd take a feature request, though I'm also not sure I understand your position on it, since I'm not sure what they could do.

What would you want or expect that interface to look and feel like? How would you want it to automate without you modifying the additional files? Would you want them to provide scripts or hooks that would update the source code in the way this PR does? Would you want "some property" on the hook definition or config definition that would determine the version of the image used, either by the hook's provider or consumer?

---

Note before I go on, please know that I'm just trying to answer some of the potential questions up front to avoid wasting your time with back-and-forth. I'm new to your project, so I don't know what your take is or what decisions you specifically disagree with on the pre-commit side, so this all may just be "noise" you can ignore.

---

Some possible examples

TL;DR: I'm not sure if there's anything feasible that could be done here, aside from manual hard-coding or automated hard-coding.

To set up my examples, from the consumer end, for example, if I want to make sure it's stable so my team's CI doesn't go down (like what triggered this effort), how would you want that to get passed between the three of us (markdownlint + pre-commit + my CI) in an automatable way?

Here are some possibilities.

Provided Scripts / Hooks

I put this first, but I'm not sure what would be built into pre-commit toolbox that would update the versions in disparate places without it being custom to every hook provider other than a custom pre-commit-config entrypoint like the one in my PR. To automate that, I had to specifically dig into the code to see where and how the version was used, parse it, and make changes that were unique to your repo. I couldn't just grep/sed the version number, for example, since that string value could just be in whatever files for non-versioning reason, or even a CHANGELOG file.

Hooks Definition

At the moment, the image version is defined in the hooks definition, (pre-commit-hooks.yaml) explicitly hard-coded on each line. One idea would be to just provide a way to define a passthrough rev -> tag. For example, it could use a jinja/nunjucks template string:

    entry: ghcr.io/igorshubovych/markdownlint-cli:{{ rev }} --fix

Then if my config looks like this, we're golden:

    rev: v0.39.0

That could easily automatically pull ghcr.io/igorshubovych/markdownlint-cli:v0.39.0.

[Un?]fortunately pre-commit revs are just git revs, though, so what if there was an incompatible-for-me bug in a tagged version and I needed a temporary even-harder-coded solution? I assume you wouldn't want to publish an image for every commit, so this surely wouldn't work:

    rev: 06facdab3b778e97b46a697baf18dc6f3b7174d8

To make those work without over-publishing for edge-cases, we're back to hard-coding. Potentially you could get around that with defaults, but that would still have to be hard-coded, so it wouldn't really help.

    entry: ghcr.io/igorshubovych/markdownlint-cli:{{ rev or "v0.39.0" }} --fix

Config Definition

I assume you wouldn't want it on the consumer's end, in the pre-commit-config.yaml. That could look something like this:

    rev: v0.39.0
    image_version: v0.39.0

But then:

1. It seems strange that I would have to type the version twice, especially since that's literally the only reason the rev property exists.

2. If I really wanted to explicitly list an image version, I already can (but this feels even more "icky" and "intrusive"):

     # - repo: https://github.com/igorshubovych/markdownlint-cli
     #   rev: v0.39.0
     #   hooks:
     #   # - id: markdownlint-docker
     #   - id: markdownlint-fix-docker
   
     - repo: local
       hooks:
         - id: markdownlint-fix-docker
           entry: ghcr.io/igorshubovych/markdownlint-cli:v0.39.0 --fix
           language: docker_image
           types: [markdown]

---

Just to have it re-stated, these are honest questions. I'm not trying to be Socratic -- by asking questions that end in a specific direction -- and I'd love to get an understanding if you wanted to share. I'm super open to contributing where I can to help the community.

[DavidAnson]

All of this could even be replaced by a single test that just makes sure your contributors don't accidentally forget to update it, just like the --version tests already do:

Yes, that's more how I'd approach this. (And have.)

At the moment, the image version is defined in the hooks definition, (pre-commit-hooks.yaml) explicitly hard-coded on each line. One idea would be to just provide a way to define a passthrough rev -> tag.

Yes, or maybe it can be simpler than that - a project can opt into this and pre-commit can append the tag value for the same result.

[Un?]fortunately pre-commit revs are just git revs, though, so what if there was an incompatible-for-me bug in a tagged version and I needed a temporary even-harder-coded solution?

How is this different than a bug in npm package version N? You can pin version N-1 and unblock yourself. Same here.

Potentially you could get around that with defaults, but that would still have to be hard-coded, so it wouldn't really help.

I propose that pre-commit could fall back to the un-decorated/un-versioned Docker container image if there's no matching auto-tagged image.

I assume you wouldn't want it on the consumer's end, in the pre-commit-config.yaml. That could look something like this:

That seems fine to me, also. rev selects the git commit and you can optionally override the Docker container image tag in exceptional cases.

If I really wanted to explicitly list an image version, I already can (but this feels even more "icky" and "intrusive"):

To be honest, I might like this better than pre-commit's current implementation. It puts the consumer in control of what happens and there's nothing uniquely defined by your example that needs to be centralized in the tool repo.