context list: temporarily add ContextType to JSON output

[thaJeztah]

• depends on / stacked on cli/command/context: improve tests and add coverage for custom metadata in contexts #5097

Docker Desktop currently ships with the "cloud integration" wrapper, which outputs an additional ContextType field in the JSON output.

While this field is non-standard, it made its way into Visual Studio's Docker integration, which uses this to exclude "aci" and "eci" context types that are not supported by Visual Studio.

This patch;

• conditionally adds a ContextType field to the JSON output
• but ONLY when using the default "{{json .}}" or "json" formats (which are the formats used by Visual Studio)
• if the context is a "aci" or "eci" context, that type is preserved, otherwise the default "moby" type is used.

- Description for the changelog

Add a temporary `ContextType` field to the JSON output of "docker context ls" for backward-compatibility with Visual Studio.

- A picture of a cute animal (not mandatory but encouraged)

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 61.37%. Comparing base (3da25f6) to head (fed9fa0).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5094   +/-   ##
=======================================
  Coverage   61.37%   61.37%           
=======================================
  Files         298      298           
  Lines       20707    20718   +11     
=======================================
+ Hits        12708    12716    +8     
- Misses       7099     7102    +3     
  Partials      900      900           

[thaJeztah]

Interesting; for some reason static check does NOT complain about using deprecated field 🤔 wondering if we have some config incorrect in golangci-lint; I would expect it to barf;

8.45 cli/command/context/list.go:70:52: directive `//nolint:staticcheck // ignore SA1019: field is deprecated, but used for backward-compatibility.` is unused for linter "staticcheck" (nolintlint)
68.45 				ContextType: getContextType(nil, opts.format), //nolint:staticcheck // ignore SA1019: field is deprecated, but used for backward-compatibility.
68.45 				                                               ^
68.45 cli/command/context/list.go:87:69: directive `//nolint:staticcheck // ignore SA1019: field is deprecated, but used for backward-compatibility.` is unused for linter "staticcheck" (nolintlint)
68.45 			ContextType: getContextType(meta.AdditionalFields, opts.format), //nolint:staticcheck // ignore SA1019: field is deprecated, but used for backward-compatibility.
68.45 			                                                                 ^
68.45 cli/command/context/list.go:105:51: directive `//nolint:staticcheck // ignore SA1019: field is deprecated, but used for backward-compatibility.` is unused for linter "staticcheck" (nolintlint)
68.45 			ContextType: getContextType(nil, opts.format), //nolint:staticcheck // ignore SA1019: field is deprecated, but used for backward-compatibility.

[krissetto]

The binary with this patch will only be used with DD for the upcoming release (and maybe one after that?), is that correct?

[thaJeztah]

Yes(ish); I wanted to avoid having to build some forked version of the CLI, so tried to create a patch with as minimal-as-possible impact, so that we could included it in a v26.1.4 patch release.

[krissetto]

so that we could included it in a v26.1.4 patch release.

if this does go out to "everyone" (and not only dd users), we need to document this in a visible way and be ready in case anything else breaks (i guess, just in case any other project depends on the shape of that json in even worse ways?)

[thaJeztah]

I added 3 commits to improve test-coverage; last commit is the change related to ContextType PTAL

[thaJeztah]

Maybe it's cleaner to move those 3 commits to a separate PR, so that this PR is only showing the changes related to ContextType; let me open a separate PR for the first 3 commits

[laurazard]

LGTM

[thaJeztah]

I'll rebase this one to get rid of the other commits

[thaJeztah]

Rebased and moved out of draft

[krissetto]

LGTM, approving as to not block this.

[krissetto]

One curiosity: could it also be possible to check if the current context being used is from DD? Do we have ways of determining that from here, aside from the context name itself (which i totally wouldn't rely on)?

[thaJeztah]

I think the custom metadata could allow for things like that; we need to better define things here; it's all quite opaque (i.e. custom metadata is supported, but nothing defines it well); some digging may be needed there, because ISTR there's some complex code present to try to match "custom" metadata to be mapped into a specific type / interface.

And that's where my head always explodes, and we need to remove a ton of unneeded complexity.

(in the end, a context is .. well.. not so much different from just key/value pairs ... but it also uses a map[string]any so it's not just "key=value" strings - which is both "great" and "not so great.."😞