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.

Sign up for GitHub

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

Jump to bottom

caddy: add suport for compiling Caddy with plugins #358586

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+80 −6
Open

caddy: add suport for compiling Caddy with plugins #358586

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 80 additions & 6 deletions pkgs/by-name/ca/caddy/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
, testers
, installShellFiles
, stdenv
, go
, xcaddy
, cacert
}:
let
version = "2.8.4";
Expand All @@ -32,7 +35,8 @@ buildGoModule {
subPackages = [ "cmd/caddy" ];

ldflags = [
"-s" "-w"
"-s"
"-w"
"-X github.com/caddyserver/caddy/v2.CustomVersion=${version}"
];

Expand Down Expand Up @@ -61,12 +65,82 @@ buildGoModule {
--zsh <($out/bin/caddy completion zsh)
'';

passthru.tests = {
inherit (nixosTests) caddy;
version = testers.testVersion {
command = "${caddy}/bin/caddy version";
package = caddy;
passthru = {
tests = {
inherit (nixosTests) caddy;
version = testers.testVersion {
command = "${caddy}/bin/caddy version";
package = caddy;
};
};
withPlugins =
{ plugins
Copy link
Member

Choose a reason for hiding this comment

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

Quick question, should we make the build fail if a plugin is provided without the version string?

For example:

pkgs.caddy.withPlugins {
  plugins = [ "github.com/caddy-dns/[email protected]" "github.com/caddy-dns/cloudflare" ];
  hash = "sha256-AoW35l7QkXunjBzZ43IlyU3UkVXw2D4eyc1jx8xpT0U=";
}

After testing this on my darwin machine, I have the following:

$ /nix/store/icp2z20hpf2ps7g4n5rzqdkg5qsjp38z-caddy-2.8.4/bin/caddy build-info
...
dep	github.com/caddy-dns/cloudflare	v0.0.0-20240703190432-89f16b99c18e
dep	github.com/caddy-dns/powerdns	v1.0.1	
...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think I can add an assertion.

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 have added an assertion (using the first non-versioned plugin as an example).

, hash ? lib.fakeHash
}: caddy.overrideAttrs (finalAttrs: prevAttrs:
let
pluginsSorted = builtins.sort builtins.lessThan plugins;
pluginsList = lib.concatMapStrings (plugin: "${plugin}-") pluginsSorted;
pluginsHash = builtins.hashString "md5" pluginsList;
pluginsWithoutVersion = builtins.filter (p: !lib.hasInfix "@" p) pluginsSorted;
in
assert lib.assertMsg (builtins.length pluginsWithoutVersion == 0)
"All plugins should have a version (eg ${builtins.elemAt pluginsWithoutVersion 0}@x.y.z)!";
{
vendorHash = null;
subPackages = [ "." ];

src = stdenv.mkDerivation {
pname = "caddy-src-with-plugins-${pluginsHash}";
Copy link
Member

Choose a reason for hiding this comment

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

Small nit, imo pluginsHash makes the build log a bit too long, maybe caddy-src-with-plugins is good enough here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's there to ensure a cached build is not used when adding/removing a plugin. This was one of the request in #317881 (comment).

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 have switched to md5 to reduce the length a bit. We could also use a subset of the hash if it's still too long.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if using md5 is a good idea, IIRC it's considered outdated and insecure (citation needed)?

I took a look into the comment you linked, it seems like it'd be a good idea to include a test to make sure specified plugins are properly installed. I came up with the following but don't have time to dig deeper (feel free to take whatever you need):

diff --git a/pkgs/by-name/ca/caddy/package.nix b/pkgs/by-name/ca/caddy/package.nix
index eea6894ce328..c052da5ef290 100644
--- a/pkgs/by-name/ca/caddy/package.nix
+++ b/pkgs/by-name/ca/caddy/package.nix
@@ -116,6 +116,31 @@ buildGoModule {
             outputHash = hash;
             outputHashAlgo = "sha256";
           };
+
+          doInstallCheck = true;
+          installCheckPhase = ''
+            runHook preInstallCheck
+
+            build_info="$($out/bin/caddy build-info)"
+
+            for plugin in ''${plugins[@]}; do
+              # this won't work :(
+              echo $plugin
+              url=$(echo "$plugin" | cut -d'@' -f1)
+              version=$(echo "$plugin" | cut -d'@' -f2)
+              echo $url
+              echo $version
+
+              if echo "$build_info" | grep -q "$url[[:space:]]*$version"; then
+                echo "$plugin found in build-info"
+              else
+                echo "$plugin not found in build-info" >&2
+                exit 1
+              fi
+            done
+
+            runHook postInstallCheck
+          '';
       });
   };

For testing:

nom-build --expr 'with import ./. { }; caddy.withPlugins { plugins = [ "github.com/caddy-dns/[email protected]" "github.com/caddy-dns/[email protected]" ]; hash = "sha256-AoW35l7QkXunjBzZ43IlyU3UkVXw2D4eyc1jx8xpT0U="; }'

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 have added a postInstallCheck as you suggest.

As for the MD5, this is not meant to be secure, just a safety to ensure the user does not forget to update the hash when modifying plugins. A mechanism like this was requested in the original PR.

version = finalAttrs.version;

nativeBuildInputs = [
go
xcaddy
cacert
];
dontUnpack = true;
buildPhase =
let
withArgs = lib.concatMapStrings (plugin: "--with ${plugin} ") pluginsSorted;
in
''
export GOCACHE=$TMPDIR/go-cache
export GOPATH="$TMPDIR/go"
XCADDY_SKIP_BUILD=1 TMPDIR="$PWD" xcaddy build v${finalAttrs.version} ${withArgs}
(cd buildenv* && go mod vendor)
'';
installPhase = ''
mv buildenv* $out
'';

outputHashMode = "recursive";
outputHash = hash;
outputHashAlgo = "sha256";
};


doInstallCheck = true;
installCheckPhase = ''
runHook preInstallCheck

${lib.toShellVar "notfound" pluginsSorted}
while read kind module version; do
[[ "$kind" = "dep" ]] || continue
module="''${module}@''${version}"
for i in "''${!notfound[@]}"; do
if [[ ''${notfound[i]} = ''${module} ]]; then
unset 'notfound[i]'
fi
done
done < <($out/bin/caddy build-info)
if (( ''${#notfound[@]} )); then
>&2 echo "Plugins not found: ''${notfound[@]}"
exit 1
fi

runHook postInstallCheck
'';
});
};

meta = with lib; {
Expand Down