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. |
meln5674
force-pushed
the
feature/remote-tls
branch
2 times, most recently
from
645a6fb to
1b75c0b
Compare
Signed-off-by: Andrew Melnick <[email protected]>
meln5674
force-pushed
the
feature/remote-tls
branch
from
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
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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.
docs/containers.conf.5.md
Outdated
|
|
||
| **identity="~/.ssh/id_rsa** | ||
|
|
||
| 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.
Signed-off-by: Andrew Melnick <[email protected]>
meln5674
force-pushed
the
feature/remote-tls
branch
from
0837c71 to
fc20da2
Compare
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 containers/podman#24601 to fix containers/podman#24583 .