CI: build an additional `arm64` docker image

[Tomaszal]

Resolves https://github.com/cs3org/wopiserver/issues/109.

Blocked by https://github.com/cs3org/reva/pull/3642.

I've never used GitHub workflows before, so I'm not 100% sure this will work as expected.

Also, I created a separate Dockerfile for arm64 as I had to switch from python:3.11-alpine to python:3.10-slim-buster base image. This is because it was failing to building on alpine even with the changes mentioned on https://github.com/grpc/grpc/issues/24722, and grpcio failed to build on python-3.11 even on slim-buster.

[glpatcern]

Hi @Tomaszal, thanks, with @vascoguita we have reviewed this and we rather propose to use slim-buster for all platforms to simplify the CI and avoid duplication. Can you approve the PR?

[vascoguita]

Hi @Tomaszal, thanks, with @vascoguita we have reviewed this and we rather propose to use slim-buster for all platforms to simplify the CI and avoid duplication. Can you approve the PR?

https://github.com/Tomaszal/wopiserver/pull/1

[Tomaszal]

Hi @Tomaszal, thanks, with @vascoguita we have reviewed this and we rather propose to use slim-buster for all platforms to simplify the CI and avoid duplication. Can you approve the PR?

Tomaszal#1

Done, also switched apk to apt package manager, as that's what the base image uses

[micbar]

@glpatcern This is bad from a maintainablility POV

I wanted to avoid a big docker base image like debian because it is badly maintained and has too many vulnerabilities.

See my trivy scan report

python:3.10-slim-buster (debian 10.13)

Total: 130 (UNKNOWN: 0, LOW: 87, MEDIUM: 19, HIGH: 23, CRITICAL: 1)

python:3.10-alpine (alpine 3.17.1)

Total: 16 (UNKNOWN: 0, LOW: 0, MEDIUM: 14, HIGH: 2, CRITICAL: 0)

This will pop up on every customer security scan in the future. In my opinion, we should not "call for trouble" if we do not need to.

Can we find something better maintained?

[glpatcern]

See my trivy scan report

All right, understood. Let's cook something tailored for arm64.

The original PR indeed had all separated, but with quite some duplication. I see we can at least parameterize the Dockerfile.

[Tomaszal]

I've seen that the job failed on the master branch after the latest commit so I checked the logs. It seems the build arguments aren't properly set. Currently they are comma separated (--build-arg VERSION=v9.4.1,BASEIMAGE=python:3.10-slim-buster), but according to the documentation, they need to be specified individually (--build-arg VERSION=v9.4.1 --build-arg BASEIMAGE=python:3.10-slim-buster). I think the parent workflow in reva repository needs to be modified first to fix this.

[glpatcern]

@Tomaszal good catch, we fixed the build arguments in master (and previously I merged this PR to master to ease the debugging process, though it's not yet complete). Now the build complains because presumably apt -y g++ fails in the Dockerfile. Can you have a look?

For reference: https://github.com/cs3org/wopiserver/actions/runs/4142747553

[Tomaszal]

@glpatcern this if statement doesn't seem to work, as it's still trying to run apk add. This is probably because of the single quotes in the if statement, as variables are not expanded then. Try to change them to double quotes.

[glpatcern]

@glpatcern this if statement doesn't seem to work

Interestingly we tried several variants of that if statement but they failed (likely because of docker's interaction with sh). You could give it a try yourself as now the release action can be fired in your fork independently from tags - at least it's configured like that.

[Tomaszal]

@glpatcern I see, I've tried playing around with it a bit and wasn't able to figure out why it wasn't expanding the variable correctly. However, I did come up with an alternative that should work just fine as well. Replace this if statement:

RUN if [ '$BASEIMAGE' = 'python:3.10-slim-buster' ]; then \
      apt -y install g++; \
    else \
      apk add g++; \
    fi

With this:

RUN if command -v apt &> /dev/null; then \
      apt -y install g++; \
    elif command -v apk &> /dev/null; then \
      apk add g++; \
    fi

This is probably a better solution anyway, as it doesn't require hard coding the base image name into the if statement.

[glpatcern]

This is probably a better solution anyway, as it doesn't require hard coding the base image name into the if statement.

Good point, thanks for this! I've pushed the change (including an extra else) and the CI is now all green :+1: