[ADD] Support tls verify flag for compose#1399
Conversation
Chethan30
force-pushed
the
feature/add-tls-verify-flag
branch
from
fda5ec2 to
6136b7b
Compare
| "push", | ||
| "build", | ||
| ): | ||
| xargs.insert(0, "--tls-verify=false") |
There was a problem hiding this comment.
It is possible to rewrite code to use only append. Inserting in the begining is unnecessarily complicated.
|
|
||
| @overload | ||
| def rec_subs(value: str, subs_dict: dict[str, Any]) -> str: ... | ||
|
|
| ) | ||
| tls_verify_env = ( | ||
| "false" | ||
| if os.environ.get("PODMAN_COMPOSE_TLS_VERIFY", "true").strip().lower() == "false" |
| "Use false for self-signed or corporate registries " | ||
| "(default: true, or PODMAN_COMPOSE_TLS_VERIFY env)" | ||
| ), | ||
| metavar="true|false", |
| class TestGetPodmanArgsTlsVerify(unittest.TestCase): | ||
| def test_tls_verify_true_pull_push_build_no_flag(self) -> None: | ||
| compose = compose_with_tls_verify("true") | ||
| for cmd in ("pull", "push", "build"): |
| for cmd in ("pull", "push", "build"): | ||
| xargs = compose.get_podman_args(cmd) | ||
| self.assertNotIn( | ||
| "--tls-verify=false", |
There was a problem hiding this comment.
Brittle test, it should verify that there are no --tls-verify* flags. Similar problem with other tests.
| def test_tls_verify_true_registry_commands_no_tls_flag(self, cmd: str) -> None: | ||
| compose = compose_with_tls_verify("true") | ||
| xargs = compose.get_podman_args(cmd) | ||
| tls_flags = _tls_verify_flags(xargs) |
There was a problem hiding this comment.
very verbose: this variable is not necessary, also assertEqual explanation is also not necessary
p12tic
left a comment
p12tic
left a comment
There was a problem hiding this comment.
tls-verify still needs documentation. Also look into git history and structure your commits like in other PRs.
Please read project requirements carefully because I don't have time to review PRs where I need to explicitly point out every little thing it misses. Such PRs will be ignored.
Signed-off-by: Chethan30 <chetan611611611@gmail.com>
Chethan30
force-pushed
the
feature/add-tls-verify-flag
branch
from
eb14eee to
19d9a83
Compare
Apologies. It's my first open PR in a while. Some of our internal practices had followed through. I've made effort to fix the style and requirements. Please let me know if any changes. Thank you for your time. |
2 participants
Summary
Added support for disabling TLS verification when podman-compose runs
podman pull,podman push, andpodman build. This allows use behind VPNs or with self-signed/invalid registry certificates where podman would otherwise fail with x509 TLS errors.Motivation
podman pull(and push/build that contact registries) can fail with x509 certificate verification errors.--tls-verify=falseforpodman pull,podman push, andpodman login, but podman-compose did not expose this when invoking these commands internally.--podman-pull-args "--tls-verify=false"(and similarly for push/build), but that is hard to discover and easy to forget for commands likeupthat trigger pulls. This change provides a single, explicit option.Changes
--tls-verifywith valuestrueorfalse(defaulttrue). When set tofalse, podman-compose passes--tls-verify=falseto all registry-related podman subcommands.get_podman_args(cmd), when--tls-verify=falseis in effect andcmdispull,push, orbuild, the returned arguments include--tls-verify=false. This automatically covers:podman-compose pulland the pull phase inpodman-compose uppodman-compose pushpodman-compose buildand the build phase inup(e.g. when using--pull=always)tests/unit/test_get_podman_args_tls_verify.pyforget_podman_args()withtls_verifytrue/false and for registry vs non-registry commands.