Add timezone information to the available fields #33
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,47 @@ | ||||||||||||||||||||||||||
| name: Build | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||
| workflow_dispatch: | ||||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||||
| branches: | ||||||||||||||||||||||||||
| - master | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
Comment on lines
+3
to
+8
There was a problem hiding this comment. Consider adding a PR build (no push) trigger, or explicitly document “master-only push” behavior. Right now this only runs on 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||
| build: | ||||||||||||||||||||||||||
| name: Build & deploy | ||||||||||||||||||||||||||
| runs-on: ubuntu-22.04 | ||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||
| - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 #v6.0.1 | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - name: Login to Docker Hub | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| uses: docker/login-action@5e57cd118135c172c3672efd75eb46360885c0ef #v3.6.0 | ||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||
| username: ${{ secrets.ACR_USERNAME }} | ||||||||||||||||||||||||||
| password: ${{ secrets.ACR_PASSWORD }} | ||||||||||||||||||||||||||
| registry: cratecache.azurecr.io/new-black | ||||||||||||||||||||||||||
|
Comment on lines
+16
to
+21
There was a problem hiding this comment. Fix the registry configuration. The 🔧 Proposed fix - name: Login to Docker Hub
uses: docker/login-action@5e57cd118135c172c3672efd75eb46360885c0ef #v3.6.0
with:
username: ${{ secrets.ACR_USERNAME }}
password: ${{ secrets.ACR_PASSWORD }}
- registry: cratecache.azurecr.io/new-black
+ registry: cratecache.azurecr.io📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsThere was a problem hiding this comment.
The Useful? React with 👍 / 👎. |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
Comment on lines
+16
to
+22
There was a problem hiding this comment. Step name is misleading (this logs into ACR, not Docker Hub). Rename the step to avoid confusion during incident/debugging. Proposed diff- - name: Login to Docker Hub
+ - name: Login to Azure Container Registry (ACR)
uses: docker/login-action@5e57cd118135c172c3672efd75eb46360885c0ef #v3.6.0
with:
username: ${{ secrets.ACR_USERNAME }}
password: ${{ secrets.ACR_PASSWORD }}
registry: cratecache.azurecr.io/new-black🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| # Switch to a different builder | ||||||||||||||||||||||||||
| - name: Set up Docker Buildx | ||||||||||||||||||||||||||
| id: buildx | ||||||||||||||||||||||||||
| uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f #3.12.0 | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify the docker/setup-buildx-action version
gh api repos/docker/setup-buildx-action/tags --jq '.[] | select(.commit.sha | startswith("8d2750c")) | {name, commit: .commit.sha}'Repository: PeterDaveHello/ipinfo.tw Length of output: 205 Update version comment to match Docker's release tag naming convention. The version comment should be 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Determine the run number | ||||||||||||||||||||||||||
| - name: Set the run number | ||||||||||||||||||||||||||
| id: commit-id | ||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||
| echo "build_number=$GITHUB_RUN_NUMBER" >> $GITHUB_ENV | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
Comment on lines
+29
to
+33
There was a problem hiding this comment. Minor: quote
Proposed diff- - name: Set the run number
- id: commit-id
+ - name: Set the run number
+ id: build-number
run: |
- echo "build_number=$GITHUB_RUN_NUMBER" >> $GITHUB_ENV
+ echo "build_number=$GITHUB_RUN_NUMBER" >> "$GITHUB_ENV"🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| # Build the production image | ||||||||||||||||||||||||||
| - name: Build production image | ||||||||||||||||||||||||||
| uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 #v6.18.0 | ||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||
| context: . | ||||||||||||||||||||||||||
| platforms: linux/amd64,linux/arm64 | ||||||||||||||||||||||||||
| push: true | ||||||||||||||||||||||||||
| target: release | ||||||||||||||||||||||||||
| build-args: | | ||||||||||||||||||||||||||
| MAXMIND_LICENSE_KEY=${{ secrets.MAXMIND_LICENSE_KEY }} | ||||||||||||||||||||||||||
| tags: | | ||||||||||||||||||||||||||
| cratecache.azurecr.io/new-black/eva-whoami:latest | ||||||||||||||||||||||||||
| cratecache.azurecr.io/new-black/eva-whoami:${{ env.build_number }} | ||||||||||||||||||||||||||
|
Comment on lines
+44
to
+46
There was a problem hiding this comment. Critical: Image names don't match the project. The workflow builds and pushes images tagged as The image names should reflect this project (e.g., 🔧 Proposed fix (verify correct names first) tags: |
- cratecache.azurecr.io/new-black/eva-whoami:latest
- cratecache.azurecr.io/new-black/eva-whoami:${{ env.build_number }}
+ cratecache.azurecr.io/new-black/ipinfo:latest
+ cratecache.azurecr.io/new-black/ipinfo:${{ env.build_number }}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+44
to
+46
Comment on lines
+45
to
+46
There was a problem hiding this comment.
This workflow only tags and pushes Useful? React with 👍 / 👎. |
||||||||||||||||||||||||||
| builder: ${{ steps.buildx.outputs.name }} | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,12 +55,12 @@ Use any http(s) client to explore the server, e.g. https://ipinfo.tw, | |
| - `wget -qO- https://ipinfo.tw` | ||
| - `curl https://ipinfo.tw` | ||
|
|
||
| Without any specified URI, the server will return IP address, country, timezone, AS, and user agent. | ||
|
There was a problem hiding this comment.
The README now states that Useful? React with 👍 / 👎. |
||
|
|
||
| If you prefer to receive a machine-readable result, use path `/json` (without trailing slash), e.g. `https://ipinfo.tw/json`, the result will look like: | ||
|
|
||
| ```json | ||
| {"ip":"3.115.123.234","country_code":"JP","country_name":"Japan","timezone":"Asia/Tokyo","asn":"16509","as_desc":"Amazon.com, Inc.","user_agent":"curl/7.58.0"} | ||
| ``` | ||
|
|
||
| #### Endpoints | ||
|
Comment on lines
55
to
66
There was a problem hiding this comment.
The description of the demo setup mentions "an reverse proxy" which should be corrected to "a reverse proxy" for grammatical accuracy. - this demo is behind an reverse proxy with https enabled
+ this demo is behind a reverse proxy with https enabled
There's a minor typographical error with "http traffic" which should be capitalized as "HTTP traffic" for consistency with standard terminology. - http traffic will be redirected to use https
+ HTTP traffic will be redirected to use https
The phrase "pass the it to the container" seems to contain an extra word. It should be corrected for clarity. - set up an `X-Real-IP` header and pass the it to the container
+ set up an `X-Real-IP` header and pass it to the container |
||
|
|
@@ -75,6 +75,7 @@ You can also specify the following URI to retrieve certain info: | |
| - `asn`: AS number | ||
| - `as_desc`: AS description | ||
| - `user_agent`: User agent string | ||
| - `timezone`: Timezone based on the city (e.g Europe/Amsterdam) | ||
|
|
||
|
|
||
| Examples: | ||
|
|
||
|
|
@@ -97,6 +98,9 @@ HK | |
| $ curl https://ipinfo.tw/country_name | ||
| South Korea | ||
|
|
||
| $ curl https://ipinfo.tw/timezone | ||
| Europe/Amsterdam | ||
|
|
||
| $ curl https://ipinfo.tw/as | ||
| AS16509 / Amazon.com, Inc. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,39 @@ | ||||||||||||||||||
| apiVersion: apps/v1 | ||||||||||||||||||
| kind: Deployment | ||||||||||||||||||
| metadata: | ||||||||||||||||||
| labels: | ||||||||||||||||||
| app: eva-whoami | ||||||||||||||||||
| name: eva-whoami | ||||||||||||||||||
| namespace: platform-tools | ||||||||||||||||||
| spec: | ||||||||||||||||||
| progressDeadlineSeconds: 600 | ||||||||||||||||||
| replicas: 2 | ||||||||||||||||||
| revisionHistoryLimit: 10 | ||||||||||||||||||
| selector: | ||||||||||||||||||
| matchLabels: | ||||||||||||||||||
| app: eva-whoami | ||||||||||||||||||
| strategy: | ||||||||||||||||||
| rollingUpdate: | ||||||||||||||||||
| maxSurge: 25% | ||||||||||||||||||
| maxUnavailable: 25% | ||||||||||||||||||
| type: RollingUpdate | ||||||||||||||||||
| template: | ||||||||||||||||||
| metadata: | ||||||||||||||||||
| labels: | ||||||||||||||||||
| app: eva-whoami | ||||||||||||||||||
| spec: | ||||||||||||||||||
| containers: | ||||||||||||||||||
| - image: cratecache.azurecr.io/new-black/eva-whoami | ||||||||||||||||||
|
There was a problem hiding this comment. WARNING: Image reference has no explicit tag.
Note: this whole |
||||||||||||||||||
| imagePullPolicy: Always | ||||||||||||||||||
| name: whoami | ||||||||||||||||||
|
Comment on lines
+25
to
+28
|
||||||||||||||||||
| ports: | ||||||||||||||||||
| - containerPort: 8080 | ||||||||||||||||||
| protocol: TCP | ||||||||||||||||||
| resources: {} | ||||||||||||||||||
|
There was a problem hiding this comment. Define resource limits and requests. The deployment has no resource constraints, which can lead to resource contention, unpredictable performance, and potential node exhaustion. Production deployments should always specify CPU and memory limits/requests. ⚙️ Proposed resource configuration- resources: {}
+ resources:
+ requests:
+ cpu: 100m
+ memory: 128Mi
+ limits:
+ cpu: 500m
+ memory: 256MiAdjust values based on actual application requirements. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| terminationMessagePath: /dev/termination-log | ||||||||||||||||||
| terminationMessagePolicy: File | ||||||||||||||||||
| dnsPolicy: ClusterFirst | ||||||||||||||||||
| restartPolicy: Always | ||||||||||||||||||
| schedulerName: default-scheduler | ||||||||||||||||||
| securityContext: {} | ||||||||||||||||||
|
Comment on lines
+25
to
+38
There was a problem hiding this comment. Add security context to prevent privilege escalation. The deployment lacks security context constraints, leaving it vulnerable to privilege escalation and running as root. This violates Kubernetes security best practices and Pod Security Standards. 🔒 Proposed security hardening containers:
- image: cratecache.azurecr.io/new-black/eva-whoami
imagePullPolicy: Always
name: whoami
ports:
- containerPort: 8080
protocol: TCP
resources: {}
+ securityContext:
+ allowPrivilegeEscalation: false
+ runAsNonRoot: true
+ runAsUser: 10001
+ capabilities:
+ drop:
+ - ALL
+ readOnlyRootFilesystem: true
terminationMessagePath: /dev/termination-log
terminationMessagePolicy: File
dnsPolicy: ClusterFirst
restartPolicy: Always
schedulerName: default-scheduler
- securityContext: {}
+ securityContext:
+ runAsNonRoot: true
+ seccompProfile:
+ type: RuntimeDefault
terminationGracePeriodSeconds: 30Note: If the application requires write access, mount an emptyDir volume for temporary storage instead of using a writable root filesystem. Based on static analysis hints. |
||||||||||||||||||
| terminationGracePeriodSeconds: 30 | ||||||||||||||||||
|
Comment on lines
+1
to
+39
There was a problem hiding this comment. Critical mismatch: Files don't match PR objectives. The PR summary states this adds "timezone information to the available fields" for the ipinfo.tw project with nginx configuration changes, but this file deploys an entirely different service (eva-whoami) in Kubernetes. This appears to be either:
Please verify that the correct files are included in this PR. 🧰 Tools🪛 Checkov (3.2.334)[medium] 1-39: Containers should not run with allowPrivilegeEscalation (CKV_K8S_20) [medium] 1-39: Minimize the admission of root containers (CKV_K8S_23) |
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| apiVersion: networking.k8s.io/v1 | ||
| kind: Ingress | ||
| metadata: | ||
| annotations: | ||
| cert-manager.io/acme-challenge-type: dns01 | ||
| cert-manager.io/cluster-issuer: letsencrypt-prod-dns | ||
| kubernetes.io/ingress.class: nginx | ||
|
There was a problem hiding this comment. Remove deprecated ingress class annotation. The deprecated annotation 🧹 Proposed cleanup annotations:
cert-manager.io/acme-challenge-type: dns01
cert-manager.io/cluster-issuer: letsencrypt-prod-dns
- kubernetes.io/ingress.class: nginx
generation: 2Also applies to: 14-14 🤖 Prompt for AI Agents |
||
| generation: 2 | ||
| labels: | ||
| name: eva-whoami | ||
| name: eva-whoami | ||
| namespace: platform-tools | ||
| spec: | ||
| ingressClassName: nginx | ||
| rules: | ||
| - host: whoami.on-eva.io | ||
| http: | ||
| paths: | ||
| - backend: | ||
| service: | ||
| name: eva-whoami | ||
| port: | ||
| number: 8080 | ||
| path: / | ||
| pathType: ImplementationSpecific | ||
| tls: | ||
| - hosts: | ||
| - whoami.on-eva.io | ||
| secretName: eva-whoami-tls | ||
|
Comment on lines
+15
to
+29
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| annotations: | ||
| service.beta.kubernetes.io/azure-load-balancer-resource-group: rg-prod-eva | ||
| labels: | ||
| component: eva-whoami | ||
| name: eva-whoami | ||
| namespace: platform-tools | ||
| spec: | ||
| ipFamilies: | ||
| - IPv4 | ||
| ipFamilyPolicy: SingleStack | ||
| ports: | ||
| - name: whoami | ||
| nodePort: 31514 | ||
| port: 8080 | ||
| protocol: TCP | ||
| targetPort: 8080 | ||
| selector: | ||
| app: eva-whoami | ||
| sessionAffinity: None | ||
| type: LoadBalancer | ||
|
Comment on lines
+4
to
+23
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,3 +11,8 @@ geoip2 /usr/share/GeoIP/GeoLite2-ASN.mmdb { | |
| $ip_aso source=$remote_addr autonomous_system_organization; | ||
| $ip_as_build_epoch metadata build_epoch; | ||
| } | ||
|
|
||
| geoip2 /usr/share/GeoIP/GeoLite2-City.mmdb { | ||
| auto_reload 1d; | ||
| $ip_time_zone source=$remote_addr location time_zone; | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,9 @@ server { | |
| location = /user_agent { | ||
| return 200 "$http_user_agent\n"; | ||
| } | ||
| location = /timezone { | ||
| return 200 "$ip_time_zone\n"; | ||
| } | ||
| location ~* ^/index.htm(l)?$ { | ||
| rewrite ^(.*)$ /; | ||
| } | ||
|
|
@@ -58,7 +61,7 @@ server { | |
| } | ||
| location = /json { | ||
| default_type application/json; | ||
| return 200 "{\"ip\":\"$remote_addr\",\"country_code\":\"$ip_country_code\",\"country_name\":\"$ip_country_name\",\"timezone\":\"$ip_time_zone\",\"asn\":\"$ip_asn\",\"as_desc\":\"$ip_aso\",\"user_agent\":\"$http_user_agent\"}\n"; | ||
| } | ||
| location = /build_epoch { | ||
| default_type application/json; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| real_ip_header X-Forwarded-For; | ||
|
There was a problem hiding this comment. WARNING: Breaking change unrelated to the timezone feature. This PR's stated scope is adding timezone data, but it also changes the trusted real-IP header from
There was a problem hiding this comment.
Switching Useful? React with 👍 / 👎. |
||
| real_ip_recursive on; | ||
| set_real_ip_from 10.0.0.0/8; | ||
| set_real_ip_from 172.16.0.0/12; | ||
| set_real_ip_from 192.168.0.0/16; | ||
|
There was a problem hiding this comment. SUGGESTION: Missing newline at end of file (the diff explicitly notes
Comment on lines
+3
to
+5
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify the deployment configuration and network topology
# Check for Kubernetes Service/Ingress configurations that show the network path
fd -e yaml -e yml . --exec rg -l 'kind:\s*(Service|Ingress|LoadBalancer)'
# Look for documentation about the deployment architecture
fd -g '*README*' -g '*DEPLOY*' -g '*ARCHITECTURE*' --exec cat {}Repository: PeterDaveHello/ipinfo.tw Length of output: 281 🏁 Script executed: # Read the Kubernetes service configuration
cat -n ./k8s/service.yamlRepository: PeterDaveHello/ipinfo.tw Length of output: 681 🏁 Script executed: # Read the Kubernetes ingress configuration
cat -n ./k8s/ingress.yamlRepository: PeterDaveHello/ipinfo.tw Length of output: 910 🏁 Script executed: # Search for README or deployment documentation
find . -maxdepth 2 -type f \( -iname "*readme*" -o -iname "*architecture*" -o -iname "*deployment*" \) | head -20Repository: PeterDaveHello/ipinfo.tw Length of output: 102 🏁 Script executed: # Check the complete nginx realip.conf for context
cat -n ./nginx/conf.d/realip.confRepository: PeterDaveHello/ipinfo.tw Length of output: 261 🏁 Script executed: # Check if there's a main nginx.conf to see if real_ip_recursive is set
cat -n ./nginx/nginx.confRepository: PeterDaveHello/ipinfo.tw Length of output: 1079 🏁 Script executed: cat -n ./k8s/deployment.yamlRepository: PeterDaveHello/ipinfo.tw Length of output: 1244 🏁 Script executed: # Check if there are network policies that restrict access
fd -e yaml . | xargs grep -l "NetworkPolicy" 2>/dev/null || echo "No NetworkPolicy found"Repository: PeterDaveHello/ipinfo.tw Length of output: 91 🏁 Script executed: # Check README for deployment/architecture info
head -100 ./README.mdRepository: PeterDaveHello/ipinfo.tw Length of output: 3683 Restrict RFC1918 ranges or verify network isolation to prevent IP spoofing attacks. The nginx configuration trusts all private address ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16) to provide the real client IP via The Kubernetes Service is configured as type
This is only secure if network policies or cloud firewalls restrict the LoadBalancer to trusted sources. Instead:
🤖 Prompt for AI Agents |
||
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.
The workflow is configured to run on pushes to
master, but this repository usesmain(there is nomasterbranch), so ordinary pushes will not execute this build-and-publish pipeline at all. That means image publication and CI validation silently stop for normal development flow until the branch filter is corrected.Useful? React with 👍 / 👎.