Change localhost references to 127.0.0.1

[pingles]

I think it was @roffe that referenced https://pracucci.com/kubernetes-dns-resolution-ndots-options-and-why-it-may-affect-application-performances.html in another issue.

I'd suggest we change all the deployment manifest examples to reference 127.0.0.1 rather than localhost to avoid DNS during health checks.

[cheeseandcereal]

The PR for this change https://github.com/uswitch/kiam/pull/163 caused me a LOT of headache trying to get cert-manager working with kiam. cert-manager (and other TLS certificate tools) don't all handle SAN extension for iPAddress correctly in order to get a working TLS cert for an IP address (namely 127.0.0.1). See HERE for more info, but generally it's probably not best practice to be using TLS certs for IP addresses, even with a private CA.

Are we even sure that DNS lookups for localhost are even going to the kubernetes dns for lookup? In basically every linux configuration I've ever seen, the /etc/hosts file ALWAYS has precedence over a remote DNS lookup/search, and I've seen no evidence to suggest that localhost dns lookups ever leave the local pod in a k8s cluster, which would mean there's no sort of performance penalty for using localhost instead of 127.0.0.1. Reverting to localhost would allow for regular dNSName fields to be used with the cert for the kiam server, which is vastly more supported.

It's especially misleading when the docs point out that 'when deploying to production please ensure you automate certificates using a tool like Vault, cert-manager etc.', when cert-manager literally doesn't work with ip address certificates, and it's not obvious you would ever need to change the local server-address for the liveliness/readiness probes, especially with no good errors even showing up in logs until you enable higher verbosity with GRPC_GO_LOG_VERBOSITY_LEVEL, and even then, it's only 'bad TLS cert' without any other explanation.

For these reasons, I personally think this change should be reverted unless it can be shown that dns lookups for localhost ACTUALLY make network calls (which I'm fairly certain they do not). And if so, it should at least be documented in https://github.com/uswitch/kiam/blob/master/docs/TLS.md that a cert must be generated with proper iPAddress SAN extension support if any ip's are going to be used for any server-address parameters in the deployment manifests.

[hakamadare]

@cheeseandcereal i got bit by the same problem, and so am manually generating my kiam mTLS certs for the time being. once jetstack/cert-manager#1128 is merged and released (hopefully the next minor release of cert-manager) we'll be able to provision IPAddress SANs with cert-manager.

[pingles]

I'll re-open the issue to make sure it's more visible. If someone could pick it up that would be appreciated!

Apologies that it's bitten people- if someone who had problems with cert-manager could maybe add some stuff to the TLS docs about how you configured/requested certs etc. that'd be super helpful.

[hakamadare]

@pingles the problem is that right now afaict it is impossible (at least with the stable release of cert-manager, and it looks like with the current RC as well) to use cert-manager to provision certs that meet kiam's requirements. i was able to work around it by forking cert-manager and implementing something very similar to jetstack/cert-manager#1128, at which point i opened a PR against cert-manager, and then discovered that @lrolaz was already working on the same functionality.

once it's possible to do so, i'd be happy to update the docs!

[sudermanjr]

It is now possible to add IP address fields to the CSR using cert-manager 0.7.0

I was able to make this work with kiam like this:

---
# Create a selfsigned Issuer, which we'll use to create a root CA certificate for kiam
apiVersion: certmanager.k8s.io/v1alpha1
kind: Issuer
metadata:
  name: kiam-selfsigning-issuer
spec:
  selfSigned: {}
---
# Generate a CA Certificate for kiam
apiVersion: certmanager.k8s.io/v1alpha1
kind: Certificate
metadata:
  name: kiam-ca-certificate
spec:
  secretName: kiam-ca-certificate
  isCA: true
  commonName: "kiam-ca"
  issuerRef:
    name: kiam-selfsigning-issuer
---
# Create an Issuer that uses the CA above to issue certs
apiVersion: certmanager.k8s.io/v1alpha1
kind: Issuer
metadata:
  name: kiam-ca-issuer
spec:
  ca:
    secretName: kiam-ca-certificate
---
## create the actual server certifcate with the above CA
apiVersion: certmanager.k8s.io/v1alpha1
kind: Certificate
metadata:
  name: kiam-server
spec:
  secretName: kiam-server-certificate
  commonName: "kiam-server"
  dnsNames:
    - "127.0.0.1:443"
    - "127.0.0.1:9610"
    - "localhost"
  ipAddresses:
    - "127.0.0.1"
  issuerRef:
    name: kiam-ca-issuer
    kind: Issuer
---
apiVersion: certmanager.k8s.io/v1alpha1
kind: Certificate
metadata:
  name: kiam-agent
spec:
  secretName: kiam-agent-certificate
  commonName: "kiam-agent"
  dnsNames:
    - "127.0.0.1:443"
    - "127.0.0.1:9610"
  ipAddresses:
    - "127.0.0.1"
  issuerRef:
    name: kiam-ca-issuer
    kind: Issuer

This creates two secrets which can be used by kiam for the certs.

[bboreham]

In basically every linux configuration I've ever seen, the /etc/hosts file ALWAYS has precedence over a remote DNS lookup/search

Here you can read of examples where this does not happen: https://github.com/golang/go/issues/22846 (broadly, when using containers where the base image does not supply /etc/nsswitch.conf, the default is DNS first)

[jacksontj]

Related, swapping to 127.0.0.1 will also preclude any ipv6 support (for all those that actually do ipv6 :) )

[cheeseandcereal]

I think with the docs updated properly (https://github.com/uswitch/kiam/pull/233) and the cert-manager update which allows ip addresses, this issue can probably be safely closed.

I don't see any reason to switch back to localhost unless people are using ipv6, which isn't really supported by kubernetes anyways.