test: buildkit with defaults

[MiahaCybersec]

Closes #595

[MiahaCybersec]

It looks like the failing test is "default buildkit address", but the test runs and passes locally. Is it possible that the test is failing because GitHub actions doesn't have a locally running BuildKit client?

The test initializes bkOpts with no values, so bkOpts.Addr is an empty string when NewClient is called. It then tries to connect to a locally running BuildKit service (autoClient) which is where the errors are originating from.

@cpuguy83 mentioned at a previous LFX mentorship meeting that the old TODO is likely referring to testing BuildKit with nothing passed in, which is why this test does not perfectly match up with the old TODO in code.

[ashnamehrotra]

@MiahaCybersec can you check what docker version is being used in the workflow? If it is not 23.0 or later, buildkit will not be enabled by default, and you would need to change the unit test workflow to enable it

[ashnamehrotra]

LGTM

[cpuguy83]

Can you have more details on this comment?
Its not clear from the call why this should be faulty and may produce something else in the future.

[MiahaCybersec]

That comment was left in by mistake, the address validation in client.New does not properly parse invalid addresses. If an address such as 300.300.300.300 is passed in, it shows no errors.

I've pushed a new commit that updates the test to be named faulty key path and added additional error validation for the tests.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 34.22%. Comparing base (1fcc6e0) to head (c68fbc4).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #682      +/-   ##
==========================================
+ Coverage   33.01%   34.22%   +1.20%     
==========================================
  Files          18       18              
  Lines        1578     1578              
==========================================
+ Hits          521      540      +19     
+ Misses       1024     1007      -17     
+ Partials       33       31       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[MiahaCybersec]

I'm not sure why the lint is failing. It says platforms.Normalize is deprecated, but then says to use platforms.Normalize.

[sozercan]

@MiahaCybersec haha that's confusing. Looks like it's deprecated in favor of platforms.Normalize in github.com/containerd/platforms. If you change the import from github.com/containerd/containerd/platforms to github.com/containerd/platforms it should work.

[MiahaCybersec]

The unit tests for GetDockerTransport have been removed because they were incomplete. The tests did not include all of the necessary checks, and mocks would be required to properly test them. The unit tests will be added in a separate pull request to increase test coverage.