Add NoAuthLocations and default it to "/.well-known/acme-challenge"

[alvaroaleman]

What this PR does / why we need it:

Allows to configure locations that should not be authenticated, very usefull e.G. for Let's encrypt/ACME.

Basically the same as #2203 but for authentication instead of TLS upgrading.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

[codecov-io]

Codecov Report

Merging #2243 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2243      +/-   ##
==========================================
+ Coverage    37.3%   37.31%   +0.01%     
==========================================
  Files          71       71              
  Lines        5042     5043       +1     
==========================================
+ Hits         1881     1882       +1     
  Misses       2876     2876              
  Partials      285      285

Impacted Files	Coverage Δ
internal/file/bindata.go	53.93% <ø> (ø)	⬆️
internal/ingress/controller/config/config.go	98.18% <100%> (+0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b4d7f2...d4ac563. Read the comment docs.

[aledbf]

@alvaroaleman please add e2e tests. Like two ingresses, one with basic authentication in the path / and another with /noauth checking the new settings allows access to /noauth.

[alvaroaleman]

@aledbf Added e2e tests. Travis currently fails because #2254 introduced a change that redeploys the ingress controller but doesn't properly wait afterwards

[aledbf]

@alvaroaleman please rebase and we are ready to merge

[aledbf]

/approve

[alvaroaleman]

@aledbf Done

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, alvaroaleman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [aledbf]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[aledbf]

@alvaroaleman thanks!

[Justkant]

Hi, I'm wondering if there is any security risks with this feature.
I created #2214 earlier about the feature, I like and need it. (Thanks btw)
Could a security warning for people using the ingress with auth be useful because of the defaults ? (If any risks)

[Jokero]

@alvaroaleman Does it actually work? :) So I'm testing in minikube with last nginx-ingress version (0.13.0):

Ingress:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: ingress
  annotations:
    kubernetes.io/ingress.class: "nginx"
    nginx.ingress.kubernetes.io/auth-type: basic
    nginx.ingress.kubernetes.io/auth-secret: basic-auth
spec:
  rules:
    - host: minikube.local
      http:
        paths:
          - path: /api
            backend:
              serviceName: api
              servicePort: 3000
          - path: /
            backend:
              serviceName: web-client
              servicePort: 8080

And I always get "Authentication required" window, even if I visit /.well-known/acme-challenge page.
nginx-ingress is installed using helm chart (https://github.com/kubernetes/charts/tree/master/stable/nginx-ingress) with overridden controller.image.tag version (set to 0.13.0)

[Jokero]

And what's inside nginx-ingress-controller container:

root@nginx-ingress-controller-857dc686c7-pr4rg:/# cat /etc/nginx/template/nginx.tmpl | grep NoAuthLocations --context 3
            }
            {{ end }}

            {{ if not (isLocationInLocationList $location $all.Cfg.NoAuthLocations) }}
            {{ if $authPath }}
            # this location requires authentication
            auth_request        {{ $authPath }};
root@nginx-ingress-controller-857dc686c7-pr4rg:/# cat /etc/nginx/nginx.conf | grep auth_request
root@nginx-ingress-controller-857dc686c7-pr4rg:/# 

[alvaroaleman]

@Jokero This only works if you have a path in your ingress definition that starts with /.well-known/acme-challenge

@Justkant I was thinking about that as well, but IMHO this is not an issue since as @Jokero noticed it will only kick in when you create a path starting with /.well-known/acme-challenge in your ingress - WDYT?

[aledbf]

@Jokero please add an ingress with the expected path. If you have / or /api in the nginx.conf you will receive 401

[Jokero]

If I am right, it's not possible right now to use this solution with for example grafana and prometheus charts which has an option to create ingress. And this is my case actually.
All options are specified in values.yaml:

ingress:
  enabled: false
  annotations: {}
    # kubernetes.io/ingress.class: nginx
    # kubernetes.io/tls-acme: "true"
  path: /
  hosts:
    - chart-example.local
  tls: []
  #  - secretName: chart-example-tls
  #    hosts:
  #      - chart-example.local

[aledbf]

@Jokero if you enable kubernetes.io/tls-acme: "true" and have kube-lego or cert-manager the path /.well-known/acme-challenge will be there (is added by one of the cert controllers)

[Justkant]

@alvaroaleman Looks good if it only bypass the auth when the path is declared (done by cert-manager w/ ingress-shim)
Thanks for the feature ✌️

[Jokero]

@aledbf Uh, I did it. Indeed, it works in GKE. Tried with cert-manager. Not sure only will it work in minikube or not (due to cert-manager), but not a big problem. Thanks a lot!

[ermik]

@alvaroaleman do I understand correctly, this limited to a single path: /.well-known/acme-challenge? Why?

[alvaroaleman]

@ermik Please do take the time to read the docs that were merged as part of this PR.

[ermik]

@alvaroaleman sorry, and thanks, couldn't find where to find them. 😃 Do I understand correctly these locations can only be explicitly defined, no regex or wildcard support?