coolify icon indicating copy to clipboard operation
coolify copied to clipboard

fix: Dockerfile ARGs and Nixpacks ARGs for preview builds

Open djsisson opened this issue 6 months ago • 1 comments

@andrasbacsai @peaklabs-dev

Please note i haven't tested this, but should give you an idea of what needs changing

There are 2 current issues regarding build ARGS

  1. build args are inserted into a dockerfile, but they are spliced after line 1 Since ARGS are scoped to the stage they are in, line 1 is usually a FROM command, so currently the ARGS are only valid in the inital stage and unavailable later when they are needed if it's a multi stage build

This is corrected by inserting them before any lines such that they are now globally scoped, as well as ensuring they remain in the same order this means later build args can now use values from a coolify build arg as this cant be done in a flag (this could be a setting to enable/disable inserting build args, some users might not want their dockerfile changing)

  1. build args are passed into the build command but are not inserted into the nixpacks dockerfile, instead you are relying purely on the .env file, however in preview builds this is called .env-pr-xxx so no framework will pick them up in a preview build, so currently you cant use build args in a nixpacks preview build

This now sets the dockerfile to the created nixpacks dockerfile, and inserts the build args at the start

this should fix any env issues

p.s im unsure if the array lines are correct, but it's to ensure the env end up in the same order

thanks

djsisson avatar May 29 '25 10:05 djsisson

so, i think i was mistaken regarding the scoping of Build ARGS

a globally scoped Build ARG still needs to be redefined in any stage it is used

so your initial splice at line 1 was sort of correct for single stage builds (but not always)

so my suggestion would be not to insert at the start, but after the first or even after ever FROM line, you would only need to set the key, as the values are passed in the cli

since you can't guarantee the first FROM line is on line 1, you would need to check where to insert

i think nixpacks are always single stage builds, though im not sure if FROM is always on line one or if there is any comments

but if nixpacks is always single stage, then you can splice at 1 for nixpacks builds, but for normal Dockerfile builds you can ask users to include a comment (#COOLIFY BUILD ARG) in their Dockerfile of where they want build args to be inserted, maybe thats a better solution than adding them after every FROM, as it's rare they are going to be single stage

but that should be quick to fix

this would be after every FROM stage

$fromIndex = 0;
foreach ($dockerfile as $index => $line) {
    if (str_starts_with($line, 'FROM')) {
        $dockerfile->splice($fromIndex + 1, 0, $argLines);
        $fromIndex = $index + 1;
    }
}

this would be after first FROM if its nixpacks, or after #COOLIFYBUILDARGS if its in dockerfile, otherwise no insertion is done

$spliceIndex = null;
foreach ($dockerfile as $index => $line) {
    if ($this->application->build_pack === 'nixpacks' && str_starts_with($line, 'FROM')) {
        $spliceIndex = $index + 1;
        break;
    } elseif (str_starts_with($line, '#COOLIFYBUILDARGS')) {
        $spliceIndex = $index + 1;
        break;
    }
}

if ($spliceIndex !== null) {
    $dockerfile->splice($spliceIndex, 0, $argLines);
}

my preference would be the latter option, since it puts the agency on the user, i.e nothing about their Dockerfile is changed unless they have requested it. And for nixpacks it would be a requirement, so that's ok.

Hope that makes sense

*edit

Of course you could always just name the .env file .env for preview builds too then you wouldn't need to insert these at all, however I honestly think no .env file should be added to the cloned repo since it might accidently get exposed if copied, and it shouldn't be needed since they are passing in build args. However for compose builds you can inject a .env file but not in the repo directory just in a tmp dir, since you pass it as a flag so docker compose can use it for interpolation. Ideally a users repo should be completely unchanged.

djsisson avatar May 30 '25 07:05 djsisson