Skip to content
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

Fix Buildx build with Dockerfile outside of Docker build context #1721

Merged
merged 1 commit into from
Nov 19, 2023

Conversation

mruzicka
Copy link
Contributor

@mruzicka mruzicka commented Nov 9, 2023

Fixes a Buildx build failure when an external Dockerfile outside of the Docker build context directory is specified.
The Dockerfile (be it an external one or the generated one) is always stored at the root of the build archive created by DockerAssemblyManager.createDockerTarArchive().
Before this patch the code which constructs the full path of the Dockerfile to pass to Buildx didn't take that into account.

@rohanKanojia
Copy link
Member

@mruzicka : Thanks for your pr, Is it possible to write some test to validate this fix?

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Merging #1721 (ff0d0f0) into master (8e0e480) will increase coverage by 0.11%.
Report is 5 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1721      +/-   ##
============================================
+ Coverage     65.11%   65.23%   +0.11%     
- Complexity     2254     2274      +20     
============================================
  Files           172      172              
  Lines         10111    10173      +62     
  Branches       1391     1403      +12     
============================================
+ Hits           6584     6636      +52     
- Misses         2977     2984       +7     
- Partials        550      553       +3     
Files Coverage Δ
...io/fabric8/maven/docker/service/BuildXService.java 75.97% <100.00%> (ø)

... and 2 files with indirect coverage changes

@mruzicka
Copy link
Contributor Author

mruzicka commented Nov 9, 2023

@mruzicka mruzicka force-pushed the master branch 3 times, most recently from f9588d3 to dd73682 Compare November 9, 2023 18:11
@rhuss
Copy link
Collaborator

rhuss commented Nov 17, 2023

Thanks! Test looks good, and the fix as well. Could you please add an entry to https://github.com/fabric8io/docker-maven-plugin/blob/master/doc/changelog.md for the fix, too ? I think we are good to merge then.

+ Fixes a Buildx build failure when an external Dockerfile outside of the
  build context directory is specified.
  The Dockerfile (be it an external one or the generated one) is always
  stored at the root of the build archive created by:
    DockerAssemblyManager.createDockerTarArchive()
  Bear that in mind when creating the full path of the Dockerfile to pass
  to Buildx.

Signed-off-by: Michal Růžička <michal.ruza@gmail.com>
@mruzicka
Copy link
Contributor Author

Hi, @rhuss, I've added the changelog entry.

Copy link

sonarcloud bot commented Nov 19, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.21) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

@rohanKanojia rohanKanojia merged commit 6be16cf into fabric8io:master Nov 19, 2023
11 checks passed
@rohanKanojia
Copy link
Member

@mruzicka : Thanks a lot!

@mruzicka
Copy link
Contributor Author

Thank you!

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.

3 participants