canonical / nginx-ingress-integrator-operator

nginx-ingress-integrator-operator - charm repository.
Apache License 2.0
1 stars 8 forks source link

Adds ExternalName Service type option #96

Closed claudiubelu closed 11 months ago

claudiubelu commented 1 year ago

Kubernetes has a Service type ExternalName, which allows the Cluster DNS Service to return a CNAME record to the set externalName. This can be useful in certain situations.

The nginx-ingress-integrator charm does not have any support for this at this moment.

This commit adds the external-name relation field and config option, which, if set, will cause the nginx-ingress-integrator charm to create a Kubernetes ExternalName Service with the externalName set instead of a ClusterIP Service.

Implements: https://github.com/canonical/nginx-ingress-integrator-operator/issues/93

cbartz commented 1 year ago

/canonical/self-hosted-runners/run-workflows d560067e7bf15dae02b4d5638eba3900990e51a4

jdkandersson commented 1 year ago

@weiiwang01 could you please take a look at this

weiiwang01 commented 1 year ago

@weiiwang01 could you please take a look at this

@claudiubelu Thanks for contributing, and sorry for the delay, I've been doing some experiments related to this. I have some concerns regarding the use of an external name with a HTTPS backend, as it seems to be a major use case. By default, Nginx ingress does not validate the certificate of the HTTPS backend, which could pose significant security risks. And make it worse, configuring Nginx ingress to work with a HTTPS backend and ensuring that it validates the backend is not straightforward at all.

Here's an example Kubernetes manifest file using an external name to connect to a website with a self-signed certificate. And the Nginx ingress doesn't complain at all.

# curl -H 'Host: self-signed.badssl.com' http://127.0.0.1 # no complaints
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: example-ingress
  annotations:
    nginx.ingress.kubernetes.io/backend-protocol: HTTPS
spec:
  ingressClassName: public
  rules:
  - host: "self-signed.badssl.com"
    http:
      paths:
      - pathType: Prefix
        path: "/"
        backend:
          service:
            name: external-name-example
            port:
              number: 443
---
apiVersion: v1
kind: Service
metadata:
  name: external-name-example
  labels:
    endpoint-group: example
spec:
  type: ExternalName
  externalName: self-signed.badssl.com
cbartz commented 11 months ago

/canonical/self-hosted-runners/run-workflows 04b5d490c8c93b26d1c72afc987be10a974e4098

github-actions[bot] commented 11 months ago

Lint checks failed for 04b5d490c8c93b26d1c72afc987be10a974e4098

```

lib/charms/nginx_ingress_integrator/v0/nginx_route.py:407: error: No overload variant of "get" of "dict" matches argument type "str" [call-overload] lib/charms/nginx_ingress_integrator/v0/nginx_route.py:407: note: Possible overload variants: lib/charms/nginx_ingress_integrator/v0/nginx_route.py:407: note: def get(self, Never, /) -> None lib/charms/nginx_ingress_integrator/v0/nginx_route.py:407: note: def get(self, Never, Never, /) -> Never lib/charms/nginx_ingress_integrator/v0/nginx_route.py:407: note: def [_T] get(self, Never, _T, /) -> _T Found 1 error in 1 file (checked 15 source files)

github-actions[bot] commented 11 months ago

Test coverage for 04b5d490c8c93b26d1c72afc987be10a974e4098

Name             Stmts   Miss Branch BrPart  Cover   Missing
------------------------------------------------------------
src/charm.py       480     26    188      6    94%   430->432, 560, 645-647, 651-656, 664-665, 673-674, 709-711, 812-821, 835->860, 1095-1099, 1155, 1198->1234, 1216
src/helpers.py      10      0      2      0   100%
------------------------------------------------------------
TOTAL              490     26    190      6    94%

Static code analysis report

Run started:2023-11-14 12:00:40.863146

Test results:
    No issues identified.

Code scanned:
    Total lines of code: 3918
    Total lines skipped (#nosec): 2
    Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
    Total issues (by severity):
        Undefined: 0
        Low: 0
        Medium: 0
        High: 0
    Total issues (by confidence):
        Undefined: 0
        Low: 0
        Medium: 0
        High: 0
Files skipped (0):
weiiwang01 commented 11 months ago

@claudiubelu I think for now we should close this pull request considering the current security concerns. I think this is more of an issue of the Kubernetes Nginx ingress controller. But we should definitely revisit this after we have a solution for enabling the HTTP security.