Skip to content

Commit 449bf6e

Browse files
authored
Merge pull request #177 from techulus/fix/dockerfile-path-rebuild
Rebuild when Dockerfile path changes
2 parents eb84adc + a130910 commit 449bf6e

5 files changed

Lines changed: 206 additions & 28 deletions

File tree

‎agent/internal/build/build.go‎

Lines changed: 67 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,15 @@ type Builder struct {
4848
const (
4949
staleBuildDirAge = 1 * time.Hour
5050
staleTempArtifactAge = 24 * time.Hour
51+
dockerfilePathKey = "TECHULUS_DOCKERFILE_PATH"
5152
)
5253

54+
type dockerfileConfig struct {
55+
directory string
56+
filename string
57+
found bool
58+
}
59+
5360
var managedTempArtifactPattern = regexp.MustCompile(`^(backup|restore)-[0-9a-fA-F-]{36}\.tar\.gz$|^restore-extract-[0-9a-fA-F-]{36}$`)
5461

5562
func NewBuilder(dataDir string, logSender LogSender) *Builder {
@@ -226,23 +233,9 @@ func (b *Builder) buildAndPush(ctx context.Context, config *Config, buildDir str
226233
b.sendLog(config, fmt.Sprintf("Using root directory: %s", config.RootDir))
227234
}
228235

229-
dockerfileRelPath := "Dockerfile"
230-
hasDockerfile := false
231-
forcedDockerfile := false
232-
if customPath := config.Secrets["TECHULUS_DOCKERFILE_PATH"]; customPath != "" {
233-
if !filepath.IsLocal(customPath) {
234-
return fmt.Errorf("TECHULUS_DOCKERFILE_PATH must be a relative path within the build context, got %q", customPath)
235-
}
236-
resolved := filepath.Join(contextDir, customPath)
237-
if info, err := os.Stat(resolved); err != nil || info.IsDir() {
238-
b.sendLog(config, fmt.Sprintf("TECHULUS_DOCKERFILE_PATH is set but %s was not found in the repository", customPath))
239-
return fmt.Errorf("TECHULUS_DOCKERFILE_PATH %q not found in build context", customPath)
240-
}
241-
dockerfileRelPath = filepath.Clean(customPath)
242-
hasDockerfile = true
243-
forcedDockerfile = true
244-
} else if _, err := os.Stat(filepath.Join(contextDir, "Dockerfile")); err == nil {
245-
hasDockerfile = true
236+
dockerfile, err := resolveDockerfile(contextDir, config.Secrets)
237+
if err != nil {
238+
return err
246239
}
247240

248241
buildkitAddr := os.Getenv("BUILDKIT_HOST")
@@ -253,6 +246,9 @@ func (b *Builder) buildAndPush(ctx context.Context, config *Config, buildDir str
253246
var secretArgs []string
254247
var secretEnv []string
255248
for key, value := range config.Secrets {
249+
if key == dockerfilePathKey {
250+
continue
251+
}
256252
secretArgs = append(secretArgs, "--secret", fmt.Sprintf("id=%s,env=%s", key, key))
257253
secretEnv = append(secretEnv, fmt.Sprintf("%s=%s", key, value))
258254
}
@@ -266,10 +262,10 @@ func (b *Builder) buildAndPush(ctx context.Context, config *Config, buildDir str
266262
archImageUri := config.ImageURI + "-" + arch
267263
archOutputFlag := fmt.Sprintf("type=image,name=%s,push=true,registry.insecure=true", archImageUri)
268264

269-
if hasDockerfile {
270-
log.Printf("[build:%s] building with Dockerfile %s via buildctl for %s", truncateStr(config.BuildID, 8), dockerfileRelPath, platform)
271-
if forcedDockerfile {
272-
b.sendLog(config, fmt.Sprintf("Using Dockerfile %s (from TECHULUS_DOCKERFILE_PATH)", dockerfileRelPath))
265+
if dockerfile.found {
266+
log.Printf("[build:%s] building with Dockerfile via buildctl for %s", truncateStr(config.BuildID, 8), platform)
267+
if configuredPath := strings.TrimSpace(config.Secrets[dockerfilePathKey]); configuredPath != "" {
268+
b.sendLog(config, fmt.Sprintf("Using Dockerfile: %s", configuredPath))
273269
} else {
274270
b.sendLog(config, "Using existing Dockerfile")
275271
}
@@ -280,8 +276,8 @@ func (b *Builder) buildAndPush(ctx context.Context, config *Config, buildDir str
280276
"build",
281277
"--frontend", "dockerfile.v0",
282278
"--local", "context=.",
283-
"--local", fmt.Sprintf("dockerfile=%s", filepath.Dir(dockerfileRelPath)),
284-
"--opt", fmt.Sprintf("filename=%s", filepath.Base(dockerfileRelPath)),
279+
"--local", fmt.Sprintf("dockerfile=%s", dockerfile.directory),
280+
"--opt", fmt.Sprintf("filename=%s", dockerfile.filename),
285281
"--opt", fmt.Sprintf("platform=%s", platform),
286282
"--output", archOutputFlag,
287283
}
@@ -350,6 +346,54 @@ func (b *Builder) buildAndPush(ctx context.Context, config *Config, buildDir str
350346
return nil
351347
}
352348

349+
func resolveDockerfile(contextDir string, secrets map[string]string) (dockerfileConfig, error) {
350+
configuredPath, configured := secrets[dockerfilePathKey]
351+
configuredPath = strings.TrimSpace(configuredPath)
352+
if configured && configuredPath == "" {
353+
return dockerfileConfig{}, fmt.Errorf("%s cannot be empty", dockerfilePathKey)
354+
}
355+
356+
if !configured {
357+
if _, err := os.Stat(filepath.Join(contextDir, "Dockerfile")); err == nil {
358+
return dockerfileConfig{directory: ".", filename: "Dockerfile", found: true}, nil
359+
} else if !os.IsNotExist(err) {
360+
return dockerfileConfig{}, fmt.Errorf("failed to inspect Dockerfile: %w", err)
361+
}
362+
return dockerfileConfig{}, nil
363+
}
364+
365+
cleanedPath := filepath.Clean(configuredPath)
366+
if filepath.IsAbs(cleanedPath) || cleanedPath == ".." || strings.HasPrefix(cleanedPath, ".."+string(filepath.Separator)) {
367+
return dockerfileConfig{}, fmt.Errorf("%s must be relative to the service root directory", dockerfilePathKey)
368+
}
369+
370+
info, err := os.Stat(filepath.Join(contextDir, cleanedPath))
371+
if err != nil {
372+
return dockerfileConfig{}, fmt.Errorf("dockerfile %s does not exist: %w", configuredPath, err)
373+
}
374+
if info.IsDir() {
375+
return dockerfileConfig{}, fmt.Errorf("dockerfile path %s is a directory", configuredPath)
376+
}
377+
resolvedContextDir, err := filepath.EvalSymlinks(contextDir)
378+
if err != nil {
379+
return dockerfileConfig{}, fmt.Errorf("failed to resolve service root directory: %w", err)
380+
}
381+
resolvedPath, err := filepath.EvalSymlinks(filepath.Join(contextDir, cleanedPath))
382+
if err != nil {
383+
return dockerfileConfig{}, fmt.Errorf("failed to resolve Dockerfile %s: %w", configuredPath, err)
384+
}
385+
relativePath, err := filepath.Rel(resolvedContextDir, resolvedPath)
386+
if err != nil || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) {
387+
return dockerfileConfig{}, fmt.Errorf("%s must resolve inside the service root directory", dockerfilePathKey)
388+
}
389+
390+
return dockerfileConfig{
391+
directory: filepath.Dir(cleanedPath),
392+
filename: filepath.Base(cleanedPath),
393+
found: true,
394+
}, nil
395+
}
396+
353397
func (b *Builder) runCommand(cmd *exec.Cmd, config *Config) (string, error) {
354398
output, err := cmd.CombinedOutput()
355399
outputStr := string(output)

‎agent/internal/build/build_test.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,88 @@ func TestCloneDeepensConfiguredBranchForSelectedCommit(t *testing.T) {
119119
}
120120
}
121121

122+
func TestResolveDockerfile(t *testing.T) {
123+
contextDir := t.TempDir()
124+
if err := os.WriteFile(filepath.Join(contextDir, "Dockerfile"), []byte("FROM scratch"), 0600); err != nil {
125+
t.Fatal(err)
126+
}
127+
if err := os.Mkdir(filepath.Join(contextDir, "docker"), 0700); err != nil {
128+
t.Fatal(err)
129+
}
130+
if err := os.WriteFile(filepath.Join(contextDir, "docker", "Dockerfile.prod"), []byte("FROM scratch"), 0600); err != nil {
131+
t.Fatal(err)
132+
}
133+
if err := os.WriteFile(filepath.Join(contextDir, "Dockerfile.custom"), []byte("FROM scratch"), 0600); err != nil {
134+
t.Fatal(err)
135+
}
136+
if err := os.Mkdir(filepath.Join(contextDir, "dockerfiles"), 0700); err != nil {
137+
t.Fatal(err)
138+
}
139+
outsideDir := t.TempDir()
140+
if err := os.WriteFile(filepath.Join(outsideDir, "Dockerfile"), []byte("FROM scratch"), 0600); err != nil {
141+
t.Fatal(err)
142+
}
143+
if err := os.Symlink(outsideDir, filepath.Join(contextDir, "outside")); err != nil {
144+
t.Fatal(err)
145+
}
146+
147+
tests := []struct {
148+
name string
149+
secrets map[string]string
150+
directory string
151+
filename string
152+
wantErr bool
153+
}{
154+
{name: "default", secrets: map[string]string{}, directory: ".", filename: "Dockerfile"},
155+
{
156+
name: "nested custom path",
157+
secrets: map[string]string{dockerfilePathKey: "docker/Dockerfile.prod"},
158+
directory: "docker",
159+
filename: "Dockerfile.prod",
160+
},
161+
{
162+
name: "root custom filename",
163+
secrets: map[string]string{dockerfilePathKey: "Dockerfile.custom"},
164+
directory: ".",
165+
filename: "Dockerfile.custom",
166+
},
167+
{name: "missing custom path", secrets: map[string]string{dockerfilePathKey: "missing.Dockerfile"}, wantErr: true},
168+
{name: "empty custom path", secrets: map[string]string{dockerfilePathKey: " "}, wantErr: true},
169+
{name: "absolute custom path", secrets: map[string]string{dockerfilePathKey: "/tmp/Dockerfile"}, wantErr: true},
170+
{name: "escaping custom path", secrets: map[string]string{dockerfilePathKey: "../Dockerfile"}, wantErr: true},
171+
{name: "directory custom path", secrets: map[string]string{dockerfilePathKey: "dockerfiles"}, wantErr: true},
172+
{name: "symlink escape", secrets: map[string]string{dockerfilePathKey: "outside/Dockerfile"}, wantErr: true},
173+
}
174+
175+
for _, tt := range tests {
176+
t.Run(tt.name, func(t *testing.T) {
177+
got, err := resolveDockerfile(contextDir, tt.secrets)
178+
if tt.wantErr {
179+
if err == nil {
180+
t.Fatal("expected an error")
181+
}
182+
return
183+
}
184+
if err != nil {
185+
t.Fatal(err)
186+
}
187+
if !got.found || got.directory != tt.directory || got.filename != tt.filename {
188+
t.Fatalf("resolveDockerfile() = %+v, want directory=%q filename=%q", got, tt.directory, tt.filename)
189+
}
190+
})
191+
}
192+
}
193+
194+
func TestResolveDockerfileFallsBackToRailpack(t *testing.T) {
195+
got, err := resolveDockerfile(t.TempDir(), map[string]string{})
196+
if err != nil {
197+
t.Fatal(err)
198+
}
199+
if got.found {
200+
t.Fatalf("resolveDockerfile() = %+v, want no Dockerfile", got)
201+
}
202+
}
203+
122204
func runGit(t *testing.T, args ...string) string {
123205
t.Helper()
124206
output, err := exec.Command("git", args...).CombinedOutput()

‎web/components/service/details/pending-changes-banner.tsx‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ import { deployService } from "@/actions/projects";
99
import { Button } from "@/components/ui/button";
1010
import { Spinner } from "@/components/ui/spinner";
1111
import type { ServiceWithDetails as Service } from "@/db/types";
12-
import type { ConfigChange } from "@/lib/service-config";
12+
import {
13+
type ConfigChange,
14+
hasBuildAffectingChanges,
15+
} from "@/lib/service-config";
1316

1417
interface PendingChangesBannerProps {
1518
service: Service;
@@ -37,8 +40,9 @@ export const PendingChangesBanner = memo(function PendingChangesBanner({
3740
0,
3841
);
3942
const hasNoDeployments = service.deployments.length === 0;
40-
const isGithubWithNoDeployments =
41-
service.sourceType === "github" && hasNoDeployments;
43+
const shouldBuild =
44+
service.sourceType === "github" &&
45+
(hasNoDeployments || hasBuildAffectingChanges(changes));
4246

4347
const hasChanges = changes.length > 0;
4448
const showBanner =
@@ -48,7 +52,7 @@ export const PendingChangesBanner = memo(function PendingChangesBanner({
4852
const handleDeploy = async () => {
4953
setIsDeploying(true);
5054
try {
51-
if (isGithubWithNoDeployments) {
55+
if (shouldBuild) {
5256
await triggerBuild(service.id);
5357
router.push(
5458
`/dashboard/projects/${projectSlug}/${envName}/services/${service.id}/builds`,
@@ -92,7 +96,7 @@ export const PendingChangesBanner = memo(function PendingChangesBanner({
9296
) : (
9397
<Rocket className="size-4" data-icon="inline-start" />
9498
)}
95-
{isGithubWithNoDeployments ? "Build" : "Deploy"}
99+
{shouldBuild ? "Build" : "Deploy"}
96100
</Button>
97101
</div>
98102
{hasChanges ? (

‎web/lib/service-config.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,17 @@ export type ConfigChange = {
7676
field: string;
7777
from: string;
7878
to: string;
79+
secretKey?: string;
7980
};
8081

82+
export const TECHULUS_DOCKERFILE_PATH = "TECHULUS_DOCKERFILE_PATH";
83+
84+
export function hasBuildAffectingChanges(changes: ConfigChange[]): boolean {
85+
return changes.some(
86+
(change) => change.secretKey === TECHULUS_DOCKERFILE_PATH,
87+
);
88+
}
89+
8190
export function buildCurrentConfig(
8291
service: {
8392
image: string;
@@ -238,6 +247,7 @@ export function diffConfigs(
238247
field: "Secret",
239248
from: "(none)",
240249
to: secret.key,
250+
secretKey: secret.key,
241251
});
242252
}
243253
for (const volume of current.volumes || []) {
@@ -482,12 +492,14 @@ export function diffConfigs(
482492
field: "Secret",
483493
from: "(none)",
484494
to: key,
495+
secretKey: key,
485496
});
486497
} else if (deployedSecret.updatedAt !== currentSecret.updatedAt) {
487498
changes.push({
488499
field: "Secret",
489500
from: key,
490501
to: `${key} (updated)`,
502+
secretKey: key,
491503
});
492504
}
493505
}
@@ -498,6 +510,7 @@ export function diffConfigs(
498510
field: "Secret",
499511
from: key,
500512
to: "(removed)",
513+
secretKey: key,
501514
});
502515
}
503516
}

‎web/tests/service-config.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ import {
33
type DeployedConfig,
44
diffConfigs,
55
getCurrentServerlessConfig,
6+
hasBuildAffectingChanges,
67
MIN_SERVERLESS_SLEEP_AFTER_SECONDS,
78
revisionSpecToDeployedConfig,
9+
TECHULUS_DOCKERFILE_PATH,
810
} from "@/lib/service-config";
911

1012
function deployedConfig(
@@ -95,4 +97,37 @@ describe("service config", () => {
9597
to: "Disabled",
9698
});
9799
});
100+
101+
it("requires a build when the Dockerfile path is added, updated, or removed", () => {
102+
const dockerfileSecret = {
103+
key: TECHULUS_DOCKERFILE_PATH,
104+
updatedAt: "2026-07-20T00:00:00.000Z",
105+
};
106+
const withoutSecret = deployedConfig({ secrets: [] });
107+
const withSecret = deployedConfig({ secrets: [dockerfileSecret] });
108+
const updatedSecret = deployedConfig({
109+
secrets: [{ ...dockerfileSecret, updatedAt: "2026-07-20T01:00:00.000Z" }],
110+
});
111+
112+
expect(
113+
hasBuildAffectingChanges(diffConfigs(withoutSecret, withSecret)),
114+
).toBe(true);
115+
expect(
116+
hasBuildAffectingChanges(diffConfigs(withSecret, updatedSecret)),
117+
).toBe(true);
118+
expect(
119+
hasBuildAffectingChanges(diffConfigs(withSecret, withoutSecret)),
120+
).toBe(true);
121+
});
122+
123+
it("does not require a build for unrelated environment variables", () => {
124+
const changes = diffConfigs(
125+
deployedConfig({ secrets: [] }),
126+
deployedConfig({
127+
secrets: [{ key: "DATABASE_URL", updatedAt: "2026-07-20" }],
128+
}),
129+
);
130+
131+
expect(hasBuildAffectingChanges(changes)).toBe(false);
132+
});
98133
});

0 commit comments

Comments
 (0)