add proper handling for golang ssh #1136

cdoern · 2022-08-26T17:49:46Z

podman machine and podman-remote need some softer handling when it comes to key verification
this ensures that podman machine will still work (until we want to make this mandatory). I made the call back function more verbose so we know what is happening from now on.

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

cdoern · 2022-08-26T17:49:57Z

@mtrmac @baude PTAL

cdoern · 2022-08-26T18:09:29Z

I have no idea why, but when I run make vendor about 500 lines are removed from go.sum....

mtrmac

I’d really prefer if we didn’t do any of these special cases. But, if we have to, let’s do that for podman machine only.
It would be nice for $someone to enumerate the cases we care about, to make sure that we didn’t forget anything, and then to see how that works (E.g. create a machine + use it; create a non-machine connection and use it; is scp special enough that we need to discuss it separately? Also see how any of those work when starting with an empty ~/.ssh.)
All the time here we are talking about connection_golang.go. Is there a reason why this does not need to be handled in connection_native.go? Is that perhaps a non-default mode that users only opt into? Or is that able to interactively prompt for confirmation on the TTY, even if we redirect stdout/stderr as we seem to do?

pkg/ssh/connection_golang.go

cdoern · 2022-08-26T18:33:08Z

@mtrmac do you have any guess as to when the next release is? If this works in podman I think I can make some of these changes if we are planning to cut a release next week, but some of these might not get done before then.

cdoern · 2022-09-01T20:57:16Z

/hold

pkg/config/config.go

rhatdan · 2022-09-04T11:53:08Z

You rever to the boolean in multiple different ways which I find confusing. Should the boolean be IsMachine?

cdoern · 2022-09-05T15:43:20Z

You rever to the boolean in multiple different ways which I find confusing. Should the boolean be IsMachine?

sure, I'll make all of the references the same

pkg/config/config.go

pkg/config/config_test.go

pkg/ssh/connection_golang.go

pkg/config/config_test.go

pkg/ssh/connection_golang.go

mtrmac · 2022-09-05T21:29:01Z

pkg/ssh/connection_golang.go

+				if os.IsNotExist(err) && insecureMachine {
+					logrus.Warn("please create a known_hosts file and recreate the machine to add the public key entry")
+					return nil
+				}
+				return err
+			}
+			hErr := known(host, remote, pubKey)
+			if errors.As(hErr, &keyErr) && len(keyErr.Want) > 0 {
+				logrus.Warnf("ssh host key mismatch for host %s, got key %s of type %s", host, ssh.FingerprintSHA256(pubKey), pubKey.Type())
+				return keyErr
+			} else if errors.As(hErr, &keyErr) && len(keyErr.Want) == 0 {


Could you write down, in words, the intended policy?

This might be correct but it seems surprising. In particular I didn’t expect that a missing known_hosts, and an existing known_hosts without an entry for the host in question, don’t cause the same behavior. Why does it warn only on a missing file? Why does it add an entry only on an existing file with a missing entry, and not with an empty file?

Is there, maybe, some expectation that in machine mode, we would never get in the len(keyErr.Want) == 0 situation? If so, this should loudly fail if we do get in that situation.

well, since we are adding the machine keys here too, this will fail for machine as well. I am adding comments to explain each condition

AFAICS if known_hosts does not exist, we either warn and succeed, or don’t warn and fail, but we never get to the known( call and never get the opportunity to add a new known_hosts entry.

To be a bit more explicit, the “the intended policy” was asking about what is the ~higher-level wanted behavior, maybe I’d phase it as asking what are the guarantees that users can rely on, and what are the compatibility compromises we have decided to use that weaken security or usability.

Explaining the code line by line might not be the same thing — i.e. please focus on why the code is using these conditions to decide, not just re-explain the details of the effects of the Go code in words. At least assuming I’ve been reading the code correctly (which might not be the case), I don’t think we are quite in disagreement in what the code does on any code path — but I find it difficult to figure out what is the overall effect.

I have added some intense documentation to this section and the code here is good to go, resolving this.

The added line-by-line documentation answers not a single one of the specific questions from this thread:

I didn’t expect that a missing known_hosts, and an existing known_hosts without an entry for the host in question, don’t cause the same behavior. Why does it warn only on a missing file? Why does it add an entry only on an existing file with a missing entry, and not with an empty file?

what are the guarantees that users can rely on

what are the compatibility compromises we have decided to use that weaken security or usability.

I will un-resolve this so we can talk about it. @mtrmac would you be opposed to getting this to a mergeable state but me filing an issue to keep track of the remaining design discussions we want to have?

You’re right, let’s collect the outstanding ideas somewhere. #1152 , feel free to edit / add entries.

mtrmac · 2022-09-05T21:29:57Z

pkg/ssh/connection_golang.go

+				logrus.Warnf("ssh host key mismatch for host %s, got key %s of type %s", host, ssh.FingerprintSHA256(pubKey), pubKey.Type())
+				return keyErr
+			} else if errors.As(hErr, &keyErr) && len(keyErr.Want) == 0 {
+				// write to known_hosts


I mean, this is an improvement over the status quo but I’ve been talking about the need to interactively get user’s approval first, for some time.

ok I will prompt

@mtrmac this will actually break the machine tests since there is no way to force through the prompt. suggestions?

and most podman-remote tests actually

For machine, maybe that’s consistent with the isMachine bool in general. But that rather depends on the specifics of the semantics of that field.

For “most podman-remote tests”, how does that fail? (It might very well be possible but just a lot of work.)

how would a simple command to a new "machine" in the e2e tests work in the testing suite if we need to prompt for y/n to continue?

I could do a timeout default yes here, where if the user starts a machine or executes podman-remote and doesn't answer y/n we default to yes after 15 seconds.

Other than that, this is a huge hindrance to regular usage and people who use this in applied scenarios... a very breaking change.

how would a simple command to a new "machine" in the e2e tests work in the testing suite if we need to prompt for y/n to continue?

Whatever the current equivalent of expect(1) is. I assume (without checking) that we have some tests of podman run -it, that’s the same issue.

I could do a timeout default yes here, where if the user starts a machine or executes podman-remote and doesn't answer y/n we default to yes after 15 seconds.

No thank you. That’s not an explicit expression of intent.

Other than that, this is a huge hindrance to regular usage and people who use this in applied scenarios... a very breaking change.

How is prompting on podman connection add, and never again, a huge hindrance?

I’ve been talking about the need to interactively prompt since the very start of the existence of this package, and bringing up the need to walk though the UX to ensure it is practical.

I agree a prompt is best but that is not something that can be introduced in 4.3 I do not think. if you exec podman-remote or any podman machine command and it idles, we will break a lot of existing builds.

cdoern · 2022-09-06T18:03:50Z

@mtrmac the Machine bool is now renamed isMachine and does nothing other than tell us if the podman-remote connection is a machine

pkg/config/config.go

mtrmac · 2022-09-07T17:42:58Z

pkg/ssh/connection_golang.go

+				knownFile, err := writeKnownHosts(host, pubKey)
+				if err != nil {
+					if os.IsNotExist(err) {
+						logrus.Warn("podman will soon require a known_hosts file to function properly.")


~~If this code is confident enough to in the public key to add an entry, why would it be reluctant to create a new file with a single line?~~

I thought our desired path was to warn people for a few podman versions and then maybe talk about creating a file? I think this is exiting the scope of this PR.

This fix is needed for 4.3 next week

I agree with @mtrmac. The stock behavior for podman machine is that a file does not need to be created. Granted, it was because it wasn't in use, but from a UX perspective, we should avoid having podman machine instances break after the upgrade. This is also the stock behavior for stock ssh (silent file creation). Change-wise should be safe and small (just adding O_CREATE to flags)

@n1hility This path is not really a podman machine concern in this sense. If the file is entirely missing, we get into the other path with the insecureIsMachineConnection check.

Even this path does not break machines, it warns and continues.

It’s the non-machine case where the behavior between existing file with no match / no file is hard for me to understand.

Actually, my original comment is way off base.

If we get to the path that start with a known() call at all, we know that the file did exist when creating known. So the only way we can get here is if someone removed known_hosts, or a parent directory, between the read and this write.

In that case I don’t think we really need any special handling for os.IsNotExist (though using O_CREATE would, I think, be closer to the right thing to do [with the absolutely right thing being worrying about locking the known_hosts writers across disparate implementations??? ugh]).

But also, the current warning text seems very unrelated to the situation at hand: it is warning about a known_hosts file that did exist just a few milliseconds ago.

@mtrmac @cdoern aha sorry! I just skimmed the shown snippets without looking at the full code. Ok, that alleviates my primary concern.

That said I agree it would make sense to auto-create in the non-machine case as well, and this could simplify the logic a bit.

_ need any special handling for os.IsNotExist (though using O_CREATE would, I think, be closer to the right thing to do [with the absolutely right thing being worrying about locking the known_hosts writers across disparate implementations??? ugh]).

I think its ok relying on O_APPENDs atomicity as long as its always one write for the record. I just double checked the openssh source and they dont use cooperative file locking on known hosts, so its effect would be limited to just concurrent usage of podman

I am mocking up a version where we create a known_hosts file which removes the need for insecureIsMachineConnection

I think its ok relying on O_APPENDs atomicity as long as its always one write for the record. I just double checked the openssh source and they dont use cooperative file locking on known hosts

Thanks for taking the time to actually check. Yes, looking at add_host_to_hostfile in https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/hostfile.c?rev=1.93&content-type=text/x-cvsweb-markup , we don’t need to do any better than that:

Just write text+\n, don’t worry about files that don’t end with a newline.

Use O_CREATE | O_APPEND, don’t do anything special about concurrency (but potentially create ~/.ssh, and in that case it is fairly important to use mode 0700).

I guess that’s for a future version?

@mtrmac do you think we should merge this as is for 4.3? I modified it locally to create ~/.ssh/known_hosts, I can push that if needed.

@cdoern I apologize, for some reason I thought that our 4.2 baseline is #1094 , and that this PR is a net decrease in security. I have only today actually checked, and 4.2 has the pre-c/common ssh implementation if I’m not mistaken.

Assuming that’s the case, any version of this is an improvement over 4.2. So, at this point I think the priorities are, in this order:

Make sure this + the corresponding Podman PRs is merged into 4.3, so that we have a working Podman machine in 4.3.

Get the implementation of the feature set as is right now correct and usable. (I think the PR as is is fine in isolation, but can we rely on tests for all relevant features? I.e. I think that rather than add more last-minute features, it would be more useful to do a manual walk-through of all the relevant use cases [and to capture that for posterity], and ensure that we haven’t overlooked any corner cases / interactions. (E.g. is there really no intersection between ssh_native and the IsMachineConnection cases?)

And for 4.4, let’s first document what we want to do, then see what pieces are missing, and finally finish this.

pkg/ssh/connection_golang.go

cdoern · 2022-09-08T17:44:16Z

/hold cancel

cdoern · 2022-09-08T17:45:27Z

@containers/podman-maintainers PTAL, this needs reviews and merging before weeks end. I think this is in a state where we can merge it as it fixes the machine issues.

Of course, everything here is up for debate and probably will be but for 4.3, this is a crucial fix that needs to be brought into podman

cc/ @n1hility

cdoern · 2022-09-15T18:02:42Z

@containers/podman-maintainers PTAL! all of the review comments are for a different version.

cdoern · 2022-09-19T20:22:03Z

@mtrmac @rhatdan @n1hility should we merge this now for 4.3?

pkg/ssh/connection_golang.go

pkg/ssh/utils.go

n1hility · 2022-09-21T20:49:21Z

My concerns were addressed so LGTM

cdoern · 2022-09-22T17:21:23Z

@mtrmac PTAL and merge

mtrmac

The known_hosts parts LGTM, just 1+2 small things.

The fairly recent changes relating to ssh:// made me look at the parser (both before and after this PR), and … I’m worried. It’s non-obvious, with a completely undocumented intent, and generally gives me the impression of a tangle that we are going to be struggling with for a few years, or something that will be just rewritten from scratch (reintroducing old long-fixed bugs) because it’s easier than to fully understand all the interactions.

Maybe this PR should be split into two, if there is no relationship between these two major themes, then? I don’t know that the split would help decrease the total time to get both in, but at least we could stop thinking about the known_hosts part for 4.3.

mtrmac · 2022-09-22T18:51:09Z

pkg/ssh/connection_golang.go

@@ -84,12 +85,15 @@ func golangConnectionDial(options ConnectionDialOptions) (*ConnectionDialReport,
 }

 func golangConnectionExec(options ConnectionExecOptions) (*ConnectionExecReport, error) {
+	if !strings.HasPrefix(options.Host, "ssh://") {
+		options.Host = "ssh://" + options.Host


Throughout, in several instances of these changes: Why does it make sense to set a “host” field to something URI-like?

IMHO fields/variables, should almost always clear and unambiguous semantics — the obvious semantics. If there are two possible formats, have two separate variables/fields.

It would at least be somewhat manageable if the ConnectionExecOptions.Host field were clearly documented that way, but… it isn’t. How is someone in the future (3 years into the future? or maybe just next week) going to update this undocumented code, without exhaustively enumerating every single caller to understand what formats are acceptable? Or perhaps even validating many past versions of Podman to see what they were writing into persistent config files?

If you currently know these things, please write them down.

If the user’s input is always supposed to be a host name and not an URI, but Validate uses an URI parser for some reason, I’d expect Validate to do this internally (or, maybe, not to use an URI parser at all).

If the user’s input can either be a URI or a non-URI, do we have to have that ambiguity? Maybe that ambiguity should only exist at some top CLI level (possibly as a helper provided by this package) but not here inside the fairly low levels of the API?

If the user’s input can be a URI, what happens if the input uses a different scheme than ssh?

mtrmac · 2022-09-22T18:51:38Z

pkg/ssh/connection_golang.go

@@ -111,11 +115,15 @@ func golangConnectionScp(options ConnectionScpOptions) (*ConnectionScpReport, er
 		return nil, err
 	}

+	// removed for parsing


I don’t understand what this comment is trying to say.

mtrmac · 2022-09-22T18:53:07Z

pkg/ssh/connection_golang.go

-		return nil, fmt.Errorf("creating host key callback function for %s: %w", keyFilePath, err)
+
+	var callback ssh.HostKeyCallback
+	var keyErr *knownhosts.KeyError


Please move keyErr inside the callback so that we clearly don’t need to worry about reuse of the value across callback invocations.

(This is the only blocking part WRT known_hosts changes.)

mtrmac · 2022-09-22T18:56:12Z

pkg/ssh/connection_golang.go

+					}
+					return err
+				}
+			case hErr != nil:


Non-blocking: Simplifying this to default: return hErr would make it a tiny bit clearer that we never return nil without being intentional about it.

mtrmac · 2022-09-22T18:57:42Z

pkg/ssh/connection_golang.go

+
+// addKnownHostsEntry adds (host, pubKey) to user’s known_hosts.
+func addKnownHostsEntry(host string, pubKey ssh.PublicKey) error {
+	hd := homedir.Get()


Absolutely non-blocking: The caller already computed this, so it could pass a path instead of this function recomputing.

(Right now this makes ~no difference, but it will eventually help if we start maintaining per-machine known_hosts files in non-default locations.)

mtrmac · 2022-09-22T19:05:37Z

pkg/ssh/utils.go

@@ -21,6 +21,7 @@ func Validate(user *url.Userinfo, path string, port int, identity string) (*conf
 	if strings.Contains(path, "/run") {
 		sock = strings.Split(path, "/run")[1]
 	}
+	// url.Parse NEEDS ssh://, if this ever fails or returns some nonsense, that is why.


See elsewhere, why do the callers need to deal with that?

Why is a “path” parameter being parsed as an URI? What is the actually intended semantics of that value? Please name parameters exactly correctly, or if that’s impossible (preferably restructure code to make that possible, or at least) clearly, and exhaustively, document the expected formats, in the function’s/field’s doc string.

mtrmac · 2022-09-22T19:09:39Z

pkg/ssh/utils.go

@@ -21,6 +21,7 @@ func Validate(user *url.Userinfo, path string, port int, identity string) (*conf
 	if strings.Contains(path, "/run") {
 		sock = strings.Split(path, "/run")[1]
 	}
+	// url.Parse NEEDS ssh://, if this ever fails or returns some nonsense, that is why.


Looking at the “sometimes we are not going to have a path” comment below?

What’s an example? I guessed ssh://foo and ssh://user@foo, but that didn’t match the code. If there is a a corner case (ssh:foo??), it should be documented clearly — perhaps in a comment, but ideally actually documented also by a unit test that ensures the behavior is not accidentally broken.

How exactly does “not going to have a path” relate to the presence, or not, of a user designation? I can‘t see the relationship.

mtrmac · 2022-09-22T19:15:22Z

pkg/ssh/utils.go

@@ -21,6 +21,7 @@ func Validate(user *url.Userinfo, path string, port int, identity string) (*conf
 	if strings.Contains(path, "/run") {


Non-blocking, about all of Validate:

This kind of parsing/transformation function, overall, seems to me like a pretty good candidate for writing fairly exhaustive unit tests: it is

non—trivial, with quite a few code paths and, apparently, corner cases (assuming they can’t be eliminated in the first place)

very isolated, so cheap to test

mtrmac · 2022-09-22T19:29:23Z

pkg/ssh/utils.go

@@ -21,6 +21,7 @@ func Validate(user *url.Userinfo, path string, port int, identity string) (*conf
 	if strings.Contains(path, "/run") {


Non-blocking: BTW naming a transformative parser like this “validate” is a bit misleading.

Non-blocking: And why are native callers passing options.Identity to this function, and immediately afterwards to ValidateAndConfigure? It seems to me that this could be simplified, e.g. by this function returning a struct (resolvedConnectionInfo[1]?), and callers can then pass around that single struct and access dest.URI and dest.Identity.

[1] I guess that struct can’t be *config.Destination, because the on-disk format and the normalized internal data seem to differ.

(Regardless of the name, AFAICS Validate does not need to be public at all; so, with the recent plans to make the c/common API stable, it should start private. We can easily make things public later, but we can’t take back public API nearly as easily. This applies to other functions in this package as well.)

mtrmac · 2022-09-22T19:43:03Z

pkg/ssh/utils.go

@@ -33,9 +34,9 @@ func Validate(user *url.Userinfo, path string, port int, identity string) (*conf

 	if uri.Port() == "" {


What happens if both uri.Port() and port is set?

Looking at various Podman callers, so many (but not all of them) call url.Parse themselves already. Why do that twice, and then need to worry about these port/port conflicts that might not be actually possible in practice?

Maybe there should be a single ssh.Destination type, containing all relevant information in a native support optimized for further use (whether that’s an url.URL, individual values, or some combination), and helpers vaguely like DestinationFromConfigDestination(*config.Destination), DestinationFromURI, DestinationFromCLIString, DestinationFromPodmanConnectionAddComponents — each with appropriate heuristics, without several round-trips back and forth.

cdoern · 2022-09-22T19:54:41Z

@mtrmac for the 4.3 version would you like all of the above comments addressed? I am unsure if I can get to them before weeks end due to exams I have

mtrmac · 2022-09-22T20:08:39Z

From my POV worrying about authentication, the keyErrone-liner is the only thing I would insist on.

For the rest… *shrug*. It’s not something I can prioritize over my own work near-term.

To the extent that c/common is vendored into Podman 4.3 and not exposed externally, the unclear data semantics and formats are an internal technical debt (unless they show up as user-visible bugs, or user-visible unexpected behavior users start relying on) that can be fixed any time, and I don’t think the 4.3 release boundary is a major influence.

I think there would be a notable difference between getting these things well defined and clearly documented before the discussed API freeze of c/common vs. afterwards. That’s a much more relevant deadline to me.

mtrmac · 2022-09-26T16:56:02Z

I am unsure if I can get to them before weeks end

@cdoern FYI I’ll prepare updates of the two ssh PRs so that they can be merged early, without last-minute scramble.

mtrmac · 2022-09-26T17:03:12Z

#1163 is the updated variant of this PR.

…mes to key verification this ensures that podman machine will still work (until we want to make this mandatory). I made the call back function more verbose so we know what is happening from now on. Signed-off-by: Charlie Doern <cdoern@redhat.com>

mtrmac · 2022-09-26T19:39:03Z

/lgtm

openshift-ci · 2022-09-26T19:39:04Z

@mtrmac: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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.

cdoern · 2022-09-26T19:39:19Z

@rhatdan ^

mtrmac · 2022-09-26T19:43:09Z

/approve
(experimenting with what is going on here… https://github.com/containers/common/blob/main/OWNERS suggests I should have some permissions.)

rhatdan · 2022-09-26T19:48:16Z

/approve
/lgtm

openshift-ci · 2022-09-26T19:48:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdoern, mtrmac, 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:

~~OWNERS~~ [rhatdan]

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

cdoern force-pushed the ssh branch 4 times, most recently from 6ec4168 to e361e89 Compare August 26, 2022 18:09

mtrmac reviewed Aug 26, 2022

View reviewed changes

cdoern force-pushed the ssh branch 3 times, most recently from cf3097c to 9a2200f Compare August 29, 2022 17:44

This was referenced Sep 1, 2022

podman machine ssh handling containers/podman#15471

Merged

Regression in podman 4.3.0-dev connection on windows (ssh handshake fails) containers/podman#15585

Closed

openshift-ci bot added the do-not-merge/hold label Sep 1, 2022

cdoern force-pushed the ssh branch 2 times, most recently from 0aaa2a1 to 55b1e51 Compare September 2, 2022 13:41

rhatdan reviewed Sep 4, 2022

View reviewed changes

pkg/config/config.go Outdated Show resolved Hide resolved

rhatdan reviewed Sep 4, 2022

View reviewed changes

pkg/config/config.go Outdated Show resolved Hide resolved

mtrmac reviewed Sep 5, 2022

View reviewed changes

cdoern force-pushed the ssh branch 2 times, most recently from 7fd7c2c to ace5da1 Compare September 7, 2022 15:14

mtrmac reviewed Sep 7, 2022

View reviewed changes

cdoern force-pushed the ssh branch from ace5da1 to e2fd6b2 Compare September 8, 2022 17:43

openshift-ci bot removed the do-not-merge/hold label Sep 8, 2022

cdoern force-pushed the ssh branch from 38078fb to 9c78cff Compare September 14, 2022 00:39

cdoern force-pushed the ssh branch 3 times, most recently from 5eaaab5 to cd1d646 Compare September 15, 2022 22:21

rhatdan reviewed Sep 19, 2022

View reviewed changes

pkg/ssh/connection_golang.go Outdated Show resolved Hide resolved

rhatdan reviewed Sep 19, 2022

View reviewed changes

pkg/ssh/connection_golang.go Outdated Show resolved Hide resolved

rhatdan reviewed Sep 19, 2022

View reviewed changes

pkg/ssh/utils.go Outdated Show resolved Hide resolved

cdoern force-pushed the ssh branch from cd1d646 to 7231b94 Compare September 19, 2022 20:36

mtrmac reviewed Sep 22, 2022

View reviewed changes

mtrmac mentioned this pull request Sep 26, 2022

Allow disabling known_hosts verification for machine connections #1163

Closed

cdoern force-pushed the ssh branch from 7231b94 to 2b55d99 Compare September 26, 2022 19:36

openshift-ci bot assigned rhatdan Sep 26, 2022

openshift-ci bot added the lgtm label Sep 26, 2022

openshift-ci bot added the approved label Sep 26, 2022

openshift-merge-robot merged commit 590004b into containers:main Sep 26, 2022

		@@ -21,6 +21,7 @@ func Validate(user url.Userinfo, path string, port int, identity string) (conf
		if strings.Contains(path, "/run") {

		@@ -33,9 +34,9 @@ func Validate(user url.Userinfo, path string, port int, identity string) (conf

		if uri.Port() == "" {

add proper handling for golang ssh #1136

add proper handling for golang ssh #1136

Conversation

cdoern commented Aug 26, 2022

cdoern commented Aug 26, 2022

cdoern commented Aug 26, 2022

mtrmac left a comment

Choose a reason for hiding this comment

cdoern commented Aug 26, 2022

cdoern commented Sep 1, 2022

rhatdan commented Sep 4, 2022

cdoern commented Sep 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdoern Sep 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdoern Sep 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdoern commented Sep 6, 2022

mtrmac Sep 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdoern commented Sep 8, 2022

cdoern commented Sep 8, 2022

cdoern commented Sep 15, 2022

cdoern commented Sep 19, 2022

n1hility commented Sep 21, 2022

cdoern commented Sep 22, 2022

mtrmac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdoern commented Sep 22, 2022 • edited Loading

mtrmac commented Sep 22, 2022

mtrmac commented Sep 26, 2022

mtrmac commented Sep 26, 2022

mtrmac commented Sep 26, 2022

openshift-ci bot commented Sep 26, 2022

cdoern commented Sep 26, 2022

mtrmac commented Sep 26, 2022

rhatdan commented Sep 26, 2022

openshift-ci bot commented Sep 26, 2022

cdoern Sep 9, 2022 •

edited

Loading

cdoern Sep 7, 2022 •

edited

Loading

mtrmac Sep 7, 2022 •

edited

Loading

cdoern commented Sep 22, 2022 •

edited

Loading