Add issuer override

[herodes1991]

PR to include the override of the issuer name as we already do with the issuer type

We have the need to have different certificate_issuers per ingress in the same application and the current FIAAS spec does not allow us to do it

Also fixed an error that appeared with python3 version

[herodes1991]

@oyvindio, can you take a look? 😄

[j18e]

@oyvindio is on vacation until next week, and I don't understand the feature well enough (or the codebase, for that matter) to be the sole approver. So this will have to wait until he's back, unfortunately.

[herodes1991]

@j18e no problem at all. I will create my own image with this change meanwhile 😄

I will add some fixes about python3 I'm just discovering while testing this image

[oyvindio]

Thanks for the PR! I will try to make some time to take a closer look at this later this week

[herodes1991]

Thank you @oyvindio! Were you able to take a quick look? 😄

[oyvindio]

Functionally, this looks good to me.

Before merging, please update the operator guide with documentation describing how to use the new configuration flag. (I'm setting request changes mainly for this).

IngressDeployer is already a bit complicated, so while it looks like this should work, I don't think it is ideal that more TLS related functionality is "leaking" into this class. Some ideas to refactor this might be;

• Wrap the _tls_issuer_* instance variables and _get_issuer_* methods in a separate class (TLSIssuerConfiguration?, TLSIssuers?) and use that in IngressDeployer. That seems relatively straightforward to do and could make the separation of concerns a bit clearer.
• Make AnnotatedIngress a proper (data?)class, move more of the of the logic around issuer types and issuer names into it (maybe combine this with using the class to encapsulate issuer name/type logic suggested above in AnnotatedIngress?). AnnotatedIngress could be passed to IngressTLSDeployer for setting the TLS annotations. I think this could leave only the ingress item grouping related logic related to TLS issuers in IngressDeployer. This probably requires moving a bit more code around than the previous point.

What do you think? Refactoring could potentially be handled in a followup PR.

[herodes1991]

Hello Oyvindio, I will try to check the comments, but for the refactor I will not be able to work on it until April, Is it okay to merge this PR and, in April, I refactor the TLS code?

[oyvindio]

Hello Oyvindio, I will try to check the comments, but for the refactor I will not be able to work on it until April, Is it okay to merge this PR and, in April, I refactor the TLS code?

Yes, I think it is okay to do refactoring in a followup PR.
Please add documentation in the operator guide for the new config flag added in this PR though.

[oyvindio]

LGTM