Containers: insecure registries: allow https (ignore cert errors), and accept config from envvar.

[tmds]

Implements dotnet/sdk-container-builds#338 (comment), dotnet/sdk-container-builds#576.

@dotnet/sdk-container-builds-maintainers ptal.

[tmds]

@baronfel @dotnet/sdk-container-builds-maintainers this is up for review.

[baronfel]

This LGTM. Thanks for expanding this functionality and bringing us more into alignment with the container ecosystem tooling!

I'll log an issue to expand the docs around insecure registries on the docs repo.

[baronfel]

We should also backport this to 8.0.4xx since the initial insecure registry support was backported there as well.

[baronfel]

This failing test is killing me 😓

[tmds]

This failing test is killing me 😓

I see a failure of:

InsecureRegistry(isInsecureRegistry: False, serverIsHttps: False, httpServerCloseAbortive: True) on FullFramework: windows (x64).

The way it fails tells me that when an https connection is made on .NET framework and the peer does an abortive TCP close, then HttpClient isn't throwing HttpRequestException with HttpRequestError.SecureConnectionError.

The http fallback doesn't kick in, and the auth handler throws ApplicationException after a number of retries.

One option is to extend ShouldAttemptFallbackToHttp so it includes this exception (System.Net.Sockets.SocketException (10054): An existing connection was forcibly closed by the remote host.). If we assume it is netfx only we can include that as a condition.

wdyt @baronfel?

[tmds]

The http fallback doesn't kick in

Actually, the fallback shouldn't kick in when isInsecureRegistry: False.
I'll take a fresh look at this tomorrow.

[tmds]

There's definitely a bug in the test in that it doesn't account for the AuthHandshakeMessageHandler to turn HttpRequestException into ApplicationException after retries. I'll fix that first, and then give it another go.

[tmds]

The test bug is fixed.

The test for this feature was more challenging than the implementation. Fortunately, the logging from the test provided a good understanding of why it failed.

[baronfel]

Lovely work. Thank you for the effort!

[baronfel]

/backport to release/8.04xx

[github-actions]

Started backporting to release/8.04xx: https://github.com/dotnet/sdk/actions/runs/9583676452

[github-actions]

@baronfel an error occurred while backporting to release/8.04xx, please check the run log for details!

Error: The specified backport target branch release/8.04xx wasn't found in the repo.

[baronfel]

/backport to release/8.0.4xx

[github-actions]

Started backporting to release/8.0.4xx: https://github.com/dotnet/sdk/actions/runs/9583690663

[github-actions]

@baronfel backporting to release/8.0.4xx failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Containers: insecure registries: allow https (ignore cert errors), and accept config from envvar.
Using index info to reconstruct a base tree...
M	src/Containers/Microsoft.NET.Build.Containers/ContainerHelpers.cs
M	src/Containers/Microsoft.NET.Build.Containers/LocalDaemons/DockerCli.cs
M	src/Containers/Microsoft.NET.Build.Containers/Registry/Registry.cs
A	test/Microsoft.NET.Build.Containers.IntegrationTests/DockerRegistryManager.cs
A	test/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Tests/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
Auto-merging src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/DockerRegistryManager.cs
Auto-merging src/Containers/Microsoft.NET.Build.Containers/Registry/Registry.cs
Auto-merging src/Containers/Microsoft.NET.Build.Containers/LocalDaemons/DockerCli.cs
Auto-merging src/Containers/Microsoft.NET.Build.Containers/ContainerHelpers.cs
Applying: Add tests.
Using index info to reconstruct a base tree...
M	src/Containers/Microsoft.NET.Build.Containers/Registry/Registry.cs
A	test/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Tests/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
Auto-merging src/Containers/Microsoft.NET.Build.Containers/Registry/Registry.cs
Applying: Fix Windows test issue.
Using index info to reconstruct a base tree...
A	test/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Tests/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
Applying: Try fix tests on Windows.
Using index info to reconstruct a base tree...
A	test/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Tests/Microsoft.NET.Build.Containers.UnitTests/RegistryTests.cs
Applying: PR feedback.
Applying: Fix EnsureRegistryLoaded.
Using index info to reconstruct a base tree...
A	test/Microsoft.NET.Build.Containers.IntegrationTests/DockerRegistryManager.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/DockerRegistryManager.cs
Applying: Update WriteToPrivateBasicRegistry test.
Using index info to reconstruct a base tree...
A	test/Microsoft.NET.Build.Containers.IntegrationTests/DockerRegistryTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/DockerRegistryTests.cs
CONFLICT (content): Merge conflict in src/Tests/Microsoft.NET.Build.Containers.IntegrationTests/DockerRegistryTests.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0007 Update WriteToPrivateBasicRegistry test.
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

[github-actions]

@baronfel an error occurred while backporting to release/8.0.4xx, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.