Build image from source #234
Conversation
|
I took from what you did but changed a few bits. You'll need to ignore some bits, as I changed them so I can request the version I want to build, as I have a wrapper script that calls this as I had issues where the HTTP API version didn't match what needed to be built. My wrapper script, automates the updating process effectively. If we move to this new method, the bitBetter C# program is no longer required, just the licenseGen. So any code regarding that can be removed from All we'd need to do now is
Use licenseGen to generate any license files. Thoughts? How I run the script, working dir is #!/bin/sh
DIR=`dirname "$0"`
DIR=`exec 2>/dev/null;(cd -- "$DIR") && cd -- "$DIR"|| cd "$DIR"; unset PWD; /usr/bin/pwd || /bin/pwd || pwd`
BW_VERSION=$1
echo "Building Api,Identity for BitWarden version $BW_VERSION (BitBetter)"
# If there aren't any keys, generate them first.
[ -e "$DIR/.keys/cert.cert" ] || "$DIR/.keys/generate-keys.sh"
# Prepare Bitwarden repository
rm -rf $DIR/server
git clone --branch "v${BW_VERSION}" --depth 1 https://github.com/bitwarden/server.git $DIR/server
old_thumbprint=$(openssl x509 -fingerprint -noout -in $DIR/server/src/Core/licensing.cer | cut -d= -f2 | tr -d ':')
new_thumbprint=$(openssl x509 -fingerprint -noout -in $DIR/.keys/cert.cert | cut -d= -f2 | tr -d ':')
cp $DIR/.keys/cert.cert $DIR/server/src/Core/licensing.cer
# Optional, has actually no effect
sed -i -e "s/$old_thumbprint/$new_thumbprint/g" $DIR/server/src/Core/Services/Implementations/LicensingService.cs
# Enable loose files for API, so Core.dll is accessible
sed -i -e 's/PublishSingleFile=true/PublishSingleFile=false/g' $DIR/server/src/Api/Dockerfile
docker build --no-cache --build-arg BITWARDEN_TAG=ghcr.io/bitwarden/api:$BW_VERSION --label com.bitwarden.product="bitbetter" $DIR/server -f $DIR/server/src/Api/Dockerfile -t bitbetter/api:$BW_VERSION
if [ $? -ne 0 ]; then
echo "Error: bitwarden/api build container failed."
exit 1
fi
docker build --no-cache --build-arg BITWARDEN_TAG=ghcr.io/bitwarden/identity:$BW_VERSION --label com.bitwarden.product="bitbetter" $DIR/server -f $DIR/server/src/Identity/Dockerfile -t bitbetter/identity:$BW_VERSION
if [ $? -ne 0 ]; then
echo "Error: bitwarden/identity container failed."
exit 1
fi
docker tag bitbetter/api:$BW_VERSION bitbetter/api:latest
docker tag bitbetter/identity:$BW_VERSION bitbetter/identity:latest
# Remove old instances of the image after a successful build.
ids=$( docker images bitbetter/* | grep -E -v -- "CREATED|latest|${BW_VERSION}" | awk '{ print $3 }' )
[ -n "$ids" ] && docker rmi $ids || true |
|
In case this is useful to anyone, on Debian 11 (Bullseye) you will get an error (for both certs) when running this, presumably because Debian 11 uses openssl 1.1.1w (Bullseye LTS expires next year):
Piping it back through openssl to convert the cert seemed to produce the correct results:
Results from Debian 12 (Bookworm):
|
|
Hi guys, I can confirm that this PR allows to successfully build bitwarden images for 2025.6.1. Thanks a lot @Genva ! Bye |
There was a problem hiding this comment.
Overall I think we can remove the files that are not included anymore in the new build: I think the whole src/bitBetter folder can be removed? I'm not really sure tho.
Nice addition the set -e line.
Also thanks to @FingerlessGlov3s for the suggestions.
A final note: do we want to support the old openssl1.1.1w version included in Bullseye LTS? Personally I'd advocate against it, since openssl 1.1.1 already reached EOL.
|
After further inspection of the code I am quite confident that the entirety of the src/bitBetter folder can now be removed.
The last point will consist in modifying some parts of the If everyone agrees then I kindly ask @Genva if you are able to update the PR to reflect the changes proposed in the first two points and submit a draft to solve the third point so that we can also help adjusting the code and testing out everything. |
|
The I kept the thumbprint replacement inside the |
There was a problem hiding this comment.
I checked the linked resources that use the cert's thumbprint.
If I read the code correctly, the thumbprint is used to retrieve the certificate only in not-self-hosted instances.
That said I think we could try to remove that part and test the code.
Kudos for the modified Program.cs retaining the retro compatibility with loose-files bitwarden images!
|
I tested out the latest code and unfortunately we were wrong about the certificate fingerprint: at some point it is very much needed! (I can't tell when unfortunately) If you are interested, these are the logs of the Api container (Error due to a failed SSO login):In conclusion we have to revert the last commit (d8465e1) I tested out the script after adding back the following lines to Now everything seems to work:
|
|
Also I think we need to update the |
This reverts commit d8465e1.
|
I reverted the latest commit. As a matter of fact, here they validate the thumbprint of the loaded certificate against the hardcoded one. Must have overlooked it when I checked the constructor. |
|
Great catch. As of now I think the PR can be considered done, as long as no one else is willing to test this implementation more thoroughly. Just a minor note at this point: it might be beneficial to edit the comment about the replacing of the cert's thumbprint having "no actual effect", to avoid anyone falsely believing that those lines are in fact not useful. |
|
Before calling it a day, I'd like to share a thought that I had in mind regarding this solution: Until the last version the BitBetter code was in fact modifying the This of course would be not ideal since we spent the past few days coding and reviewing another solution, but I wonder if we should just ignore the other possibility. |
|
@Fly7113 You were right, I changed the comment. About modifying the single file application in a way, we did before. It might be possible, but I don't know how. If you are interested feel free to check it in a separate PR. For now, I'd take this solution as it works. |
|
@captainhook @h44z What's your take on this PR? Ready to be merged? |
I'll run some tests on my end before I approve. Will invite a few others to test and approve since this is a big change. Please kindly include comments in your testing including license gen/import as well as general use of the application. |
captainhook
requested review from
Fly7113,
h44z and
captainhook
and removed request for
FingerlessGlov3s
There was a problem hiding this comment.
Tests done:
- Image patching: works as expected, both from fresh install and already patched with previous versions; build time is sensibly higher as expected (5-10 times higher than before, depending on machine specs);
- User license generation: works as expected;
- User license validation: works as expected, all premium features available;
- Org license generation: works as expected;
- Org license validation: works as expected, all premium features available (SSO, Key Connector, SCIM, AD);
|
I can confirm that this method works to build the new images. |
|
Agreed on the TARGETPLATFORM - or set a default for amd64. Could anyone add this in please? I will run a test tomorrow - apologies for the delay. @h44z if you have any capacity to test, would be grateful since this is a large change. |
|
Merged #236 - not blocking. Please consider for main. |
As proposed in #233. Building images using the official Bitwarden repository.