kie-issues#1583: [sonataflow-operator] Add external built image integrity validation #3013

treblereel · 2025-03-19T02:19:16Z

Closes: apache/incubator-kie-kogito-serverless-operator#405

ricardozanini · 2025-03-19T22:41:53Z

packages/sonataflow-operator/container-builder/examples/dockerfiles/SonataFlow.dockerfile

@@ -40,6 +40,7 @@ COPY --from=builder --chown=185 /home/kogito/serverless-workflow-project/target/
 COPY --from=builder --chown=185 /home/kogito/serverless-workflow-project/target/quarkus-app/*.jar /deployments/
 COPY --from=builder --chown=185 /home/kogito/serverless-workflow-project/target/quarkus-app/app/ /deployments/app/
 COPY --from=builder --chown=185 /home/kogito/serverless-workflow-project/target/quarkus-app/quarkus/ /deployments/quarkus/
+COPY --from=builder --chown=185 /home/kogito/serverless-workflow-project/target/classes/workflow.sw.json /deployments/app/workflow.sw.json


So, should the builder image add the workflow file to the /deployments/app? Is there no other way to do it? This file can also be a YAML named anything (bananas.sw.yml).

So, if we imagine a custom builder or an image built by users, they must know where to put the file in their environment.

Docs and migration from a previous version will be severely impacted. Can you also add a configuration to the manager to skip this validation? It will be needed in case of a migration from a previous version.

Currently, users may build their workflow images as they wish. To migrate to this new version, they might need to turn the validation off and turn it on once everything is migrated.

treblereel · 2025-03-25T23:51:40Z

blocked by #3028

…rity validation

…s-operator_405_fix_2 Incubator kie kogito serverless operator 405 fix 2

wmedvede · 2025-04-01T16:58:55Z

packages/sonataflow-operator/internal/controller/validation/ImageValidator.go

@@ -0,0 +1,128 @@
+// Copyright 2024 Apache Software Foundation (ASF)


wrong licence format, copyright not needed

wmedvede · 2025-04-01T17:02:03Z

packages/sonataflow-operator/internal/controller/sonataflow_controller.go

@@ -118,6 +122,11 @@ func (r *SonataFlowReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, err
 	}

+	if err := r.Validate(ctx, workflow, req); err != nil {
+		klog.V(log.E).ErrorS(err, "Failed to validate SonataFlow")


I think the error message should include the name / namespace of the failing workflow. That information will be very helpful when we need to see the logs in case of failues.

This invocation to the validator should go after the code below.
That if indicates that the WF being deleted right now, so, we don't need to validate it.

if workflow.DeletionTimestamp != nil { return r.applyFinalizers(ctx, workflow) }

wmedvede · 2025-04-01T17:09:17Z

packages/sonataflow-operator/internal/controller/validation/Validator.go

+
+var validators = []Validator{ImageValidator{}}
+
+type Validator interface {


Nice approach!

wmedvede · 2025-04-01T17:10:07Z

packages/sonataflow-operator/internal/controller/validation/Validator.go

+	Validate(ctx context.Context, client client.Client, sonataflow *operatorapi.SonataFlow, req ctrl.Request) error
+}
+
+// validate the SonataFlow object, right now it's only check if workflow is in deployment has image declared as that image


nitpick, validate here, goes with uppercase Validate instead.

wmedvede · 2025-04-01T17:24:58Z

packages/sonataflow-operator/internal/controller/validation/ImageValidator.go

+
+func (v ImageValidator) Validate(ctx context.Context, client client.Client, sonataflow *operatorapi.SonataFlow, req ctrl.Request) error {
+	if sonataflow.HasContainerSpecImage() {
+		err := client.Get(ctx, req.NamespacedName, sonataflow)


If we are validating a `workflow' that was just read the instant before calling the Validators sequence, I don't believe we need to re-read it again.

But if you do, I don't believe we should read it and storing the value in the parameter sonataflow *operatorapi.SonataFlow paramter, I think this could make the validator has side effects on that variable value. When multiple validators are executed I think this code wont be clear.

Another nitpick, when we do reading of the resource as part of the reconciliation, we normally guard the code with something like this:

err := r.Client.Get(ctx, req.NamespacedName, workflow) if err != nil { if errors.IsNotFound(err) { not good, return the error or whatever, but we know exactly that the given resource don't exist at this time. } not good, return the error or whatever, something went wrong, we don't know what. }

wmedvede · 2025-04-01T17:27:03Z

packages/sonataflow-operator/internal/controller/validation/ImageValidator.go

+func (v ImageValidator) Validate(ctx context.Context, client client.Client, sonataflow *operatorapi.SonataFlow, req ctrl.Request) error {
+	if sonataflow.HasContainerSpecImage() {
+		err := client.Get(ctx, req.NamespacedName, sonataflow)
+		equals, err := validateImage(sonataflow, sonataflow.Spec.PodTemplate.Container.Image)


if we don't do the consideration above, in case of error the sonataflow value is uncertain.

wmedvede · 2025-04-01T17:35:18Z

packages/sonataflow-operator/internal/controller/validation/ImageValidator.go

+			return err
+		}
+		if !equals {
+			return fmt.Errorf("Workflow, defined in the image %s doesn't match deployment workflow", sonataflow.Spec.PodTemplate.Container.Image)


workflow name/namepace would be nice to include in the message.

wmedvede · 2025-04-01T17:53:11Z

packages/sonataflow-operator/internal/controller/validation/ImageValidator.go

+func jsonFromDockerImage(reader io.Reader) (operatorapi.Flow, error) {
+	data, err := io.ReadAll(reader)
+	workflow := &operatorapi.Flow{}
+	if err = yaml.Unmarshal(data, workflow); err != nil {


Excuse me if I'm missing something, but do this Unmarshalling work both when the workflow inside the image is in json and yaml format? If so, maybe this function can be called workflowFromDockerImage.

wmedvede · 2025-04-01T17:53:38Z

packages/sonataflow-operator/internal/controller/validation/Validator.go

@@ -0,0 +1,41 @@
+// Copyright 2024 Apache Software Foundation (ASF)


same here, copyright we don't need

wmedvede · 2025-04-01T18:04:23Z

Hi @treblereel I had tried the feature in OpenShift 4.15.17 with the following scenario

I have a workflow with a pre-built image
In SonataFlow I modify the flow intentionally to force validation to fail
When I deploy the workflow I can see this message:

E0401 16:52:11.167885 1 sonataflow_controller.go:126] "Failed to validate SonataFlow" err="Get \"https://quay.io/v2/\": tls: failed to verify certificate: x509: certificate signed by unknown authority"

``

It looks like it was not possible for the validator to read the image,

wmedvede · 2025-04-02T06:26:19Z

@treblereel I'd recommend rebasing with main, since there has been some recent updates in sontaflow_controller.go

ricardozanini · 2025-04-02T14:52:00Z

@treblereel the docker api needs to authenticate with OpenShift internal registry

treblereel requested a review from tiagobento as a code owner March 19, 2025 02:19

treblereel requested review from ricardozanini and domhanak and removed request for tiagobento March 19, 2025 02:19

treblereel self-assigned this Mar 19, 2025

treblereel added the area:sonataflow label Mar 19, 2025

ricardozanini requested a review from wmedvede March 19, 2025 13:44

ricardozanini reviewed Mar 19, 2025

View reviewed changes

tiagobento changed the title ~~kie-issues#1583: [sonataflow-operator] Add external built image integ…~~ kie-issues#1583: [sonataflow-operator] Add external built image integrity validation Mar 20, 2025

kie-issues#1583: [sonataflow-operator] Add external built image integ…

f4eaddc

…rity validation

treblereel force-pushed the incubator-kie-kogito-serverless-operator_405 branch from 6713238 to f4eaddc Compare March 28, 2025 17:27

treblereel and others added 3 commits March 28, 2025 10:27

added pattern for yaml, json and yml extension of the workflow image

96a4b89

add config param to switch workflow image validation

e225aa3

Merge pull request #44 from treblereel/incubator-kie-kogito-serverles…

c9f1816

…s-operator_405_fix_2 Incubator kie kogito serverless operator 405 fix 2

treblereel force-pushed the incubator-kie-kogito-serverless-operator_405 branch from c51a1c9 to c9f1816 Compare March 30, 2025 02:24

wmedvede requested changes Apr 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

kie-issues#1583: [sonataflow-operator] Add external built image integrity validation #3013

kie-issues#1583: [sonataflow-operator] Add external built image integrity validation #3013

Uh oh!

treblereel commented Mar 19, 2025 •

edited by tiagobento

Loading

Uh oh!

ricardozanini Mar 19, 2025

Uh oh!

treblereel commented Mar 25, 2025

Uh oh!

wmedvede Apr 1, 2025

Uh oh!

wmedvede Apr 1, 2025

Uh oh!

wmedvede Apr 1, 2025

Uh oh!

wmedvede Apr 1, 2025

Uh oh!

wmedvede Apr 1, 2025

Uh oh!

wmedvede Apr 1, 2025

Uh oh!

wmedvede Apr 1, 2025

Uh oh!

wmedvede Apr 1, 2025

Uh oh!

wmedvede Apr 1, 2025

Uh oh!

wmedvede Apr 1, 2025

Uh oh!

wmedvede commented Apr 1, 2025

Uh oh!

wmedvede commented Apr 2, 2025

Uh oh!

ricardozanini commented Apr 2, 2025

Uh oh!

Uh oh!

		@@ -0,0 +1,128 @@
		// Copyright 2024 Apache Software Foundation (ASF)


		var validators = []Validator{ImageValidator{}}

		type Validator interface {

		@@ -0,0 +1,41 @@
		// Copyright 2024 Apache Software Foundation (ASF)

kie-issues#1583: [sonataflow-operator] Add external built image integrity validation #3013

Are you sure you want to change the base?

kie-issues#1583: [sonataflow-operator] Add external built image integrity validation #3013

Uh oh!

Conversation

treblereel commented Mar 19, 2025 • edited by tiagobento Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

treblereel commented Mar 25, 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!

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!

wmedvede commented Apr 1, 2025

Uh oh!

wmedvede commented Apr 2, 2025

Uh oh!

ricardozanini commented Apr 2, 2025

Uh oh!

Uh oh!

treblereel commented Mar 19, 2025 •

edited by tiagobento

Loading