Support plain http registries that are not localhost/127.x.x.x #2301
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carolynvs
force-pushed
the
use-image-relocation-fork
branch
8 times, most recently
from
f9c5ac0 to
ab97b1d
Compare
After fixing insecure registries, that use an untrusted TLS certificate, it was discovered that unsecured registries (plain http) don't work when communicating with a non-localhost or loopback address. Basically the library has no way to automatically determine that we should use http, and we need to rely on the --insecure-registry flag to know that plain http is okay too. The porter publish --archive and porter copy commands were affected by this because they used the github.com/pivotal/image-relocation library, which never supported configuring plain http, only detecting based on the hostname (e.g. localhost/127.0.0.1). I have created a fork of the image relocation library at https://github.com/cnabio/image-relocation that has a workaround for not being able to configure plain http. I am checking if skipTLS is configured for the http transport passed to the image-relocation library, and also allowing plain http in that case too. This means that the --insecure-registry flag now properly controls plain http too for that library. The fork has a different go module name so that we don't need to forever maintain a replace statement for that library since it's archived/unmaintained. I have updated the airgap smoke test to check insecure and unsecured registry functions and included a copy as well so that the bulk of our --insecure-registry test cases are in that one test. Signed-off-by: Carolyn Van Slyck <[email protected]>
carolynvs
force-pushed
the
use-image-relocation-fork
branch
from
ab97b1d to
939480e
Compare
* Remove the mybuns-with-img-ref and use an image reference in mybuns so that it's a more complete test case. * Use mybuns in the airgap test instead of mybuns-with-img-ref. * Install the mybuns bundle in the airgap test so that we know that what we relocated still runs. I am running it without its dependencies to keep things simple for now but long term, after we rewrite dependencies, understanding how to move bundles with deps across an airgap is something we need to document, add regression tests for, etc. * Add plain http test case to the airgap test that doesn't require using --insecure-registry or a registry alias. Signed-off-by: Carolyn Van Slyck <[email protected]>
carolynvs
force-pushed
the
use-image-relocation-fork
branch
from
059d5b6 to
0bc0e08
Compare
Signed-off-by: Carolyn Van Slyck <[email protected]>
Signed-off-by: Carolyn Van Slyck <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this change
After fixing insecure registries, that use an untrusted TLS certificate, it was discovered that unsecured registries (plain http) don't work when communicating with a non-localhost or loopback address. Basically the library has no way to automatically determine that we should use http, and we need to rely on the --insecure-registry flag to know that plain
http is okay too.
The porter publish --archive and porter copy commands were affected by this because they used the github.com/pivotal/image-relocation library, which never supported configuring plain http, only detecting based on the hostname (e.g. localhost/127.0.0.1).
I have created a fork of the image relocation library at https://github.com/cnabio/image-relocation that has a workaround for not being able to configure plain http. I am checking if skipTLS is configured for the http transport passed to the image-relocation library, and also allowing plain http in that case too. This means that the --insecure-registry flag now properly controls plain http too for that library.
The fork has a different go module name so that we don't need to forever maintain a replace statement for that library since it's archived/unmaintained.
I have updated the airgap smoke test to check insecure and unsecured registry functions and included a copy as well so that the bulk of our --insecure-registry test cases are in that one test.
What issue does it fix
Closes #2297
Notes for the reviewer
I will update the go.mod with a tagged release of github.com/cnabio/image-relocation once that is merged.
Checklist
Reviewer Checklist