Extract tsuru.yaml from docker context #49
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to extract tsuru.yaml from the Docker build context when it's not present in the container filesystem. This provides a fallback mechanism for containerfile-based builds to access Tsuru configuration files from the build context.
Key changes:
- New
findAndReadTsuruYamlfunction to search for and read tsuru.yaml files from the build context directory - Integration in
buildFromContainerFileto use context-based tsuru.yaml when not found in the built container image - Test case demonstrating the extraction behavior with testdata files
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/build/buildkit/build.go | Adds findAndReadTsuruYaml function and integrates tsuru.yaml extraction from context as a fallback in buildFromContainerFile |
| pkg/build/buildkit/build_test.go | Adds test case verifying tsuru.yaml extraction from context when not present in container filesystem |
| pkg/build/buildkit/testdata/tsuru-files-from-context/Dockerfile | Test fixture with minimal Dockerfile that doesn't copy config files to container |
| pkg/build/buildkit/testdata/tsuru-files-from-context/tsuru.yaml | Test fixture containing sample tsuru.yaml configuration |
| pkg/build/buildkit/testdata/tsuru-files-from-context/Procfile | Test fixture containing sample Procfile configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if tc.TsuruYaml == "" { | ||
| tsuruYamlData, err := findAndReadTsuruYaml(tmpDir) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if tsuruYamlData != "" { | ||
| tc.TsuruYaml = tsuruYamlData | ||
| } | ||
| } |
There was a problem hiding this comment.
The test data directory contains a Procfile, but the implementation only extracts tsuru.yaml from the context directory. Consider implementing similar fallback logic for Procfile extraction when it's not present in the container filesystem. The test case "Dockerfile w/ build context (adds tsuru.yaml and Procfile)" (line 649) shows that both files should be handled consistently when they exist in the build context.
| appFiles, err := NewBuildKit(bc, BuildKitOptions{TempDir: t.TempDir()}).Build(context.TODO(), req, os.Stdout) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, &pb.TsuruConfig{ | ||
| Procfile: "", |
There was a problem hiding this comment.
The test data includes a Procfile but the test expects an empty Procfile field. This is inconsistent with the presence of the Procfile in the testdata directory. Either the Procfile should be removed from the testdata if it's not intended to be extracted, or the expected value should be updated to match the Procfile content and the implementation should be enhanced to extract it from context (similar to how tsuru.yaml is extracted).
1 participant
Sometimes users forget do add this ugly line:
ADD tsuru.yaml /my suggestion is to auto identify tsuru.yaml from context when user use:
tsuru app deploy -a APP --dockerfile .