Fix pod logs in case in case empty data from odh-ca-bundle.crt key

[atheo89]

Related to: https://issues.redhat.com/browse/RHOAIENG-4165

Description

This pull request resolves the issue with pod logs by adjusting it to display an informative message instead of raising an error when encountering the odh-ca-bundle.crt key, as there might be instances where the data associated with this key is empty.

How Has This Been Tested?

• Change the deployment on the odh-notebook-controller by using the image created from this PR
  quay.io/opendatahub/odh-notebook-controller:pr-264

In case of empty odh-ca-bundle.crt

{"level":"info","ts":"2024-03-07T15:00:12Z","logger":"controllers.Notebook","msg":"Certificates in 'ca-bundle.crt' are valid and can be mounted","notebook":"emptycert","namespace":"test"}
{"level":"info","ts":"2024-03-07T15:00:12Z","logger":"controllers.Notebook","msg":"odh-ca-bundle.crt data are empty","notebook":"emptycert","namespace":"test"}
{"level":"info","ts":"2024-03-07T15:00:12Z","logger":"controllers.Notebook","msg":"Certificates in 'odh-ca-bundle.crt' are empty, skipping mounting","notebook":"emptycert","namespace":"test"}

Mounts only the valid cert

In case of wrong cert

{"level":"info","ts":"2024-03-07T15:02:33Z","logger":"controllers.Notebook","msg":"Certificates in 'ca-bundle.crt' are valid and can be mounted","notebook":"wrondcert","namespace":"test"}
{"level":"error","ts":"2024-03-07T15:02:33Z","logger":"controllers.Notebook","msg":"invalid certificate format for key 'odh-ca-bundle.crt' in ConfigMap odh-trusted-ca-bundle","notebook":"wrondcert","namespace":"test","stacktrace":"{..}
{"level":"error","ts":"2024-03-07T15:02:33Z","logger":"controllers.Notebook","msg":"Error validating certificates for odh-ca-bundle.crt. They cannot be mounted","notebook":"wrondcert","namespace":"test","error":"invalid certificate format for key 'odh-ca-bundle.crt' in ConfigMap odh-trusted-ca-bundle","stacktrace":"{...}

Mounts only the valid one

In case of both certs are valid

{"level":"info","ts":"2024-03-07T15:11:28Z","logger":"controllers.Notebook","msg":"Certificates in 'ca-bundle.crt' are valid and can be mounted","notebook":"valid","namespace":"test"}
{"level":"info","ts":"2024-03-07T15:11:28Z","logger":"controllers.Notebook","msg":"Certificates in 'odh-ca-bundle.crt' are valid and can be mounted","notebook":"valid","namespace":"test"}

Mounts both cert

Merge criteria:

• The commits are squashed in a cohesive manner and have meaningful messages.
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has manually tested the changes and verified that the changes work

[harshad16]

Did we test this in a cluster, i feel this would result in volumemount of empty bit.

[jstourac]

On lines 301 and 316 of this file, there is following code:

		if err := certValidator(cm, "odh-ca-bundle.crt", log); err == nil {
			log.Info("Validating certificates for odh-ca-bundle.crt")

Do I understand correctly that by that time the validation had been done already? So shouldn't we rather say something like:

Certificates in 'odh-ca-bundle.crt' are valid and can be mounted.

Just a proposal.

---

I put a few comments. Basically it can be merged as is, just maybe some of them make sense so the code/logs are slightly clearer.

[atheo89]

Thank you very much for your patience and your review! 🙂

I believe we are now in a good state. To address the scenario of empty data, I have introduced a boolean variable isEmpty to handle this situation. Now, when encountering empty data, the code will navigate to this section: https://github.com/atheo89/kubeflow/blob/RHOAIENG-4165/components/odh-notebook-controller/controllers/notebook_webhook.go#L322

@jstourac and @harshad16 could you take a look?

[jstourac]

Thank you LGTM and if it really behaves like described in the description, I love it 😍

[atheo89]

Thank you LGTM and if it really behaves like described in the description, I love it 😍

Yeah, I updated the description as well!

[jiridanek]

Does this need any further automated testing?

[atheo89]

Does this need any further automated testing?

Nope, this pr only fixes the displayed logs on the odh-notebook-operator

[harshad16]

/lgtm
/approve

thank you 💯

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: harshad16, jstourac

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [harshad16]

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

[harshad16]

/cherrypick stable

[openshift-cherrypick-robot]

@harshad16: new pull request created: #271

In response to this:

/cherrypick stable

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.