Add HTTPS option #19
Add HTTPS option #19
Conversation
We're currently testing out DuckyAPI on Kubernetes and we have a practice of using HTTPS even in internal cluster communications. Others might find this helpful for different reasons.
Add HTTPS option
Add config for HTTPS option
There was a problem hiding this comment.
Hi @CheyenneForbes, thank you very much for the PR! I have some feedback. Additionally, could you add these values to the example.env? They should be commented out as they're optional.
I hadn't really set this project up for contributions yet, sorry about that. In a second CLA assistant should ask you to sign a CLA. It just makes sure that I retain full rights to the entirety of this app :). Thanks!
| @IsOptional() | ||
| @IsNotEmpty() | ||
| @IsString() | ||
| TLS_KEY_PATH: string |
There was a problem hiding this comment.
| TLS_KEY_PATH: string | |
| TLS_KEY_PATH?: string |
| @IsOptional() | ||
| @IsNotEmpty() | ||
| @IsString() | ||
| TLS_CERT_PATH: string |
There was a problem hiding this comment.
| TLS_CERT_PATH: string | |
| TLS_CERT_PATH?: string |
| const server = express.default() | ||
| const app = await NestFactory.create( | ||
| AppModule, | ||
| new ExpressAdapter(server) |
There was a problem hiding this comment.
The only reason to do it this way I can see from the documention if for starting multiple servers at the same time (for example an http server next to an https server), which is not something that's happening right now. Is there a reason you did it this way as opposed to the standard nestjs config?
| const https = require('https') | ||
| const fs = require('fs') |
There was a problem hiding this comment.
Please make these import statements at the top of the file instead of requires.
|
|
We're currently testing out DuckyAPI on Kubernetes and we have a practice of using HTTPS even in internal cluster communications. Others might find this helpful for different reasons.