feat: make cert manager certificate's commonName optional #major

[owais-rehman]

Making certificate.commonName default to null. It should not default to admin-app as this field is optional. It can be set according to specific needs but it should not default to an irrelevant value.

https://cert-manager.io/docs/usage/certificate/#:~:text=%23%20Avoid%20using%20commonName,%3A%20example.com

The main reason for making this change is that whenever we try to create Certificate, its commonName field defaults to admin-app which is unexpected behavior in default scenario. Setting it to nil will make more sense.

[rasheedamir]

also according to the documentation it seems one doesn't need to set the commonName for the leaf certificates which belong to the applications

[aslafy-z]

I'm wondering if the field in the certificate value reacts correctly to a null value
We might want to add a guard to prevent null values to enter the templating helperml.

[owais-rehman]

Is there anything left to modify before you guys can approve this PR?

[aslafy-z]

Can you please check my previous comment, have you done some tests ? Can you implement a unit test ?

[owais-rehman]

I'm wondering if the field in the certificate value reacts correctly to a null value

I have tried by setting commonName in certificate to null vs removing this field completely. In both case tls.crt gets populated with one of the SAN value. So I think we are good in that regard.

We might want to add a guard to prevent null values to enter the templating helperml.

I am not exactly sure what changes do I need to make for this. But should we keep this PR open till this change is made?
Suggestions?

[aslafy-z]

I made the test here: #370. It looks like it's ok.

This PR introduces a breaking change, I will make it bump major version. Thank you for your contribution!