Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ HubertStefanski
❌ matt-delaney-cruise
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Currently awaiting approval from my employer to sign CLA.

Thanks a lot for this @mdelaney .
I will try to take a look at this during the weekend.

To me, this looks like it works. Thanks allot @mdelaney for your help on this one.
Do you think your company will approve the CLA some time next week?

@smuda can you verify that this fix is working for you?

Sure, I'll test that tonight

Do you think your company will approve the CLA some time next week?

I'm hoping so. Will update soon as I know more. 🙂

I'm a bit confused. I span up the kind cluster and adding a grafanadatasource with appropriate secureJsonData, testing the datasource in grafana and then checks the http message in the "thanos" pod and it misses the Authorization header:

GET /api/v1/status/buildinfo HTTP/1.1
Host: thanos.default.svc:8080
User-Agent: Grafana/9.5.17
Accept: application/json, text/plain, */*
Accept-Encoding: gzip, deflate, br, zstd
Accept-Language: sv,en-US;q=0.9,en;q=0.8
Sec-Ch-Ua: "Google Chrome";v="123", "Not:A-Brand";v="8", "Chromium";v="123"
Sec-Ch-Ua-Mobile: ?0
Sec-Ch-Ua-Platform: "macOS"
Sec-Fetch-Dest: empty
Sec-Fetch-Mode: cors
Sec-Fetch-Site: same-origin
X-Forwarded-For: 192.168.65.1, 10.244.0.8, 10.244.0.8
X-Forwarded-Scheme: https
X-Grafana-Nocache: true
X-Grafana-Org-Id: 1
X-Grafana-Referer: https://grafana.127.0.0.1.nip.io/datasources/edit/af828ddc-5207-4ecd-894f-d9110bbf04ea
X-Real-Ip: 192.168.65.1
X-Request-Id: 6edbfac1bab0855309c37182aaa400d8
X-Scheme: https

POST /api/v1/query HTTP/1.1
Host: thanos.default.svc:8080
User-Agent: Grafana/9.5.17
Content-Length: 31
Content-Type: application/x-www-form-urlencoded
X-Datasource-Uid: af828ddc-5207-4ecd-894f-d9110bbf04ea
X-Grafana-Org-Id: 1
Accept-Encoding: gzip

I verified in another kind cluster that (roughly) the same settings created a HTTP post with the Authorization header with an older grafana-operator (v5.6.2):

POST /api/v1/query HTTP/1.1
Host: thanos.addon-grafana-operator.svc:8080
User-Agent: Grafana/9.1.6
Content-Length: 31
Authorization: Bearer eyJh.....gaJkQ
Content-Type: application/x-www-form-urlencoded
Accept-Encoding: gzip

I might have done something wrong in the verification but it seems this fix is insufficient. I'm going to create a PR from the tag of 5.8.0 with the files to make sure my test work correctly.

apiVersion: apps/v1
kind: Deployment
metadata:
  name: thanos
  labels:
    app: thanos
spec:
  selector:
    matchLabels:
      app: thanos
  template:
    metadata:
      labels:
        app: thanos
    spec:
      terminationGracePeriodSeconds: 3
      containers:
        - name: netcat
          image: alpine
          command:
            - sh
            - -c
            - |
              set -eu
              echo "Starting pod"
              while true; do echo -e 'HTTP/1.1 200 OK\n\n{"asdf":"date"}' | nc -l -p 8080; done
          ports:
            - containerPort: 8080
              name: http
              protocol: TCP
---
apiVersion: v1
kind: Service
metadata:
  name: thanos
spec:
  selector:
    app: thanos
  ports:
    - port: 8080
      name: http
      protocol: TCP
      targetPort: 8080

apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaDatasource
metadata:
  name: thanos
spec:
  datasource:
    access: proxy
    basicAuth: false
    editable: true
    isDefault: true
    jsonData:
      httpHeaderName1: Authorization
      timeInterval: 5s
      tlsSkipVerify: true
    name: Thanos
    orgId: 1
    secureJsonData:
      httpHeaderValue1: Bearer ${token}
    type: prometheus
    url: http://thanos.default.svc:8080
  instanceSelector:
    matchLabels:
      dashboards: grafana
  valuesFrom:
    - targetPath: 'secureJsonData.httpHeaderValue1'
      valueFrom:
        secretKeyRef:
          name: grafana-instance-sa-token
          key: token

apiVersion: v1
kind: Secret
metadata:
  name: grafana-instance-sa-token
  annotations:
    kubernetes.io/service-account.name: grafana-instance-sa
type: kubernetes.io/service-account-token

apiVersion: v1
kind: ServiceAccount
metadata:
  name: grafana-instance-sa
secrets:
  - name: grafana-instance-sa-token

I went back to the 5.8.0 release commit, created a branch and added the test files, verifying that they work correctly. Then I rebased on master again to create a PR with the test files here:
#1494

If we could merge that PR and rebase this PR on top, perhaps it would be easer to develop this fix?

The fix seems to work for me:

But I have only verified that the authorization header shows up in the UI, not that it is actually sent when fetching from the datasource.

@smuda so th is fix is not working for you?

@pb82 I'm not saying that I haven't messed anything up during testing, but yes, I do not see the header in the call to my emulator which I did with 5.8.0. But if it works for you, it's likely I have messed up.

Here are some example calls I'm seeing with netcat:

GET /api/v1/status/buildinfo HTTP/1.1
Host: thanos.default.svc:8080
User-Agent: Grafana/9.5.17
Accept: application/json, text/plain, */*
Accept-Encoding: gzip, deflate
Accept-Language: sv,en-US;q=0.9,en;q=0.8
X-Forwarded-For: 192.168.65.1, 10.244.0.7, 10.244.0.7
X-Forwarded-Scheme: http
X-Grafana-Nocache: true
X-Grafana-Org-Id: 1
X-Grafana-Referer: http://grafana.127.0.0.1.nip.io/connections/your-connections/datasources/edit/55442e8f-5329-478b-a53d-537b195c38fe
X-Real-Ip: 192.168.65.1
X-Request-Id: 682b216d649ebec0fffe87565ae2855b
X-Scheme: http

@smuda I see the token in your test:

I would suggest to double-check if you're using a build from this branch.

:face_palm:
Yup, running on the correct branch works much better, thank you!

GET /api/v1/status/buildinfo HTTP/1.1
Host: thanos.default.svc:8080
User-Agent: Grafana/9.1.6
Accept: application/json, text/plain, */*
Accept-Encoding: gzip, deflate
Accept-Language: sv,en-US;q=0.9,en;q=0.8
Authorization: Bearer eyJh...PhkA
X-Forwarded-For: 192.168.65.1, 10.244.0.7, 10.244.0.7
X-Forwarded-Scheme: http
X-Grafana-Nocache: true
X-Grafana-Org-Id: 1
X-Real-Ip: 192.168.65.1
X-Request-Id: 2d318799494425cc29026493725cbc5a
X-Scheme: http

I'm really sorry for making such a mess.

@smuda No worries, it happens to all of us :)

I have some concerns about this implementation as Datasource and UpdateDataSourceCommand are not entirely equivalent. At least, orgId would stop working. We'll discuss it during the upcoming meeting today.

Stuff that happens. Thanks allot for the verification and writing the test @smuda .
Then all we wait for is the CLA which hopefully comes soon, and then we can merge this PR.

@weisdd we don't support orgID at all today so I don't think it's a problem. But let's talk about during today's meeting.

My concern was that models.DataSource and models.UpdateDataSourceCommand are not entirely equal in terms of the list of supported fields. It turned out, the only fields that are exposed through our CRD, but not supported in models.UpdateDataSourceCommand are editable and orgId. editable has no effect in Grafana, and we did not intend to support multiple organizations anyway, so we will simply remove both fields from CRD.

We should be good to merge the PR once @mdelaney signs CLA. Nice work, btw! :)

Hi all,

Unfortunately I haven’t heard back around the CLA and unfortunately can’t provide a good idea of when it will be reviewed. I very much understand if you decide to fix this outside of this pull request.

Apologies for the delay on this

Hi @mdelaney , thanks for the update. Such is life some times

I will grab the code and create a separate PR for this so we can solve this issue, but will of course point to this one and credit you for the contribution.

Thanks once again for solving the issue.