Skip to content
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

TLS and WebSocket support in NodeRED instance Ingress (the one created by kubernetes.js) #83

Closed
adaptivegarage opened this issue May 2, 2023 · 9 comments · Fixed by #131

Comments

@adaptivegarage
Copy link

Hi,
is there any reason that I might be overlooking in kubernetes.js for not to support TLS and WebSocket in ingressTemplate and createIngress?

In order to get the NodeRED instances working on a K8s cluster one would need to adjust the two mentioned somehow along these lines:

const ingressTemplate = {
    apiVersion: 'networking.k8s.io/v1',
    kind: 'Ingress',
    metadata: {
        // name: "k8s-client-test-ingress",
        // namespace: 'flowforge',
        annotations: {
            // ***** Added:
            // nginx.org/websocket-services: "k8s-client-test-service"
        }
    },
    spec: {
        rules: [
            {
                // host: "k8s-client-test" + "." + "ubuntu.local",
                http: {
                    paths: [
                        {
                            pathType: 'Prefix',
                            path: '/',
                            backend: {
                                service: {
                                    // name: 'k8s-client-test-service',
                                    port: { number: 1880 }
                                }
                            }
                        }
                    ]
                }
            }
        ],
        // ***** Added:
        tls: [
            {
                hosts: [
                    // "k8s-client-test" + "." + "ubuntu.local"
                ]
            }
        ]
    }
}
const createIngress = async (project, options) => {
    const prefix = project.safeName.match(/^[0-9]/) ? 'srv-' : ''

    const localIngress = JSON.parse(JSON.stringify(ingressTemplate))
    localIngress.metadata.name = project.safeName
    // ***** Added:
    localIngress.metadata.annotations['nginx.org/websocket-services'] = `${prefix}${project.safeName}`
    localIngress.spec.rules[0].host = project.safeName + '.' + options.domain
    localIngress.spec.rules[0].http.paths[0].backend.service.name = `${prefix}${project.safeName}`
    // ***** Added:
    localIngress.spec.tls[0].hosts.push(project.safeName + '.' + options.domain)

    return localIngress
}
@MarianRaphael MarianRaphael moved this to 💬 Support cases & under review in ☁️ Product Planning May 4, 2023
@hardillb
Copy link
Contributor

hardillb commented May 4, 2023

Hello,

We haven't had a need for the either of these just yet with the configurations we've been running with.

All the ingress controllers have automatically worked with WebSockets without the need for an annotation.

And for the TLS we have configured the controller to redirect all traffic to the https port and we've been doing TLS termination at the Ingress Controller with a wildcard certificate, so no need for the tls section or to use things like CertManager just yet.

I hope to merge some changes in the next release that will allow the specifying of extra annotations to make ingress objects which would include the first point.

I'll have a think about the options for setting tls

@adaptivegarage
Copy link
Author

Hello,

We haven't had a need for the either of these just yet with the configurations we've been running with.

All the ingress controllers have automatically worked with WebSockets without the need for an annotation.

And for the TLS we have configured the controller to redirect all traffic to the https port and we've been doing TLS termination at the Ingress Controller with a wildcard certificate, so no need for the tls section or to use things like CertManager just yet.

I hope to merge some changes in the next release that will allow the specifying of extra annotations to make ingress objects which would include the first point.

I'll have a think about the options for setting tls

Hi,
thanks for getting back to me.

WebSocket: I am not familiar with all Ingress controllers but Nginx from Nginx (CE edition). And this one requires the annotation to be present in order to support the WS connections. Without the annotation the NodeRED applications complain on failed WS connections.

TLS: I understand that it may work in the scenario you mention. However, in our environment (and I believe we wouldn't be alone) the TLS section needs to be explicitly configured for every Ingress in every namespace. We cannot reconfigure the environment to work as you describe.

Could I perhaps hope that you might consider these suggestions in any near future, or would you appreciate a pull request, that I would take care of, or would you rather suggest that we make our own patch?

Thanks,
regards
Roman

@dfulgham
Copy link
Contributor

dfulgham commented Jul 4, 2023

I was having this same issue with websockets, and by adding the annotation for nginx.org/websocket-services into the ingress for the node-red service fixed the issue. So I think there is something needed here.

@hardillb
Copy link
Contributor

hardillb commented Jul 4, 2023

@dfulgham You can pass additional ingress annotations as part of the helm install by setting the value ingress.annotations

These should be added to all the instance ingress objects that are created.

@adaptivegarage
Copy link
Author

@dfulgham You can pass additional ingress annotations as part of the helm install by setting the value ingress.annotations

These should be added to all the instance ingress objects that are created.

Hi, I would like to refresh and bring to your attention that my comment (and I believe the one from @dfulgham too) referred to the NodeRED instance ingress created by kubernetes.js, not the Flowforge ingress created by helmchart.

@adaptivegarage adaptivegarage changed the title TLS and WebSocket support in Ingress TLS and WebSocket support in NodeRED instance Ingress (the one created by kubernetes.js) Jul 4, 2023
@hardillb
Copy link
Contributor

hardillb commented Jul 4, 2023

@adaptivegarage those annoations also get added to both, they are added to the ingress template for the forge app here and get added as an environment variable here which is then picked up by the kubernetes driver here

@adaptivegarage
Copy link
Author

@adaptivegarage those annoations also get added to both, they are added to the ingress template for the forge app here and get added as an environment variable here which is then picked up by the kubernetes driver here

Oh, I see. Thanks a lot, I'll give it a try.
My comment regarding the TLS section is, however, still valid, is that right?

@hardillb
Copy link
Contributor

hardillb commented Jul 4, 2023

Yes, TLS entries do still need looking at.

There will be some overlap with some other work we need to look at soon to potentially support custom hostname/domains for instances.

hardillb added a commit that referenced this issue Jul 12, 2023
part of #83

Basic TLS ingress entry
@hardillb
Copy link
Contributor

hardillb commented Jan 2, 2024

Cert-Manager TLS support to be added via #131

@hardillb hardillb linked a pull request Jan 2, 2024 that will close this issue
11 tasks
@github-project-automation github-project-automation bot moved this from 💬 Support cases & under review to Closed / Done in ☁️ Product Planning Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed / Done
Development

Successfully merging a pull request may close this issue.

3 participants