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

podman machine ssh handling #15471

Merged
merged 1 commit into from
Sep 27, 2022
Merged

Conversation

cdoern
Copy link
Contributor

@cdoern cdoern commented Aug 25, 2022

add the key used in newly initialized machines to the user's known_hosts file. This ensures that golang will be able to ssh into the machine using
podman-remote. Also, remove the /dev/null redirection for podman machine ssh's known_hosts file.

resolves #15347

Signed-off-by: Charlie Doern cdoern@redhat.com

Does this PR introduce a user-facing change?

introduce the new ssh interface to podman machine, initializing new keys properly, and warning users about the upcoming change.

@cdoern
Copy link
Contributor Author

cdoern commented Aug 25, 2022

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 25, 2022
@cdoern cdoern force-pushed the ssh branch 2 times, most recently from d76d627 to bf2331c Compare August 26, 2022 18:10
@cdoern cdoern force-pushed the ssh branch 2 times, most recently from 6ab47c0 to 9414a29 Compare August 30, 2022 20:53
@cdoern
Copy link
Contributor Author

cdoern commented Aug 30, 2022

@containers/podman-maintainers this and its common counterpart are going to block any new releases PTAL

@edsantiago
Copy link
Member

@cdoern did you really mean to include all this new ginkgo stuff that's blowing up?

@cdoern
Copy link
Contributor Author

cdoern commented Aug 31, 2022

@cdoern did you really mean to include all this new ginkgo stuff that's blowing up?

@edsantiago , nope not sure where it is coming from either

@cdoern
Copy link
Contributor Author

cdoern commented Aug 31, 2022

I think bringing a new c/common commit in is force upgrading gomega, and they recently made the change that is impacting this PR. I think if has to do with checking for two things in the same line.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 31, 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 Aug 31, 2022
@cdoern
Copy link
Contributor Author

cdoern commented Sep 1, 2022

ok, all of these are working which means the common changes are good to go. PTAL at containers/common#1136

test/e2e/secret_test.go Outdated Show resolved Hide resolved
@cdoern cdoern force-pushed the ssh branch 2 times, most recently from c9abda4 to cdc3ddf Compare September 8, 2022 17:47
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 10, 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 Sep 13, 2022
@rhatdan
Copy link
Member

rhatdan commented Sep 14, 2022

/approve
@mtrmac PTAL again

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdoern, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 14, 2022
Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

(but do not merge right now, with a replace directive)

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 23, 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 Sep 26, 2022
add the key used in newly initialized machines to the user's known_hosts file. This ensures that golang will be able to ssh into the machine using
podman-remote. Also, remove the /dev/null redirection for podman machine ssh's known_hosts file.

resolves containers#15347

Signed-off-by: Charlie Doern <cdoern@redhat.com>
Signed-off-by: cdoern <cbdoer23@g.holycross.edu>
@mtrmac
Copy link
Collaborator

mtrmac commented Sep 26, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 26, 2022
@cdoern
Copy link
Contributor Author

cdoern commented Sep 27, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 27, 2022
@openshift-merge-robot openshift-merge-robot merged commit 98e2627 into containers:main Sep 27, 2022
@cdoern
Copy link
Contributor Author

cdoern commented Sep 27, 2022

sigh of relief

@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 Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
5 participants