F5Networks / k8s-bigip-ctlr

Repository for F5 Container Ingress Services for Kubernetes & OpenShift.
Apache License 2.0
357 stars 195 forks source link

RFE: Support for automatic creation of https monitors with SNI #3236

Closed alonsocamaro closed 8 months ago

alonsocamaro commented 8 months ago

Title

Support for automatic creation of https monitors with SNI

Description

At time of this writing it is required for customers to create https monitors in order to health check an application that is hosting several domain names. This is the typical case of sending traffic to an ingress controller (2-tier deployments).

This requires:

as follows:

ltm profile server-ssl serverssl.www.mc-nginx.com {
    app-service none
    defaults-from serverssl
    server-name www.mc-nginx.com
}
ltm monitor https www.mc-nginx.com {
    defaults-from https
    send "GET / HTTP/1.1\r\nHost: www.mc-nginx.com\r\nConnection: close\r\n\r\n"
    ssl-profile /Common/serverssl.www.mc-nginx.com
}

Actual Problem

Customers cannot automate the monitoring for this typical use case because the monitors need to be pre-created

Solution Proposed

When indicating an https monitor in the manifest, this should be by default SNI enabled

Additional context

In order for SNI enabled monitors to work in BIG-IP Classic it is necessary to enable the following DB key

tmsh modify sys db bigd.tmm value enable

See https://my.f5.com/manage/s/article/K85210713 for more details. Given that in-TMM monitoring has been affected by issues in the past and has an impact in data plane's performance. I would recommend that https monitors with SNI (which require in-TMM right now) have a different name than https in the manifests, for example: https-sni

alonsocamaro commented 8 months ago

Note that one of the TLS server profiles of the monitors will need to have ticked the option

    sni-default true

otherwise AS3 complains (I believe it shouldn´t in this case), maybe an AS3 bug should be raised?

"2024/01/17 14:26:38 [ERROR] [AS3] Raw response from Big-IP: map[code:422 declaration:HTML Tag-like Content in the Request URL/Body results:[map[code:422 host:localhost message:declaration failed response:0107180b:3: Virtual Server /mc-twotier/Shared/crd_10_1_10_115_443 has more than one clientssl/serverssl profile but none of them is default for SNI. runTime:3097 tenant:mc-twotier]]]"

lavanya-f5 commented 8 months ago

@alonsocamaro This may not be AS3 issue, but requirement from bigip to support multiple profiles on same virtual. See https://community.f5.com/t5/technical-forum/unable-to-assign-different-certificate-to-same-virtual-server/td-p/215962 . Before AS3 3.44 it used to set default sni true on first cert by default.This behavior is changed with fix https://github.com/F5Networks/f5-appsvcs-extension/issues/274. So CIS now sets default_sni to true on first certificate from 2.13 version https://github.com/F5Networks/k8s-bigip-ctlr/blob/master/docs/upgradeProcess.md#upgrading-from-2121-to-2130

alonsocamaro commented 8 months ago

@lavanya-f5 please note that in this case the profile is attached to the monitor, not to the virtual server. Anyway please note that the important topic of the issue is instead allowing to have HTTPS with SNI support

alonsocamaro commented 8 months ago

Given that at present we don´t create automatically TLS profiles even for the data plane (see TLSProfile CRD) it seems that for now we should just:

Expose the ssl-profile in the monitor section (it is actually exposed in NextGen Routes with the field sslProfile) in which case the customer will be left to do:

It would be nice if in CIS 3.0 it is possible to create SSL profiles with a manifests so DevOps can fully configure a service without having to do a separate automation or manually creating these

alonsocamaro commented 8 months ago

Additionally, at present the only the Ingress resource supports specifying a ssl profile in the monitor, see:

https://clouddocs.f5.com/containers/latest/userguide/ingress.html

That is, the Route resource doesn´t support it as of yet. This should be implemented as well to have consistent health checking across the different resource types.

lavanya-f5 commented 8 months ago

@alonsocamaro dev build with support for setting sslProfile in https monitor with nextgen routes and virtualserver quay.io/f5networks/k8s-bigip-ctlr-devel:b447bdff59b9194a0ee9e1d8dba595d85965e726.

alonsocamaro commented 8 months ago

I´ve found the following when not using the the attribute:

2024/02/08 18:49:48 [ERROR] [AS3] Raw response from Big-IP: map[code:422 declarationFullId: errors:[/sc-twotier/Shared/router_default_route_a_openshift_ingress_www_sc_twotier_com_https_443/clientTLS: should NOT h
ave fewer than 1 properties] message:declaration is invalid] 

while using AS3 3.45.0

The object referenced is the monitor itself:

                "router_default_route_a_openshift_ingress_www_sc_twotier_com_https_443": {
                    "adaptive": false,
                    "class": "Monitor",
                    "clientTLS": {},
                    "monitorType": "https",
                    "receive": "^HTTP/1.1 200",
                    "send": "GET / HTTP/1.1\r\nHost: www.sc-twotier.com\r\nConnection: close\r\n\r\n",
                    "targetAddress": ""
                },

on the other hand when defining sslProfile attribute with a value it works like a charm

I´m using the CRDs from your private repository https://raw.githubusercontent.com/lavanya-f5/k8s-bigip-ctlr/6cf56bc014c65ca81a51ff5ef98f392661aa1474/docs/config_examples/customResourceDefinitions/incubator/customresourcedefinitions.yml

lavanya-f5 commented 8 months ago

@alonsocamaro dev build with fix for handling default value quay.io/f5networks/k8s-bigip-ctlr-devel:343cf970ef31d6e2de3b1b059e5e8c7a3f379098

alonsocamaro commented 8 months ago

It worked like a charm. Closing this issue.

Thanks!