[Bug] grafana-alerting service creates a race condition for OpenShift clusters using oauth-proxy

[paulczar]

Describe the bug

Many OpenShift deployments use the oauth-proxy sidecar to provide authentication to Grafana. They add the service annotation service.alpha.openshift.io/serving-cert-secret-name: grafana-instance which tells OpenShift to create a TLS certificate for the service. Because the change below adds the annotation to both services, the certificate will sometimes be created for the grafana-alert service instead of the grafana service which means the TLS cert will be invalid (its named for the dns of the service that its created for).

Version
4.10.0

To Reproduce

helm upgrade -n "custom-logging" aro-thanos-af   --install mobb/aro-thanos-af --version 0.4.1   --set "aro.storageAccount=sdasdsdasdsadsa"   --set "aro.storageAccountKey=AAAAAA"   --set "aro.storageContainer=aro-metrics"   --set "enableUserWorkloadMetrics=true"

the grafana resource created by ^ looks like

apiVersion: integreatly.org/v1alpha1
kind: Grafana
metadata:
  annotations:
    meta.helm.sh/release-name: aro-thanos-af
    meta.helm.sh/release-namespace: custom-logging
  creationTimestamp: "2023-05-13T14:01:27Z"
  generation: 1
  labels:
    app.kubernetes.io/instance: aro-thanos-af
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: grafana-cr
    helm.sh/chart: grafana-cr-0.2.1
  name: aro-thanos-af-grafana-cr
  namespace: custom-logging
  resourceVersion: "150443"
  uid: a596515c-c728-45d6-adec-65d72ed461fc
spec:
  config:
    analytics:
      check_for_updates: false
      reporting_enabled: false
    auth:
      disable_login_form: true
      disable_signout_menu: true
      sigv4_auth_enabled: true
    auth.basic:
      enabled: false
    auth.proxy:
      auto_sign_up: true
      enabled: true
      header_name: X-Forwarded-User
    log:
      level: warn
      mode: console
    security:
      admin_user: system:does-not-exist
      cookie_secure: true
    users:
      auto_assign_org: true
      auto_assign_org_role: Viewer
      default_theme: light
      editors_can_admin: true
      viewers_can_edit: true
  containers:
  - args:
    - -provider=openshift
    - -https-address=:9091
    - -http-address=
    - -email-domain=*
    - -upstream=http://localhost:3000
    - '-openshift-sar={"resource": "namespaces", "verb": "get"}'
    - '-openshift-delegate-urls={"/": {"resource": "namespaces", "verb": "get"}}'
    - -tls-cert=/etc/tls/private/tls.crt
    - -tls-key=/etc/tls/private/tls.key
    - -client-secret-file=/var/run/secrets/kubernetes.io/serviceaccount/token
    - -openshift-service-account=grafana-serviceaccount
    - -openshift-ca=/etc/pki/tls/cert.pem
    - -openshift-ca=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt
    - -cookie-secret-file=/etc/proxy/secrets/session_secret
    - -cookie-expire=24h
    - -scope=user:info user:check-access user:list-projects
    - -pass-access-token=true
    - -pass-basic-auth=false
    - -display-htpasswd-form=false
    - -htpasswd-file=/etc/proxy/htpasswd/auth
    - -skip-provider-button=true
    image: quay.io/openshift/origin-oauth-proxy:4.12
    name: grafana-proxy
    ports:
    - containerPort: 9091
      name: grafana-proxy
      protocol: TCP
    resources: {}
    volumeMounts:
    - mountPath: /etc/tls/private
      name: secret-aro-thanos-af-grafana-cr-tls
      readOnly: false
    - mountPath: /etc/proxy/secrets
      name: secret-aro-thanos-af-grafana-cr-proxy
      readOnly: false
    - mountPath: /etc/proxy/htpasswd
      name: secret-aro-thanos-af-grafana-cr-htpasswd
      readOnly: true
  dashboardLabelSelector:
  - matchExpressions:
    - key: app
      operator: In
      values:
      - grafana
  deployment:
    envFrom:
    - secretRef:
        name: aro-thanos-af-grafana-cr-creds
    skipCreateAdminAccount: true
  ingress:
    enabled: true
    targetPort: grafana-proxy
    termination: reencrypt
  secrets:
  - aro-thanos-af-grafana-cr-tls
  - aro-thanos-af-grafana-cr-proxy
  - aro-thanos-af-grafana-cr-htpasswd
  service:
    annotations:
      service.alpha.openshift.io/serving-cert-secret-name: aro-thanos-af-grafana-cr-tls
    ports:
    - name: grafana-proxy
      port: 9091
      protocol: TCP
      targetPort: grafana-proxy
  serviceAccount:
    annotations:
      serviceaccounts.openshift.io/oauth-redirectreference.primary: '{"kind":"OAuthRedirectReference","apiVersion":"v1","reference":{"kind":"Route","name":"grafana-route"}}'
status:
  message: success
  phase: reconciling
  previousServiceName: grafana-service

Suspect component/Location where the bug might be occurring

bug introduced by b1f3635

Screenshots
If applicable, add screenshots to help explain your problem.

Runtime (please complete the following information):

• OS: [e.g. Linux,Fedora,Mac]
• Grafana Operator Version [e.g. v5.0.0]
• Environment: [e.g Openshift,Kubernetes,minikube etc. please specify versions]
• Deployment type: [e.g running the operator locally, or deployed]
• Other: [Other variables/things that might be relevant to this bug, versions of other services e.g. operator-sdk]

Additional context
Add any other context about the problem here.

[pb82]

@paulczar a possible workaround: do not specify the annotation through the Grafana CR, but instead add it manually to the service where you want it to be. The operator preserves existing annotations. This way you can avoid the annotation being added to both services.

[KhizerJaan]

Facing same issue . is there any way to add annotation to only one service through Grafana CR ?

[paulczar]

@pb82 while that works, its not really scalable to manually modify resources that the operator should manage. Some operators do not preserve annotations, so its not something people should expect from an operator.

@KhizerJaan the following works around it

    oc patch -n custom-logging service grafana-alert -p '{ "metadata": { "annotations": null }}'
    oc -n custom-logging delete secret aro-thanos-af-grafana-cr-tls
    oc patch -n custom-logging service grafana-service \
      -p '{"metadata":{"annotations":{"retry": "true" }}}'
    sleep 5
    oc -n custom-logging rollout restart deployment grafana-deployment

[NissesSenap]

@paulczar i agree with you.
Would you be willing to send in a PR for this? And I will be happy to fix a OLM release.