Add .zip file support for docker context import

[goksu]

- What I did
I've introduced a way for docker context import to accept .zip files.

- How I did it
I've tried to implement it in a way that it can land itself better for what future additions also bring, with addition of Cli enum type. Once we identify which type of import we are trying to get for context, then I go ahead and use the specific import functions, although these functions share as much functionality as possible. I've tried to also make use of an interface of fileData to tie these two functionalities together.

- How to verify it
I've updated the existing tests to define their import types as Cli representing the source. One can test the functionality with using the existing any Docker Context tar file and using the contents to create a .zip file. docker context import will work the same. I've also added new tests to check if the import functions properly to determine these different import types.

- Description for the changelog

Introduces a .zip import support for docker context.

[codecov-io]

Codecov Report

Merging #1895 into master will decrease coverage by 0.04%.
The diff coverage is 49.58%.

@@            Coverage Diff             @@
##           master    #1895      +/-   ##
==========================================
- Coverage   56.75%   56.71%   -0.05%     
==========================================
  Files         309      310       +1     
  Lines       21680    21773      +93     
==========================================
+ Hits        12305    12349      +44     
- Misses       8476     8517      +41     
- Partials      899      907       +8

[thaJeztah]

GitHub is having issues, and looks like they have a full outage of delivering webhooks

[codecov-io]

Codecov Report

Merging #1895 into master will increase coverage by 0.01%.
The diff coverage is 62.79%.

@@            Coverage Diff             @@
##           master    #1895      +/-   ##
==========================================
+ Coverage    56.7%   56.71%   +0.01%     
==========================================
  Files         309      310       +1     
  Lines       21721    21792      +71     
==========================================
+ Hits        12316    12359      +43     
- Misses       8504     8518      +14     
- Partials      901      915      +14

[goksu]

@thaJeztah — All comments are resolved, and checks are passing, let me know about the next steps please, thank you! 💯

[thaJeztah]

Some quick comments

[thaJeztah]

I took a quick stab at making some of the changes that were suggested here; pushed it as a PR, but feel free to include it here if it looks good: #1908

[goksu]

@thaJeztah — The changes look good, I'll bring them in, just one question. I've noticed that you've used the limited reader for the file itself, but not the individual archived files. Is that on purpose, or should I also use limited reader for those files too when brining the changes in.

Thank you for your help, let me know.

[thaJeztah]

I left them out for the individual files as that would be a bit of a corner case, but I'm not against doing it for both

[goksu]

@thaJeztah — Following your very helpful PR, I've implemented all those changes here as well. Was not able to cherry-pick across forks, but all updates you made are here as well. I've added a test to check the content type generation & added limited reader usage for individual Zipped files.

I will keep an eye on the CI builds, I am hoping all will pass and will land this PR for merging possibilities well. Thank you!

[thaJeztah]

Thanks! Could you also squash some (or all) of the commits?

[thaJeztah]

Ideally we'd have a test as well to try importing from a ZIP

[goksu]

Squashed commits, added tests for specifically testing .zip & CI is looking good, I think we are almost there with the PR.

Let me know, thank you!

[thaJeztah]

@tiborvass @silvin-lubecki PTAL

[simonferquel]

lgtm

[tiborvass]

io.LimitReader does not only error with EOF. https://golang.org/src/io/io.go#L448

I would much rather use io.LimitReader. Unless I'm misunderstanding something.

[tiborvass]

OK I understand now, what you want is https://golang.org/pkg/net/http/#MaxBytesReader but with io instead of http.

[goksu]

I think I wasn't clear in the comment there. I think the main concern here from @thaJeztah & me was the fact that, if reader hits the limit it won't tell us it's actually over the limit, but instead will just return an EOF.

[goksu]

Ah, yes, the comment in there returns a non-EOF error for a Read beyond the limit is basically what we wanted.

[tiborvass]

Ok so you can simplify this further, because all you need is just a 2 lines change around https://golang.org/src/io/io.go?s=14885:14942#L443

// LimitedReader is a fork of io.LimitedReader to override Read.
type LimitedReader struct {
	R Reader // underlying reader
	N int64  // max bytes remaining
}

// Read is a fork of io.LimitReader.Read that returns an error when limit is exceeded (instead of io.EOF).
func (l *LimitedReader) Read(p []byte) (n int, err error) {
	if l.N < 0 {
		return 0, errors.New("read limit exceeded")
	}
	if l.N == 0 {
		return 0, io.EOF
	}
	if int64(len(p)) > l.N {
		p = p[0:l.N]
	}
	n, err = l.R.Read(p)
	l.N -= int64(n)
	return
}

This should possibly in its own ioutils package, although I see we have on in docker/docker/pkg/ioutils.
Maybe we keep it here for now (with the risk of forgetting about it forever).

[tiborvass]

You don't need limitedReadAll. Just call ioutil.ReadAll inline.

[goksu]

I've implemented the Read as discussed, and called LimitedReader inline from all the places used for context import. I'll resolve this conversation just for the sake of the iteration causing the code being outdated. Please feel free to open another thread if need be.

[thaJeztah]

Got some failure in CI;

=== FAIL: cli/command/context TestCreateFromContext/no-override (600.02s)
SIGQUIT: quit
PC=0x405b9a m=0 sigcode=0

goroutine 20 [running]:
runtime.chanrecv(0xc0004ca600, 0x0, 0x0, 0x0)
	/usr/local/go/src/runtime/chan.go:456 +0x6a fp=0xc0003357d0 sp=0xc000335740 pc=0x405b9a
runtime.selectnbrecv(0x0, 0xc0004ca600, 0xc00003e050)
	/usr/local/go/src/runtime/chan.go:636 +0x3a fp=0xc000335800 sp=0xc0003357d0 pc=0x40644a
io.(*pipe).Read(0xc0002a20f0, 0xc00033a000, 0x1000, 0x1000, 0x0, 0x11a8fa0, 0xc00003e050)
	/usr/local/go/src/io/pipe.go:45 +0x4d fp=0xc0003358b8 sp=0xc000335800 pc=0x468d9d
io.(*PipeReader).Read(0xc000320868, 0xc00033a000, 0x1000, 0x1000, 0x0, 0x11a8fa0, 0xc00003e050)
	/usr/local/go/src/io/pipe.go:127 +0x4c fp=0xc000335900 sp=0xc0003358b8 pc=0x4696ec
bufio.(*Reader).Read(0xc00034e2a0, 0xc000385468, 0x1f9, 0x200, 0x0, 0x11a8fa0, 0xc00003e050)
	/usr/local/go/src/bufio/bufio.go:223 +0x23e fp=0xc000335958 sp=0xc000335900 pc=0x504d9e
github.com/docker/cli/cli/context/store.(*LimitedReader).Read(0xc00047dc60, 0xc000385468, 0x1f9, 0x200, 0x0, 0x0,

[goksu]

Pushed a fix for the build. My recent push to fix the linter on empty return statement, err was carried as nil, I've properly changed that statement now, tests should pass as expected.

[thaJeztah]

LGTM if green

[thaJeztah]

And it's green 🎉