Ensure read access when resolving run image location

[pbusko]

Summary

pack performs it's own run-image resolution logic, and must be compliant with the platform spec, in particular with the buildpacks/spec#357

Output

$ docker logout unauthorized.registry.io
Removing login credentials for unauthorized.registry.io
$ pack build --verbose --publish private-registry.corp/my-image:latest
...
CheckReadAccess failed for the run-image unauthorized.registry.io/run:latest, error: GET ***: : Authentication is required
CheckReadAccess succeeded for the run-image authorized.registry.io/run:latest
Selected run image mirror authorized.registry.io/run:latest
...

Before

pack is not compliant with https://github.com/buildpacks/spec/pull/357/files and avoids the run-image resolution baked into the lifecycle (buildpacks/lifecycle#1024)

After

pack will ensure read access during the run-image resolution

Documentation

• Should this change be documented?
  • Yes
  • No

Open Questions

Currently the lifecycle already has read-access and preferable registry resolution for the run-image. What would be the correct place for this? Should it be kept in both places or be left only in a single one?

[jjbustamante]

Hi @pbusko could you rebase your branch?

[jjbustamante]

You also have some issues when running make verify

[natalieparellano]

Currently the lifecycle already has read-access and preferable registry resolution for the run-image.

There are some subtleties around run image mirrors in pack that I'm not sure I fully understand, but I agree that it's confusing that it happens in both places (see also: buildpacks/rfcs#285 (comment)). If anyone has thoughts around how to improve it, please share!

[jjbustamante]

You also have some issues when running make verify

@pbusko I think you just need to run make format to fix them, if you have the time to rebase and run the fix the format issue will merge and cut a pack release candidate today

[loewenstein]

@jjbustamante I just had a look, but what I am seeing is

$ make format verify
=====> Installing goimports...
cd tools && go install golang.org/x/tools/cmd/goimports
=====> Formatting code...
=====> Verifying format...
=====> Installing golangci-lint...
cd tools && go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1
=====> Linting code...
cmd/docker_init.go:73:3: naked return in func `readSecret` with 32 lines of code (nakedret)
		return
		^
pkg/client/common_test.go:122: unnecessary leading newline (whitespace)
		when("desirable run-image is not accessible", func() {

internal/sshdialer/ssh_dialer.go:141:3: naked return in func `networkAndAddressFromRemoteDockerHost` with 49 lines of code (nakedret)
		return
		^
internal/sshdialer/ssh_dialer.go:147:3: naked return in func `networkAndAddressFromRemoteDockerHost` with 49 lines of code (nakedret)
		return
		^
internal/sshdialer/ssh_dialer.go:153:3: naked return in func `networkAndAddressFromRemoteDockerHost` with 49 lines of code (nakedret)
		return
		^
internal/sshdialer/server_test.go:106:3: naked return in func `prepareSSHServer` with 140 lines of code (nakedret)
		return
		^
internal/sshdialer/server_test.go:118:3: naked return in func `prepareSSHServer` with 140 lines of code (nakedret)
		return
		^
internal/sshdialer/server_test.go:122:3: naked return in func `prepareSSHServer` with 140 lines of code (nakedret)
		return
		^
internal/sshdialer/server_test.go:234:4: naked return in func `setupServerAuth` with 59 lines of code (nakedret)
			return
			^
internal/sshdialer/server_test.go:239:4: naked return in func `setupServerAuth` with 59 lines of code (nakedret)
			return
			^
internal/sshdialer/server_test.go:269:4: naked return in func `setupServerAuth` with 59 lines of code (nakedret)
			return
			^
make: *** [lint] Error 1

[jjbustamante]

@jjbustamante I just had a look, but what I am seeing is

$ make format verify
=====> Installing goimports...
cd tools && go install golang.org/x/tools/cmd/goimports
=====> Formatting code...
=====> Verifying format...
=====> Installing golangci-lint...
cd tools && go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1
=====> Linting code...
cmd/docker_init.go:73:3: naked return in func `readSecret` with 32 lines of code (nakedret)
		return
		^
pkg/client/common_test.go:122: unnecessary leading newline (whitespace)
		when("desirable run-image is not accessible", func() {

internal/sshdialer/ssh_dialer.go:141:3: naked return in func `networkAndAddressFromRemoteDockerHost` with 49 lines of code (nakedret)
		return
		^
internal/sshdialer/ssh_dialer.go:147:3: naked return in func `networkAndAddressFromRemoteDockerHost` with 49 lines of code (nakedret)
		return
		^
internal/sshdialer/ssh_dialer.go:153:3: naked return in func `networkAndAddressFromRemoteDockerHost` with 49 lines of code (nakedret)
		return
		^
internal/sshdialer/server_test.go:106:3: naked return in func `prepareSSHServer` with 140 lines of code (nakedret)
		return
		^
internal/sshdialer/server_test.go:118:3: naked return in func `prepareSSHServer` with 140 lines of code (nakedret)
		return
		^
internal/sshdialer/server_test.go:122:3: naked return in func `prepareSSHServer` with 140 lines of code (nakedret)
		return
		^
internal/sshdialer/server_test.go:234:4: naked return in func `setupServerAuth` with 59 lines of code (nakedret)
			return
			^
internal/sshdialer/server_test.go:239:4: naked return in func `setupServerAuth` with 59 lines of code (nakedret)
			return
			^
internal/sshdialer/server_test.go:269:4: naked return in func `setupServerAuth` with 59 lines of code (nakedret)
			return
			^
make: *** [lint] Error 1

Hi @loewenstein, what lint version do you have? let me try to double check, sorry for late response I am on PTO but I want to ship a pack release candidate version :)

[loewenstein]

Hi @jjbustamante,

The only hint at a version, I would draw from the execution log above, i.e.

=====> Installing golangci-lint...
cd tools && go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.1

However, as this seems to install as part of the Makefile, I would also expect comparable results. Maybe the Go versions has an influence?

$ go version
go version go1.21.5 darwin/amd64

Anything else I should check?

[codecov]

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (c08b289) 79.52% compared to head (4566bb5) 79.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2010      +/-   ##
==========================================
- Coverage   79.52%   79.50%   -0.01%     
==========================================
  Files         174      175       +1     
  Lines       13077    13114      +37     
==========================================
+ Hits        10398    10425      +27     
- Misses       2016     2022       +6     
- Partials      663      667       +4     

Flag	Coverage Δ
os_linux	78.42% <78.27%> (-0.01%)	⬇️
os_macos	76.24% <78.27%> (-<0.01%)	⬇️
os_windows	78.88% <78.27%> (-0.01%)	⬇️
unit	79.50% <78.27%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

[pbusko]

Hi @jjbustamante

The styling issue has been fixed, however the build / test (windows-lcow) now fails due to unclear to me reason.

[natalieparellano]

@pbusko the failures are due to a flake that we haven't been able to get to the bottom of. We will try to devote some cycles to it soon

[jjbustamante]

Awesome!!!
Thanks @loewenstein @pbusko