Skip to content

Conversation

@Cali0707
Copy link
Member

@Cali0707 Cali0707 commented Sep 3, 2025

This adds the Request Reply control plane (as a first pass). In follow ups, TLS/authn/authz/observability/AES key rotation will be handled.

This depends on:

Proposed Changes

  • Add controller and reconciler for the Request
feat: the RequestReply resource can now be deployed from eventing core

Signed-off-by: Calum Murray <cmurray@redhat.com>
Signed-off-by: Calum Murray <cmurray@redhat.com>
Signed-off-by: Calum Murray <cmurray@redhat.com>
@knative-prow knative-prow bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 3, 2025
@knative-prow knative-prow bot requested review from Leo6Leo and matzew September 3, 2025 14:58
@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 3, 2025
@Cali0707
Copy link
Member Author

Cali0707 commented Sep 3, 2025

/cc @creydr @matzew

I'll add unit tests to this today, but besides that this is ready for review

@knative-prow knative-prow bot requested a review from creydr September 3, 2025 14:58
@Cali0707
Copy link
Member Author

Cali0707 commented Sep 3, 2025

The broken builds/tests are coming from #8698 not being in yet - they should work once it is in

@Cali0707
Copy link
Member Author

Cali0707 commented Sep 3, 2025

/hold

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 3, 2025
@Cali0707
Copy link
Member Author

Cali0707 commented Sep 4, 2025

For testing (with the dependent PRs merged), I did:

  1. Create a knative function with the following handle func:
func Handle(ctx context.Context, e event.Event) (*event.Event, error) {
	/*
	 * YOUR CODE HERE
	 *
	 * Try running `go test`.  Add more test as you code in `handle_test.go`.
	 */

	fmt.Println("Received event")
	fmt.Println(e) // echo to local output
	correlationId, ok := e.Extensions()["correlationid"]
	if !ok {
		fmt.Println("warning: failed to extract correlation id from event, reply may not be routed properly")
	} else {
		delete(e.Extensions(), "correlationid")
		e.SetExtension("replyid", correlationId)
	}
	return &e, nil // echo to caller
}
  1. Deploy the following yaml resources:
apiVersion: eventing.knative.dev/v1
kind: Broker
metadata:
  name: example
---
apiVersion: eventing.knative.dev/v1
kind: Trigger
metadata:
  name: example
spec:
  broker: example
  subscriber:
    ref:
      apiVersion: serving.knative.dev/v1
      kind: Service
      name: echo-reply
  filters:
    - cesql: "EXISTS correlationid AND NOT EXISTS replyid"
---
apiVersion: eventing.knative.dev/v1alpha1
kind: RequestReply
metadata:
  name: rr-test
spec:
  brokerRef:
    name: example
    kind: Broker
    apiVersion: eventing.knative.dev/v1

Curl the request reply url within the cluster:

kubectl run curl --image=docker.io/curlimages/curl --rm=true --restart=Never -ti \
-- -X POST -v \
-H "content-type: application/json" \
-H "ce-specversion: 1.0" \
-H "ce-source: my/curl/command" \
-H "ce-type: my.demo.event" \
-H "ce-id: 6cf17c7b-30b1-45a6-80b0-4cf58c92b947" \
-d '{"name":"Request Reply demo"}' \
http://request-reply.knative-eventing.svc.cluster.local/default/rr-test
If you don't see a command prompt, try pressing enter.
< HTTP/1.1 200 OK
< Allow: POST, OPTIONS
< Ce-Correlationid: 6cf17c7b-30b1-45a6-80b0-4cf58c92b947:N3RDobCYCVeP73fuzrXg1x4tYE2c3VkFAMqRADCM1uISFeKTvpKoN7L9YSrRMTvOKcbtQk1CGzvq5L3HEheAfw==:0
< Ce-Id: 6cf17c7b-30b1-45a6-80b0-4cf58c92b947
< Ce-Knativearrivaltime: 2025-09-02T21:38:07.133319085Z
< Ce-Replyid: 6cf17c7b-30b1-45a6-80b0-4cf58c92b947:N3RDobCYCVeP73fuzrXg1x4tYE2c3VkFAMqRADCM1uISFeKTvpKoN7L9YSrRMTvOKcbtQk1CGzvq5L3HEheAfw==:0
< Ce-Source: my/curl/command
< Ce-Specversion: 1.0
< Ce-Type: my.demo.event
< Content-Length: 29
< Content-Type: application/json
< Date: Tue, 02 Sep 2025 21:38:07 GMT
< 
* Connection #0 to host request-reply.knative-eventing.svc.cluster.local left intact
{"name":"Request Reply demo"}pod "curl" deleted

deleteContext context.Context // used to delete triggers in a async cleanup operation
}

func (r *Reconciler) ReconcileKind(ctx context.Context, rr *v1alpha1.RequestReply) reconciler.Event {
Copy link
Member

@matzew matzew Sep 8, 2025

Choose a reason for hiding this comment

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

can we have a test for the reconciler?

@matzew
Copy link
Member

matzew commented Sep 8, 2025

@Cali0707 Mind sharing the old feature track document for that ?

@Cali0707
Copy link
Member Author

Cali0707 commented Sep 8, 2025

@Cali0707 Mind sharing the old feature track document for that ?

Yeah - it is here https://docs.google.com/document/d/1vrCfIH9wRrsl8j7XpHK5XiVRgnx5nB4mLHsp0AxU_0Y/edit?usp=sharing @matzew

@knative-prow knative-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 9, 2025
@Cali0707
Copy link
Member Author

Cali0707 commented Sep 9, 2025

/cc @matzew

@knative-prow knative-prow bot requested a review from matzew September 9, 2025 20:24
Signed-off-by: Calum Murray <cmurray@redhat.com>
@codecov
Copy link

codecov bot commented Sep 9, 2025

Codecov Report

❌ Patch coverage is 27.12215% with 352 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.19%. Comparing base (e804605) to head (c072edc).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
pkg/reconciler/requestreply/requestreply.go 53.90% 92 Missing and 20 partials ⚠️
pkg/reconciler/requestreply/controller.go 0.00% 61 Missing ⚠️
pkg/reconciler/testing/v1/statefulset.go 0.00% 58 Missing ⚠️
pkg/reconciler/testing/v1/pod.go 0.00% 52 Missing ⚠️
pkg/reconciler/testing/v1alpha1/requestreply.go 0.00% 50 Missing ⚠️
pkg/reconciler/testing/v1/trigger.go 0.00% 11 Missing ⚠️
cmd/controller/main.go 0.00% 4 Missing ⚠️
pkg/reconciler/testing/v1/listers.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8701      +/-   ##
==========================================
- Coverage   51.58%   50.19%   -1.40%     
==========================================
  Files         401      409       +8     
  Lines       25446    26659    +1213     
==========================================
+ Hits        13126    13381     +255     
- Misses      11518    12436     +918     
- Partials      802      842      +40     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Calum Murray <cmurray@redhat.com>
Signed-off-by: Calum Murray <cmurray@redhat.com>
@Cali0707
Copy link
Member Author

/unhold
/cc @creydr

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 17, 2025
Copy link
Member

@creydr creydr left a comment

Choose a reason for hiding this comment

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

Thanks @Cali0707
I tested it as (after rebased this PR) and it worked 🎉

Maybe we can add e2e tests for it as well (as a separate PR) and create issues to add TLS and auth support

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 17, 2025
@knative-prow
Copy link

knative-prow bot commented Sep 17, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Cali0707, creydr

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

The pull request process is described here

Details 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 knative-prow bot merged commit d4a30b2 into knative:main Sep 17, 2025
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants