Repository: knative-sandbox/eventing-kafka-broker
Pull request #2616 (2022-09-08 05:35:22)

[release-1.7] Checking if received config has data or not
This is an automated cherry-pick of #2567

```release-note
When finalizing the broker and the actual configuration can not be correctly received (e.g. an empty ConfigMap), we move on, to prevent blocking the finalization of the broker, as without deletion is stuck
```

knative-prow[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/eventing-kafka-broker/pull/2616#" title="Author self-approved">knative-prow-robot</a>*
**Once this PR has been reviewed and has the lgtm label**, please ask for approval from matzew by writing `/assign @matzew` in a comment. For more information see:[The Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Feventing-kafka-broker).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/knative-sandbox/eventing-kafka-broker/blob/release-1.7/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["matzew"]} -->
knative-prow[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/eventing-kafka-broker/pull/2616#" title="Author self-approved">knative-prow-robot</a>*, *<a href="https://github.com/knative-sandbox/eventing-kafka-broker/pull/2616#pullrequestreview-1100116143" title="Approved">matzew</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Feventing-kafka-broker).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative-sandbox/eventing-kafka-broker/blob/release-1.7/OWNERS)~~ [matzew]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) Report
> Merging [#2616](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (bfc669e) into [release-1.7](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/commit/fcb2051d6a40d9fee2ec70f5070253101ce240f9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (fcb2051) will **increase** coverage by `25.03%`.
> The diff coverage is `n/a`.

```diff
@@                Coverage Diff                 @@
##             release-1.7    #2616       +/-   ##
==================================================
+ Coverage          56.25%   81.28%   +25.03%     
- Complexity             0      715      +715     
==================================================
  Files                 74       72        -2     
  Lines               7640     2501     -5139     
  Branches               0      229      +229     
==================================================
- Hits                4298     2033     -2265     
+ Misses              3043      336     -2707     
+ Partials             299      132      -167     
```

| Flag | Coverage Δ | |
|---|---|---|
| java-unittests | `81.28% <ø> (?)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) | Coverage Δ | |
|---|---|---|
| [control-plane/pkg/reconciler/broker/broker.go](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29udHJvbC1wbGFuZS9wa2cvcmVjb25jaWxlci9icm9rZXIvYnJva2VyLmdv) | | |
| [control-plane/pkg/receiver/routing.go](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29udHJvbC1wbGFuZS9wa2cvcmVjZWl2ZXIvcm91dGluZy5nbw==) | | |
| [control-plane/pkg/contract/log\_marshaller.go](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29udHJvbC1wbGFuZS9wa2cvY29udHJhY3QvbG9nX21hcnNoYWxsZXIuZ28=) | | |
| [...fka/eventing/v1alpha1/consumer\_group\_validation.go](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29udHJvbC1wbGFuZS9wa2cvYXBpcy9pbnRlcm5hbHMva2Fma2EvZXZlbnRpbmcvdjFhbHBoYTEvY29uc3VtZXJfZ3JvdXBfdmFsaWRhdGlvbi5nbw==) | | |
| [control-plane/pkg/receiver/address.go](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29udHJvbC1wbGFuZS9wa2cvcmVjZWl2ZXIvYWRkcmVzcy5nbw==) | | |
| [...ane/pkg/apis/eventing/v1alpha1/kafka\_sink\_types.go](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29udHJvbC1wbGFuZS9wa2cvYXBpcy9ldmVudGluZy92MWFscGhhMS9rYWZrYV9zaW5rX3R5cGVzLmdv) | | |
| [control-plane/pkg/prober/cache.go](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29udHJvbC1wbGFuZS9wa2cvcHJvYmVyL2NhY2hlLmdv) | | |
| [control-plane/pkg/config/env\_config.go](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29udHJvbC1wbGFuZS9wa2cvY29uZmlnL2Vudl9jb25maWcuZ28=) | | |
| [...afka/eventing/v1alpha1/consumer\_group\_lifecycle.go](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29udHJvbC1wbGFuZS9wa2cvYXBpcy9pbnRlcm5hbHMva2Fma2EvZXZlbnRpbmcvdjFhbHBoYTEvY29uc3VtZXJfZ3JvdXBfbGlmZWN5Y2xlLmdv) | | |
| [...plane/pkg/reconciler/trigger/namespaced\_trigger.go](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29udHJvbC1wbGFuZS9wa2cvcmVjb25jaWxlci90cmlnZ2VyL25hbWVzcGFjZWRfdHJpZ2dlci5nbw==) | | |
| ... and [136 more](https://codecov.io/gh/knative-sandbox/eventing-kafka-broker/pull/2616/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)


Patch:
diff --git a/control-plane/pkg/reconciler/broker/broker.go b/control-plane/pkg/reconciler/broker/broker.go
index b6f957063..2fef78ed7 100644
--- a/control-plane/pkg/reconciler/broker/broker.go
+++ b/control-plane/pkg/reconciler/broker/broker.go
@@ -341,6 +341,13 @@ func (r *Reconciler) finalizeKind(ctx context.Context, broker *eventing.Broker)
 	}
 
 	_, brokerConfig, err := r.brokerConfigMap(logger, broker)
+
+	// If the broker config data is empty we simply return,
+	// as the configuration may already be gone
+	if len(brokerConfig.Data) == 0 {
+		return nil
+	}
+
 	if err != nil && !apierrors.IsNotFound(err) {
 		return err
 	}


Test patch:
diff --git a/control-plane/pkg/reconciler/broker/broker_test.go b/control-plane/pkg/reconciler/broker/broker_test.go
index 8b76f88df..52f214a9b 100644
--- a/control-plane/pkg/reconciler/broker/broker_test.go
+++ b/control-plane/pkg/reconciler/broker/broker_test.go
@@ -1993,6 +1993,32 @@ func brokerFinalization(t *testing.T, format string, env config.Env) {
 				testProber: probertesting.MockProber(prober.StatusNotReady),
 			},
 		},
+		{
+			Name: "Reconciled normal - no ConfigMap and no annotations",
+			Objects: []runtime.Object{
+				NewDeletedBrokerWithoutConfigMapAnnotations(),
+				NewConfigMapFromContract(&contract.Contract{
+					Resources: []*contract.Resource{
+						{
+							Uid:     BrokerUUID,
+							Topics:  []string{BrokerTopic()},
+							Ingress: &contract.Ingress{Path: receiver.Path(BrokerNamespace, BrokerName)},
+						},
+					},
+					Generation: 1,
+				}, env.DataPlaneConfigMapNamespace, env.DataPlaneConfigMapName, env.DataPlaneConfigFormat),
+			},
+			Key: testKey,
+			WantUpdates: []clientgotesting.UpdateActionImpl{
+				ConfigMapUpdate(env.DataPlaneConfigMapNamespace, env.DataPlaneConfigMapName, env.DataPlaneConfigFormat, &contract.Contract{
+					Resources:  []*contract.Resource{},
+					Generation: 2,
+				}),
+			},
+			OtherTestData: map[string]interface{}{
+				testProber: probertesting.MockProber(prober.StatusNotReady),
+			},
+		},
 		{
 			Name: "Reconciled failed - probe not ready",
 			Objects: []runtime.Object{
diff --git a/control-plane/pkg/reconciler/testing/objects_broker.go b/control-plane/pkg/reconciler/testing/objects_broker.go
index 54df4d70d..2aca5f9f4 100644
--- a/control-plane/pkg/reconciler/testing/objects_broker.go
+++ b/control-plane/pkg/reconciler/testing/objects_broker.go
@@ -106,6 +106,17 @@ func NewDeletedBroker(options ...reconcilertesting.BrokerOption) runtime.Object
 	)
 }
 
+func NewDeletedBrokerWithoutConfigMapAnnotations(options ...reconcilertesting.BrokerOption) runtime.Object {
+	return NewBroker(
+		append(
+			options,
+			func(broker *eventing.Broker) {
+				WithDeletedTimeStamp(broker)
+			},
+		)...,
+	)
+}
+
 func WithDelivery(mutations ...func(spec *eventingduck.DeliverySpec)) func(*eventing.Broker) {
 	service := NewService()
 
diff --git a/test/e2e_new/broker_test.go b/test/e2e_new/broker_test.go
index 70ae883dd..8d997dc00 100644
--- a/test/e2e_new/broker_test.go
+++ b/test/e2e_new/broker_test.go
@@ -57,3 +57,17 @@ func TestBrokerConfigMapDeletedFirst(t *testing.T) {
 
 	env.Test(ctx, t, features.BrokerConfigMapDeletedFirst())
 }
+
+func TestBrokerConfigMapDoesNotExist(t *testing.T) {
+	t.Parallel()
+
+	ctx, env := global.Environment(
+		knative.WithKnativeNamespace(system.Namespace()),
+		knative.WithLoggingConfig,
+		knative.WithTracingConfig,
+		k8s.WithEventListener,
+		environment.Managed(t),
+	)
+
+	env.Test(ctx, t, features.BrokerConfigMapDoesNotExist())
+}
diff --git a/test/e2e_new/features/broker_deleted_recreated.go b/test/e2e_new/features/broker_deleted_recreated.go
index 8d250f284..0caf43b0c 100644
--- a/test/e2e_new/features/broker_deleted_recreated.go
+++ b/test/e2e_new/features/broker_deleted_recreated.go
@@ -18,9 +18,12 @@ package features
 
 import (
 	"k8s.io/apimachinery/pkg/types"
+
+	"knative.dev/pkg/system"
+
 	"knative.dev/eventing/test/rekt/resources/broker"
 	"knative.dev/eventing/test/rekt/resources/trigger"
-	"knative.dev/pkg/system"
+
 	"knative.dev/reconciler-test/pkg/eventshub"
 	"knative.dev/reconciler-test/pkg/feature"
 	"knative.dev/reconciler-test/resources/svc"
@@ -66,3 +69,16 @@ func BrokerConfigMapDeletedFirst() *feature.Feature {
 
 	return f
 }
+
+// BrokerConfigMapDoesNotExist tests that a broker can be deleted without the ConfigMap existing.
+func BrokerConfigMapDoesNotExist() *feature.Feature {
+	f := feature.NewFeatureNamed("delete broker ConfigMap first")
+
+	brokerName := feature.MakeRandomK8sName("broker")
+
+	f.Setup("install broker", broker.Install(brokerName, append(broker.WithEnvConfig(), broker.WithConfig("doesNotExist"))...))
+
+	f.Assert("delete broker", featuressteps.DeleteBroker(brokerName))
+
+	return f
+}