Add fields for TLS material to destination config#2249
Conversation
|
If you are going to add more fields, you need to document them in the containers.conf file and the containers.conf.5.md file. |
645a6fb to
1b75c0b
Compare
1b75c0b to
d1b90b3
Compare
|
@Luap99 PTAL |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: meln5674, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Luap99
left a comment
There was a problem hiding this comment.
Also please add some basic test to pkg/config/connections_test.go to ensure the config files can be parsed correctly and the serialization/deserialization works/looks as expected.
Before merging this here we should properly have a formal agreement on the podman PR that we like to merge TLS support. I don't think anyone objects there but I like to be sure.
|
|
||
| Path to file containing ssh identity key | ||
|
|
||
| **tls_cert_file="~/certs/podman/tls.crt"** |
There was a problem hiding this comment.
I see identity sets the same example with ~ but AFAIK our code will never expand this as we just pass it to the open syscall and normally the shell will expand ~ so this will most likely not work.
So this example might mislead some people to thinking it works, I would recommend using full paths in the example same for all the other cases here.
There was a problem hiding this comment.
Ah, I think you're right. Do you think it'd be a good idea to require consumers (e.g. podman) to expand the tilde to be consistent with the identity field? Or does the identity example also need to be updated?
There was a problem hiding this comment.
I would just update the docs here, so far nobody has asked for it. And if you create the connections with podman system connection add --identity ~/... the shell already did it for us and we just see the full path anyway so I don't think it matters to users.
It only matters for users that manually write the connections in the config file but I am not aware of anyone doing this.
0837c71 to
fc20da2
Compare
67a1b07 to
df12dcd
Compare
Done. As an aside, it seems pkg/config/modules_test.go is failing due to a sync.Once capturing the wrong XDG_CONFIG_HOME value, but this failure doesn't appear to be reflected in the CI because it doesn't test rootless. |
Ah yes you are right. I hit this a while back but then forget about it/ignored it. Maybe good to file an issue for that, these unit tests should work rootless |
| package version | ||
|
|
||
| // Version is the version of the build. | ||
| const Version = "0.62.0-dev" |
There was a problem hiding this comment.
This looks to have been caused by a bad rebase, should be fixed now.
|
Needs a rebase as well. Before we merge, let's test everything in the podman PR too. |
696c588 to
40cbbe1
Compare
fb281f9 to
1f0251a
Compare
|
Ephemeral COPR build failed. @containers/packit-build please check. |
Signed-off-by: Andrew Melnick <meln5674@kettering.edu>
Signed-off-by: Andrew Melnick <meln5674@kettering.edu>
Signed-off-by: Andrew Melnick <meln5674@kettering.edu>
1f0251a to
cc079d8
Compare
|
Hi, and thank you for your contribution! We’ve recently migrated this repository into a new monorepo: containers/container-libs along with other repositories As part of this migration, this repository is no longer accepting new Pull-Requests and therefore this Pull-Request is being closed. Thank you very much for your contribution. We would appreciate your continued help in migrating this PR to the new container-libs repository. Please let us know if you are facing any issues. You can read more about the migration and the reasoning behind it in our blog post: Upcoming migration of three containers repositories to monorepo. Thanks again for your work and for supporting the containers ecosystem! |
(Resuming from containers/common#2249) Signed-off-by: Andrew Melnick <meln5674.5674@gmail.com>
Adds optional fields
tls_cert_file,tls_key_file, andtls_cafileto the configuration TOML to support connecting to TLS and mTLS podman API sockets.This is in support of podman-container-tools/podman#24601 to fix podman-container-tools/podman#24583 .