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

Kubernetes Extension Should be Aware of Standard TLS Properties #29999

Closed
csviri opened this issue Dec 21, 2022 · 7 comments · Fixed by #30302
Closed

Kubernetes Extension Should be Aware of Standard TLS Properties #29999

csviri opened this issue Dec 21, 2022 · 7 comments · Fixed by #30302
Assignees
Labels
Milestone

Comments

@csviri
Copy link

csviri commented Dec 21, 2022

Description

When generating Kubernetes resources using the Kubernetes extension
I would expect that when set an the tls port, for example: quarkus.http.ssl-port=443

This will be reflected in the generated Deployment resource, thus an additional port will be added to the container definition:

  - containerPort: 443
    name: tls
    protocol: TCP

In addition to that, if I turn off insecure requests, using:
quarkus.http.insecure-requests=disabled
The http port will not be generated anymore:

 - containerPort: 80
   name: http
   protocol: TCP

Implementation ideas

No response

@csviri csviri added the kind/enhancement New feature or request label Dec 21, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Dec 21, 2022

/cc @Sgitario(kubernetes), @geoand(kubernetes), @iocanel(kubernetes)

@Sgitario
Copy link
Contributor

The problem I see is determining at build time that we should provide the SSL port over the HTTP port.
By default, we're using the quarkus.http.ssl-port=8443 and quarkus.http.insecure-requests=enabled properties, so the default values would imply that the SSL port is enabled.
At the moment, Quarkus determines the usage of HTTP over SSL at runtime based on the presence of the certificates, so we can't use this logic for generating the Kubernetes manifests.
Any ideas?
Some solutions:

  • Generate the two ports unless the user explicitly says quarkus.http.insecure-requests=disabled
  • Add a new property in Kubernetes to use the SSL port.

@iocanel @geoand @csviri

@csviri
Copy link
Author

csviri commented Dec 22, 2022

Some solutions:

  • Generate the two ports unless the user explicitly says quarkus.http.insecure-requests=disabled
  • Add a new property in Kubernetes to use the SSL port.

IMO this makes sense. Only thing is that I might not generate the SSL port automatically, so only if the new Kubernetes property is present.

@geoand
Copy link
Contributor

geoand commented Dec 22, 2022

Add a new property in Kubernetes to use the SSL port.

Yeah, I think we can do this

@Sgitario
Copy link
Contributor

So, this issue should be addressed as:

  • Generate the HTTPS port as part of the service (and the container). At the moment, this is only generating the HTTP port.
  • Add a new property in the Ingress config: quarkus.kubernetes.ingress.target-port, so a user can select between the HTTP or the HTTPS port name.

If nobody works on this, I will when I'm back after my PTO in the second week of January.

@iocanel
Copy link
Contributor

iocanel commented Jan 4, 2023

If nobody works on this, I will when I'm back after my PTO in the second week of January.

Perfect! Assigned the issue to you.

Sgitario added a commit to quarkiverse/quarkus-helm that referenced this issue Jan 9, 2023
@Sgitario
Copy link
Contributor

FYI: I will start working in this tomorrow

Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 11, 2023
These changes will add an additional container port HTTPS in the generated manifests:

```yaml
containers:
        - image: ...
          imagePullPolicy: IfNotPresent
          name: kubernetes-kind
          ports:
            - containerPort: 8080
              name: http
              protocol: TCP
            - containerPort: 8443
              name: https
              protocol: TCP
```

By default, the Ingress and Route resources will use the "http" port. However, as part of these changes, I've added a new property to select between "https" or "http", or any other that user might have added as part of the configuration. Example:

```
quarkus.kubernetes.ingress.target-port=https
quarkus.openshift.route.target-port=https
```

Finally, note that the https container won't be added for the Knative resources because of the following Dekorate issue: dekorateio/dekorate#1119. 
Also, that the nodeport for Kind and Minikube resources will only be added for the http ports due to this Dekorate limitation: dekorateio/dekorate#1120.
Both issues should be addressed in Dekorate and then fixed in a later pull requested.

Fix quarkusio#29999
Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 11, 2023
These changes will add an additional container port HTTPS in the generated manifests:

```yaml
containers:
        - image: ...
          imagePullPolicy: IfNotPresent
          name: kubernetes-kind
          ports:
            - containerPort: 8080
              name: http
              protocol: TCP
            - containerPort: 8443
              name: https
              protocol: TCP
```

By default, the Ingress and Route resources will use the "http" port. However, as part of these changes, I've added a new property to select between "https" or "http", or any other that user might have added as part of the configuration. Example:

```
quarkus.kubernetes.ingress.target-port=https
quarkus.openshift.route.target-port=https
```

Finally, note that the https container won't be added for the Knative resources because of the following Dekorate issue: dekorateio/dekorate#1119. 
Also, that the nodeport for Kind and Minikube resources will only be added for the http ports due to this Dekorate limitation: dekorateio/dekorate#1120.
Both issues should be addressed in Dekorate and then fixed in a later pull requested.

Fix quarkusio#29999
Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 12, 2023
These changes will add an additional container port HTTPS in the generated manifests:

```yaml
containers:
        - image: ...
          imagePullPolicy: IfNotPresent
          name: kubernetes-kind
          ports:
            - containerPort: 8080
              name: http
              protocol: TCP
            - containerPort: 8443
              name: https
              protocol: TCP
```

By default, the Ingress and Route resources will use the "http" port. However, as part of these changes, I've added a new property to select between "https" or "http", or any other that user might have added as part of the configuration. Example:

```
quarkus.kubernetes.ingress.target-port=https
quarkus.openshift.route.target-port=https
```

Finally, note that the https container won't be added for the Knative resources because of the following Dekorate issue: dekorateio/dekorate#1119. 
Also, that the nodeport for Kind and Minikube resources will only be added for the http ports due to this Dekorate limitation: dekorateio/dekorate#1120.
Both issues should be addressed in Dekorate and then fixed in a later pull requested.

Fix quarkusio#29999
Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 16, 2023
These changes will add an additional container port HTTPS in the generated manifests:

```yaml
containers:
        - image: ...
          imagePullPolicy: IfNotPresent
          name: kubernetes-kind
          ports:
            - containerPort: 8080
              name: http
              protocol: TCP
            - containerPort: 8443
              name: https
              protocol: TCP
```

By default, the Ingress and Route resources will use the "http" port. However, as part of these changes, I've added a new property to select between "https" or "http", or any other that user might have added as part of the configuration. Example:

```
quarkus.kubernetes.ingress.target-port=https
quarkus.openshift.route.target-port=https
```

Finally, note that the https container won't be added for the Knative resources since there seems to be a limitation at Knative side.

Fix quarkusio#29999
asd
Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 18, 2023
These changes will add an additional container port HTTPS in the generated manifests:

```yaml
containers:
        - image: ...
          imagePullPolicy: IfNotPresent
          name: kubernetes-kind
          ports:
            - containerPort: 8080
              name: http
              protocol: TCP
            - containerPort: 8443
              name: https
              protocol: TCP
```

By default, the Ingress and Route resources will use the "http" port. However, as part of these changes, I've added a new property to select between "https" or "http", or any other that user might have added as part of the configuration. Example:

```
quarkus.kubernetes.ingress.target-port=https
quarkus.openshift.route.target-port=https
```

Finally, note that the https container won't be added for the Knative resources since there seems to be a limitation at Knative side.

Fix quarkusio#29999
asd
Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 26, 2023
These changes will add an additional container port HTTPS in the generated manifests:

```yaml
containers:
        - image: ...
          imagePullPolicy: IfNotPresent
          name: kubernetes-kind
          ports:
            - containerPort: 8080
              name: http
              protocol: TCP
            - containerPort: 8443
              name: https
              protocol: TCP
```

By default, the Ingress and Route resources will use the "http" port. However, as part of these changes, I've added a new property to select between "https" or "http", or any other that user might have added as part of the configuration. Example:

```
quarkus.kubernetes.ingress.target-port=https
quarkus.openshift.route.target-port=https
```

Finally, note that the https container won't be added for the Knative resources since there seems to be a limitation at Knative side.

Fix quarkusio#29999
asd
@quarkus-bot quarkus-bot bot added this to the 2.17 - main milestone Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants