-
Notifications
You must be signed in to change notification settings - Fork 343
K8s practice #5
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
base: main
Are you sure you want to change the base?
K8s practice #5
Conversation
WalkthroughNamespace changed from chat-app to chatapp across manifests. Backend and frontend deployments switched images; backend added PORT and updated Secret reference. Ingress host, annotations, and paths updated. Mongo storage reworked: PV path/class changed, PVC re-added in new namespace, deployment volume names adjusted. Secrets replaced and renamed. Minor formatting/whitespace edits. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Ingress as Ingress (chat-sunil.com)
participant FE_SVC as Service: frontend
participant FE as Deployment: frontend pods
participant BE_SVC as Service: backend
participant BE as Deployment: backend pods (PORT 5001)
participant DB_SVC as Service: mongodb
participant DB as Deployment: mongodb pods
User->>Ingress: HTTP GET /
Ingress->>FE_SVC: route to port 80 (rewrite /)
FE_SVC->>FE: forward request
FE-->>User: response
User->>Ingress: HTTP GET /backend/*
Ingress->>BE_SVC: route to port 5001 (rewrite /)
BE_SVC->>BE: forward request
BE->>DB_SVC: TCP 27017
DB_SVC->>DB: query
DB-->>BE: result
BE-->>User: response
Note over Ingress: Annotations: rewrite-target "/", ssl-redirect "false"
Note over DB: Uses PVC "mongodb-pvc" with storageClass "mongo-storage"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
ingess no configure |
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.
Actionable comments posted: 7
🧹 Nitpick comments (3)
k8s/secret.yml (1)
6-7: Add MongoDB connection string into the Secret.To move database credentials out of the deployment manifest, please extend this Secret with an additional key (for example
mongodb-uri) holding the base64‑encoded connection string so the deployment can consume it viavalueFrom.Apply this diff once you have the base64 value:
data: jwt: WnJqcFRBcWdwQUZHZVh1QWMxMXpPQVhsQnBwRDI1NjkyWU5qNzVPVnRHaGI3RTAyM3FjN2wyT1F3ZE4yTUNaYlJURmd6aFU2WVFuRDJKcHZXODRBbjBvVU0zU1FRTVpydjlDQVN2RGR2dTA5YktrQ0ZPM2RFR3NoS096RDZhczhkQXVOQnA0S2Q1ZzhhT2U0Sm1GTUNPTzB4U3pocjd2bU9NRUdMWmZ5cTg2dnByd0R1UklEUlEwUGNZYzlRUEJwSVh5MEtqRlg0aXpWNERzQjlGRU5WaTljRW1MdUVCb3cyUFhDNEtHaFpFaUhJVTdndHR5Ukd2dExod3JTVGY5aA== + mongodb-uri: <base64-encoded-connection-string>k8s/backend_deployment.yml (2)
14-16: Dropnamespacefrom the pod template.Pods created by the Deployment automatically inherit the Deployment’s namespace; leaving this field here is redundant and can confuse tools. Please remove it.
Apply this diff:
- namespace: chatapp labels: app: backend
19-21: Align with YAML linting expectations.
image:has a double space after the colon, which YAMLlint flags. Trim it to keep the manifest lint-clean.Apply this diff:
- image: sunilchouha/backend:latest + image: sunilchouha/backend:latest
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
k8s/backend-svc.yml(1 hunks)k8s/backend_deployment.yml(2 hunks)k8s/frontend-svc.yml(1 hunks)k8s/frontend_deployment.yml(2 hunks)k8s/ingress.yml(1 hunks)k8s/mongo-pv.yml(1 hunks)k8s/mongo-pvc.yml(1 hunks)k8s/mongo-service.yml(1 hunks)k8s/mongodb-deployment.yml(2 hunks)k8s/mongodb-pvc.yml(0 hunks)k8s/namespace.yml(1 hunks)k8s/secret.yml(1 hunks)k8s/secrets.yml(0 hunks)
💤 Files with no reviewable changes (2)
- k8s/mongodb-pvc.yml
- k8s/secrets.yml
🧰 Additional context used
🪛 Checkov (3.2.334)
k8s/mongodb-deployment.yml
[medium] 1-34: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 1-34: Minimize the admission of root containers
(CKV_K8S_23)
k8s/backend_deployment.yml
[medium] 27-28: Basic Auth Credentials
(CKV_SECRET_4)
🪛 YAMLlint (1.37.1)
k8s/backend_deployment.yml
[warning] 20-20: too many spaces after colon
(colons)
k8s/ingress.yml
[error] 9-9: duplication of key "annotations" in mapping
(key-duplicates)
[error] 19-19: duplication of key "path" in mapping
(key-duplicates)
[error] 27-27: duplication of key "path" in mapping
(key-duplicates)
| value: "mongodb://mongoadmin:secret@mongodb:27017/dbname?authSource=admin" | ||
| - name: PORT | ||
| value: "5001" | ||
| - name: JWT_SECRET | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: chatapp-secrets | ||
| key: jwt | ||
| - name: PORT | ||
| value: "5001" | ||
| name: chatapp-secret | ||
| key: jwt |
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.
Move MongoDB credentials into a Secret.
Hard-coding mongoadmin:secret in the manifest exposes credentials in git and every kubectl dump. Load the connection string from the Secret you manage (see k8s/secret.yml) instead.
Apply this diff after adding the mongodb-uri key to chatapp-secret:
- - name: MONGODB_URI
- value: "mongodb://mongoadmin:secret@mongodb:27017/dbname?authSource=admin"
+ - name: MONGODB_URI
+ valueFrom:
+ secretKeyRef:
+ name: chatapp-secret
+ key: mongodb-uri🧰 Tools
🪛 Checkov (3.2.334)
[medium] 1-34: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 1-34: Minimize the admission of root containers
(CKV_K8S_23)
[medium] 27-28: Basic Auth Credentials
(CKV_SECRET_4)
🤖 Prompt for AI Agents
In k8s/backend_deployment.yml around lines 27 to 34, the MongoDB connection
string is hard-coded; replace the literal value with a secret reference: add an
env entry that uses valueFrom.secretKeyRef pointing to the existing secret name
chatapp-secret and key mongodb-uri (after you add that key to k8s/secret.yml),
leaving the PORT and JWT_SECRET entries intact; ensure the value field is
removed for MongoDB and the secretKeyRef uses name: chatapp-secret and key:
mongodb-uri so credentials are no longer stored in the manifest.
| name: backend | ||
| namespace: chatapp |
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.
Fix trailing space in Service name.
metadata.name currently ends with a space, which violates the DNS-1123 label requirement and will make kubectl apply fail. Drop the trailing space.
Apply this diff:
- name: backend
+ name: backend📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: backend | |
| namespace: chatapp | |
| name: backend | |
| namespace: chatapp |
🤖 Prompt for AI Agents
In k8s/backend-svc.yml around lines 4 to 5, the Service metadata.name has a
trailing space ("backend "), which violates DNS-1123 label rules and causes
kubectl apply to fail; remove the trailing space so the name is "backend" (no
extra whitespace) and ensure there are no other leading/trailing spaces in
metadata.name.
| metadata: | ||
| name: frontend | ||
| namespace: chat-app | ||
| name: frontend |
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.
Remove the trailing space from the Service name.
metadata.name currently renders as frontend (note the trailing space), which violates the DNS-1123 naming rules and prevents the Service from being created. Please drop the trailing space so the name is exactly frontend.
- name: frontend
+ name: frontend📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: frontend | |
| name: frontend |
🤖 Prompt for AI Agents
In k8s/frontend-svc.yml around line 4, metadata.name contains a trailing space
("frontend "), which violates DNS-1123 rules and prevents Service creation;
remove the trailing whitespace so the name is exactly "frontend" (no
leading/trailing spaces) and verify there are no other hidden whitespace
characters in the file.
| annotations: | ||
| labels: | ||
| name: chatapp-ingress | ||
| annotations: | ||
| nginx.ingress.kubernetes.io/rewrite-target: / | ||
| nginx.ingress.kubernetes.io/ssl-redirect: "false" |
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.
Deduplicate the annotations block.
metadata currently declares annotations twice (Line 6 empty, Line 9 populated). YAML rejects duplicate keys, so the ingress manifest cannot be applied. Remove the first empty annotations: entry and keep a single block containing the desired annotations.
namespace: chatapp
- annotations:
labels:
name: chatapp-ingress
annotations:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| annotations: | |
| labels: | |
| name: chatapp-ingress | |
| annotations: | |
| nginx.ingress.kubernetes.io/rewrite-target: / | |
| nginx.ingress.kubernetes.io/ssl-redirect: "false" | |
| namespace: chatapp | |
| labels: | |
| name: chatapp-ingress | |
| annotations: | |
| nginx.ingress.kubernetes.io/rewrite-target: / | |
| nginx.ingress.kubernetes.io/ssl-redirect: "false" |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 9-9: duplication of key "annotations" in mapping
(key-duplicates)
🤖 Prompt for AI Agents
In k8s/ingress.yml around lines 6 to 11, remove the duplicate empty
"annotations:" key at line 6 so there is only one annotations block (the
populated one at lines 9–11); ensure the metadata contains a single annotations
mapping with the nginx.ingress.kubernetes.io/rewrite-target and
nginx.ingress.kubernetes.io/ssl-redirect entries and no duplicate keys.
| - path: /frontend | ||
| pathType: Prefix | ||
| path: "/" | ||
| backend: | ||
| service: | ||
| name: frontend | ||
| port: | ||
| port: | ||
| number: 80 | ||
| - pathType: Prefix | ||
| - path: /backend | ||
| pathType: Prefix | ||
| path: "/api" | ||
| backend: | ||
| service: | ||
| name: backend | ||
| port: | ||
| number: 5001 No newline at end of file | ||
| port: | ||
| number: 5001 |
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.
Separate each ingress path into its own list item.
Within a single path item you define path twice (/frontend then "/"; similarly /backend then "/api"). YAML disallows duplicate keys, so this manifest is invalid. Make each route its own list entry with a single path, pathType, and backend block.
paths:
- - path: /frontend
- pathType: Prefix
- path: "/"
- backend:
- service:
- name: frontend
- port:
- number: 80
- - path: /backend
- pathType: Prefix
- path: "/api"
- backend:
- service:
- name: backend
- port:
- number: 5001
+ - path: /frontend
+ pathType: Prefix
+ backend:
+ service:
+ name: frontend
+ port:
+ number: 80
+ - path: /
+ pathType: Prefix
+ backend:
+ service:
+ name: frontend
+ port:
+ number: 80
+ - path: /backend
+ pathType: Prefix
+ backend:
+ service:
+ name: backend
+ port:
+ number: 5001
+ - path: /api
+ pathType: Prefix
+ backend:
+ service:
+ name: backend
+ port:
+ number: 5001📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - path: /frontend | |
| pathType: Prefix | |
| path: "/" | |
| backend: | |
| service: | |
| name: frontend | |
| port: | |
| port: | |
| number: 80 | |
| - pathType: Prefix | |
| - path: /backend | |
| pathType: Prefix | |
| path: "/api" | |
| backend: | |
| service: | |
| name: backend | |
| port: | |
| number: 5001 | |
| \ No newline at end of file | |
| port: | |
| number: 5001 | |
| paths: | |
| - - path: /frontend | |
| - pathType: Prefix | |
| - path: "/" | |
| - backend: | |
| - service: | |
| - name: frontend | |
| - port: | |
| - number: 80 | |
| - - path: /backend | |
| - pathType: Prefix | |
| - path: "/api" | |
| - backend: | |
| - service: | |
| - name: backend | |
| - port: | |
| - path: /frontend | |
| pathType: Prefix | |
| backend: | |
| service: | |
| name: frontend | |
| port: | |
| number: 80 | |
| - path: / | |
| pathType: Prefix | |
| backend: | |
| service: | |
| name: frontend | |
| port: | |
| number: 80 | |
| - path: /backend | |
| pathType: Prefix | |
| backend: | |
| service: | |
| name: backend | |
| port: | |
| number: 5001 | |
| - path: /api | |
| pathType: Prefix | |
| backend: | |
| service: | |
| name: backend | |
| port: | |
| number: 5001 |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 19-19: duplication of key "path" in mapping
(key-duplicates)
[error] 27-27: duplication of key "path" in mapping
(key-duplicates)
🤖 Prompt for AI Agents
In k8s/ingress.yml around lines 17 to 32, the ingress list items currently
contain duplicate path keys (e.g., "path: /frontend" and then "path: \"/\"" in
the same item), which is invalid YAML; split each route into its own list entry
so each item has a single path, pathType, and backend block (one entry for
/frontend with its path and backend, another entry for / with its path and
backend, and similarly separate entries for /backend and /api), ensuring no
duplicate keys within an item and preserving the correct service names and
ports.
| env: | ||
| - name: MONGO_INITDB_ROOT_USERNAME | ||
| value: mongoadmin | ||
| value: mongoadmin |
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.
Fix the Mongo root username value.
There’s a trailing space in value: mongoadmin , so the pod will launch with the username mongoadmin instead of mongoadmin. Any client (and your Secret) will send credentials without the space and authentication will fail. Drop the trailing space.
- value: mongoadmin
+ value: mongoadmin📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| value: mongoadmin | |
| value: mongoadmin |
🤖 Prompt for AI Agents
In k8s/mongodb-deployment.yml around line 25 the env value for the Mongo root
username contains a trailing space ("mongoadmin "), which causes the container
to use the wrong username; remove the trailing space so the value is exactly
"mongoadmin" (update the YAML value field to have no extra whitespace) and
verify the Secret and any clients use the identical username string.
| # volumeMounts: | ||
| # - name: mongo-data | ||
| # mountPath: /data/db | ||
| volumes: | ||
| - name: mongo-data | ||
| - name: mongo-storage | ||
| persistentVolumeClaim: | ||
| claimName: mongodb-pvc | ||
|
|
||
| claimName: mongodb-pvc |
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.
Restore the MongoDB volume mount.
You still declare the PVC under volumes, but the corresponding volumeMounts block is now commented out. As a result, the database runs entirely on the container filesystem and you lose data on every restart. Please reinstate the volumeMounts section (updated to the new volume name).
- # volumeMounts:
- # - name: mongo-data
- # mountPath: /data/db
+ volumeMounts:
+ - name: mongo-storage
+ mountPath: /data/db🧰 Tools
🪛 Checkov (3.2.334)
[medium] 1-34: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 1-34: Minimize the admission of root containers
(CKV_K8S_23)
🤖 Prompt for AI Agents
In k8s/mongodb-deployment.yml around lines 28 to 34 the PVC is declared under
volumes but the corresponding container volumeMounts block was commented out,
causing MongoDB to use ephemeral container storage; restore a volumeMounts
section in the container spec using the new volume name "mongo-storage" and
mount it at /data/db (ensure correct indentation under the container spec and
that the mount name matches the volumes entry).
Summary by CodeRabbit