Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom HTTP headers when connecting to Prometheus #4323

Closed
libesz opened this issue Sep 1, 2021 · 16 comments 路 Fixed by #4350
Closed

Custom HTTP headers when connecting to Prometheus #4323

libesz opened this issue Sep 1, 2021 · 16 comments 路 Fixed by #4350
Assignees
Labels
backlog Triaged Issue added to backlog enhancement This is the preferred way to describe new end-to-end features.

Comments

@libesz
Copy link
Contributor

libesz commented Sep 1, 2021

Once I asked about this topic on Slack and I was told that: this is not yet supported and it is better to open an issue for further consideration. So here it is 馃槃

Is your feature request related to a problem? Please describe.
There are certain managed public cloud offerings, where a multi-tenant monitoring endpoint however, is a standard Prometheus API, but the client authentication is not a standard bearer token of basic authentication based solution. In the environment, where I would like to operate Kiali, the Prometheus API requires custom HTTP headers to authenticate the client.

Describe the solution you'd like
A generic new feature for Kiali would be to add custom HTTP header options for the Prometheus configuration. Basically a new string list could be added to here: https://github.com/kiali/kiali-operator/blob/76242369299c35db350119516c6db6fd87f47822/deploy/kiali/kiali_cr.yaml#L551
This then would be consumed by the actual HTTP client which is connecting to the Prometheus API. Whenever the extra headers are to be changed/renewed, it would be out of scope of Kiali. External automation would take care of the reconfiguration and restart of the Kiali instance.

Describe alternatives you've considered
Some transparend or non-transparent HTTP proxy could be employed in the same cluster to add the extra headers. The downside is that it requires an extra dedicated component (with all the security considerations of it) just for the proper Kiali operation.

Additional context
IBM Cloud monitoring documentation, which explains the usage of the custom HTTP headers for it's Prometheus API. It basically requires an instance ID reference to be set in the queries.
https://cloud.ibm.com/docs/monitoring?topic=monitoring-metrics_api#metrics_api-curl

@libesz libesz added the enhancement This is the preferred way to describe new end-to-end features. label Sep 1, 2021
@jshaughn jshaughn added the backlog Triaged Issue added to backlog label Sep 1, 2021
@jshaughn
Copy link
Collaborator

jshaughn commented Sep 1, 2021

Hi @libesz , thanks for creating the issue. I think this is a reasonable enhancement and am adding to the backlog.

@libesz
Copy link
Contributor Author

libesz commented Sep 1, 2021

Thanks @jshaughn . Our team might offer some help with the actual contribution in the coming months if you need it.

@jshaughn
Copy link
Collaborator

jshaughn commented Sep 1, 2021

@libesz If you'd like to see it sooner than later please feel free to submit a PR and we will be more than happy to review/guide the submission. Just let us know if you are starting so that we don't also start at some point.

@jmazzitelli
Copy link
Collaborator

Since this requires adding new configuration settings to the server, this checklist will help tell you what needs to be done to add the new config - https://github.com/kiali/kiali-operator/blob/master/DEVELOPING.adoc#are-you-altering-a-kiali-server-configuration-setting

@libesz
Copy link
Contributor Author

libesz commented Sep 1, 2021

Okay, sure! It is unlikely that we will have any free cycles in 1-2 months, but whenever we get there, I will get back to you.

@jmazzitelli jmazzitelli self-assigned this Sep 13, 2021
@jmazzitelli jmazzitelli added this to Backlog in Sprint 63 (v1.41) via automation Sep 13, 2021
@jmazzitelli
Copy link
Collaborator

@libesz I'm going to take a stab at this. Can you confirm if this is what you would need? This is a snippet of the Kiali CR with the new setting:

spec:
  external_services:
    prometheus:
      auth:
        custom_headers:
          some_header: some_value
          IBMInstanceID: MyGUID

I place them under the auth subsection since it appears likely these custom headers would have to do with logging in or authenticating with Prometheus. But I'm not sure. Perhaps the potential exists for needing custom headers that do not have anything to do with auth? Would like to hear what people think - should custom_headers section stay under auth or should it be directly under prometheus so as not to confuse people in thinking these headers must only be related to auth headers?

@libesz
Copy link
Contributor Author

libesz commented Sep 13, 2021

@jmazzitelli thanks for taking a look! Let me discuss the proposal within our team in the coming days. Meanwhile... for our particular use-case these are indeed auth related headers only, though not really any secret as far as I can tell (i.e. instance ID is not very sensitive). I think other people might have other use-cases outside of auth, so probably having it as generic header section makes more sense.

jmazzitelli added a commit to jmazzitelli/kiali-operator that referenced this issue Sep 13, 2021
side note: this also makes sure the prom config is the same for custom_dashboards.prometheus since that follows the identical config schema as the external_services.prometheus config.

part of: kiali/kiali#4323
jmazzitelli added a commit to jmazzitelli/kiali-operator that referenced this issue Sep 13, 2021
side note: this also makes sure the prom config is the same for custom_dashboards.prometheus since that follows the identical config schema as the external_services.prometheus config.

part of: kiali/kiali#4323
@jmazzitelli
Copy link
Collaborator

draft operator PR adds the following:

spec.
  external_services:
    custom_dashboards:
      prometheus:
        custom_headers: {}
    ...
    prometheus:
      custom_headers: {}

@libesz
Copy link
Contributor Author

libesz commented Sep 14, 2021

@jmazzitelli Sounds good, thanks! We are more than happy to test this change whenever becomes available.

@jmazzitelli
Copy link
Collaborator

I'll be working in this draft server-side PR: #4350

jmazzitelli added a commit to jmazzitelli/kiali that referenced this issue Sep 14, 2021
jmazzitelli added a commit to jmazzitelli/kiali that referenced this issue Sep 14, 2021
jmazzitelli added a commit to jmazzitelli/kiali-operator that referenced this issue Sep 14, 2021
side note: this also makes sure the prom config is the same for custom_dashboards.prometheus since that follows the identical config schema as the external_services.prometheus config.

part of: kiali/kiali#4323
jmazzitelli added a commit to jmazzitelli/kiali that referenced this issue Sep 14, 2021
@jmazzitelli
Copy link
Collaborator

jmazzitelli commented Sep 14, 2021

@libesz can you test this out and see how it works for you?

Here's a quick way to install the Kiali Operator (and a Kiali CR) via helm - you can add your own --set cr.spec.XYZ settings if you want to set additional config. Make sure you set the new custom_headers correctly, obviously. Or you can just create your own Kiali CR after you install the operator (leave out the --set cr.create=true option) - but make sure your Kiali CR overrides spec.deployment.image_name and spec.deployment.image_version using the values you see below to pick up the patched server I uploaded to my quay repo (which will support this new custom_headers feature). Note I have a bogus prometheus url here - just change it to yours.

helm install \
  --create-namespace \
  --namespace kiali-operator \
  --repo https://kiali.org/helm-charts \
  --set cr.create=true \
  --set cr.namespace=istio-system \
  --set cr.spec.deployment.image_name=quay.io/jmazzitelli/kiali \
  --set cr.spec.deployment.image_version=issue4323 \
  --set cr.spec.external_services.prometheus.url=http://127.0.0.1:8088 \
  --set cr.spec.external_services.prometheus.custom_headers.IBMInstanceID=XYZ \
  --set cr.spec.external_services.prometheus.custom_headers.SysdigTeamID=XYZ \
  --set cr.spec.auth.strategy=anonymous \
  --set allowAdHocKialiImage=true \
  --set image.repo=quay.io/jmazzitelli/kiali-operator \
  --set image.tag=issue4323 \
  kiali-operator \
  kiali-operator

@libesz
Copy link
Contributor Author

libesz commented Sep 15, 2021

@jmazzitelli quick update from my side. FYI I am trying to use this change along with some other extraordinary things:

  • the experimental remote cluster feature added by this change.
  • my environment is based on Maistra

Anyways, once every configuration (i.e. prometheus and remote K8S API) seems to be in place, I run into the following crash:

2021-09-15T12:44:56Z INF Kiali: Version: v1.41.0-SNAPSHOT, Commit: 007126f8f28c8e16ffddf404b06b8468dd346edb

2021-09-15T12:44:56Z INF Using authentication strategy [anonymous]
2021-09-15T12:44:56Z WRN Kiali auth strategy is configured for anonymous access - users will not be authenticated.
2021-09-15T12:44:56Z INF Kiali: Console version: 1.40.0-local-43f11aac24550865168cd08dafb727ffca88fdb9
2021-09-15T12:44:56Z INF Server endpoint will start at [:20001/kiali]
2021-09-15T12:44:56Z INF Server endpoint will serve static content from [/opt/kiali/console]
2021-09-15T12:44:56Z INF Starting Metrics Server on [:9090]


2021-09-15T12:45:05Z INF Kiali Cache is active for namespaces [.*]
2021-09-15T12:45:05Z INF [Prom Cache] Enabled
2021-09-15T12:45:05Z INF Waiting for Kiali cache for [namespace: mesh-newtest2] to sync
2021-09-15T12:45:05Z INF Kiali cache for [namespace: mesh-newtest2] started
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x140e3c9]

goroutine 43 [running]:
github.com/kiali/kiali/status.IsMaistra(...)
        /home/jmazzite/source/kiali/kiali/src/github.com/kiali/kiali/status/status.go:114
github.com/kiali/kiali/models.(*Workload).parseObjectMeta(0xc0001c05a0, 0xc0009dd6a0, 0xc0009dd7a8)
        /home/jmazzite/source/kiali/kiali/src/github.com/kiali/kiali/models/workload.go:172 +0x1c9
github.com/kiali/kiali/models.(*Workload).ParseDeployment(0xc0001c05a0, 0xc0009dd680)
        /home/jmazzite/source/kiali/kiali/src/github.com/kiali/kiali/models/workload.go:202 +0x67
github.com/kiali/kiali/business.fetchWorkloads(0xc000337d40, 0xc00059f910, 0xd, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        /home/jmazzite/source/kiali/kiali/src/github.com/kiali/kiali/business/workloads.go:811 +0x340f
github.com/kiali/kiali/business.(*IstioStatusService).getComponentNamespacesWorkloads.func1(0xc000394110, 0xc000337da8, 0xc00059f910, 0xd, 0xc0004016e0, 0xc000401740)
        /home/jmazzite/source/kiali/kiali/src/github.com/kiali/kiali/business/istio_status.go:112 +0x9f
created by github.com/kiali/kiali/business.(*IstioStatusService).getComponentNamespacesWorkloads
        /home/jmazzite/source/kiali/kiali/src/github.com/kiali/kiali/business/istio_status.go:108 +0x4ed

If I am not mistaken, it is here.

@jmazzitelli
Copy link
Collaborator

jmazzitelli commented Sep 15, 2021

Ignore that seg fault - see this for the bug and workaround. We might ship a patch release/fix for this (1.40.1) - haven't decided yet. #4351

Just shutdown your Kiali UI browser tabs when you restart a kiali pod. How is the custom headers working is the important question.

UPDATE: 1.40.1 patch release has just been published. That segfault should not happen anymore.

jmazzitelli added a commit to jmazzitelli/kiali-operator that referenced this issue Sep 15, 2021
side note: this also makes sure the prom config is the same for custom_dashboards.prometheus since that follows the identical config schema as the external_services.prometheus config.

part of: kiali/kiali#4323
@jmazzitelli
Copy link
Collaborator

jmazzitelli commented Sep 15, 2021

I don't have anything special setup (so these headers are essentially a no-op) but here's what Kiali's HTTP request looks like when passing in custom headers to the Prometheus endpoint:

Kiali CR snippet:

spec:
  external_services:
    prometheus:
      custom_headers:
        Mazz-Was-Here: "tada"
        IBMInstanceID: "MyGUID"
        SysdigTeamID: "My Team ID"

HTTP request that the internal client sends to prometheus:

POST /api/v1/query_range HTTP/1.1
Host: localhost:9091
User-Agent: Go-http-client/1.1
Content-Length: 365
Content-Type: application/x-www-form-urlencoded
Ibminstanceid: MyGUID
Mazz-Was-Here: tada
Sysdigteamid: My Team ID
Accept-Encoding: gzip

I will assume case does not matter (looks like the client library is modifying the header names to just be capitalized). Other than that, the custom headers are being sent over the wire to Prometheus.

@libesz
Copy link
Contributor Author

libesz commented Sep 15, 2021

Thanks for the update. I was also able to set up my kiali instance, just stuck with the actual (relevant) metrics push to my monitoring backend from the proxies. Will get back to you once I got working graphs within Kiali.

@libesz
Copy link
Contributor Author

libesz commented Sep 15, 2021

Another update: I am now having a working graph with live data. We can consider the custom_header changes as working. Thanks @jmazzitelli 馃憤

Sprint 63 (v1.41) automation moved this from Backlog to Done Sep 20, 2021
jmazzitelli added a commit to kiali/kiali-operator that referenced this issue Sep 20, 2021
side note: this also makes sure the prom config is the same for custom_dashboards.prometheus since that follows the identical config schema as the external_services.prometheus config.

part of: kiali/kiali#4323
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Triaged Issue added to backlog enhancement This is the preferred way to describe new end-to-end features.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants