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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the adapter doesn't support access to prometheus which enabled ssl/tls #52

Closed
huxiaoliang opened this issue Mar 8, 2018 · 5 comments · Fixed by #53
Closed

the adapter doesn't support access to prometheus which enabled ssl/tls #52

huxiaoliang opened this issue Mar 8, 2018 · 5 comments · Fixed by #53

Comments

@huxiaoliang
Copy link
Contributor

I checked the codes, that adapter use http.DefaultClient to access prometheus instance and it doesn't support https client to access ssl/tls enabled prometheus. It is a gap here if prometheus behind a proxy or
prometheus enable ssl/tls according to official guideline Why don't the Prometheus server components support TLS or authentication? Can I add those?

This is a common case that prometheus in production, do we have any plan for this? Thanks in advance.

@huxiaoliang
Copy link
Contributor Author

@luxas @DirectXMan12 hi, is this issue make sense? I will create a PR to fix it, what do you think?

huxiaoliang added a commit to huxiaoliang/k8s-prometheus-adapter that referenced this issue Mar 11, 2018
fix kubernetes-sigs#52

Currently, the adapter use http.DefaultClient to access prometheus
instance and it doesn't support https client to access ssl/tls is
enabled prometheus. It is a gap here if prometheus behind a proxy
or prometheus is enable ssl/tls according to official guideline.
This is a common case that prometheus in production, this patch
will fix the gap described above.
@DirectXMan12
Copy link
Contributor

The issue makes sense. I think we should have something like a kubeconfig (maybe just use the kubeconfig code?), so that we can support other auth mechanisms too, like tokens (because in OpenShift, we deploy with kind-of Kube-native proxy that can use SA tokens, for instance)

@huxiaoliang
Copy link
Contributor Author

@DirectXMan12 kubeconfig supports below authN refer to here for details (same like curl as well) :

case 1: ssl/tls based
-certificate-authority
--client-certificate
--client-key
--insecure-skip-tls-verify

case 2 : simple based
--username
--password

case 3: based
--token

my patch only cover case 1 now, did you mean we should support all above and leverage kubeconfig codes and implementation?

@DirectXMan12
Copy link
Contributor

Yeah, I'm saying it might not be a bad idea to just have a separate argument for prom-kubeconfig, use-prom-auth. By default, if use-prom-auth is set to true, prom-kubeconfig it just uses InClusterConfig, so if you have your Prometheus set up behind an auth proxy that auths against Kubernetes, everything should just work.

@huxiaoliang
Copy link
Contributor Author

@DirectXMan12 I am ok for your postal, append some code snippet for you check in advance. (will make it more modularization later)

	var prometheusClientConfig *rest.Config
	if o.PrometheusUseAuth && len(o.PrometheusKubeConfig) > 0 {
		loadingRules := &clientcmd.ClientConfigLoadingRules{ExplicitPath: o.RemoteKubeConfigFile}
		configOverrides := &clientcmd.ConfigOverrides{ClusterInfo: api.Cluster{Server: baseURL.String()}}
		loader := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, configOverrides)
		prometheusClientConfig, err = loader.ClientConfig()
	} else {
		prometheusClientConfig, err = rest.InClusterConfig()
		prometheusClientConfig.Host = baseURL.String()
	}
	transport, err := rest.TransportFor(prometheusClientConfig)
	if err != nil {
		return fmt.Errorf("failed to create Prometheus client transport %v", err)
	}

	var httpClient *http.Client
	if transport != http.DefaultTransport {
		httpClient = &http.Client{Transport: transport}
		if prometheusClientConfig.Timeout > 0 {
			httpClient.Timeout = prometheusClientConfig.Timeout
		}
	}
	genericPromClient := prom.NewGenericAPIClient(httpClient, baseURL)
  1. if user doesn't specified prom-kubeconfig, use InClusterConfig conf to access to prometheus, but need to overwrite api server address to prometheus url in conf

  2. if user specified prom-kubeconfig, load it and create client by k8s/client-go
    for example, access to an enabled ssl/tls prometheus

