Feature Adding TLS-HTTPS configuration #288
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sigrid maintainability feedback✅ You wrote maintainable code and achieved your objective of 3.8 stars Show detailsSigrid compared your code against the baseline of 2024-12-12. 👍 What went well?
👎 What could be better?
📚 Remaining technical debt
View this system in Sigrid** to explore your technical debt ⭐️ Sigrid ratings
💬 Did you find this feedback helpful?We would like to know your thoughts to make Sigrid better. |
Closed
RabbITCybErSeC
requested review from
MaartendeKruijf and
lucamrgs
and removed request for
MaartendeKruijf
MaartendeKruijf
requested changes
Dec 11, 2024
internal/controller/controller.go
Outdated
| certFile := utils.GetEnv("CERT_FILE", "/certs/server.crt") | ||
| certKey := utils.GetEnv("CERT_KEY", "/certs/server.key") | ||
| enableFins := utils.GetEnv("ENABLE_FINS", "false") | ||
| maxExecutionsStr := utils.GetEnv("MAX_EXECUTIONS", strconv.Itoa(defaultCacheSize)) |
There was a problem hiding this comment.
create an int directly ...
There was a problem hiding this comment.
Move the refactor to an other branch
docker-compose.yaml
Outdated
There was a problem hiding this comment.
Only minimal config in here
RabbITCybErSeC
force-pushed
the
feature/287-feature-adding-https-tls
branch
from
78b68d0 to
b69c3da
Compare
RabbITCybErSeC
force-pushed
the
feature/287-feature-adding-https-tls
branch
from
6a5f164 to
376b466
Compare
MaartendeKruijf
requested changes
Dec 19, 2024
internal/controller/controller.go
Outdated
Comment on lines
183
to
194
| enableTLS, _ := strconv.ParseBool(utils.GetEnv("ENABLE_TLS", "false")) | ||
| certFile := utils.GetEnv("CERT_FILE", "./certs/server.crt") | ||
| keyFile := utils.GetEnv("CERT_KEY_FILE", "./certs/server.key") | ||
|
|
||
| if enableTLS { | ||
| if certFile == "" || keyFile == "" { | ||
| err := fmt.Errorf("TLS enabled but certificate or key file not provided") | ||
| log.Error(err) | ||
| return err | ||
| } | ||
| } | ||
| err = runServer(app, port, enableTLS, certFile, keyFile) |
There was a problem hiding this comment.
Move to separate function
| @@ -178,7 +180,18 @@ func Initialize() error { | |||
| } | |||
|
|
|||
| port := utils.GetEnv("PORT", "8080") | |||
| err = app.Run(":" + port) | |||
internal/controller/controller.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func runServer(app *gin.Engine, port string, enableTlS bool, certFile string, keyFile string) error { |
There was a problem hiding this comment.
Limit to app and enableTLS
MaartendeKruijf
previously approved these changes
Dec 19, 2024
MaartendeKruijf
approved these changes
Dec 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.