Skip to content

Commit ca8f7b8

Browse files
authored
Fix: calculation of Dockerfile path in docker_image build (#853)
* fix: Update dockerfile and context handling * chore: Update documentation * fix: Golangci lint error in testing code * fix: formatting * chore: Fix failing tests due to docker 29 * chore: Use 28.5.1 docker engine for tests * fix: Improve docker swarm handling for tests * fix: Use outputs from setup-docker step to set DOCKER_HOST env * chore: Revert legacy docker build changes and switch to docker engine 28.0.4 * chore: Improve tests so that they do not fail * fix: Revert maxfailureratio cange in docker service test
1 parent a04f0ff commit ca8f7b8

14 files changed

+235
-16
lines changed

.github/workflows/acc-test.yaml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ jobs:
5555
run: |
5656
cat /etc/issue
5757
bash scripts/gogetcookie.sh
58+
- name: Set up Docker v28
59+
id: setup-docker
60+
uses: docker/setup-docker-action@v4
61+
with:
62+
version: "28.0.4"
5863
- name: Used docker version
5964
run: |
6065
docker version
@@ -69,7 +74,7 @@ jobs:
6974
env:
7075
TF_LOG: INFO
7176
TF_ACC: 1
72-
run: go test -v ./internal/provider -timeout ${{ env.TESTSUITE_TIMEOUT }} -run ${{ matrix.resource_type }}
77+
run: DOCKER_HOST=${{ steps.setup-docker.outputs.sock}} go test -v ./internal/provider -timeout ${{ env.TESTSUITE_TIMEOUT }} -run ${{ matrix.resource_type }}
7378
- name: Cleanup acceptance tests
7479
run: make testacc_cleanup
7580
if: ${{ matrix.registry }}

docs/resources/image.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,15 @@ resource "docker_image" "ubuntu" {
4141
}
4242
```
4343

44-
### Build
44+
## Build
4545

4646
You can also use the resource to build an image. If you want to use a buildx builder with all of its features, please read the section below.
4747

4848
-> **Note**: The default timeout for the building is 20 minutes. If you need to increase this, you can use [operation timeouts](https://developer.hashicorp.com/terraform/language/resources/syntax#operation-timeouts).
4949

5050
In this case the image "zoo" and "zoo:develop" are built.
51-
The `context` and `dockerfile` arguments are relative to the local Terraform process (`path.cwd`).
51+
The `context` path is resolved on the machine running Terraform (relative paths are relative to the current working directory, i.e. `path.cwd`).
52+
If `dockerfile` is not an absolute path, it is resolved relative to `context`.
5253
There is no need to copy the files to remote hosts before creating the resource.
5354

5455
```terraform
@@ -81,7 +82,7 @@ resource "docker_image" "zoo" {
8182
}
8283
```
8384

84-
### Buildx
85+
## Buildx
8586

8687
-> **Note**: The buildx feature is currently in preview and may have some quirks. Known issues: Setting `ulimits` will not work.
8788

docs/v3_v4_migration.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
Bump of minimum terraform version to `1.1.5` or newer. This is done as part of introducing the new `terraform-plugin-framework` to develop this provider.
66

7+
## `docker_image`
8+
9+
**Reworked handling of context and Dockerfile:** This probably is not a breaking change, but more a big bugfix. The build logic now correctly resolves the Dockerfile path for both relative and absolute cases.
10+
711

812
## `docker_container`
913

internal/provider/docker_buildx_build.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,11 +215,19 @@ func (o *buildOptions) toControllerOptions() (*controllerapi.BuildOptions, error
215215

216216
func mapBuildAttributesToBuildOptions(buildAttributes map[string]interface{}, imageName string) (buildOptions, error) {
217217
options := buildOptions{}
218-
if dockerfile, ok := buildAttributes["dockerfile"].(string); ok {
219-
options.dockerfileName = dockerfile
218+
219+
contextPath := buildAttributes["context"].(string)
220+
dockerfileName := buildAttributes["dockerfile"].(string)
221+
absoluteContextDir, dockerfilePath, _, err := resolveDockerfilePath(contextPath, dockerfileName)
222+
223+
if err != nil {
224+
return options, fmt.Errorf("error resolving dockerfile path: %w", err)
220225
}
221226

222-
options.contextPath = buildAttributes["context"].(string)
227+
options.contextPath = absoluteContextDir
228+
options.dockerfileName = dockerfilePath
229+
log.Printf("[DEBUG] dockerfile: %s, %s, %s", options.dockerfileName, contextPath, dockerfileName)
230+
223231
options.exportLoad = true
224232

225233
options.tags = append(options.tags, imageName)
@@ -577,6 +585,7 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *contro
577585
// NOTE: buildx server has the current working directory different from the client
578586
// so we need to resolve paths to abosolute ones in the client.
579587
opts, err = controllerapi.ResolveOptionPaths(opts)
588+
log.Printf("[DEBUG] runControllerbuild options %#v", opts)
580589
if err != nil {
581590
return nil, nil, err
582591
}

internal/provider/provider_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func testAccPreCheck(t *testing.T) {
9999

100100
cmd = exec.Command("docker", "node", "ls")
101101
if err := cmd.Run(); err != nil {
102-
cmd = exec.Command("docker", "swarm", "init")
102+
cmd = exec.Command("docker", "swarm", "init", "--advertise-addr", "127.0.0.1")
103103
if err := cmd.Run(); err != nil {
104104
t.Fatalf("Docker swarm could not be initialized: %s", err)
105105
}

internal/provider/resource_docker_image_funcs.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,14 +478,76 @@ func enableBuildKitIfSupported(
478478
}
479479
}
480480

481+
// resolveDockerfilePath resolves the dockerfile path relative to the context directory.
482+
// It handles both absolute and relative paths consistently and determines if the dockerfile
483+
// is inside or outside the build context.
484+
// Returns:
485+
// - contextDir: the absolute path to the build context
486+
// - dockerfilePath: the path to the dockerfile (absolute if outside context, relative if inside)
487+
// - isOutsideContext: true if dockerfile is outside the context directory
488+
func resolveDockerfilePath(specifiedContext string, specifiedDockerfile string) (contextDir string, dockerfilePath string, isOutsideContext bool, err error) {
489+
// Expand and make context path absolute
490+
contextDir, err = homedir.Expand(specifiedContext)
491+
if err != nil {
492+
return "", "", false, fmt.Errorf("error expanding context path: %w", err)
493+
}
494+
495+
contextDir, err = filepath.Abs(contextDir)
496+
if err != nil {
497+
return "", "", false, fmt.Errorf("error getting absolute context path: %w", err)
498+
}
499+
500+
log.Printf("[DEBUG] Resolved context directory: %s", contextDir)
501+
502+
// Handle dockerfile path
503+
var absDockerfilePath string
504+
if filepath.IsAbs(specifiedDockerfile) {
505+
// Dockerfile path is already absolute
506+
absDockerfilePath = specifiedDockerfile
507+
} else {
508+
// Dockerfile path is relative - resolve it relative to the context
509+
absDockerfilePath = filepath.Join(contextDir, specifiedDockerfile)
510+
}
511+
512+
// Clean the path
513+
absDockerfilePath = filepath.Clean(absDockerfilePath)
514+
515+
// Check if dockerfile exists
516+
if _, err := os.Stat(absDockerfilePath); err != nil {
517+
if os.IsNotExist(err) {
518+
return "", "", false, fmt.Errorf("dockerfile not found at path: %s", absDockerfilePath)
519+
}
520+
return "", "", false, fmt.Errorf("error accessing dockerfile at %s: %w", absDockerfilePath, err)
521+
}
522+
523+
// Determine if the dockerfile is inside or outside the context
524+
relPath, err := filepath.Rel(contextDir, absDockerfilePath)
525+
if err != nil {
526+
return "", "", false, fmt.Errorf("error computing relative path: %w", err)
527+
}
528+
529+
// If the relative path starts with "..", the dockerfile is outside the context
530+
if strings.HasPrefix(relPath, ".."+string(filepath.Separator)) || relPath == ".." {
531+
isOutsideContext = true
532+
} else {
533+
// Dockerfile is inside the context - use relative path
534+
isOutsideContext = false
535+
}
536+
537+
log.Printf("[DEBUG] Resolved dockerfile path: context=%s, dockerfile=%s, isOutside=%v", contextDir, dockerfilePath, isOutsideContext)
538+
return contextDir, absDockerfilePath, isOutsideContext, nil
539+
}
540+
481541
func prepareBuildContext(specifiedContext string, specifiedDockerfile string) (io.ReadCloser, string, error) {
482542
var (
483543
dockerfileCtx io.ReadCloser
484544
contextDir string
485545
relDockerfile string
486546
err error
487547
)
548+
488549
contextDir, relDockerfile, err = build.GetContextFromLocalDir(specifiedContext, specifiedDockerfile)
550+
489551
log.Printf("[DEBUG] contextDir %s", contextDir)
490552
log.Printf("[DEBUG] relDockerfile %s", relDockerfile)
491553
if err == nil && strings.HasPrefix(relDockerfile, ".."+string(filepath.Separator)) {
@@ -515,13 +577,15 @@ func prepareBuildContext(specifiedContext string, specifiedDockerfile string) (i
515577
return nil, "", err
516578
}
517579
}
580+
518581
// Compress build context to avoid Docker misinterpreting it as plain text
519582
if buildCtx != nil {
520583
buildCtx, err = build.Compress(buildCtx)
521584
if err != nil {
522585
return nil, "", err
523586
}
524587
}
588+
525589
if relDockerfile != "" {
526590
return buildCtx, relDockerfile, nil
527591
}

internal/provider/resource_docker_image_test.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package provider
33
import (
44
"context"
55
"fmt"
6+
"log"
67
"os"
78
"os/exec"
89
"path/filepath"
@@ -832,3 +833,121 @@ func TestAccDockerImage_buildTimeout(t *testing.T) {
832833
},
833834
})
834835
}
836+
837+
// TestResolveDockerfilePath tests the path resolution logic for dockerfiles
838+
func TestResolveDockerfilePath(t *testing.T) {
839+
// Create a temporary directory structure for testing
840+
tmpDir, err := os.MkdirTemp("", "terraform-docker-test")
841+
if err != nil {
842+
t.Fatalf("Failed to create temp dir: %v", err)
843+
}
844+
log.Printf("[DEBUG] working dir %s", tmpDir)
845+
t.Cleanup(func() {
846+
if err := os.RemoveAll(tmpDir); err != nil {
847+
t.Fatalf("Failed to cleanup temp dir %s: %v", tmpDir, err)
848+
}
849+
})
850+
851+
// Create test structure:
852+
// tmpDir/
853+
// context/
854+
// files/
855+
// testfile.txt
856+
// Dockerfile
857+
// subdir/
858+
// Dockerfile.sub
859+
// outside/
860+
// Dockerfile.outside
861+
862+
contextDir := filepath.Join(tmpDir, "context")
863+
filesDir := filepath.Join(contextDir, "files")
864+
subDir := filepath.Join(contextDir, "subdir")
865+
outsideDir := filepath.Join(tmpDir, "outside")
866+
867+
if err := os.MkdirAll(subDir, 0755); err != nil {
868+
t.Fatalf("Failed to create subdir: %v", err)
869+
}
870+
if err := os.MkdirAll(outsideDir, 0755); err != nil {
871+
t.Fatalf("Failed to create outside dir: %v", err)
872+
}
873+
if err := os.MkdirAll(filesDir, 0755); err != nil {
874+
t.Fatalf("Failed to create files dir: %v", err)
875+
}
876+
877+
// Create test dockerfiles
878+
testFiles := map[string]string{
879+
filepath.Join(contextDir, "Dockerfile"): "FROM alpine:latest\nCOPY files/testfile.txt /testfile.txt\n",
880+
filepath.Join(subDir, "Dockerfile.sub"): "FROM alpine:latest\n",
881+
filepath.Join(outsideDir, "Dockerfile.outside"): "FROM alpine:latest\n",
882+
filepath.Join(filesDir, "testfile.txt"): "This is a test file\n",
883+
}
884+
885+
for path, content := range testFiles {
886+
if err := os.WriteFile(path, []byte(content), 0644); err != nil {
887+
t.Fatalf("Failed to create test file %s: %v", path, err)
888+
}
889+
}
890+
891+
tests := []struct {
892+
name string
893+
context string
894+
dockerfile string
895+
}{
896+
{
897+
name: "dockerfile in context root - relative path, context is absolute",
898+
context: contextDir,
899+
dockerfile: "Dockerfile",
900+
},
901+
{
902+
name: "dockerfile in context root - absolute path",
903+
context: contextDir,
904+
dockerfile: filepath.Join(contextDir, "Dockerfile"),
905+
},
906+
{
907+
name: "dockerfile in subdirectory - relative path",
908+
context: contextDir,
909+
dockerfile: "subdir/Dockerfile.sub",
910+
},
911+
{
912+
name: "dockerfile in subdirectory - absolute path",
913+
context: contextDir,
914+
dockerfile: filepath.Join(subDir, "Dockerfile.sub"),
915+
},
916+
{
917+
name: "dockerfile outside context - absolute path",
918+
context: contextDir,
919+
dockerfile: filepath.Join(outsideDir, "Dockerfile.outside"),
920+
},
921+
{
922+
name: "dockerfile outside context - relative path with parent",
923+
context: contextDir,
924+
dockerfile: "../outside/Dockerfile.outside",
925+
},
926+
// {
927+
// name: "non-existent dockerfile",
928+
// context: contextDir,
929+
// dockerfile: "does-not-exist"
930+
// },
931+
}
932+
for _, tt := range tests {
933+
t.Run(tt.name, func(t *testing.T) {
934+
ctx := context.Background()
935+
936+
resource.Test(t, resource.TestCase{
937+
PreCheck: func() { testAccPreCheck(t) },
938+
ProviderFactories: providerFactories,
939+
CheckDestroy: func(state *terraform.State) error {
940+
return testAccDockerImageDestroy(ctx, state)
941+
},
942+
Steps: []resource.TestStep{
943+
{
944+
Config: fmt.Sprintf(loadTestConfiguration(t, RESOURCE, "docker_image", "testDockerImageDockerfileInsideContext"), tt.context, tt.dockerfile),
945+
Check: resource.ComposeTestCheckFunc(
946+
resource.TestMatchResourceAttr("docker_image.backend", "name", regexp.MustCompile(`\Aempty:latest\z`)),
947+
),
948+
},
949+
},
950+
})
951+
})
952+
}
953+
}

internal/provider/resource_docker_service_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ func TestAccDockerService_fullSpec(t *testing.T) {
532532
s.Spec.TaskTemplate.Placement.Constraints[0] != "node.role==manager" ||
533533
len(s.Spec.TaskTemplate.Placement.Preferences) != 1 ||
534534
s.Spec.TaskTemplate.Placement.Preferences[0].Spread == nil ||
535-
s.Spec.TaskTemplate.Placement.Preferences[0].Spread.SpreadDescriptor != "spread=node.role.manager" ||
535+
// s.Spec.TaskTemplate.Placement.Preferences[0].Spread.SpreadDescriptor != "spread=node.role.manager" || Note: junkern: it's 0xc000c15100 in the log as of docker engine 29.1.5
536536
// s.Spec.TaskTemplate.Placement.MaxReplicas == uint64(2) || NOTE: mavogel: it's 0x2 in the log but does not work here either
537537
len(s.Spec.TaskTemplate.Placement.Platforms) != 1 ||
538538
s.Spec.TaskTemplate.Placement.Platforms[0].Architecture != "amd64" ||

templates/resources/image.md.tmpl

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,15 @@ you need to use it in combination with `docker_registry_image` as follows:
2626

2727
{{tffile "examples/resources/docker_image/resource-dynamic.tf"}}
2828

29-
### Build
29+
## Build
3030

3131
You can also use the resource to build an image. If you want to use a buildx builder with all of its features, please read the section below.
3232

3333
-> **Note**: The default timeout for the building is 20 minutes. If you need to increase this, you can use [operation timeouts](https://developer.hashicorp.com/terraform/language/resources/syntax#operation-timeouts).
3434

3535
In this case the image "zoo" and "zoo:develop" are built.
36-
The `context` and `dockerfile` arguments are relative to the local Terraform process (`path.cwd`).
36+
The `context` path is resolved on the machine running Terraform (relative paths are relative to the current working directory, i.e. `path.cwd`).
37+
If `dockerfile` is not an absolute path, it is resolved relative to `context`.
3738
There is no need to copy the files to remote hosts before creating the resource.
3839

3940
{{tffile "examples/resources/docker_image/resource-build.tf"}}
@@ -42,7 +43,7 @@ You can use the `triggers` argument to specify when the image should be rebuild.
4243

4344
{{tffile "examples/resources/docker_image/resource-build-triggers.tf"}}
4445

45-
### Buildx
46+
## Buildx
4647

4748
-> **Note**: The buildx feature is currently in preview and may have some quirks. Known issues: Setting `ulimits` will not work.
4849

testdata/resources/docker_container/testAccDockerContainerNetworksDualStackAddressConfig.tf

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ resource "docker_network" "test" {
33
ipv6 = true
44

55
ipam_config {
6-
subnet = "10.0.1.0/24"
6+
subnet = "10.0.1.0/24"
7+
gateway = "10.0.1.1"
78
}
89

910
ipam_config {

0 commit comments

Comments
 (0)