apiVersion: v1
clusters:
- cluster:
    insecure-skip-tls-verify: true
    server: https://monitoring-prometheus:9090
  name: mycluster.icp
contexts: []
current-context: ""
kind: Config
preferences: {}
users:
- name: test
  user:
    client-certificate: /tmp/client.crt
    client-key: /tmp/client.key

any comments is welcome, thanks.

huxiaoliang added a commit to huxiaoliang/k8s-prometheus-adapter that referenced this issue Mar 18, 2018
fix kubernetes-sigs#52

Currently, the adapter use http.DefaultClient to access prometheus
instance and it doesn't support https client to access ssl/tls is
enabled prometheus. It is a gap here if prometheus behind a proxy
or prometheus is enable ssl/tls according to official guideline.
This is a common case that prometheus in production, this patch
will fix the gap described above.
huxiaoliang added a commit to huxiaoliang/k8s-prometheus-adapter that referenced this issue Mar 18, 2018
fix kubernetes-sigs#52

Currently, the adapter use http.DefaultClient to access prometheus
instance and it doesn't support https client to access ssl/tls is
enabled prometheus. It is a gap here if prometheus behind a proxy
or prometheus is enable ssl/tls according to official guideline.
This is a common case that prometheus in production, this patch
will fix the gap described above.
huxiaoliang added a commit to huxiaoliang/k8s-prometheus-adapter that referenced this issue Mar 18, 2018
fix kubernetes-sigs#52

Currently, the adapter use http.DefaultClient to access prometheus
instance and it doesn't support https client to access ssl/tls is
enabled prometheus. It is a gap here if prometheus behind a proxy
or prometheus is enable ssl/tls according to official guideline.
This is a common case that prometheus in production, this patch
will fix the gap described above.
huxiaoliang added a commit to huxiaoliang/k8s-prometheus-adapter that referenced this issue Mar 18, 2018
fix kubernetes-sigs#52

Currently, the adapter use http.DefaultClient to access prometheus
instance and it doesn't support https client to access ssl/tls is
enabled prometheus. It is a gap here if prometheus behind a proxy
or prometheus is enable ssl/tls according to official guideline.
This is a common case that prometheus in production, this patch
will fix the gap described above.
huxiaoliang added a commit to huxiaoliang/k8s-prometheus-adapter that referenced this issue Mar 20, 2018
fix kubernetes-sigs#52

Currently, the adapter use http.DefaultClient to access prometheus
instance and it doesn't support https client to access ssl/tls is
enabled prometheus. It is a gap here if prometheus behind a proxy
or prometheus is enable ssl/tls according to official guideline.
This is a common case that prometheus in production, this patch
will fix the gap described above.
DirectXMan12 pushed a commit to huxiaoliang/k8s-prometheus-adapter that referenced this issue Mar 22, 2018
Currently, the adapter uses http.DefaultClient to access Prometheus.
This means that it does support using TLS client certificates, custom
CA certificates, or token-base authentication, which are all common
setups when connecting to Prometheus behind an auth proxy.

This commit adds support for using a separate kubeconfig, or in-cluster
config, to configure auth when connecting to Prometheus.

Fixes kubernetes-sigs#52
DirectXMan12 pushed a commit to huxiaoliang/k8s-prometheus-adapter that referenced this issue Mar 22, 2018
Currently, the adapter uses http.DefaultClient to access Prometheus.
This means that it does support using TLS client certificates, custom
CA certificates, or token-base authentication, which are all common
setups when connecting to Prometheus behind an auth proxy.

This commit adds support for using a separate kubeconfig, or in-cluster
config, to configure auth when connecting to Prometheus.

Fixes kubernetes-sigs#52

[directxman12: cleanups, spelling corrections, and slight refactor]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants