Event Transform JSONata TLS #8515

pierDipi · 2025-03-04T17:00:32Z

Follow up to knative-extensions/eventing-integrations#73

Proposed Changes

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

Docs

knative-prow · 2025-03-04T17:00:34Z

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

matzew · 2025-03-05T08:05:26Z

pkg/certificates/certificate.go

+	cert := &cmv1.Certificate{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      CertificateName(name),
+			Name:      CertificateName(obj.GetName()),


thx for these

linux-foundation-easycla · 2025-03-06T07:04:15Z

The committers listed above are authorized under a signed CLA.

✅ login: pierDipi / name: Pierangelo Di Pilato (69d9c5d, fc13b1e, 1d7ccc9, 3e0ccab, 0fe8158, c738e17, a979172)

pierDipi · 2025-03-06T07:08:34Z

/easycla

pierDipi · 2025-03-06T07:09:46Z

/check-cla

codecov · 2025-03-06T07:19:01Z

Codecov Report

Attention: Patch coverage is 67.12871% with 166 lines in your changes missing coverage. Please review.

Project coverage is 52.53%. Comparing base (ec8a208) to head (1d7ccc9).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/reconciler/eventtransform/eventtransform.go	70.25%	36 Missing and 11 partials ⚠️
pkg/eventingtls/trust_bundle.go	37.68%	40 Missing and 3 partials ⚠️
pkg/reconciler/eventtransform/controller.go	11.53%	23 Missing ⚠️
...apis/eventing/v1alpha1/eventtransform_lifecycle.go	0.00%	22 Missing ⚠️
pkg/reconciler/eventtransform/resources_jsonata.go	93.44%	11 Missing and 1 partial ⚠️
pkg/reconciler/integration/sink/integrationsink.go	0.00%	4 Missing ⚠️
pkg/reconciler/testing/v1/factory.go	0.00%	4 Missing ⚠️
pkg/reconciler/testing/v1/listers.go	0.00%	4 Missing ⚠️
pkg/reconciler/testing/v1alpha1/eventtransform.go	0.00%	4 Missing ⚠️
cmd/controller/main.go	0.00%	2 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8515      +/-   ##
==========================================
+ Coverage   52.22%   52.53%   +0.30%     
==========================================
  Files         397      397              
  Lines       24354    24778     +424     
==========================================
+ Hits        12719    13016     +297     
- Misses      10863    10978     +115     
- Partials      772      784      +12

☔ 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.

pierDipi · 2025-03-06T14:56:00Z

/check-cla

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

pierDipi · 2025-03-07T14:32:41Z

/retest-required

matzew · 2025-03-10T07:13:35Z

pkg/reconciler/eventtransform/eventtransform.go

-	expected := jsonataDeployment(ctx, r.configWatcher, expression, transform)
+func (r *Reconciler) reconcileJsonataTransformationCertificate(ctx context.Context, transform *eventing.EventTransform) (*cmapis.Certificate, error) {
+	if f := feature.FromContext(ctx); !f.IsStrictTransportEncryption() && !f.IsPermissiveTransportEncryption() {
+		return nil, r.deleteJsonataTransformationCertificate(ctx, transform)


that;s a good point for integration sink as well

matzew · 2025-03-10T07:14:43Z

pkg/reconciler/eventtransform/controller.go

 )

+func init() {
+	// TODO: Use dynamic (filtered) informer factory since cert-manager is an optional dependency: https://github.com/knative/eventing/pull/8517


matzew · 2025-03-10T07:17:09Z

pkg/reconciler/eventtransform/eventtransform.go

+		}
+		if !transform.Status.PropagateJsonataCertificateStatus(created.Status) {
+			// Wait for Certificate to become ready before continuing.
+			return nil, controller.NewSkipKey("")


why the skip key?

should we factor this out for some more generic reuse, e.g into the certififcates?

Because the certificate is not ready and we need to wait for it to become ready to continue, so we skip and the object will get re-queued when the certificate status changes

matzew · 2025-03-10T07:17:55Z

pkg/reconciler/eventtransform/eventtransform.go

+
+func (r *Reconciler) reconcileJsonataTransformationDeployment(ctx context.Context, expression *corev1.ConfigMap, certificate *cmapis.Certificate, transform *eventing.EventTransform) error {
+	withCombinedTrustBundle := false
+	if isPresent, _ := eventingtls.CombinedBundlePresent(r.trustBundleConfigMapLister); isPresent {


why this check here, for combined / trustBundle?

This is to check if SinkBinding will inject the trust bundle volume and configure nodejs to read it for outgoing requests

OK, for integration we have this, since we leverage existing API via Containersource.

thx

for IntegrationSource this might be relevant, how are we reading trust bundles on the kamelet/data plane side?

matzew · 2025-03-10T07:18:52Z

pkg/eventingtls/trust_bundle.go

 	return nil
 }

+func CombinedBundlePresent(trustBundleLister corev1listers.ConfigMapLister) (bool, error) {


mind adding a little comment on top?

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

matzew

/lgtm
/approve

knative-prow · 2025-03-10T11:39:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matzew, pierDipi

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:

~~OWNERS~~ [matzew,pierDipi]

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

* Event Transform JSONata TLS Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * HA and security context Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Reconcile Certificate Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Additional unit tests cases Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Fix tests Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * EventTransform trust bundle propagation and tests Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Fix tests and address comments Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

* Partial implementation of certmanager for integration sink (knative#8481) * Adding cert-mananger bits Basics for Cert-manager Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Adding create cert manifest function Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Use the make cert Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Reconcile Certificate manifest and add RBAC Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Add secret filter to controller Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Look up secrets/certs for sink Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Remove old/wrong Certificate manifest and reference Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Fix controller compile Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Formatting Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * change the reconcile of cert manifest, and run it only w/ some form of TLS Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * using filtered informer Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Adding Rekt test for IntegrationSink TLS support Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * adding a bit of https port for deployment/service of the IntegrationSink Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Mounting secrets and setting quarkus env vars for TLS support Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Adding flags for different TLS levels resulting in diffeerent quarkus env vars Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Remove bad comment Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Adding label Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Update codegen Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Disable tmp. the cert manager feature Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Fixing header/year Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * More test tweaking Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Remove unused and comment out the hooks for now Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> --------- Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Run make generate-release Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Remove unused code (knative#8485) Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * IntegrationSink prefactors (knative#8486) Little refactors on IntegrationSink Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Add initial iteration of eventing-integration resources (knative#8493) * Generic code for Certificates (knative#8489) Make Certifacte factory more generic Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Jsonata Event transform e2e tests (knative#8499) * E2E tests Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * EventTransform Jsonata e2e tests Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Use eventing-integrations images and fix tests Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Verify response status code received by source Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Update JSONata transformations Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Inject integration images from ConfigMap (knative#8500) Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Run make generate-release Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Update to the correct file (knative#8507) Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * EventTransform: Inject tracing configurations (knative#8479) Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * EventTransform: add reconciler unit tests (knative#8513) * EventTransform: add reconciler unit tests Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Update pkg/reconciler/testing/v1alpha1/eventtransform.go Co-authored-by: Marek Schmidt <maschmid@redhat.com> --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> Co-authored-by: Marek Schmidt <maschmid@redhat.com> * Enable Cert Manager (knative#8509) * Re-enable Cert Mgr Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Manual informer Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Update main Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * update controller Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Adding label on cert Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * add enqueue Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Create Cert Informer, on demand, and cancle when flag is disabled Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Update unit tests that broke during rrefactors Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> --------- Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Event Transform JSONata TLS (knative#8515) * Event Transform JSONata TLS Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * HA and security context Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Reconcile Certificate Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Additional unit tests cases Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Fix tests Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * EventTransform trust bundle propagation and tests Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Fix tests and address comments Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Run make generate-release Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Add dynamic cert-manager certificates informer (knative#8517) * Add dynamic informer, part I Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Add Dynamic certificates informer for EventTransform Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Add Dynamic certificates Informer to IntegrationSink Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Fix error message Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> --------- Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> Co-authored-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Improve CM Cert reconcile and deletion if feature is turned off (knative#8519) refactor CM Cert reconciler and take care of delete if feature is disabled Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Remove transformations and integrations ConfigMaps (#1201) We don't use the ConfigMaps to inject the values, the operator will replace the value to the images directly. Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> Co-authored-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Run make generate-release Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Add configmaps/finalizers permissions Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Add update finalizers permissions for EventTransform reconciled resources Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> * Volume mounts use direct Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> * Fix EventTransform with TLS Sink test Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> --------- Signed-off-by: Matthias Wessendorf <mwessend@redhat.com> Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> Co-authored-by: David Simansky <dsimansk@redhat.com> Co-authored-by: Pierangelo Di Pilato <pierdipi@redhat.com> Co-authored-by: Marek Schmidt <maschmid@redhat.com>

knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 4, 2025

knative-prow bot requested review from creydr and lionelvillard March 4, 2025 17:00

knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 4, 2025

matzew reviewed Mar 5, 2025

View reviewed changes

knative-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 5, 2025

pierDipi force-pushed the event-transform-jsonata-tls branch from 86cd0fc to 9c72a97 Compare March 6, 2025 14:08

pierDipi added 6 commits March 7, 2025 13:10

Event Transform JSONata TLS

fc13b1e

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

HA and security context

3e0ccab

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

Reconcile Certificate

0fe8158

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

Additional unit tests cases

c738e17

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

Fix tests

a979172

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

EventTransform trust bundle propagation and tests

69d9c5d

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

pierDipi force-pushed the event-transform-jsonata-tls branch from 9c72a97 to 69d9c5d Compare March 7, 2025 13:23

knative-prow bot added the area/test-and-release Test infrastructure, tests or release label Mar 7, 2025

pierDipi marked this pull request as ready for review March 7, 2025 14:35

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 7, 2025

knative-prow bot requested a review from mgencur March 7, 2025 14:35

matzew reviewed Mar 10, 2025

View reviewed changes

pierDipi force-pushed the event-transform-jsonata-tls branch from 5e2a3fd to cc9fc58 Compare March 10, 2025 09:29

Fix tests and address comments

1d7ccc9

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

pierDipi force-pushed the event-transform-jsonata-tls branch from cc9fc58 to 1d7ccc9 Compare March 10, 2025 09:31

pierDipi requested a review from matzew March 10, 2025 11:19

matzew approved these changes Mar 10, 2025

View reviewed changes

knative-prow bot assigned matzew Mar 10, 2025

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2025

knative-prow bot merged commit 7be5c19 into knative:main Mar 10, 2025
34 of 36 checks passed

pierDipi deleted the event-transform-jsonata-tls branch March 10, 2025 11:41

Event Transform JSONata TLS #8515

Event Transform JSONata TLS #8515

Uh oh!

Conversation

pierDipi commented Mar 4, 2025

Proposed Changes

Pre-review Checklist

Uh oh!

knative-prow bot commented Mar 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linux-foundation-easycla bot commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pierDipi commented Mar 6, 2025

Uh oh!

pierDipi commented Mar 6, 2025

Uh oh!

codecov bot commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pierDipi commented Mar 6, 2025

Uh oh!

pierDipi commented Mar 7, 2025

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!

pierDipi Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pierDipi Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

matzew left a comment

Choose a reason for hiding this comment

Uh oh!

knative-prow bot commented Mar 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

linux-foundation-easycla bot commented Mar 6, 2025 •

edited

Loading

codecov bot commented Mar 6, 2025 •

edited

Loading

pierDipi Mar 10, 2025 •

edited

Loading

pierDipi Mar 10, 2025 •

edited

Loading