Add functionality to pull image manifests to DockerClient #4140
Conversation
amogh09
force-pushed
the
manifest-fetch-func
branch
from
5043ff2
to
71a2210
Compare
amogh09
force-pushed
the
manifest-fetch-func
branch
2 times, most recently
from
64ce955
to
e76bf47
Compare
amogh09
changed the title
amogh09
force-pushed
the
manifest-fetch-func
branch
from
e76bf47
to
b9437d4
Compare
amogh09
force-pushed
the
manifest-fetch-func
branch
from
b9437d4
to
bdb3741
Compare
| }, | ||
| expectedDistributionInspect: testDistributionInspect, | ||
| }, | ||
| func() testCase { |
There was a problem hiding this comment.
TIL: Didn't know we could do this!
There was a problem hiding this comment.
Yes how cool is that!
There was a problem hiding this comment.
Allows tighter scoping of variables.
| // Get auth creds | ||
| sdkAuthConfig, err := dg.getAuthdata(imageRef, authData) | ||
| if err != nil { | ||
| return registry.DistributionInspect{}, wrapPullErrorAsNamedError(imageRef, err) |
There was a problem hiding this comment.
Q: Just curious, any reasons why we can't use nil if there were errors?
There was a problem hiding this comment.
Ah that's because this method has a value return type and not a pointer so the compiler won't allow returning nil. I think it's standard practice to return zero value (that's what var d registry.DistributionInspect would initialize to as well). I just followed the method signature of Docker SDK which is
DistributionInspect(ctx context.Context, imageRef, encodedRegistryAuth string) (registry.DistributionInspect, error)
It is just like how functions that have a (string, error) return type would return "", nil for error cases and not a nil. The user must check the returned error before using the returned value.
Summary
This PR adds
PullImageManifestmethod toDockerClientinterface and adds an implementation of the method for*dockerGoClient. The method is supposed to get image manifest for the provided image reference and return the manifest as a value ofregistry.DistributionInspecttype.A new integration test is added for
*dockerGoClient's implementation ofPullImageManifestthat checks that the implementation works for public and private registries. For the test,scripts/setup-test-registryscript is updated to start a private registry on the host in addition to the public registry it already starts.Implementation details
PullImageManifestis added toDockerClientinterface.*dockerGoClienttype. The implementation resolves registry credentials in the same way asPullImageand then calls Docker client's DistributionInspect API to fetch the image manifest from the registry.TestImageManifestPullIntegis added that checks thatPullImageManifestmethod of*dockerGoClientis able to pull image manifests from public and private registries.scripts/setup-test-registryscript is updated to start a private registry in addition to the public registry that it already starts. The private registry is used by the test. The private registry uses basic auth credentials to authenticate and authorize requests. The script generates an htpasswd file which is provided to the registry. The file contains a single basic auth username/password pair where the username is "username" and password is "password". Password in the file is hashed using bcrypt algorithm as required by the registry container. So, requests to the registry need to include basic auth credentials that match this username and password pair.sdkclient.Clientinterface that is a wrapper around Docker client's API is updated to includeDistributionInspectmethod.Testing
Added new unit and integration tests.
New tests cover the changes: yes
Description for the changelog
Does this PR include breaking model changes? If so, Have you added transformation functions?
Enhancement: Add image manifest pull functionality to DockerClient
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.