Skip to content

fix: Dockerfile ARGs and Nixpacks ARGs for preview builds #5909

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

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

djsisson
Copy link
Contributor

@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
Copy link
Contributor Author

djsisson commented May 30, 2025

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.

@peaklabs-dev peaklabs-dev changed the title fix for dockerfile ARG and nixpacks ARG in dockerfiles (fix for preview builds) fix: Dockerfile ARGs and Nixpacks ARGs for preview builds Jun 1, 2025
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