Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for --ssh=native in podman-remote #16601

Closed
wants to merge 1 commit into from

Conversation

cdoern
Copy link
Contributor

@cdoern cdoern commented Nov 23, 2022

podman's new global flag --sah=native only works with scp and system connection add at the moment modify this so that --ssh=native can be used with podman-remote. This means users can plug in their custom ssh config and known_host files.

Signed-off-by: Charlie Doern [email protected]

Does this PR introduce a user-facing change?

add support for --ssh=native in all podman-remote use cases

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Nov 23, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 23, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cdoern
Once this PR has been reviewed and has the lgtm label, please assign mheon for approval by writing /assign @mheon in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added release-note and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Nov 23, 2022
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 28, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 30, 2022
@cdoern cdoern force-pushed the ssh branch 2 times, most recently from 40e4f22 to eec0c76 Compare November 30, 2022 16:29
@cdoern
Copy link
Contributor Author

cdoern commented Nov 30, 2022

@cevich, this is failing when I try to cp /etc/ssh/sshd_config filepath.Join(u.HomeDir, ".ssh", "config") with no output or error but only fails rootless. Is there a chance user.Current() when rootless does not have a home dir?

@cevich
Copy link
Member

cevich commented Dec 1, 2022

cp /etc/ssh/sshd_config filepath.Join(u.HomeDir, ".ssh", "config")

👀 you sure that's not suppose to be /etc/ssh/ssh_config? 👀

only fails rootless. Is there a chance user.Current() when rootless does not have a home dir?

Yes actually. Well actually, it depends on how the call-stack was setup. If it originated with an ssh rootlessuser@localhost then $HOME should most certainly be defined. If otherwise, then there's a chance $HOME is not setup or exported properly, or (gasp) set to the wrong value (like /root/).

There's a hack in place to try and ensure this can't happen. But it's entirely possible that's not working as intended for your use-case. Analysis of the call-stack is likely necessary. But as a first-step, is there a dummy-way of printing out $HOME from your code and/or u.HomeDir?

@cdoern
Copy link
Contributor Author

cdoern commented Dec 1, 2022

@cevich both ssh_config and sshd_config should exist and sshd_config is the right one. However, I'll add some debugs and repush.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2022
@cdoern cdoern force-pushed the ssh branch 12 times, most recently from f8958f0 to 10cfb2e Compare December 7, 2022 21:15
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 17, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 19, 2022
@cdoern
Copy link
Contributor Author

cdoern commented Dec 19, 2022

@cevich PTAL at this one when you get a chance, integration tests keep failing in setup due to my ssh dir changes

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 21, 2022
@cdoern
Copy link
Contributor Author

cdoern commented Dec 21, 2022

@containers/podman-maintainers PTAL, the aarch tests keep failing on different tests each time I re-run them...

podman's new global flag --sah=native only works with scp and system connection add at the moment
modify this so that --ssh=native can be used with podman-remote. This means users can plug in their custom ssh config and known_host files.

Signed-off-by: Charlie Doern <[email protected]>
@openshift-merge-robot openshift-merge-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 21, 2022
@openshift-merge-robot
Copy link
Collaborator

@cdoern: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Mar 1, 2023

@Luap99 Any chance you could take this over?

@Luap99
Copy link
Member

Luap99 commented Mar 1, 2023

Unlikely, I am not sure what is needed here?

@github-actions github-actions bot removed the stale-pr label Mar 2, 2023
@vrothberg
Copy link
Member

I think a rebase should fix the aarch flakes.

@cdoern is there anything else needed?

@vrothberg vrothberg closed this May 3, 2023
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Aug 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants