Reconciliation of MutatingWebhookConfiguration values didnt happen by Danil-Grigorev · Pull Request #682 · openshift/machine-api-operator

Danil-Grigorev · 2020-08-20T14:25:33Z

Fix webhook spec update recognition

Enforce all the values we expect to be set on the webhooks. This ensures the hash
will always represent the full resource spec as we view it, so any specified field update
will be recognized and processed by the operator.

openshift-ci-robot · 2020-08-20T14:25:40Z

@Danil-Grigorev: This pull request references Bugzilla bug 1870158, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Details

In response to this:

BUG 1870158: Reconciliation of MutatingWebhookConfiguration values didnt happen

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.

openshift-ci-robot · 2020-08-20T14:25:47Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign danil-grigorev
You can assign the PR to them by writing /assign @danil-grigorev in a comment when ready.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

Danil-Grigorev · 2020-08-21T11:19:45Z

/retest

Danil-Grigorev · 2020-08-21T12:27:52Z

/retest

Danil-Grigorev · 2020-08-21T13:30:44Z

/retest

Danil-Grigorev · 2020-08-21T17:59:13Z

/retest

Danil-Grigorev · 2020-08-21T19:59:51Z

/retest

Danil-Grigorev · 2020-08-22T06:39:01Z

/retest

Danil-Grigorev · 2020-08-24T10:19:28Z

/retest

Danil-Grigorev · 2020-08-25T11:00:27Z

/retest

Danil-Grigorev · 2020-08-31T09:16:28Z

@JoelSpeed @enxebre This PR implements one of the possible approaches to handling webhooks. Merging this will allow dropping generation management and spec hashing which is not currently used with our implementation, and rely solely on spec hash.

Another approach could be use generation comparison, and drop defaulting/spec hashing instead, but this way the operator will frequently poll API server for updates without value changes.

Danil-Grigorev · 2020-09-02T13:18:18Z

/retest

JoelSpeed · 2020-09-03T14:32:34Z

Another approach could be use generation comparison, and drop defaulting/spec hashing instead, but this way the operator will frequently poll API server for updates without value changes.

Does the operator use an informer? If such, this shouldn't be a problem, we can use a cached reader and that should be sufficient.

If we have informers set up already for this, then we should consider if the generation approach may be simpler, I believe this is what other implementation in library go have been using as one of their indicators, there must be a good reason for that. We may want to speak to the API folks to try and work out a bit more about the history of the apply code

JoelSpeed

Would be good to see some comments on the defaulting functions with the sources of the defaults, ie, how did you work these out, is there anywhere in the code we can link to for the defaulting in the API so that we can see if any of these change easily at a later date.

Would also be good to get some test cases for these changes. This area is sparsely tested and without proper tests we aren't going to be able to ship this.

Danil-Grigorev · 2020-09-07T13:24:59Z

Another approach could be use generation comparison, and drop defaulting/spec hashing instead, but this way the operator will frequently poll API server for updates without value changes.

Does the operator use an informer? If such, this shouldn't be a problem, we can use a cached reader and that should be sufficient.

If we have informers set up already for this, then we should consider if the generation approach may be simpler, I believe this is what other implementation in library go have been using as one of their indicators, there must be a good reason for that. We may want to speak to the API folks to try and work out a bit more about the history of the apply code

Yes, the implementation is intended to be used with an informer. I'll see how far I could test current implementation, and I think it would be good to merge it this release with the approach we already choose. Then the upstream discussion could raise the question, and the library-go implementation could implement this in my opinion simpler approach (assuming we won't be able to figure out reasoning to do otherwhise). If you agree I'll open a JIRA ticket to that.

For now, I'll see how well tested the current implementation is, and do the changes. Do you think some heavier refactoring is allowed here? The observedGeneration is a dead code with the introduced changes, so it's either one way or another if we'd like to be clear.

Danil-Grigorev · 2020-09-08T12:35:34Z

Would be good to see some comments on the defaulting functions with the sources of the defaults, ie, how did you work these out, is there anywhere in the code we can link to for the defaulting in the API so that we can see if any of these change easily at a later date.

Would also be good to get some test cases for these changes. This area is sparsely tested and without proper tests we aren't going to be able to ship this.

@JoelSpeed I added the comments on the code. The code is currently tested as much as it could be with the structure we got. Most of the absent coverage (38%) comes from structured <-> unstructured conversion, and error checking. I would adjust the code to ignore errors at these points, as we expect a structured resource on the methods like apply, so there should be no error-prone behavior there.

