fix insecure manifest inspect with restrictive certs perms #1378
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1378 +/- ##
=========================================
- Coverage 54.88% 54.79% -0.1%
=========================================
Files 293 293
Lines 19428 19428
=========================================
- Hits 10663 10645 -18
- Misses 8089 8110 +21
+ Partials 676 673 -3 |
vdemeester
requested review from
justincormack,
silvin-lubecki,
thaJeztah and
vdemeester
| serviceOpts := registry.ServiceOptions{} | ||
| if insecure { | ||
| logrus.Debugf("allowing insecure registry for: %s", reference.Domain(namedRef)) | ||
| serviceOpts.InsecureRegistries = []string{reference.Domain(namedRef)} |
There was a problem hiding this comment.
The name was a bit confusing, so checked if reference.Domain() returned the port-number as well, but looks like it does 😅 👍 https://github.com/docker/distribution/blob/6170ac53daed40ebbebc9fe5f482495d01f09f12/reference/reference_test.go#L51-L55
There was a problem hiding this comment.
yes it does. i double-checked that b/c i had the same concern. :) thanks @thaJeztah!
|
ping @justincormack @silvin-lubecki ptal |
cli/registry/client/fetcher.go
Outdated
| } | ||
| registryService, err := registry.NewService(registry.ServiceOptions{}) | ||
|
|
||
| serviceOpts := registry.ServiceOptions{} |
There was a problem hiding this comment.
nit: var serviceOpts registry.ServiceOptions
There was a problem hiding this comment.
fixed that and re-pushed
If, for some reason, the certs directory has permissions that are inaccessible by docker, we should still be able to fetch manifests using the `insecure` flag. Since the cli doesn't access the engine's list of insecure registries, the registry client should make a singleton list of the registry being queried with the `insecure` flag. Closes docker#1358 Signed-off-by: Christy Norman <[email protected]>
clnperez
force-pushed
the
manifest-inspect-insecure-fix
branch
from
7e84791 to
d57adbc
Compare
|
Thanks! |
…e-fix fix insecure manifest inspect with restrictive certs perms Signed-off-by: Lifubang <[email protected]>
6 participants
If, for some reason, the certs directory has permissions that are
inaccessible by docker, we should still be able to fetch manifests using
the
insecureflag.Since the cli doesn't access the engine's list of insecure registries,
the registry client should make a singleton list of the registry being queried with the
insecureflag.Closes #1358
Signed-off-by: Christy Norman [email protected]