Added AppProtocol for Default Grafana Port

[kebroad]

Description

This adds the AppProtocol field to the default Grafana service port, which allows it to integrate with Istio's explicit protocol selection, among other things.

Relevant issues/tickets

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• This change requires a documentation update
• I have added tests that prove my fix is effective or that my feature works
• I have added a test case that will be used to verify my changes
• Verified independently on a cluster by reviewer

Verification steps

[NissesSenap]

Hi @kebroad , first of all thanks for your contribution.

In this case you hard code the value. My understanding from reading the isito docs is that you will use http by default.
Why is this feature needed?
I personally don´t use istio day to day so I only know the basics but it would be nice to get some understanding.

[kebroad]

Hey @NissesSenap ,

Thanks for your response!

Istio doc above states the following:

Istio can automatically detect HTTP and HTTP/2 traffic.

However, from our experience, this seems best effort and is not reliable in most cases. We are seeing Istio not identify the traffic as http but rather plain TCP traffic, which checks out according to this description on the Istio page:

If the protocol cannot automatically be determined, traffic will be treated as plain TCP traffic.

This technically works fine from a raw routing standpoint, however when using specific use cases such as http-api-authorization, this traffic cannot get recognized as http traffic.

Let me know if you have any questions or need further explanation.

[NissesSenap]

Thanks allot for the explanation @kebroad , im just trying to think though if this could create any issues for other istio use-cases or some sidecar solution or similar. But at the same time i assume all of them should be http.

I will take a second look at it tomorrow and i don't come up with anything I'm happy to merge. If you think of some use case that might get issues please share.

[kebroad]

I appreciate you looking into this @NissesSenap. I've been looking into this as well, and the two main issues I think could possibly arise from this would be:

1. Clusters that are <=v1.18 Kubernetes, prior to the AppProtocol field being established. I am not sure how k8s handles forward compatibility for unknown fields but I also assume people using the latest version of this operator will not have a really outdated cluster version. FWIW, v1.18 end of life was 1 year and 8 months ago
2. Teams that enable TLS termination on the actual grafana container itself. This seems like it would be pretty rare for k8s workloads considering the overhead of management versus using a service mesh, and even if implemented, I don't see how the AppProtocol could interfere with it, especially since without a service mesh, the communication would happen on a lower network layer like layer 4.

I don't think this should cause any breaking changes but understand if you are hesitant to merge this. I can parameterize this through the CRD rather than hardcoding if you want to take that route.

[NissesSenap]

<=v1.18 Kubernetes isn't supported by the operator any way. We only support version 1 of CRD.

But the second option is a good point and this is actually something that I know that people are doing.
So if you can create this as a CR option that is omitempty so we don't make a breaking change for those users that would be great.

[kebroad]

Sounds good @NissesSenap! Just added that functionality and tested it with/without the appProtocol field, is working as expected on my end. I am not super familiar with the operator SDK so i looked at a previous PR to determine where to update the doc/CRDs. Let me know if that all looks good or there is anything else needed. Thanks!

[NissesSenap]

LGTM

[NissesSenap]

Merged, thanks allot for your contribution @kebroad.