Danil-Grigorev · 2020-09-09T11:31:47Z

/retest

JoelSpeed · 2020-09-09T11:42:06Z

pkg/operator/webhook_configuration.go

+	// Ignore the error, as the object types are identical, and the path
+	// in the resource was already found
+	unstructured.SetNestedField(toWrite.Object, requiredSpec, path)


I don't really see the benefit of removing this error check, we know it shouldn't fail, but we should still catch it in case it does (eg we change the input and create some runtime error because we've given a reference not a pointer or something)

You are right, it presense was justified by the code which didn't belong there, so I removed it. Now we are returning the error as expected.

JoelSpeed · 2020-09-09T11:44:53Z

pkg/operator/webhook_configuration_helper.go

+// setDefaultsValidatingWebhookConfiguration sets defaults for webhooks in the ValidatingWebhookConfiguration
+// source: https://github.com/kubernetes/kubernetes/blob/c7f9dd0bafe2f3328148a85eed9c18322b9f308e/pkg/apis/admissionregistration/v1/zz_generated.defaults.go#L47-L87
+func setDefaultsValidatingWebhookConfiguration(in *admissionregistrationv1.ValidatingWebhookConfiguration) {
+	for i := range in.Webhooks {
+		a := &in.Webhooks[i]
+		setDefaultsValidatingWebhook(a)
+		if a.ClientConfig.Service != nil {
+			setDefaultsServiceReference(a.ClientConfig.Service)
+		}
+		for j := range a.Rules {
+			b := &a.Rules[j]
+			setDefaultsRule(&b.Rule)
+		}
+	}
+}
+
+// setDefaultsMutatingWebhookConfiguration sets defaults for webhooks in the MutatingWebhookConfiguration
+// source: https://github.com/kubernetes/kubernetes/blob/c7f9dd0bafe2f3328148a85eed9c18322b9f308e/pkg/apis/admissionregistration/v1/zz_generated.defaults.go#L47-L87
+func setDefaultsMutatingWebhookConfiguration(in *admissionregistrationv1.MutatingWebhookConfiguration) {
+	for i := range in.Webhooks {
+		a := &in.Webhooks[i]
+		setDefaultsMutatingWebhook(a)
+		if a.ClientConfig.Service != nil {
+			setDefaultsServiceReference(a.ClientConfig.Service)
+		}
+		for j := range a.Rules {
+			b := &a.Rules[j]
+			setDefaultsRule(&b.Rule)
+		}
+	}
+}
+
+// setDefaultsValidatingWebhook set defaults for the given ValidatingWebhook
+// source: https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/admissionregistration/v1/defaults.go#L30-L80
+func setDefaultsValidatingWebhook(obj *admissionregistrationv1.ValidatingWebhook) {
+	if obj.FailurePolicy == nil {
+		policy := admissionregistrationv1.Fail
+		obj.FailurePolicy = &policy
+	}
+	if obj.MatchPolicy == nil {
+		policy := admissionregistrationv1.Equivalent
+		obj.MatchPolicy = &policy
+	}
+	if obj.NamespaceSelector == nil {
+		selector := metav1.LabelSelector{}
+		obj.NamespaceSelector = &selector
+	}
+	if obj.ObjectSelector == nil {
+		selector := metav1.LabelSelector{}
+		obj.ObjectSelector = &selector
+	}
+	if obj.TimeoutSeconds == nil {
+		obj.TimeoutSeconds = new(int32)
+		*obj.TimeoutSeconds = 10
+	}
+}
+
+// setDefaultsMutatingWebhook set defaults for the given MutatingWebhook
+// source: https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/admissionregistration/v1/defaults.go#L30-L80
+func setDefaultsMutatingWebhook(obj *admissionregistrationv1.MutatingWebhook) {
+	if obj.FailurePolicy == nil {
+		policy := admissionregistrationv1.Fail
+		obj.FailurePolicy = &policy
+	}
+	if obj.MatchPolicy == nil {
+		policy := admissionregistrationv1.Equivalent
+		obj.MatchPolicy = &policy
+	}
+	if obj.NamespaceSelector == nil {
+		selector := metav1.LabelSelector{}
+		obj.NamespaceSelector = &selector
+	}
+	if obj.ObjectSelector == nil {
+		selector := metav1.LabelSelector{}
+		obj.ObjectSelector = &selector
+	}
+	if obj.TimeoutSeconds == nil {
+		obj.TimeoutSeconds = new(int32)
+		*obj.TimeoutSeconds = 10
+	}
+	if obj.ReinvocationPolicy == nil {
+		never := admissionregistrationv1.NeverReinvocationPolicy
+		obj.ReinvocationPolicy = &never
+	}
+}
+
+// setDefaultsRule set default rule for both MutatingWebhook and ValidatingWebhook
+func setDefaultsRule(obj *admissionregistrationv1.Rule) {
+	if obj.Scope == nil {
+		s := admissionregistrationv1.AllScopes
+		obj.Scope = &s
+	}
+}
+
+// setDefaults_ServiceReference sets defaults for Webhook's ServiceReference
+func setDefaultsServiceReference(obj *admissionregistrationv1.ServiceReference) {
+	if obj.Port == nil {
+		obj.Port = pointer.Int32Ptr(443)
+	}
+}


If this is all a direct copy from the standard defaulting implementation, why not just use the defaulting implementation? Could you not use scheme.Default(obj) to do this defaulting for you?

@JoelSpeed Are we ok importing kubernetes into our repo?

JoelSpeed · 2020-09-09T11:47:02Z

pkg/operator/webhook_configuration.go

+	existingSpec, exists, err := unstructured.NestedFieldNoCopy(existingCopy.Object, path)
 	if err != nil {
 		return nil, false, err
 	}
-
-	if !modified {
-		return existingCopy, false, nil
+	if !exists {
+		return nil, false, fmt.Errorf("Object returned by the client not contain the specified path: %s", path)
 	}
-
-	requiredSpec, exists, err := unstructured.NestedFieldNoCopy(required.Object, path)
-	if err != nil {
+	if err := setSpecHashAnnotation(existingCopy, existingSpec); err != nil {
 		return nil, false, err
 	}
-	if !exists {
-		// No spec to update
+	if modified := ensureObjectMeta(existingCopy, required); !modified {


Why do we need to change the order of these? Should they not still work in the order they were in if we've introduced the defaulting?

JoelSpeed · 2020-09-09T11:51:33Z

pkg/operator/suite_test.go

+		[]Reporter{printer.NewlineReporter{}})
+}
+
+var _ = BeforeSuite(func(done Done) {


There's nothing happening in a go routine within this function, so you don't need to have done here, the following should suffice, then remove all reference to done and the timeout param on line 39

var _ = BeforeSuite(func() {

JoelSpeed · 2020-09-09T11:53:13Z

pkg/operator/sync_test.go


 func TestSyncValidatingWebhooks(t *testing.T) {
 	defaultConfiguration := mapiv1.NewValidatingWebhookConfiguration()
+	setDefaultsValidatingWebhookConfiguration(defaultConfiguration)


Why do we set the defaults on this? Shouldn't that be handled as part of the apply method? Does this interfere with the testing?

It is interfering with testing. If I don't set defaults here then I'm providing a webhook configuration that is not fully populated, and as we are using a fake client here, nothing will fix it. So all update tests will independently on the changes I do or don't do, will execute the update operation.

Another option would be putting this defaulting into api package, and apply during NewValidatingWebhookConfiguration call. But if we choose any other approach, we would have to make changes to our API, which is not good and under practices.

JoelSpeed · 2020-09-09T11:53:32Z

pkg/operator/sync_test.go


 func TestSyncMutatingWebhooks(t *testing.T) {
 	defaultConfiguration := mapiv1.NewMutatingWebhookConfiguration()
+	setDefaultsMutatingWebhookConfiguration(defaultConfiguration)


Ditto on above, I don't think this is the right place to do this

JoelSpeed · 2020-09-09T11:56:44Z

pkg/operator/sync_test.go

+			name: "Successfully create an object",
+			path: "webhooks",
+			object: func() *unstructured.Unstructured {
+				obj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(mapiv1.NewMutatingWebhookConfiguration())


Should check the errors on this

func(t testing.T) *unstructured.Unstructured { obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(mapiv1.NewMutatingWebhookConfiguration()) if err != nil { t.Fatalf(...) }

JoelSpeed · 2020-09-09T11:58:35Z

pkg/operator/sync_test.go

+				return &unstructured.Unstructured{Object: obj}
+			},
+			existing: invalidWebhookConfiguration,
+			err:      fmt.Errorf("Object returned by the client not contain the specified path: webhooks"),


You might be better using errors.New from the "errors" package here, you're not using format strings at all

Danil-Grigorev · 2020-09-10T17:31:56Z

/retest

JoelSpeed · 2020-09-22T15:48:43Z

/retitle Reconciliation of MutatingWebhookConfiguration values didnt happen

This shouldn't be needed to resolve the attached bug, let's try to get it verified with just the other PR and see if that goes through

openshift-ci-robot · 2020-09-22T15:49:00Z

@Danil-Grigorev: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

Reconciliation of MutatingWebhookConfiguration values didnt happen

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.

…perator Prevent user from setting malitious values in the rest of the webhook configuration fields, which could cause major diviations in mutation/validation procedure.

JoelSpeed

Could do with some testing ideally, see comments inline

JoelSpeed · 2020-09-23T14:57:48Z

pkg/apis/machine/v1beta1/machine_webhook.go

+	webhookMatchPolicy        = admissionregistrationv1.Equivalent
+	webhookResourceSelector   = metav1.LabelSelector{}
+	webhookScope              = admissionregistrationv1.NamespacedScope
+	webhookReinvocationPolicy = admissionregistrationv1.IfNeededReinvocationPolicy


Can you highlight which of these are defaults? With a comment, eg is IfNeeded the default reinvocation policy? If not, why have we chose it

It is not a default one, but I set it explicitly as someone could try to negate our changes from mutation with his own webhook. This way our webhook will be invoked for each additional call.

JoelSpeed · 2020-09-24T14:29:22Z

pkg/operator/webhook_configuration.go

-// the previously required spec and metadata. For further detail, check the top-level comment.
-func applyMutatingWebhookConfiguration(client dynamic.Interface, recorder events.Recorder,
+// the previously required spec and metadata based on generation change.
+func applyMutatingWebhookConfiguration(client v1.MutatingWebhookConfigurationInterface, recorder events.Recorder,


I would suggest just using kubernetes.Interface and then making into a MutatingWebhookConfigurationInterface within the code itself

JoelSpeed · 2020-09-24T14:33:39Z

pkg/operator/webhook_configuration.go

-	if klog.V(4) {
-		klog.Infof("%s %q changes: %v", required.GetKind(), required.GetNamespace()+"/"+required.GetName(), jsonPatchNoError(existing, toWrite))
+func copyValidatingWebhookCABundle(existing, required *admissionregistrationv1.ValidatingWebhookConfiguration) {
+	existingMutatingWebhooks := make(map[string]admissionregistrationv1.ValidatingWebhook)


This should probably be validating 😛

Suggested change

existingMutatingWebhooks := make(map[string]admissionregistrationv1.ValidatingWebhook)

existingValidatingWebhooks := make(map[string]admissionregistrationv1.ValidatingWebhook)

JoelSpeed · 2020-09-24T14:36:38Z

pkg/operator/webhook_configuration.go

-	if klog.V(4) {
-		klog.Infof("%s %q changes: %v", required.GetKind(), required.GetNamespace()+"/"+required.GetName(), jsonPatchNoError(existing, toWrite))
+func copyValidatingWebhookCABundle(existing, required *admissionregistrationv1.ValidatingWebhookConfiguration) {
+	existingMutatingWebhooks := make(map[string]admissionregistrationv1.ValidatingWebhook)


This should be validating

Suggested change

existingMutatingWebhooks := make(map[string]admissionregistrationv1.ValidatingWebhook)

existingValidatingWebhooks := make(map[string]admissionregistrationv1.ValidatingWebhook)

JoelSpeed · 2020-09-24T14:38:03Z

pkg/operator/suite_test.go

+)
+
+func TestMachinesetController(t *testing.T) {
+	RegisterFailHandler(Fail)


If we're using klog at all within this code, let's set the output

Suggested change

RegisterFailHandler(Fail)

RegisterFailHandler(Fail)

klog.SetOutput(GinkgoWriter)

openshift-ci-robot · 2020-09-24T18:00:59Z

@Danil-Grigorev: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/unit	`455cee3`	link	`/test unit`
ci/prow/e2e-gcp	`455cee3`	link	`/test e2e-gcp`
ci/prow/e2e-azure	`455cee3`	link	`/test e2e-azure`
ci/prow/e2e-azure-operator	`455cee3`	link	`/test e2e-azure-operator`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

openshift-merge-robot · 2020-11-06T16:28:03Z

@Danil-Grigorev: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/generate	`455cee3`	link	`/test generate`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

Danil-Grigorev · 2020-12-01T17:22:02Z

/close

Closing this in favor of Joel's implementation currently merged #707 and an upcoming library-go approach - openshift/library-go#836

openshift-ci-robot · 2020-12-01T17:22:19Z

@Danil-Grigorev: Closed this PR.

Details

In response to this:

/close

Closing this in favor of Joel's implementation currently merged #707 and an upcoming library-go approach - openshift/library-go#836

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.

openshift-ci-robot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Aug 20, 2020

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Aug 20, 2020

openshift-ci-robot requested review from enxebre and michaelgugino August 20, 2020 14:25

Danil-Grigorev force-pushed the fix-webhooks-on-update branch from c80b935 to 7d1adca Compare August 20, 2020 16:07

JoelSpeed reviewed Sep 3, 2020

View reviewed changes

Danil-Grigorev requested a review from JoelSpeed September 7, 2020 13:25

Danil-Grigorev force-pushed the fix-webhooks-on-update branch 2 times, most recently from c0f8796 to 014a83a Compare September 8, 2020 11:17

Danil-Grigorev force-pushed the fix-webhooks-on-update branch from 014a83a to 52ce291 Compare September 9, 2020 11:19

Danil-Grigorev force-pushed the fix-webhooks-on-update branch from 52ce291 to ad91f27 Compare September 9, 2020 11:40

JoelSpeed reviewed Sep 9, 2020

View reviewed changes

Danil-Grigorev force-pushed the fix-webhooks-on-update branch 2 times, most recently from cb8d099 to 649ec9d Compare September 10, 2020 12:09

Danil-Grigorev force-pushed the fix-webhooks-on-update branch from 649ec9d to 01d5af3 Compare September 10, 2020 12:14

Danil-Grigorev requested a review from JoelSpeed September 10, 2020 12:15

openshift-ci-robot mentioned this pull request Sep 22, 2020

Bug 1870158: Enforce webhooks service reference to port 443 #708

Merged

openshift-ci-robot changed the title ~~BUG 1870158: Reconciliation of MutatingWebhookConfiguration values didnt happen~~ Reconciliation of MutatingWebhookConfiguration values didnt happen Sep 22, 2020

openshift-ci-robot removed the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Sep 22, 2020

openshift-ci-robot removed the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Sep 22, 2020

Danil-Grigorev force-pushed the fix-webhooks-on-update branch 2 times, most recently from 07f302e to ef811a8 Compare September 23, 2020 14:35

Danil-Grigorev added 3 commits September 24, 2020 10:59

Refactor applyUnstructured and add/refactor tests

56c279c

Enforce values for MachineWebhooks, which should be strictly set by o…

0326bca

…perator Prevent user from setting malitious values in the rest of the webhook configuration fields, which could cause major diviations in mutation/validation procedure.

Apply webhook after generation change

7f73f90

Danil-Grigorev force-pushed the fix-webhooks-on-update branch from ef811a8 to 7f73f90 Compare September 24, 2020 11:08

Make webhook apply typed and add tests

455cee3

JoelSpeed reviewed Sep 24, 2020

View reviewed changes

openshift-ci-robot closed this Dec 1, 2020

	existingMutatingWebhooks := make(map[string]admissionregistrationv1.ValidatingWebhook)
	existingValidatingWebhooks := make(map[string]admissionregistrationv1.ValidatingWebhook)

	RegisterFailHandler(Fail)
	RegisterFailHandler(Fail)
	klog.SetOutput(GinkgoWriter)

Conversation

Danil-Grigorev commented Aug 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Aug 20, 2020

Uh oh!

openshift-ci-robot commented Aug 20, 2020

Uh oh!

Danil-Grigorev commented Aug 21, 2020

Uh oh!

Danil-Grigorev commented Aug 21, 2020

Uh oh!

Danil-Grigorev commented Aug 21, 2020

Uh oh!

Danil-Grigorev commented Aug 21, 2020

Uh oh!

Danil-Grigorev commented Aug 21, 2020

Uh oh!

Danil-Grigorev commented Aug 22, 2020

Uh oh!

Danil-Grigorev commented Aug 24, 2020

Uh oh!

Danil-Grigorev commented Aug 25, 2020

Uh oh!

Danil-Grigorev commented Aug 31, 2020

Uh oh!

Danil-Grigorev commented Sep 2, 2020

Uh oh!

JoelSpeed commented Sep 3, 2020

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

Danil-Grigorev commented Sep 7, 2020

Uh oh!

Danil-Grigorev commented Sep 8, 2020

Uh oh!

Danil-Grigorev commented Sep 9, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Danil-Grigorev commented Sep 10, 2020

Uh oh!

JoelSpeed commented Sep 22, 2020

Uh oh!

openshift-ci-robot commented Sep 22, 2020

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Danil-Grigorev commented Aug 20, 2020 •

edited

Loading