-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add hoppscotch Helm Chart #246
base: main
Are you sure you want to change the base?
Conversation
This commit adds the initial implementation of hoppscotch Helm chart.
PR Reviewer Guide 🔍
|
This commit adds the option to use existingSecret and reference the sensitive environment variables from created secrets instead of hard coding in enviornment variables.
/improve |
/improve |
/improve |
1 similar comment
/improve |
PR Code Suggestions ✨Latest suggestions up to f152175
Previous suggestionsSuggestions up to commit b81f1e4
Suggestions up to commit 4ecab93
Suggestions up to commit 3f7cd5e
Suggestions up to commit 9e80703
|
f152175
to
b9fb929
Compare
@sourcery-ai review |
Reviewer's Guide by SourceryThis PR introduces a new Helm chart for deploying Hoppscotch, implementing a complete Kubernetes deployment solution. The implementation includes all necessary Kubernetes resources with PostgreSQL integration, proper secret management, and configurable deployment options through values.yaml. Entity relationship diagram for Hoppscotch Helm CharterDiagram
CHART ||--o{ DEPLOYMENT : includes
CHART ||--o{ SERVICE : includes
CHART ||--o{ INGRESS : includes
CHART ||--o{ VALUES : includes
CHART ||--o{ SECRETS : includes
CHART ||--o{ POSTGRESQL : depends_on
CHART {
string name
string version
string appVersion
}
DEPLOYMENT {
string name
int replicas
string image
}
SERVICE {
string name
string type
int port
}
INGRESS {
string name
string hostname
bool tls
}
VALUES {
string postgresql
string hoppscotch
}
SECRETS {
string jwt_secret
string session_secret
}
POSTGRESQL {
string name
string version
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ching-kuo - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider moving sensitive values (JWT_SECRET, SESSION_SECRET, etc.) out of values.yaml into a separate secrets management solution or using existing secrets. Hardcoding these in values.yaml is a security risk.
- Add resource limits and requests to the deployment spec to ensure proper resource management and prevent potential container resource exhaustion.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
pullPolicy: IfNotPresent | ||
|
||
auth: | ||
JWT_SECRET: "secret1233" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 issue (security): Default secrets should not be included in the values file
Including default secrets in the values file poses a security risk if users forget to change them. Consider removing the default values and requiring users to provide their own secrets, or generate them automatically.
- name: {{ $key }} | ||
value: "{{ $value }}" | ||
{{- end }} | ||
resources: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (performance): Missing container resource limits
Resource limits should be defined to prevent potential resource exhaustion. Consider adding appropriate CPU and memory limits based on the application's requirements.
User description
This commit adds the initial implementation of hoppscotch Helm chart.
PR Type
Enhancement
Description
Changes walkthrough 📝
Chart.yaml
Initial Helm Chart Configuration for Hoppscotch
charts/hoppscotch/Chart.yaml
Chart.yaml
for Helm chart configuration.aio-deployment.yaml
Deployment Configuration for Hoppscotch Application
charts/hoppscotch/templates/aio-deployment.yaml
aio-deployment.yaml
for deploying Hoppscotch application.aio-service.yaml
Service Configuration for Hoppscotch Application
charts/hoppscotch/templates/aio-service.yaml
aio-service.yaml
to expose the application via a Kubernetesservice.
ingress.yaml
Ingress Configuration for Hoppscotch Application
charts/hoppscotch/templates/ingress.yaml
ingress.yaml
for routing external traffic to theapplication.
values.yaml
Customizable Values for Hoppscotch Helm Chart
charts/hoppscotch/values.yaml
values.yaml
for customizable Helm chart values.variables.
Summary by Sourcery
Add a Helm chart for the Hoppscotch application, configuring deployment, service, ingress, and PostgreSQL integration.
New Features:
Enhancements:
Deployment: