Improve Authorino TLS validation about authorino-operator HOT 3 CLOSED

Perhaps one thing we could do to improve usability is to make the listener and oidcServer properties both mandatory.

I think this would be the best. It would force the customer to choose what he wants explicitly. Thanks for looking into this again :)

from authorino-operator.

@pehala, failing when missing the secret is actually by design. It was implemented this way to avoid issues when both resources are applied together and handled concurrently by the API server.

Users and automation should rely status.conditions to read the state of the Authorino instance.

from authorino-operator.

Taking a second look on this after we spoke, @pehala...

Perhaps one thing we could do to improve usability is to make the listener and oidcServer properties both mandatory.

Although enabled: true is inferred by default, no assumption is made about the name of the secrets that store the TLS server certs. Consequently, (listener|oidcServer).tls.certSecretRef.name must be provided. On the other hand, if the user wants to disable TLS, then (listener|oidcServer).enabled: false is required. That all reduces to virtually listener and oidcServer being always needed – either to enable or to disable TLS. Requiring those fields is something easy to enforce via OAS.

Another option would be to make (listener|oidcServer).enabled: false the default. The sample CR you provided in the descriptions of the issue would be accepted instead of to fail. However, I believe TLS enabled by default would be perceived as a better practice, so I'd probably advocate for the other solution instead of this one.

WDYT?

from authorino-operator.