Test ApiServerSource with Broker as sink

[Leo6Leo]

Fixes #6933

Proposed Changes

• A rekt-based e2e test for PingSource TLS support when using Broker as sink

Pre-review Checklist

• At least 80% unit test coverage
• E2E tests for any new behavior
• Docs PR for any user-facing impact
• Spec PR for any new API feature
• Conformance test for any change to the spec

Release Note

Added a rekt-based e2e test for PingSource TLS support when using Broker as sink

Docs

[knative-prow]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (0fd9aa1) 77.92% compared to head (4ea09c3) 77.92%.

❗ Current head 4ea09c3 differs from pull request most recent head a851f92. Consider uploading reports for the commit a851f92 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7096   +/-   ##
=======================================
  Coverage   77.92%   77.92%           
=======================================
  Files         248      248           
  Lines       13257    13257           
=======================================
  Hits        10330    10330           
  Misses       2400     2400           
  Partials      527      527           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Leo6Leo]

/retest

[Leo6Leo]

/cc @pierDipi
Would like to submit the work I have done so far as an early review to gain some feedback!

[Leo6Leo]

/cc @pierDipi
Last week during our weekly sync meeting with Christoph @creydr , I asked him some questions about this ticket. He suggested that we should only enable TLS for the broker as sink, not subscriber, because that should be handled in another test. WDYT?

[Leo6Leo]

/retest

[Leo6Leo]

@pierDipi Would you mind helping me review this ticket? Thanks!

[Leo6Leo]

/retest

[Leo6Leo]

/cc @Cali0707

[pierDipi]

The test will not run in CI since transport-encryption is disabled on CI, did you try running this test locally while setting transport-encryption to strict?

[Leo6Leo]

@pierDipi @creydr I have tried in the local env, unfortunately it didn't work. When I configured the transport-encryption to "strict", it is just starting to fail when creating the broker. Are there something I might be missing?

[pierDipi]

@Leo6Leo describe the error you're getting

[Leo6Leo]

@pierDipi It just stuck there forever to wait for broker and trigger to become ready.

Here are some details:

The code snippet that related to this

eventing/test/rekt/features/apiserversource/data_plane.go

Line 781 in 8b4298b

f.Setup("broker is ready", broker.IsReady(brokerName))

What I did

1. Apply the following yaml file to activate strict mode

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-features
  namespace: knative-eventing
  labels:
    knative.dev/config-propagation: original
    knative.dev/config-category: eventing
data:
  transport-encryption: "strict"

2. Run the rekt test with the command

SYSTEM_NAMESPACE=knative-eventing go test -v -tags=e2e -count=1 -run TestApiServerSourceDataPlane_BrokerAsSinkTLS -parallel=12 -timeout=30m ./test/rekt

The error log

=== NAME  TestApiServerSourceDataPlane_BrokerAsSinkTLS/SendsEventsWithBrokerAsSinkTLS/Setup/broker_is_ready
    wait.go:200: test-rlvwfksg/broker-hifpjhla condition is {"type":"Ready","status":"False","lastTransitionTime":"2023-08-02T15:09:23Z","reason":"NoAddress","message":"Channel does not have an address."}
=== NAME  TestApiServerSourceDataPlane_BrokerAsSinkTLS/SendsEventsWithBrokerAsSinkTLS/Setup/Wait_for_Trigger_to_become_ready
    wait.go:200: test-rlvwfksg/trigger-imdhsydn condition is {"type":"Ready","status":"False","lastTransitionTime":"2023-08-02T15:09:23Z","reason":"NoAddress","message":"Channel does not have an address."}

My guess on what is happening

"Channel does not have an address" might suggesting that channel need some TLS config, but I don't get it why channel is involved here

[creydr]

@Leo6Leo Thanks for the summary. This makes it easier to follow.
The channel is created, because the MTChannelBasedBroker is a broker, which is based on a channel. docs/mt-channel-based-broker gives an overview on the MTChannelBasedBroker.

Can you check on the involved components, while the tests are running against your local cluster?
-> start the tests and then check in the test namespace (check via kubectl get ns which got created latest) on the involved components (Broker, channel, etc.).

[pierDipi]

@Leo6Leo make sure to rebase/merge main, so that you pick up the latest changes that fixed a few issues (de2e8ff and 5e76ff2)

[Leo6Leo]

@Leo6Leo Thanks for the summary. This makes it easier to follow. The channel is created, because the MTChannelBasedBroker is a broker, which is based on a channel. docs/mt-channel-based-broker gives an overview on the MTChannelBasedBroker.

Can you check on the involved components, while the tests are running against your local cluster? -> start the tests and then check in the test namespace (check via kubectl get ns which got created latest) on the involved components (Broker, channel, etc.).

As I can see, only sink is running. Broker is not running.

Currently I am in the process of debugging, with the help from @Cali0707 to set up the debugger and guide me how to start debugging with kubernetes.

What I have tried

1. Write a yaml file to create a broker

apiVersion: eventing.knative.dev/v1
kind: Broker
metadata:
  annotations:
    eventing.knative.dev/broker.class: MTChannelBasedBroker
  name: hahabrokersss
  namespace: knative-eventing

2. Set up the break point at here
   

   eventing/pkg/reconciler/broker/broker.go

   Line 92 in 4ea09c3

   logging.FromContext(ctx).Infow("Reconciling", zap.Any("Broker", b))

The broker can be created, but I checked the log, and it is still saying Channel does not have an address..

And the debugger didn't stop at that line.

I will continue working on this tomorrow. If you could provide any insights that would be really helpful! @pierDipi @creydr

[pierDipi]

@Leo6Leo I believe your installation is not done correctly, creating a broker with strict transport encryption works fine for me

[pierDipi]

Can you show how are you installing eventing? Are you using hack/install.sh?

[Leo6Leo]

export KO_DOCKER_REPO=docker.io/leo6leoredhat
ko apply -f config/
ko apply -Rf config/channels/in-memory-channel/
ko apply -f config/brokers/mt-channel-broker/
kubectl apply -f third_party/cert-manager
ko apply -f test/config/sugar.yaml
ko apply -f test/config-transport-encryption/features.yaml

This is the way I install eventing. I didn't run install.sh
@pierDipi
@creydr
But I am trying to run it right now, encountering some errors while running install.sh. It is prompting ERROR: Knative Monitoring did not come up . Debugging right now

It is working right now by running install.sh

[knative-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Leo6Leo

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

[knative-prow]

@Leo6Leo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
upgrade-tests_eventing_main	37a5651	link	true	/test upgrade-tests

Your PR dashboard.

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.