use absolute paths to the container_image.tar in bazel build.

[reltuk]

Fixes: #7600
Related: #7251, #7281, #7430

Description

This PR changes the bazel build task to get an absolute path to the built container image. The correct path is the concatenation of the bazel execution_root and the result of the cquery for the output path of the configured target.

As per #7600, the current approach is broken if workspace symlinks are disabled. It's actually also broken if workspace symlinks are anything other than bazel-. The current approach is also broken if skaffold.yaml is not in the workspace root and Artifact.Workspace does not refer to the directory that contains the workspace symlinks. Previously, the Artifact.Workspace did not need to refer to the root of the workspace for a bazel build target, since the target can be an absolute path and it will build regardless of where bazel is invoked.

I believe using the absolute path is more robust and handles more use cases than the current approach.

User facing changes

• skaffold bazel builds and pushes with --symlink_prefix passed to bazel will work.
• skaffold bazel builds and pushes in directories that are not the root of the bazel workspace and where context: is not set to navigate to the root of the bazel workspace will work.

[codecov]

Codecov Report

Merging #7671 (53223ca) into main (290280e) will decrease coverage by 3.79%.
The diff coverage is 53.87%.

@@            Coverage Diff             @@
##             main    #7671      +/-   ##
==========================================
- Coverage   70.48%   66.68%   -3.80%     
==========================================
  Files         515      587      +72     
  Lines       23150    28228    +5078     
==========================================
+ Hits        16317    18824    +2507     
- Misses       5776     8033    +2257     
- Partials     1057     1371     +314     

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/credits/export.go	0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go	40.90% <0.00%> (-12.94%)	⬇️
cmd/skaffold/app/cmd/flags.go	93.00% <ø> (+2.18%)	⬆️
cmd/skaffold/app/cmd/test.go	44.44% <0.00%> (ø)
cmd/skaffold/skaffold.go	0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go	62.50% <14.28%> (-1.14%)	⬇️
cmd/skaffold/app/cmd/render.go	35.48% <18.18%> (-5.90%)	⬇️
cmd/skaffold/app/cmd/lsp.go	28.12% <28.12%> (ø)
cmd/skaffold/app/cmd/run.go	64.28% <33.33%> (-9.63%)	⬇️
cmd/skaffold/app/cmd/fix.go	56.41% <35.71%> (-20.07%)	⬇️
... and 346 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[aaron-prindle]

Thanks for the PR @reltuk! Seems there is currently a Skaffold unit test failing on Windows related to this change:

--- FAIL: TestBazelTarPath (0.03s)
    --- FAIL: TestBazelTarPath/EmptyExecutionRoot (0.02s)
        build_test.go:10[8](https://github.com/GoogleContainerTools/skaffold/runs/7527004738?check_suite_focus=true#step:6:9): string differ (-got, +want):   string(
            - 	`\absolute\path\bin`,
            + 	"/absolute/path/bin",
              )
    --- FAIL: TestBazelTarPath/AbsoluteExecutionRoot (0.02s)
        build_test.go:[12](https://github.com/GoogleContainerTools/skaffold/runs/7527004738?check_suite_focus=true#step:6:13)2: string differ (-got, +want):   strings.Join({
            - 	`\var\tmp\`,
            + 	"/var/tmp/",
              	"bazel-execution-roots",
            - 	`\abcdefg\execroot\workspace_name\bazel-bin\`,
            + 	"/abcdefg/execroot/workspace_name/bazel-bin/",
              	"darwin-fastbuild-ST-confighash",
            - 	`\path\to\`,
            + 	"/path/to/",
              	"bin",
              }, "")

Seems the abs path logic might not have some issues on Windows atm. Once we fix test we should be able to merge!

[reltuk]

@aaron-prindle Thanks for the feedback and for bringing the test failure on windows to my attention. I think filepath.Join is normalizing the path separator, but the mocked data in the test setup does not take that behavior into account. I suspect adding a normalizing call in the test setup will fix it. I’m currently traveling but will hopefully be able to fix this soon.

Thanks again!

[reltuk]

@tejal29 Thank you so much for looking at this while I’m traveling! Much appreciated :)