Use the ingress domain for the tls secret #35

LucasRoesler · 2020-10-31T15:58:51Z

Description

Change the name of the TLS secret to use the domain name. This allows
multiple functions to use the same secret instead of creating multiple
secrets. This reduces the complexity for REST style APIs that need
multiple paths corresponding to multiple Functions and FunctionIngress
on the same domain.

Motivation and Context

I have raised an issue to propose this change (required)

Closes #34

How Has This Been Tested?

new unit test

Manual testing locally with

ingress-operator using theaxer/ingress-operator:latest-35
latest nginx-ingress
latest cert-manager

A secret with the expected name nodeinfo-tls.myfaas.club-cert-9q9wm is created

$ kubectl get secret -n openfaas
NAME                                  TYPE                                  DATA   AGE
basic-auth                            Opaque                                2      12m
default-token-df84n                   kubernetes.io/service-account-token   3      19m
ingress-operator-token-m87z4          kubernetes.io/service-account-token   3      12m
nodeinfo-tls.myfaas.club-cert-9q9wm   Opaque                                1      60s
openfaas-controller-token-fhcsv       kubernetes.io/service-account-token   3      12m
openfaas-prometheus-token-7gd24       kubernetes.io/service-account-token   3      12m
sh.helm.release.v1.openfaas.v1        helm.sh/release.v1                    1      12m

$ kubectl get ing nodeinfo-tls -oyaml
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    cert-manager.io/issuer: letsencrypt-staging
    com.openfaas.spec: '{"metadata":{"name":"nodeinfo-tls","namespace":"openfaas","selfLink":"/apis/openfaas.com/v1alpha2/namespaces/openfaas/functioningresses/nodeinfo-tls","uid":"0e8c9e99-69b9-4ef5-bf8f-b0d421be2104","resourceVersion":"5069","generation":1,"creationTimestamp":"2020-10-31T16:56:16Z","annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"openfaas.com/v1alpha2\",\"kind\":\"FunctionIngress\",\"metadata\":{\"annotations\":{},\"name\":\"nodeinfo-tls\",\"namespace\":\"openfaas\"},\"spec\":{\"domain\":\"nodeinfo-tls.myfaas.club\",\"function\":\"nodeinfo\",\"ingressType\":\"nginx\",\"path\":\"/\",\"tls\":{\"enabled\":true,\"issuerRef\":{\"kind\":\"Issuer\",\"name\":\"letsencrypt-staging\"}}}}\n"},"managedFields":[{"manager":"kubectl","operation":"Update","apiVersion":"openfaas.com/v1alpha2","time":"2020-10-31T16:56:16Z","fieldsType":"FieldsV1","fieldsV1":{"f:metadata":{"f:annotations":{".":{},"f:kubectl.kubernetes.io/last-applied-configuration":{}}},"f:spec":{".":{},"f:domain":{},"f:function":{},"f:ingressType":{},"f:path":{},"f:tls":{".":{},"f:enabled":{},"f:issuerRef":{".":{},"f:kind":{},"f:name":{}}}}}}]},"spec":{"domain":"nodeinfo-tls.myfaas.club","function":"nodeinfo","path":"/","ingressType":"nginx","tls":{"enabled":true,"issuerRef":{"name":"letsencrypt-staging","kind":"Issuer"}}}}'
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"openfaas.com/v1alpha2","kind":"FunctionIngress","metadata":{"annotations":{},"name":"nodeinfo-tls","namespace":"openfaas"},"spec":{"domain":"nodeinfo-tls.myfaas.club","function":"nodeinfo","ingressType":"nginx","path":"/","tls":{"enabled":true,"issuerRef":{"kind":"Issuer","name":"letsencrypt-staging"}}}}
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/rewrite-target: /function/nodeinfo/$1
  creationTimestamp: "2020-10-31T16:56:16Z"
  generation: 1
    manager: ingress-operator
    operation: Update
    time: "2020-10-31T16:56:16Z"
    manager: nginx-ingress-controller
    operation: Update
    time: "2020-10-31T16:56:55Z"
  name: nodeinfo-tls
  namespace: openfaas
  ownerReferences:
  - apiVersion: openfaas.com/v1alpha2
    blockOwnerDeletion: true
    controller: true
    kind: FunctionIngress
    name: nodeinfo-tls
    uid: 0e8c9e99-69b9-4ef5-bf8f-b0d421be2104
  resourceVersion: "5184"
  selfLink: /apis/extensions/v1beta1/namespaces/openfaas/ingresses/nodeinfo-tls
  uid: ebaa395f-6047-4cac-83a7-2f1a09b54d34
spec:
  rules:
  - host: nodeinfo-tls.myfaas.club
    http:
      paths:
      - backend:
          serviceName: gateway
          servicePort: 8080
        path: /
        pathType: ImplementationSpecific
  tls:
  - hosts:
    - nodeinfo-tls.myfaas.club
    secretName: nodeinfo-tls.myfaas.club-cert
status:
  loadBalancer:
    ingress:
    - ip: 172.20.0.2

kubectl get functioningress nodeinfo-tls  -oyaml
apiVersion: openfaas.com/v1alpha2
kind: FunctionIngress
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"openfaas.com/v1alpha2","kind":"FunctionIngress","metadata":{"annotations":{},"name":"nodeinfo-tls","namespace":"openfaas"},"spec":{"domain":"nodeinfo-tls.myfaas.club","function":"nodeinfo","ingressType":"nginx","path":"/","tls":{"enabled":true,"issuerRef":{"kind":"Issuer","name":"letsencrypt-staging"}}}}
  creationTimestamp: "2020-10-31T16:56:16Z"
  generation: 1
  name: nodeinfo-tls
  namespace: openfaas
  resourceVersion: "5069"
  selfLink: /apis/openfaas.com/v1alpha2/namespaces/openfaas/functioningresses/nodeinfo-tls
  uid: 0e8c9e99-69b9-4ef5-bf8f-b0d421be2104
spec:
  domain: nodeinfo-tls.myfaas.club
  function: nodeinfo
  ingressType: nginx
  path: /
  tls:
    enabled: true
    issuerRef:
      kind: Issuer
      name: letsencrypt-staging

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Impact to existing users

Existing FunctionIngress will recreate new secrets when this update is installed. Users may want to remove the existing secrets.

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I've read the CONTRIBUTION guide
I have signed-off my commits with git commit -s
I have added tests to cover my changes.
All new and existing tests passed.

alexellis · 2020-10-31T17:26:04Z

Hi Lucas, thanks for taking this up. Could you deploy a second FunctionIngress on the same domain, but using a different path and function to show that only one certificate is obtained?

LucasRoesler · 2020-10-31T19:10:08Z

Hi Lucas, thanks for taking this up. Could you deploy a second FunctionIngress on the same domain, but using a different path and function to show that only one certificate is obtained?

$ kubectl get ing -A
NAMESPACE   NAME                CLASS    HOSTS                      ADDRESS      PORTS     AGE
openfaas    nodeinfo-tls        <none>   nodeinfo-tls.myfaas.club   172.20.0.2   80, 443   132m
openfaas    nodeinfo-tls-auth   <none>   nodeinfo-tls.myfaas.club   172.20.0.2   80, 443   14s
$ kubectl get secrets -n openfaas
NAME                                  TYPE                                  DATA   AGE
basic-auth                            Opaque                                2      137m
default-token-df84n                   kubernetes.io/service-account-token   3      144m
ingress-operator-token-m87z4          kubernetes.io/service-account-token   3      137m
nodeinfo-tls.myfaas.club-cert-9q9wm   Opaque                                1      126m
openfaas-controller-token-fhcsv       kubernetes.io/service-account-token   3      137m
openfaas-prometheus-token-7gd24       kubernetes.io/service-account-token   3      137m
sh.helm.release.v1.openfaas.v1        helm.sh/release.v1                    1      137m
$ kubectl get certificate -A
NAMESPACE   NAME                            READY   SECRET                          AGE
openfaas    nodeinfo-tls.myfaas.club-cert   False   nodeinfo-tls.myfaas.club-cert   126m

alexellis · 2020-10-31T19:46:52Z

Do you have more than one FunctionIngress records with differing paths, but the same domain? I can just see the NodeInfo one

LucasRoesler · 2020-10-31T19:51:31Z

Do you have more than one FunctionIngress records with differing paths, but the same domain? I can just see the NodeInfo one

One is / and the other is /auth, both on the same domain

Waterdrips · 2020-10-31T22:29:06Z

pkg/controller/controller_test.go

+			expected: []v1beta1.IngressTLS{},
+		},
+		{
+			name: "tls enabled creates TLS object with corret host and secret with matching the host",


corret --> correct

**What** - Change the name of the TLS secret to use the domain name. This allows multiple functions to use the same secret instead of creating multiple secrets. This reduces the complexity for REST style APIs that need multiple paths corresponding to multiple Funcions and FunctionIngress on the same domain. Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>

alexellis · 2020-11-01T11:24:20Z

You are right @LucasRoesler. Thanks

LucasRoesler marked this pull request as ready for review October 31, 2020 17:05

Waterdrips approved these changes Oct 31, 2020

View reviewed changes

LucasRoesler force-pushed the fix-34-use-domain-for-tls-secret branch from 23b83be to 8e83a73 Compare November 1, 2020 10:27

alexellis merged commit df2332d into openfaas:master Nov 1, 2020

alexellis mentioned this pull request Nov 1, 2020

Make Path optional #38

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the ingress domain for the tls secret #35

Use the ingress domain for the tls secret #35

LucasRoesler commented Oct 31, 2020 •

edited

Loading

alexellis commented Oct 31, 2020

LucasRoesler commented Oct 31, 2020

alexellis commented Oct 31, 2020

LucasRoesler commented Oct 31, 2020

Waterdrips Oct 31, 2020

LucasRoesler Nov 1, 2020

alexellis commented Nov 1, 2020

Use the ingress domain for the tls secret #35

Use the ingress domain for the tls secret #35

Conversation

LucasRoesler commented Oct 31, 2020 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Impact to existing users

Checklist:

alexellis commented Oct 31, 2020

LucasRoesler commented Oct 31, 2020

alexellis commented Oct 31, 2020

LucasRoesler commented Oct 31, 2020

Waterdrips Oct 31, 2020

Choose a reason for hiding this comment

LucasRoesler Nov 1, 2020

Choose a reason for hiding this comment

alexellis commented Nov 1, 2020

LucasRoesler commented Oct 31, 2020 •

edited

Loading