Skip to content

Add skip verification #6

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add skip verification #6

wants to merge 1 commit into from

Conversation

kerneltime
Copy link
Contributor

Not detected by mc as it depends on the customer's environment and prometheus may have the root CA while mc does not.
Add documentation for skip verification.
I am ok with skipping this as we are capturing prometheus documentation. Maybe we can add an option to mc generate command.

Not detected by mc as it depends on the customer's environment and prometheus may have the root CA while mc does not.
Add documentation for skip verification.
I am ok with skipping this as we are capturing prometheus documentation. Maybe we can add an option to mc generate command.
@kerneltime kerneltime requested a review from ravindk89 March 3, 2021 16:44
@@ -62,6 +62,20 @@ The command returns a bearer token similar to the following:
static_configs:
- targets: ['HOSTNAME:9000']

If using a self signed certificate, add the following to skip verification.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kerneltime just to make sure we're clear here - this is if the MinIO Tenant is using self-signed certificates (i.e. auto-cert)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct @ravindk89

Comment on lines +76 to +77
tls_config:
insecure_skip_verify: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's possible that

Suggested change
tls_config:
insecure_skip_verify: true
tls_config:
ca_file: /var/run/secrets/kubernetes.io/serviceaccount/ca.crt

This would be better instead?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes that auto-cert was enabled and used, right? There's also the option of the user having deployed with externalCertSecret so there are an unknown number of CAs which might be included.

Rather I'd like to just make a minor tweak to this stating that users can either:

  • Configure prometheus to skip TLS verification, or
  • Pass the CA used by the MinIO Tenant (given the hostname specified to static_configs.

@harshavardhana I think your suggestion only works if the Prometheus instance is also deployed in K8s and is using the K8s TLS API, so we can be generic ("Specify the path to the CA used to sign the MinIO Tenant. For MinIO Tenants deployed with autoCert enabled, Kubernetes cluster CA (Some link to docs here). If you deployed the Tenant using custom TLS certificates, specify the CA used to sign the certificate associated to the static_configs[0].target hostname.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this scenario we can expect that its deployed in k8s, since its a vsphere docs - I don't know if we need to talk about baremetal configs here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah fair point - I made a suggested change above to integrate your suggestion, as I'd prefer that over wholesale insecure TLS verification. Given that assumption, using --insecure on our end shouldn't be strictly required either...maybe the vsphere docs should include a "Connect to your Cluster" page that includes configuring mc and adding the Kubernetes CA to the system trust store. Would simplify things, I think.

@@ -62,6 +62,20 @@ The command returns a bearer token similar to the following:
static_configs:
- targets: ['HOSTNAME:9000']

If using a self signed certificate, add the following to skip verification.
Copy link
Collaborator

@ravindk89 ravindk89 Mar 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If using a self signed certificate, add the following to skip verification.
Prometheus by default validates the TLS certificate presented by the MinIO
Tenant. You can modify the `scrape_config` to explicitly point the Prometheus
service at the Certificate Authority (CA) to use for validating the MinIO Tenant
certificate, *or* disable TLS verification entirely.
MinIO Pods use SNI to determine which TLS certificate to respond with during the
TLS handshake. Specifically, the ``static_configs.targets`` hostname determines
the TLS certificate the MinIO Tenant responds with:
- If using the MinIO Kubernetes Service DNS name or Cluster IP address as the
scrape target, specify the path to the Kubernetes cluster CA:
.. code-block:: shell
:class: copyable
scrape_config:
- job_name: minio-job
...
tls_config:
ca_file: /var/run/secrets/kubernetes.io/serviceaccount/ca.crt
- If using a DNS name associated to a custom certificate configured
using the :kubeconf:`spec.externalCertSecret` configuration setting,
specify the path to the CA used to sign that certificate:
.. code-block:: shell
:class: copyable
scrape_config:
- job_name: minio-job
...
tls_config:
ca_file: /path/to/minio-ca.crt
- To disable TLS verification for the MinIO scrape job, specify
``tls_config.insecure_skip_verify: true``
.. code-block:: shell
:class: copyable
scrape_config:
- job_name: minio-job
...
tls_config:
insecure_skip_verify: true
Disabling TLS verification may increase the risk of certain attacks or
exploits associated to unverified TLS connections.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @kerneltime @harshavardhana

This might also be a better fit under step 2, since that's the part associated to the Prometheus config itself.

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 this pull request may close these issues.

3 participants