additional authentication files

[vrothberg]

Allow specifying additional authentication files passed through
types.SystemContext. Those additional files are only used for reading
credentials. The AuthFilePath remains the center of auth management
and is the only authentication file used for storing new and removing
old credentials.

Having the ability to specify additional authentication files can come
in handy to unify multiple entities that have dedicated auth files, for
instance, the K8s kubelet. See the below bug for additional details.

Also add debug logs to findAuthentication() to ease debugging
potential future regressions.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1686556
Signed-off-by: Valentin Rothberg [email protected]

[rhatdan]

Small nit, but LGTM

[mtrmac]

The code is plausible, but the purpose of the code is a bit surprising to me.

The config file already supports multiple entries in one file; it’s counterintuitive that a single caller (with a single privilege level) would ever really need to / want to maintain two distinct config files.

More importantly, my initial impression of https://bugzilla.redhat.com/show_bug.cgi?id=1686556 (just skimming the discussions, without having read all the relevant code or reproduced it) is that in that case we are not passing any credentials, or that we are passing one incorrect set; not that there genuinely are two sets that need merging. Is that incorrect?

[mtrmac]

I don’t quite think there four lines are with a function… it’s three lines in the caller, which already has a paths variable, and we have ~5 lines of overhead with defining the function.

Rather, the full computation of all candidate paths should probably be unified between GetAuthentication and GetUserLoggedIn; it seems a bit unlikely that the existing divergence was intentional (OTOH I haven’t checked the past discussions.)

[mtrmac]

I don’t quite think there four lines are with a function

“are worth a function”… macOS autocorrection is infuriating sometimes.

[ashcrow]

Travis report:

 github.com/containers/image/storage
storage/storage_image.go:735:55: not enough arguments in call to s.imageRef.transport.store.SetImageBigData
	have (string, string, []byte)
	want (string, string, []byte, func([]byte) (digest.Digest, error))
storage/storage_image.go:768:54: not enough arguments in call to s.imageRef.transport.store.SetImageBigData
	have (string, string, []byte)
	want (string, string, []byte, func([]byte) (digest.Digest, error))
storage/storage_image.go:775:54: not enough arguments in call to s.imageRef.transport.store.SetImageBigData
	have (string, string, []byte)
	want (string, string, []byte, func([]byte) (digest.Digest, error))
storage/storage_image.go:784:55: not enough arguments in call to s.imageRef.transport.store.SetImageBigData
	have (string, string, []byte)
	want (string, string, []byte, func([]byte) (digest.Digest, error))
Makefile:39: recipe for target 'build-internal' failed

[vrothberg]

Travis report:

 github.com/containers/image/storage
storage/storage_image.go:735:55: not enough arguments in call to s.imageRef.transport.store.SetImageBigData
	have (string, string, []byte)
	want (string, string, []byte, func([]byte) (digest.Digest, error))
storage/storage_image.go:768:54: not enough arguments in call to s.imageRef.transport.store.SetImageBigData
	have (string, string, []byte)
	want (string, string, []byte, func([]byte) (digest.Digest, error))
storage/storage_image.go:775:54: not enough arguments in call to s.imageRef.transport.store.SetImageBigData
	have (string, string, []byte)
	want (string, string, []byte, func([]byte) (digest.Digest, error))
storage/storage_image.go:784:55: not enough arguments in call to s.imageRef.transport.store.SetImageBigData
	have (string, string, []byte)
	want (string, string, []byte, func([]byte) (digest.Digest, error))
Makefile:39: recipe for target 'build-internal' failed

Thanks! That comes from containers/storage and containers/image not liking each other over at Skopeo. We need to fix this as well.

[mtrmac]

Thanks! That comes from containers/storage and containers/image not liking each other over at Skopeo.

This is the fallout from containers/storage#299 .

[vrothberg]

Closing as we found another solution for CRI-O :)

[wking]

I think #588 would give an API that makes this sort of thing convenient, even if we may be going with openshift/machine-config-operator#535 in this particular case ;).