Skip to content
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.

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

Configure renovate for apk, gem, cargo, pip and npm dependencies #4588

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

bdovaz
Copy link
Collaborator

@bdovaz bdovaz commented Jan 26, 2025

  • I have configured renovate for apk, gem, cargo, pip and npm.
  • As there are some dependencies that were added as “core” in build.py I have moved the definition to constants.py and edited renovate.json5 to add a new regular expression very similar to the previous ones.
  • I had to change how various types of ARG are generated in Dockerfile because now there were problems with the multi-stage/layer build and I had to solve problems that depending on the scope I was in, certain ARG didn't exist.
  • I have changed it to try to expand the cli_docker_image_version parameter, since it is a special case that instead of ARG (image generation time), it uses it in execution and requires that ENV expanded.
  • I have had to downgrade eslint from 9.x to 8.x because there is some other package that required that library to be in 8.x. This problem did not occur when the dependencies were not pinned because I imagine that npm would be “smart” in that sense and would be able to establish which one is the right one.
  • Since there are many libraries that are used between different linters through different descriptors, I have used a more or less objective and consistent {PACKAGE_SYSTEM]_{PACKAGE_NAME}_VERSION criterion. Examples: APK_ICU_LIBS_VERSION, NPM_ESLINT_VERSION, ...
  • There are several repology libraries (apk) that in Alpine 3.21 did not exist and I had to remove them. I don't understand why when they were not pinned it didn't complain that they didn't exist.

I do not know if I have left something without pinning but there are so many things that has been a huge work.... I would appreciate help after merging this PR in case I have left something... Some of them were difficult to detect as the ones added through build.py were more hidden.

Copy link
Contributor

github-actions bot commented Jan 27, 2025

🦙 MegaLinter status: ⚠️ WARNING

Descriptor Linter Files Fixed Errors Elapsed time
✅ API spectral 1 0 1.68s
⚠️ BASH bash-exec 6 1 0.01s
✅ BASH shellcheck 6 0 0.14s
✅ BASH shfmt 6 0 0 0.54s
✅ COPYPASTE jscpd yes no 3.24s
✅ DOCKERFILE hadolint 129 0 38.63s
✅ JSON jsonlint 20 0 0.28s
✅ JSON v8r 22 0 12.01s
⚠️ MARKDOWN markdownlint 267 0 302 21.13s
✅ MARKDOWN markdown-table-formatter 267 0 0 167.7s
⚠️ PYTHON bandit 215 66 3.42s
✅ PYTHON black 215 0 0 4.93s
✅ PYTHON flake8 215 0 2.03s
✅ PYTHON isort 215 0 0 1.54s
✅ PYTHON mypy 215 0 15.57s
✅ PYTHON pylint 215 0 32.65s
✅ PYTHON ruff 215 0 0 0.83s
✅ REPOSITORY checkov yes no 36.35s
✅ REPOSITORY git_diff yes no 0.95s
⚠️ REPOSITORY grype yes 24 12.6s
✅ REPOSITORY secretlint yes no 10.95s
✅ REPOSITORY trivy yes no 18.7s
✅ REPOSITORY trivy-sbom yes no 0.27s
⚠️ REPOSITORY trufflehog yes 1 54.27s
✅ SPELL cspell 718 0 13.35s
⚠️ SPELL lychee 349 29 66.2s
✅ XML xmllint 3 0 0 0.97s
✅ YAML prettier 160 0 0 3.96s
✅ YAML v8r 102 0 14.53s
✅ YAML yamllint 161 0 2.95s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 28, 2025

@nvuillam @echoix ready for review! All yours!

@echoix
Copy link
Collaborator

echoix commented Jan 28, 2025

I can't manage to see the last files (for descriptors and renovate) on my phone, it's too big, but I'll try to take a look before or during this weekend (as I'd have to look on my computer).
I wouldn't look too much at generated files though, but the source ones yes

@echoix
Copy link
Collaborator

echoix commented Jan 28, 2025

I have had to downgrade eslint from 9.x to 8.x because there is some other package that required that library to be in 8.x. This problem did not occur when the dependencies were not pinned because I imagine that npm would be “smart” in that sense and would be able to establish which one is the right one.

This is too impacting. It's not fine with me. The configuration format changed, and users have already adapted if they updated. We had some issues filed in this summer if I'm not mistaken, and some community member helped out a bit. You could take a look if you'd.

@echoix
Copy link
Collaborator

echoix commented Jan 28, 2025

There are several repology libraries (apk) that in Alpine 3.21 did not exist and I had to remove them. I don't understand why when they were not pinned it didn't complain that they didn't exist.
Can you point to me an example of it?

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 28, 2025

There are several repology libraries (apk) that in Alpine 3.21 did not exist and I had to remove them. I don't understand why when they were not pinned it didn't complain that they didn't exist.
Can you point to me an example of it?

This package:

https://pkgs.alpinelinux.org/packages?name=libc6-compat&branch=v3.21&repo=&arch=x86_64&origin=&flagged=&maintainer=

It only exists up to 3.18, after that it does not exist.

Why without being pinned it doesn't fail saying that it doesn't exist? No idea, but as you can see, it does not exist.

It was referenced here:

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 28, 2025

I have had to downgrade eslint from 9.x to 8.x because there is some other package that required that library to be in 8.x. This problem did not occur when the dependencies were not pinned because I imagine that npm would be “smart” in that sense and would be able to establish which one is the right one.

This is too impacting. It's not fine with me. The configuration format changed, and users have already adapted if they updated. We had some issues filed in this summer if I'm not mistaken, and some community member helped out a bit. You could take a look if you'd.

I am no npm expert so I can't tell you the answer to why without being pinned (which I understand installs the latest version of each npm package) this library works if it explicitly says that with 9.x it doesn't work:

https://github.com/airbnb/javascript/blob/11f986fdc7d6b4c80e396437e9c45c939362bdee/packages/eslint-config-airbnb/package.json#L76

https://github.com/airbnb/javascript/blob/11f986fdc7d6b4c80e396437e9c45c939362bdee/packages/eslint-config-airbnb/package.json#L88

And as I have posted, there is an issue where you can see that it is not supported: airbnb/javascript#2961

@nvuillam
Copy link
Member

Wow, what a nice PR , bringing us closer to "bring your own flavor" that we'll build someday :)

Agreed with @echoix , can't downgrade eslint ^^

About apk packages... I wonder if we really need to pin versions, especially those who are "core librairies" and linters: why wouldn't we want to have the latest, as no specific test cases depend on them ?

I don't remember a case when we had to downgrade an apk package

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 28, 2025

About eslint, as I say, if you have more experience than me in npm maybe you can explain me how to solve the problem I mention.

And about the apk “core libraries”, why differentiate them and not be deterministic? We have renovate now.

With this PR and all the PRs of #4543 I want to make the MegaLinter image 100% deterministic, I want that no matter if I build a Dockerfile today or a year from now, the result will be the same.

@nvuillam
Copy link
Member

@bdovaz do you mean you also want to be able to change python alpine base image version ? ^^

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 28, 2025

@bdovaz do you mean you also want to be able to change python alpine base image version ? ^^

Maybe I am not making myself clear. I mean that NO reference to any image or package is unpinned. This way you get deterministic results, that's all I mean.

It is clear that you will not be able to change the versions at will in your flavor for free because some are cascaded as the most core ones: alpine version, python version, ...

But at least as I say, it assures you the security of determinism.

@echoix
Copy link
Collaborator

echoix commented Jan 28, 2025

It's kinda ok to have apk packages pinned, but in a package manager environment like that you don't really have the choice on what versions to install. The whole ecosystem is updated to work together. Alpine keeps only one version of each, per os version. You can't change. But having the version in there helps illustrate what was there at that time. Maybe using ~= to more loosely pin.

I'm pretty sure that we would need to update all the apk packages versions all at once with renovate, possibly at the same time as the alpine versions. That will also mean that we would have to adjust all these pins at the same time we change Python+alpine version (the base image).

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 29, 2025

I'm pretty sure that we would need to update all the apk packages versions all at once with renovate, possibly at the same time as the alpine versions. That will also mean that we would have to adjust all these pins at the same time we change Python+alpine version (the base image).

Effectively, we have to use: https://docs.renovatebot.com/configuration-options/#groupname

Although there is this problem: # renovate: datasource=repology depName=**alpine_3_21**/git.

Which gives the conflict that it would take “2 runs”. One that changes alpine_3_21 and another that updates the package version according to that alpine version, no?

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 29, 2025

I would also appreciate if you could answer me / help me to unlock and merge this PR as soon as possible because it is very important and the more time passes, the more conflicts may arise because many files (descriptors and python core scripts code) have been touched.

@echoix
Copy link
Collaborator

echoix commented Jan 29, 2025

I would also appreciate if you could answer me / help me to unlock and merge this PR as soon as possible because it is very important and the more time passes, the more conflicts may arise because many files (descriptors and python core scripts code) have been touched.

Do we try to fix dotnet linters and release a patch before this?

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 29, 2025

I would also appreciate if you could answer me / help me to unlock and merge this PR as soon as possible because it is very important and the more time passes, the more conflicts may arise because many files (descriptors and python core scripts code) have been touched.

Do we try to fix dotnet linters and release a patch before this?

Yes, sure, I mean merge it after releasing the patch for the .NET 9 issue.

@bdovaz
Copy link
Collaborator Author

bdovaz commented Jan 31, 2025

I have had to downgrade eslint from 9.x to 8.x because there is some other package that required that library to be in 8.x. This problem did not occur when the dependencies were not pinned because I imagine that npm would be “smart” in that sense and would be able to establish which one is the right one.

This is too impacting. It's not fine with me. The configuration format changed, and users have already adapted if they updated. We had some issues filed in this summer if I'm not mistaken, and some community member helped out a bit. You could take a look if you'd.

Wow, what a nice PR , bringing us closer to "bring your own flavor" that we'll build someday :)

Agreed with @echoix , can't downgrade eslint ^^

@nvuillam @echoix about this eslint 9.x vs 8.x topic...

I have been testing and I would say that we were already on 8.x even if you think we were on 9.x....

Look:

With this Dockerfile:

FROM python:3.12.8-alpine3.21

RUN apk add --no-cache \
                npm=10.9.1-r0 \
                nodejs-current=23.2.0-r1

ENV NODE_OPTIONS="--max-old-space-size=8192" \
    NODE_ENV=production

WORKDIR /node-deps
RUN npm --no-cache install --ignore-scripts --omit=dev \
                eslint \
                eslint-config-airbnb

RUN npm ls

The result of npm ls is:

#8 [5/5] RUN npm ls
#8 1.044 (node:1) ExperimentalWarning: CommonJS module /usr/lib/node_modules/npm/node_modules/debug/src/node.js is loading ES Module /usr/lib/node_modules/npm/node_modules/supports-color/index.js using require().
#8 1.044 Support for loading ES Module in require() is an experimental feature and might change at any time
#8 1.044 (Use `node --trace-warnings ...` to show where the warning was created)
#8 1.197 node-deps@ /node-deps
#8 1.197 ├── [email protected]
#8 1.197 └── [email protected]
#8 1.197
#8 DONE 1.3s

And instead with this Dockerfile:

FROM python:3.12.8-alpine3.21

RUN apk add --no-cache \
                npm=10.9.1-r0 \
                nodejs-current=23.2.0-r1

ENV NODE_OPTIONS="--max-old-space-size=8192" \
    NODE_ENV=production

WORKDIR /node-deps
RUN npm --no-cache install --ignore-scripts --omit=dev \
                [email protected] \
                [email protected]

RUN npm ls

It does not work:

#7 1.731 npm error code ERESOLVE
#7 1.732 npm error ERESOLVE unable to resolve dependency tree
#7 1.732 npm error
#7 1.734 npm error While resolving: undefined@undefined
#7 1.734 npm error Found: [email protected]
#7 1.734 npm error node_modules/eslint
#7 1.734 npm error   eslint@"9.19.0" from the root project
#7 1.734 npm error
#7 1.734 npm error Could not resolve dependency:
#7 1.734 npm error peer eslint@"^7.32.0 || ^8.2.0" from [email protected]
#7 1.734 npm error node_modules/eslint-config-airbnb
#7 1.734 npm error   eslint-config-airbnb@"19.0.4" from the root project
#7 1.734 npm error
#7 1.734 npm error Fix the upstream dependency conflict, or retry
#7 1.734 npm error this command with --force or --legacy-peer-deps
#7 1.734 npm error to accept an incorrect (and potentially broken) dependency resolution.

Which is why I was telling you that I had to downgrade from 9.x to 8.x, although as I show you, it is not really a downgrade but we were already on 8.x.

And without pinning it seems that by installing N dependencies in a single command it is “smart” in the sense that it is able to calculate which maximum version of each dependency it can install based on the dependencies/requirements of the rest. And it calculated: 8.x

@echoix
Copy link
Collaborator

echoix commented Jan 31, 2025

Does that mean that we don't have the same eslint versions in our different images?

But anyways, in that case, I agree with you for the downgrade (if it is what the users already have). So, after publishing the patch release, it could be ok to merge this PR. I know it's a lot of work

@bdovaz
Copy link
Collaborator Author

bdovaz commented Feb 1, 2025

Does that mean that we don't have the same eslint versions in our different images?

This is the conclusion that I have come to, which is more dangerous because it depends on the flavor that you use, it may solve a different version of a package as in this case.

With this PR we are sure that all will have exactly the same version.

cc @nvuillam

@echoix
Copy link
Collaborator

echoix commented Feb 1, 2025

Maybe removing a plugin is less disruptive if the version in a major flavor is higher

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants