my-new-feature changes - #49
Conversation
Updated CI/CD workflow to improve build and code quality processes.
|
approve |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe pull request adds container packaging, Kubernetes manifests, a Helm chart, and a GitHub Actions workflow for building, testing, linting, publishing, and updating the deployed image tag. It also updates static page attribution text. ChangesGo application delivery
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant GoBuild as Go build and test
participant Linter as golangci-lint
participant DockerHub as Docker Hub
participant HelmValues as Helm values
GitHub->>GoBuild: build and test on main push
GoBuild->>Linter: run lint after build succeeds
Linter->>DockerHub: push image tagged with run ID
DockerHub->>HelmValues: update image tag and commit values.yaml
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
changes done in home.html file
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
.github/workflows/cicd.yaml (1)
23-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winValidate with the declared Go toolchain.
go.moddeclares Go 1.21.10, but both validation jobs use Go 1.22. This CI configuration does not verify support for the declared target. Usego-version: "1.21.10"in both jobs, or change the module target intentionally.Also applies to: 43-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/cicd.yaml around lines 23 - 27, Update the Go setup steps in both validation jobs to use go-version "1.21.10", matching the module target declared in go.mod; preserve the existing setup-go action and cache configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/cicd.yaml:
- Around line 5-12: Add a pull_request trigger targeting main so build, test,
and lint checks run for pull requests, while keeping publishing and chart-update
jobs restricted to push events. Update the workflow conditions for
update-newtag-in-helm-chart and related image publication steps to explicitly
require a push event, preserving the existing push path filters.
- Around line 94-100: Update the “Commit and push changes” workflow step to
check whether values.yaml has changes using git diff --quiet before committing.
Only run git commit and git push when a diff exists, while preserving the
existing Git configuration and staging behavior.
- Line 14: Set workflow-level permissions to read-only, then grant contents:
write only to the update-newtag-in-helm-chart job. In read-only checkout steps,
disable credential persistence; update that job’s checkout to use the scoped
repository GITHUB_TOKEN instead of secrets.TOKEN when permitted by repository
policy.
In `@Dockerfile`:
- Around line 26-38: Set an unprivileged USER in the final Docker image stage
before the CMD instruction, ensuring the application launched by ./main runs
without root privileges.
In `@helm/go-web-app-chart/templates/deployment.yml`:
- Around line 9-20: The Deployment template’s container settings ignore declared
Helm values. In helm/go-web-app-chart/templates/deployment.yml lines 9-20,
replace the hardcoded replica count and image repository with
.Values.replicaCount and .Values.image.repository, and add imagePullPolicy using
.Values.image.pullPolicy. helm/go-web-app-chart/values.yaml lines 5-11 requires
no direct change; it defines the values consumed by the template.
In `@helm/go-web-app-chart/templates/ingress.yml`:
- Around line 6-7: Remove the nginx.ingress.kubernetes.io/rewrite-target
annotation from the Ingress manifest so requests preserve their original
application paths, including /home and /about. Leave the remaining Ingress
configuration unchanged.
In `@k8s/manifests/deployment.yml`:
- Around line 17-22: Update the pod spec in k8s/manifests/deployment.yml at
lines 17-22 and helm/go-web-app-chart/templates/deployment.yml at lines 17-22
with identical security settings: pod-level runAsNonRoot and a RuntimeDefault
seccomp profile, plus container-level allowPrivilegeEscalation false,
readOnlyRootFilesystem true, and all capabilities dropped. Apply the settings to
both Deployment paths without changing unrelated configuration.
In `@static/home.html`:
- Line 72: Remove the test-only “test message” text from the home page content
near the existing “from basics for Free” copy, preserving the surrounding
production-facing message.
---
Nitpick comments:
In @.github/workflows/cicd.yaml:
- Around line 23-27: Update the Go setup steps in both validation jobs to use
go-version "1.21.10", matching the module target declared in go.mod; preserve
the existing setup-go action and cache configuration.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a98106ad-3a2a-4ab2-aea6-2579db75fb92
📒 Files selected for processing (14)
.github/workflows/cicd.yamlDockerfilego.modhelm/go-web-app-chart/.helmignorehelm/go-web-app-chart/Chart.yamlhelm/go-web-app-chart/templates/deployment.ymlhelm/go-web-app-chart/templates/ingress.ymlhelm/go-web-app-chart/templates/service.ymlhelm/go-web-app-chart/values.yamlk8s/manifests/deployment.ymlk8s/manifests/ingress.ymlk8s/manifests/service.ymlstatic/about.htmlstatic/home.html
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| paths-ignore: | ||
| - 'helm/**' | ||
| - 'k8s/**' | ||
| - 'README.md' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run build and quality checks for pull requests.
This workflow runs only after a push to main. Pull request changes can merge without go build, go test, or golangci-lint validation. Add a pull_request trigger for main. Restrict push and update-newtag-in-helm-chart to push events so pull requests do not publish images or modify the chart.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/cicd.yaml around lines 5 - 12, Add a pull_request trigger
targeting main so build, test, and lint checks run for pull requests, while
keeping publishing and chart-update jobs restricted to push events. Update the
workflow conditions for update-newtag-in-helm-chart and related image
publication steps to explicitly require a push event, preserving the existing
push path filters.
| - 'k8s/**' | ||
| - 'README.md' | ||
|
|
||
| jobs: |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced workflow and nearby relevant lines.
if [ -f ".github/workflows/cicd.yaml" ]; then
echo "== .github/workflows/cicd.yaml outline =="
wc -l ".github/workflows/cicd.yaml"
echo "== relevant workflow content =="
nl -ba ".github/workflows/cicd.yaml" | sed -n '1,130p'
else
echo ".github/workflows/cicd.yaml not found"
fi
echo "== token and checkout usages in workflow =="
rg -n "permissions:|secrets\.TOKEN|persist-credentials|checkout|contents:" .github/workflows/cicd.yaml || trueRepository: iam-veeramalla/go-web-app
Length of output: 309
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .github/workflows/cicd.yaml content with line numbers =="
awk '{printf "%6d %s\n", NR, $0}' .github/workflows/cicd.yaml
echo "== token and checkout usages in workflow =="
grep -nE "permissions:|secrets\.TOKEN|persist-credentials|checkout|contents:" .github/workflows/cicd.yaml || trueRepository: iam-veeramalla/go-web-app
Length of output: 3958
Scope write permissions to the update job.
This workflow has no permissions: block, and update-newtag-in-helm-chart checks out with token: ${{ secrets.TOKEN }} without persist-credentials: false. Set read-only default permissions, grant contents: write only to update-newtag-in-helm-chart, and disable credential persistence for the read-only checkout steps. Use the scoped repository GITHUB_TOKEN instead of secrets.TOKEN if repository policy allows it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/cicd.yaml at line 14, Set workflow-level permissions to
read-only, then grant contents: write only to the update-newtag-in-helm-chart
job. In read-only checkout steps, disable credential persistence; update that
job’s checkout to use the scoped repository GITHUB_TOKEN instead of
secrets.TOKEN when permitted by repository policy.
Source: Linters/SAST tools
| - name: Commit and push changes | ||
| run: | | ||
| git config --global user.email "bibhishandhagate@gmail.com" | ||
| git config --global user.name "Bibhishan Dhagate" | ||
| git add helm/go-web-app-chart/values.yaml | ||
| git commit -m "Update tag in Helm chart to ${{ github.run_id }}" | ||
| git push |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Skip the commit when the Helm value is unchanged.
A workflow rerun keeps the same github.run_id. If the first attempt already committed that tag, git commit exits with status 1 because values.yaml has no diff. Guard the commit and push with git diff --quiet.
Proposed fix
git config --global user.name "Bibhishan Dhagate"
git add helm/go-web-app-chart/values.yaml
- git commit -m "Update tag in Helm chart to ${{ github.run_id }}"
- git push
+ if ! git diff --cached --quiet; then
+ git commit -m "Update tag in Helm chart to ${{ github.run_id }}"
+ git push
+ fi📝 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: Commit and push changes | |
| run: | | |
| git config --global user.email "bibhishandhagate@gmail.com" | |
| git config --global user.name "Bibhishan Dhagate" | |
| git add helm/go-web-app-chart/values.yaml | |
| git commit -m "Update tag in Helm chart to ${{ github.run_id }}" | |
| git push | |
| - name: Commit and push changes | |
| run: | | |
| git config --global user.email "bibhishandhagate@gmail.com" | |
| git config --global user.name "Bibhishan Dhagate" | |
| git add helm/go-web-app-chart/values.yaml | |
| if ! git diff --cached --quiet; then | |
| git commit -m "Update tag in Helm chart to ${{ github.run_id }}" | |
| git push | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/cicd.yaml around lines 94 - 100, Update the “Commit and
push changes” workflow step to check whether values.yaml has changes using git
diff --quiet before committing. Only run git commit and git push when a diff
exists, while preserving the existing Git configuration and staging behavior.
| FROM gcr.io/distroless/base | ||
|
|
||
| # Copy the binary from the previous stage | ||
| COPY --from=base /app/main . | ||
|
|
||
| # Copy the static files from the previous stage | ||
| COPY --from=base /app/static ./static | ||
|
|
||
| # Expose the port on which the application will run | ||
| EXPOSE 8080 | ||
|
|
||
| # Command to run the application | ||
| CMD ["./main"] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Run the application as a non-root user.
The final stage does not set USER. Trivy resolves the image user as root. Set an unprivileged user before CMD.
Proposed fix
COPY --from=base /app/static ./static
+USER 65532:65532
+
# Expose the port on which the application will run
EXPOSE 8080📝 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.
| FROM gcr.io/distroless/base | |
| # Copy the binary from the previous stage | |
| COPY --from=base /app/main . | |
| # Copy the static files from the previous stage | |
| COPY --from=base /app/static ./static | |
| # Expose the port on which the application will run | |
| EXPOSE 8080 | |
| # Command to run the application | |
| CMD ["./main"] | |
| FROM gcr.io/distroless/base | |
| # Copy the binary from the previous stage | |
| COPY --from=base /app/main . | |
| # Copy the static files from the previous stage | |
| COPY --from=base /app/static ./static | |
| USER 65532:65532 | |
| # Expose the port on which the application will run | |
| EXPOSE 8080 | |
| # Command to run the application | |
| CMD ["./main"] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` around lines 26 - 38, Set an unprivileged USER in the final
Docker image stage before the CMD instruction, ensuring the application launched
by ./main runs without root privileges.
Source: Linters/SAST tools
| replicas: 1 | ||
| selector: | ||
| matchLabels: | ||
| app: go-web-app | ||
| template: | ||
| metadata: | ||
| labels: | ||
| app: go-web-app | ||
| spec: | ||
| containers: | ||
| - name: go-web-app | ||
| image: bibhishandhagate/go-web-app:{{ .Values.image.tag }} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked relevant files:\n'
git ls-files | rg '(^|/)helm/go-web-app-chart/(templates/deployment\.yml|values\.yaml)$|(^|/)k8s/manifests/deployment\.yml$' || true
printf '\nhelm/go-web-app-chart/templates/deployment.yml:\n'
if [ -f helm/go-web-app-chart/templates/deployment.yml ]; then
cat -n helm/go-web-app-chart/templates/deployment.yml
fi
printf '\nhelm/go-web-app-chart/values.yaml:\n'
if [ -f helm/go-web-app-chart/values.yaml ]; then
cat -n helm/go-web-app-chart/values.yaml
fi
printf '\nSearch for replicaCount/image.repository/image.pullPolicy usages in chart:\n'
rg -n 'replicaCount|image:|repository:|pullPolicy:|tag:' helm/go-web-app-chart || trueRepository: iam-veeramalla/go-web-app
Length of output: 2212
Use declared Helm values in the Deployment template.
helm/go-web-app-chart/templates/deployment.yml hardcodes replicas: 1, omits image.pullPolicy, and uses bibhishandhagate/go-web-app instead of image.repository. Render .Values.replicaCount, .Values.image.repository, and .Values.image.pullPolicy so custom values.yaml overrides work.
📍 Affects 2 files
helm/go-web-app-chart/templates/deployment.yml#L9-L20(this comment)helm/go-web-app-chart/values.yaml#L5-L11
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/go-web-app-chart/templates/deployment.yml` around lines 9 - 20, The
Deployment template’s container settings ignore declared Helm values. In
helm/go-web-app-chart/templates/deployment.yml lines 9-20, replace the hardcoded
replica count and image repository with .Values.replicaCount and
.Values.image.repository, and add imagePullPolicy using
.Values.image.pullPolicy. helm/go-web-app-chart/values.yaml lines 5-11 requires
no direct change; it defines the values consumed by the template.
| annotations: | ||
| nginx.ingress.kubernetes.io/rewrite-target: / |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not rewrite application routes to /.
The annotation rewrites /home, /about, and other paths to /. main.go:28-39 does not register a / handler, so these requests return 404 through the Ingress. Remove this annotation to preserve the request path.
Proposed fix
metadata:
name: go-web-app
- annotations:
- nginx.ingress.kubernetes.io/rewrite-target: /
spec:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/go-web-app-chart/templates/ingress.yml` around lines 6 - 7, Remove the
nginx.ingress.kubernetes.io/rewrite-target annotation from the Ingress manifest
so requests preserve their original application paths, including /home and
/about. Leave the remaining Ingress configuration unchanged.
| spec: | ||
| containers: | ||
| - name: go-web-app | ||
| image: bibhishandhagate/go-web-app:v1 | ||
| ports: | ||
| - containerPort: 8080 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)(deployment\.ya?ml|values\.ya?ml|Chart\.ya?ml)$|helm/go-web-app-chart|k8s/manifests/deployment.yml' || true
echo
echo "Manifest snippets:"
for f in k8s/manifests/deployment.yml helm/go-web-app-chart/templates/deployment.yml; do
if [ -f "$f" ]; then
echo "--- $f"
wc -l "$f"
cat -n "$f"
else
echo "MISSING $f"
fi
done
echo
echo "Chart values:"
if [ -f helm/go-web-app-chart/values.yaml ]; then
cat -n helm/go-web-app-chart/values.yaml
fi
echo
echo "Security context searches:"
rg -n "securityContext|runAsNonRoot|allowPrivilegeEscalation|readOnlyRootFilesystem|seccompProfile|capabilities|drop:|add:" k8s helm go-web-app-chart 2>/dev/null || true
echo
echo "Diff stat/name-only:"
git diff --stat
git diff --name-onlyRepository: iam-veeramalla/go-web-app
Length of output: 2638
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe: check whether a representative Kubernetes manifest has the minimum recommended
# security context fields on the Pod spec and container spec.
python3 - <<'PY'
from pathlib import Path
import re
paths = [
Path("k8s/manifests/deployment.yml"),
Path("helm/go-web-app-chart/templates/deployment.yml"),
]
for path in paths:
print(path)
text = path.read_text() if path.exists() else ""
checks = {
"spec.securityContext.runAsNonRoot": bool(re.search(r'spec:\s*\n(?P<block>(?:[ \t]+[^\n#]*\n)*)?(?:[ \t]+securityContext:\n(?:[ \t]+runAsNonRoot:\s*true)|\s*runAsNonRoot:\s*true)', text)),
"container.securityContext.allowPrivilegeEscalation": bool(re.search(r'name:\s*go-web-app(?P<block>[\s\S]*?)(?:\n\s{6,8}allowPrivilegeEscalation:\s*false|allowPrivilegeEscalation:\s*false)', text)),
"container.securityContext.readOnlyRootFilesystem": bool(re.search(r'containerPort:\s*8080(?P<block>[\s\S]*?)(?:\n\s{6,8}readOnlyRootFilesystem:\s*true|readOnlyRootFilesystem:\s*true)', text)),
"container.securityContext.seccompProfileRuntimeDefault": bool(re.search(r'containerPort:\s*8080(?P<block>[\s\S]*?)(?:\n\s{6,8}seccompProfile:\s*\n\s{8,10}type:\s*RuntimeDefault|seccompProfile:\s*\n\s{8,10}type:\s*RuntimeDefault)', text)),
"container.securityContext.capabilities.drop": bool(re.search(r'containerPort:\s*8080(?P<block>[\s\S]*?)(?:\n\s{6,8}capabilities:\s*\n\s{8,10}drop:\s*\n(?:\s{10,12}-\s+ALL|\s{10,12}-\s+(\p{L}|\p{N})+))', text)),
}
for name, ok in checks.items():
print(f" {name}: {'present' if ok else 'absent'}")
PYRepository: iam-veeramalla/go-web-app
Length of output: 155
Apply one restricted security policy to both Deployment paths.
Both manifests rely on Kubernetes default container security settings. Add pod-level runAsNonRoot, container-level allowPrivilegeEscalation: false, readOnlyRootFilesystem: true, a RuntimeDefault seccomp profile, and dropped capabilities.
k8s/manifests/deployment.yml#L17-L22: Add the pod and container security contexts.helm/go-web-app-chart/templates/deployment.yml#L17-L22: Add the same settings to the Helm template.
🧰 Tools
🪛 Checkov (3.3.9)
[medium] 2-22: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[medium] 2-22: Minimize the admission of root containers
(CKV_K8S_23)
🪛 Trivy (0.72.0)
[error] 19-22: Root file system is not read-only
Container 'go-web-app' of Deployment 'go-web-app' should set 'securityContext.readOnlyRootFilesystem' to true
Rule: KSV-0014
(IaC/Kubernetes)
[error] 19-22: Default security context configured
container go-web-app in default namespace is using the default security context
Rule: KSV-0118
(IaC/Kubernetes)
[error] 17-22: Default security context configured
deployment go-web-app in default namespace is using the default security context, which allows root privileges
Rule: KSV-0118
(IaC/Kubernetes)
📍 Affects 2 files
k8s/manifests/deployment.yml#L17-L22(this comment)helm/go-web-app-chart/templates/deployment.yml#L17-L22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@k8s/manifests/deployment.yml` around lines 17 - 22, Update the pod spec in
k8s/manifests/deployment.yml at lines 17-22 and
helm/go-web-app-chart/templates/deployment.yml at lines 17-22 with identical
security settings: pod-level runAsNonRoot and a RuntimeDefault seccomp profile,
plus container-level allowPrivilegeEscalation false, readOnlyRootFilesystem
true, and all capabilities dropped. Apply the settings to both Deployment paths
without changing unrelated configuration.
Source: Linters/SAST tools
2 participants
test changes related to ticket 1
Summary by CodeRabbit
New Features
Documentation
Style