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

ci/renovate: update pinned alpine packages #5

Merged
merged 1 commit into from
Sep 18, 2024
Merged

Conversation

nadiamoe
Copy link
Member

@nadiamoe nadiamoe commented Jul 12, 2024

This adds a regex manager and a custom datasource so renovate can update pinned alpine packages.

Proof of concept (and proof of work) was done in nadiamoe/renovate-alpine#5

@nadiamoe nadiamoe requested a review from a team as a code owner July 12, 2024 11:46
@nadiamoe nadiamoe force-pushed the ci-renovate-alpine branch 2 times, most recently from e9b2d1c to 368e01f Compare July 15, 2024 12:26
"matchStrings": [
// Lines that loosely look like "apk add --repository community something=version".
// To keep this regex simple, only one package per "apk add" is supported.
"apk.+?add.+?(--repository|-X)[= ](?<alpineRepo>[a-z]+).+?(?<depName>[a-z0-9-]+)=(?<currentValue>[a-z0-9-.]+)"
Copy link
Contributor

Choose a reason for hiding this comment

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

do try to match blanks, not anything.

Is it unlikely to happen? Yes.
Does this match apk something something add? Yes.

\s should do it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Does this match apk something something add? Yes.

This is intended. apk --update add tini is a legal command for example (don't ask me why).

Copy link
Contributor

Choose a reason for hiding this comment

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

😭

Copy link
Contributor

Choose a reason for hiding this comment

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

Then match word boundaries at least? So that it matches add and not whateveraddwhatever?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm trying the word boundary approach but I don't get the regex to match.

\sapk\s.+?\sadd\s.+?(--repository|-X)[= ](?<alpineRepo>[a-z]+).+?(?<depName>[a-z0-9-]+)=(?<currentValue>[a-z0-9-.]+)

This does not match any of

  apk --no-cache add --repository community tini=0.19.0-r3 && \
  apk --no-cache add --repository community chromium-swiftshader=126.0.6478.126-r1
  apk add --no-cache --repository community chromium-swiftshader=126.0.6478.126-r1

Removing the last boundary, \sapk\s.+?\sadd.+?(--repository|-X)..., matches

  apk --no-cache add --repository community tini=0.19.0-r3 && \
  apk --no-cache add --repository community chromium-swiftshader=126.0.6478.126-r1

But not

  apk add --no-cache --repository community chromium-swiftshader=126.0.6478.126-r1

I think this is the more correct vs more readable/debuggable regex discussion again 😢
Do you think there is a potential string that would be matched by the simpler one?

Copy link
Contributor

@mem mem Sep 10, 2024

Choose a reason for hiding this comment

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

\bapk\b.+?\badd\b.+?--repository[ \t]+(?<alpineRepo>[a-z]+)[ \t]+(?<depName>[-\w]+?)=(?<currentValue>[-.\w]+)

should do it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately it doesn't seem to capture equal signs for flags
image

Copy link
Member Author

@nadiamoe nadiamoe Sep 13, 2024

Choose a reason for hiding this comment

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

Kay, what about:

\bapk\b.+?\badd\b.+?(--repository|-X)[ =\t]+(?<alpineRepo>[a-z]+)\s+(?<depName>[-\w]+?)=(?<currentValue>[-.\w]+)

Few small changes, highlighted in bold:
image

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately it doesn't seem to capture equal signs for flags

I read English and I have no idea what you are trying to say...

Let me see...

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, you mean the = in --repository=xxx

I was going to suggest that this is fixed by adding a level of indirection, but I can already see that conversation is not going to get anywhere.

So, this one it is.

Base automatically changed from ci-renovate to main July 29, 2024 10:51
@nadiamoe nadiamoe force-pushed the ci-renovate-alpine branch from 368e01f to ab61628 Compare July 29, 2024 10:52
@nadiamoe nadiamoe requested a review from mem July 29, 2024 11:08
@nadiamoe
Copy link
Member Author

nadiamoe commented Aug 1, 2024

Container image build will fail until #8 is merged and the chromium version is updated, that's expected.

@nadiamoe nadiamoe force-pushed the ci-renovate-alpine branch from 39d104a to 660bc39 Compare August 28, 2024 10:57
nadiamoe added a commit that referenced this pull request Sep 6, 2024
Dirty workaround until we have reproducible builds with
#5
nadiamoe added a commit that referenced this pull request Sep 6, 2024
Dirty workaround until we have reproducible builds with
#5
@nadiamoe nadiamoe force-pushed the ci-renovate-alpine branch 2 times, most recently from d7fabfc to 4da23fa Compare September 13, 2024 10:07
Comment on lines +35 to +36
RUN apk --no-cache add --repository community tini=0.19.0-r3 && \
apk --no-cache add --repository community chromium-swiftshader=128.0.6613.119-r0
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm going to leave this here...

There's a meme in widespread circulation that says you should prefer this:

RUN xxx && yyy

over this:

RUN xxx
RUN yyy

Most people tend to ignore the footnote to this meme, namely, that this recommendation has everything to do with an implementation detail of docker filesystem layers. Since the default (?) driver uses a unionfs, if yyy ends up deleting a file that xxx added, that results in behavior that is annoying: the file is added to the layer created by the first command, shipped over the network (during fetch), and then the layer created by yyy deletes it.

I have no idea if that applies here or not, because I don't know if the second command overwrites files from the first command (it's very likely that the answer is yes), but I will assert that it doesn't matter. However big the difference is, it's not large enough. And in the cases where the difference is big enough, there are other ways to deal with this.

At any rate, this solves a number of headaches:

# renovate: datasource=alpine-community
RUN apk --no-cache add --repository community tini=0.19.0-r3

# renovate: datasource=alpine-community
RUN apk --no-cache add --repository community chromium-swiftshader=128.0.6613.119-r0

And your regular expression is something like:

# renovate: datasource=(?<datasource>[a-z-]+)\s+RUN\s+apk\s+.+?\s+(?<depName>[a-z0-9-]+)=(?<currentValue>[a-z0-9-.]+)

(I hate that this thing doesn't use the x modifier)

One of the "other ways" I mentioned above is something like this:

COPY my-script /local/bin
RUN /local/bin/my-script

and that one can be written in a way that is less wacky, e.g.

REPO=community PACKAGE=tini VERSION=0.19.0-r3
apk --no-cache add --repository $REPO $PACKAGE=$VERSION

Anyways...

Copy link
Member Author

Choose a reason for hiding this comment

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

I too agree that the cargo cult of adding a gazillion &&s is a bit annoying, that being the reason why I went for the "one apk add per line" and not per RUN instruction: The regex should support both.

REPO=community PACKAGE=tini VERSION=0.19.0-r3
apk --no-cache add --repository $REPO $PACKAGE=$VERSION

This was the naive option I first considered: Set up a simple regex that looks for three specific variables in a very specific order. Then I thought "I can do better! More readable!" and came up with a complex regex that works for 80% of apk commands. I'm... not entirely sure that's a good tradeoff.

Just to get done with this, let's merge with our fusion regex and then I'll open a second PR with the "three variables" syntax, and see if we like it.

@nadiamoe nadiamoe merged commit b577bd7 into main Sep 18, 2024
5 checks passed
@nadiamoe nadiamoe deleted the ci-renovate-alpine branch September 18, 2024 09:21
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.

2 participants