carverauto/serviceradar

Fork 0

WIP kong rpm updates #2241

Merged

mfreeman451 merged 13 commits from refs/pull/2241/head into main

2025-09-26 16:30:11 +00:00

mfreeman451 commented

2025-09-25 03:19:01 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1661
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1661
Original created: 2025-09-25T03:19:01Z
Original updated: 2025-09-26T16:30:35Z
Original head: carverauto/serviceradar:kong/rpm_packaging_fixes
Original base: main
Original merged: 2025-09-26T16:30:11Z by @mfreeman451

PR Type

Enhancement

Description

Add systemd service integration for Kong Gateway
Fix certificate timezone/clock skew issues
Improve RPM packaging with user management
Add Kong wrapper script for proper service execution

Diagram Walkthrough

flowchart LR
  A["Certificate Generation"] -- "Fix timing" --> B["TLS Certificates"]
  C["Kong Wrapper Script"] -- "Manages" --> D["Kong Service"]
  E["Systemd Service"] -- "Uses" --> C
  F["RPM Package"] -- "Installs" --> E
  F -- "Creates" --> G["Kong User/Group"]

File Walkthrough

Relevant files

Bug fix

tls.go

Fix certificate timing for clock skew

pkg/cli/tls.go

Set certificate NotBefore time to 1 hour ago
Prevents timezone and clock skew issues
Applied to both root CA and service certificates

+2/-2

Enhancement

kong-wrapper.sh `Add Kong systemd service wrapper` packaging/kong/scripts/kong-wrapper.sh New wrapper script for Kong systemd service Sets up Kong prefix directory and permissions Ensures proper ownership for kong user Executes Kong commands with configuration	+24/-0
postinstall.sh `Enhance postinstall with user creation` packaging/kong/scripts/postinstall.sh Create kong user and group during installation Update systemd service name to `serviceradar-kong` Add systemd daemon reload before service operations	+12/-3
preremove.sh `Add service cleanup on removal` packaging/kong/scripts/preremove.sh Stop and disable `serviceradar-kong` service before removal Ensure clean service shutdown during uninstall	+6/-0
serviceradar-kong.service `Add systemd service configuration` packaging/kong/systemd/serviceradar-kong.service New systemd service file for ServiceRadar Kong Gateway Configures service type, environment, and execution commands Sets resource limits and restart policies Uses kong-wrapper.sh for service management	+29/-0
serviceradar-kong.spec `Update RPM spec for systemd support` packaging/specs/serviceradar-kong.spec Install systemd service file and kong-wrapper script Add systemd macros for proper service lifecycle Include new files in package manifest Update post/preun/postun sections for systemd integration	+13/-2

Imported from GitHub pull request. Original GitHub pull request: #1661 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1661 Original created: 2025-09-25T03:19:01Z Original updated: 2025-09-26T16:30:35Z Original head: carverauto/serviceradar:kong/rpm_packaging_fixes Original base: main Original merged: 2025-09-26T16:30:11Z by @mfreeman451 --- ### **PR Type** Enhancement ___ ### **Description** - Add systemd service integration for Kong Gateway - Fix certificate timezone/clock skew issues - Improve RPM packaging with user management - Add Kong wrapper script for proper service execution ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Certificate Generation"] -- "Fix timing" --> B["TLS Certificates"] C["Kong Wrapper Script"] -- "Manages" --> D["Kong Service"] E["Systemd Service"] -- "Uses" --> C F["RPM Package"] -- "Installs" --> E F -- "Creates" --> G["Kong User/Group"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>tls.go</strong><dd><code>Fix certificate timing for clock skew</code>                                        </dd></summary> <hr> pkg/cli/tls.go <ul><li>Set certificate <code>NotBefore</code> time to 1 hour ago<br> <li> Prevents timezone and clock skew issues<br> <li> Applied to both root CA and service certificates</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1661/files#diff-534c10f465835ba604a2ed428ec8924f5be332786a3bdb67cc351c800af65cdc">+2/-2</a>      </td> </tr> </table></td></tr><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>kong-wrapper.sh</strong><dd><code>Add Kong systemd service wrapper</code>                                                  </dd></summary> <hr> packaging/kong/scripts/kong-wrapper.sh <ul><li>New wrapper script for Kong systemd service<br> <li> Sets up Kong prefix directory and permissions<br> <li> Ensures proper ownership for kong user<br> <li> Executes Kong commands with configuration</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1661/files#diff-f2be05ff9301bc7c31ed63663d444abaa92ee28ab76f98ab99d883c287fe307a">+24/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>postinstall.sh</strong><dd><code>Enhance postinstall with user creation</code>                                      </dd></summary> <hr> packaging/kong/scripts/postinstall.sh <ul><li>Create kong user and group during installation<br> <li> Update systemd service name to <code>serviceradar-kong</code><br> <li> Add systemd daemon reload before service operations</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1661/files#diff-4bb6bba6f4cee56343c5049aa2ffb923204a798bcb170311a50b3a18f5256629">+12/-3</a>    </td> </tr> <tr> <td> <details> <summary><strong>preremove.sh</strong><dd><code>Add service cleanup on removal</code>                                                      </dd></summary> <hr> packaging/kong/scripts/preremove.sh <ul><li>Stop and disable <code>serviceradar-kong</code> service before removal<br> <li> Ensure clean service shutdown during uninstall</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1661/files#diff-25b974b8effdac6366c6f573b0311b59c2cb937f8cd16535f887ee262c5a4d1c">+6/-0</a>      </td> </tr> <tr> <td> <details> <summary><strong>serviceradar-kong.service</strong><dd><code>Add systemd service configuration</code>                                                </dd></summary> <hr> packaging/kong/systemd/serviceradar-kong.service <ul><li>New systemd service file for ServiceRadar Kong Gateway<br> <li> Configures service type, environment, and execution commands<br> <li> Sets resource limits and restart policies<br> <li> Uses kong-wrapper.sh for service management</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1661/files#diff-7e89139af8658e78c44d4e80e4798ba4d29848cc8bf2b684a8b9680a766c6879">+29/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>serviceradar-kong.spec</strong><dd><code>Update RPM spec for systemd support</code>                                            </dd></summary> <hr> packaging/specs/serviceradar-kong.spec <ul><li>Install systemd service file and kong-wrapper script<br> <li> Add systemd macros for proper service lifecycle<br> <li> Include new files in package manifest<br> <li> Update post/preun/postun sections for systemd integration</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1661/files#diff-e1be0e25d1d167a40c46c2b32773948a0e0df715f27f9cf4c011fec892b27583">+13/-2</a>    </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

CLAassistant commented

2025-09-25 03:19:07 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @CLAassistant
Original URL: https://github.com/carverauto/serviceradar/pull/1661#issuecomment-3331891581
Original created: 2025-09-25T03:19:07Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

mfreeman seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Imported GitHub PR comment. Original author: @CLAassistant Original URL: https://github.com/carverauto/serviceradar/pull/1661#issuecomment-3331891581 Original created: 2025-09-25T03:19:07Z --- [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/carverauto/serviceradar?pullRequest=1661) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/carverauto/serviceradar?pullRequest=1661) before we can accept your contribution.<br/><hr/>**mfreeman** seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please [add the email address used for this commit to your account](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user).<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/carverauto/serviceradar?pullRequest=1661) it.</sub>

qodo-code-review[bot] commented

2025-09-25 03:19:46 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1661#issuecomment-3331896851
Original created: 2025-09-25T03:19:46Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 Security concerns

Service privilege and permissions:
The systemd unit runs as root and the wrapper chowns /etc/kong and /usr/local/kong recursively. Running Kong as root increases blast radius; consider running the service as the 'kong' user and limiting chown scope to necessary paths. Additionally, creating a system user with shell '/sbin/nologin' is good, but ensure config files and logs don't become world-readable after ownership changes.

⚡ Recommended focus areas for review

Behavior Change

Setting certificate NotBefore to one hour in the past changes validity windows; confirm this aligns with compliance and testing expectations, and consider making the skew configurable instead of hard-coded.

NotBefore:             time.Now().Add(-1 * time.Hour), // Set to 1 hour ago to avoid timezone/clock skew issues
NotAfter:              time.Now().Add(defaultDaysValid * 24 * time.Hour),

Privilege Model

Service runs as root while wrapper ensures kong-owned dirs; verify Kong itself drops privileges or run service as kong user to minimize risk, and ensure file permissions allow non-root operation.

User=root
Group=root
StandardOutput=journal

Exec Arg Handling

Appending '-c /etc/kong/kong.conf' after all user args may conflict with commands that expect options before subcommands; validate for 'start', 'reload', etc., and consider placing config earlier or allowing override.

# Export necessary environment variables
export KONG_PREFIX="$KONG_PREFIX"

# Execute the Kong command passed as arguments
exec /usr/bin/kong "$@" -c "$KONG_CONF"

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1661#issuecomment-3331896851 Original created: 2025-09-25T03:19:46Z --- ## PR Reviewer Guide 🔍 Here are some key observations to aid the review process: <table> <tr><td>⏱️ <strong>Estimated effort to review</strong>: 3 🔵🔵🔵⚪⚪</td></tr> <tr><td>🧪 <strong>No relevant tests</strong></td></tr> <tr><td>🔒 <strong>Security concerns</strong><br><br> <strong>Service privilege and permissions:</strong><br> The systemd unit runs as root and the wrapper chowns /etc/kong and /usr/local/kong recursively. Running Kong as root increases blast radius; consider running the service as the 'kong' user and limiting chown scope to necessary paths. Additionally, creating a system user with shell '/sbin/nologin' is good, but ensure config files and logs don't become world-readable after ownership changes.</td></tr> <tr><td>⚡ <strong>Recommended focus areas for review</strong><br><br> <details><summary><a href='https://github.com/carverauto/serviceradar/pull/1661/files#diff-534c10f465835ba604a2ed428ec8924f5be332786a3bdb67cc351c800af65cdcR300-R301'><strong>Behavior Change</strong></a> Setting certificate NotBefore to one hour in the past changes validity windows; confirm this aligns with compliance and testing expectations, and consider making the skew configurable instead of hard-coded. </summary> ```go NotBefore: time.Now().Add(-1 * time.Hour), // Set to 1 hour ago to avoid timezone/clock skew issues NotAfter: time.Now().Add(defaultDaysValid * 24 * time.Hour), ``` </details> <details><summary><a href='https://github.com/carverauto/serviceradar/pull/1661/files#diff-7e89139af8658e78c44d4e80e4798ba4d29848cc8bf2b684a8b9680a766c6879R16-R18'><strong>Privilege Model</strong></a> Service runs as root while wrapper ensures kong-owned dirs; verify Kong itself drops privileges or run service as kong user to minimize risk, and ensure file permissions allow non-root operation. </summary> ```txt User=root Group=root StandardOutput=journal ``` </details> <details><summary><a href='https://github.com/carverauto/serviceradar/pull/1661/files#diff-f2be05ff9301bc7c31ed63663d444abaa92ee28ab76f98ab99d883c287fe307aR20-R24'><strong>Exec Arg Handling</strong></a> Appending '-c /etc/kong/kong.conf' after all user args may conflict with commands that expect options before subcommands; validate for 'start', 'reload', etc., and consider placing config earlier or allowing override. </summary> ```shell # Export necessary environment variables export KONG_PREFIX="$KONG_PREFIX" # Execute the Kong command passed as arguments exec /usr/bin/kong "$@" -c "$KONG_CONF" ``` </details> </td></tr> </table>

qodo-code-review[bot] commented

2025-09-25 03:20:47 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1661#issuecomment-3331904515
Original created: 2025-09-25T03:20:47Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Drop root privileges before executing To mitigate a security risk, modify `kong-wrapper.sh` to use `runuser -u kong` to execute the `kong` binary, dropping root privileges and running the service with the dedicated `kong` user. packaging/kong/scripts/kong-wrapper.sh [23-24] `-# Execute the Kong command passed as arguments -exec /usr/bin/kong "$@" -c "$KONG_CONF" +# Execute the Kong command passed as arguments as the 'kong' user +exec runuser -u kong -- /usr/bin/kong "$@" -c "$KONG_CONF"` Apply / Chat Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical security vulnerability where the Kong service runs as `root` and proposes a valid fix to drop privileges to the `kong` user before execution.	High
High-level	Run Kong service as non-root user The systemd service for Kong should run as the dedicated `kong` user instead of `root` to enhance security. This aligns with the PR's creation of a `kong` user and follows the principle of least privilege. Examples: packaging/kong/systemd/serviceradar-kong.service [16-17] `User=root Group=root` Solution Walkthrough: Before: `# packaging/kong/systemd/serviceradar-kong.service [Service] Type=forking Environment=KONG_PREFIX=/usr/local/kong ExecStartPre=/usr/share/serviceradar-kong/scripts/kong-wrapper.sh prepare ExecStart=/usr/share/serviceradar-kong/scripts/kong-wrapper.sh start ... User=root Group=root ...` After: `# packaging/kong/systemd/serviceradar-kong.service [Service] Type=forking Environment=KONG_PREFIX=/usr/local/kong ExecStartPre=/usr/share/serviceradar-kong/scripts/kong-wrapper.sh prepare ExecStart=/usr/share/serviceradar-kong/scripts/kong-wrapper.sh start ... User=kong Group=kong ...` Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical security flaw by pointing out that the service runs as `root` despite a dedicated `kong` user being created, which undermines the principle of least privilege.	High
Possible issue	Remove error suppression for chown Remove the `\|\| true` from the `chown` commands in `kong-wrapper.sh`. This will ensure the script fails immediately if ownership cannot be set, preventing the service from starting in a broken state. packaging/kong/scripts/kong-wrapper.sh [16-17] `-chown -R kong:kong "$KONG_PREFIX" \|\| true -chown -R kong:kong /etc/kong \|\| true +chown -R kong:kong "$KONG_PREFIX" +chown -R kong:kong /etc/kong` Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies that suppressing `chown` errors can lead to runtime failures. Removing `\|\| true` improves script robustness and ensures the service does not start in a misconfigured state.	Medium
Possible issue	Remove error suppression for mkdir In `kong-wrapper.sh`, remove the `\|\| true` from the `mkdir -p` command. This will allow the script to fail on legitimate errors, improving its robustness, as the `-p` flag already prevents failure if the directory exists. packaging/kong/scripts/kong-wrapper.sh [12] `-mkdir -p "$KONG_PREFIX" \|\| true +mkdir -p "$KONG_PREFIX"` Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that suppressing the exit code of `mkdir -p` can hide serious filesystem issues, and removing `\|\| true` improves the script's robustness by allowing it to fail fast.	Low
Update

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1661#issuecomment-3331904515 Original created: 2025-09-25T03:20:47Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>Security</td> <td> <details><summary>Drop root privileges before executing</summary> ___ **To mitigate a security risk, modify <code>kong-wrapper.sh</code> to use <code>runuser -u kong</code> to <br>execute the <code>kong</code> binary, dropping root privileges and running the service with <br>the dedicated <code>kong</code> user.** [packaging/kong/scripts/kong-wrapper.sh [23-24]](https://github.com/carverauto/serviceradar/pull/1661/files#diff-f2be05ff9301bc7c31ed63663d444abaa92ee28ab76f98ab99d883c287fe307aR23-R24) ```diff -# Execute the Kong command passed as arguments -exec /usr/bin/kong "$@" -c "$KONG_CONF" +# Execute the Kong command passed as arguments as the 'kong' user +exec runuser -u kong -- /usr/bin/kong "$@" -c "$KONG_CONF" ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 10</summary> __ Why: The suggestion correctly identifies a critical security vulnerability where the Kong service runs as `root` and proposes a valid fix to drop privileges to the `kong` user before execution. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>High-level</td> <td> <details><summary>Run Kong service as non-root user</summary> ___ **The systemd service for Kong should run as the dedicated <code>kong</code> user instead of <br><code>root</code> to enhance security. This aligns with the PR's creation of a <code>kong</code> user and <br>follows the principle of least privilege.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1661/files#diff-7e89139af8658e78c44d4e80e4798ba4d29848cc8bf2b684a8b9680a766c6879R16-R17">packaging/kong/systemd/serviceradar-kong.service [16-17]</a> </summary> ```ini User=root Group=root ``` </details> ### Solution Walkthrough: #### Before: ```ini # packaging/kong/systemd/serviceradar-kong.service [Service] Type=forking Environment=KONG_PREFIX=/usr/local/kong ExecStartPre=/usr/share/serviceradar-kong/scripts/kong-wrapper.sh prepare ExecStart=/usr/share/serviceradar-kong/scripts/kong-wrapper.sh start ... User=root Group=root ... ``` #### After: ```ini # packaging/kong/systemd/serviceradar-kong.service [Service] Type=forking Environment=KONG_PREFIX=/usr/local/kong ExecStartPre=/usr/share/serviceradar-kong/scripts/kong-wrapper.sh prepare ExecStart=/usr/share/serviceradar-kong/scripts/kong-wrapper.sh start ... User=kong Group=kong ... ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: This suggestion correctly identifies a critical security flaw by pointing out that the service runs as `root` despite a dedicated `kong` user being created, which undermines the principle of least privilege. </details></details></td><td align=center>High </td></tr><tr><td rowspan=2>Possible issue</td> <td> <details><summary>Remove error suppression for chown</summary> ___ **Remove the <code>|| true</code> from the <code>chown</code> commands in <code>kong-wrapper.sh</code>. This will ensure <br>the script fails immediately if ownership cannot be set, preventing the service <br>from starting in a broken state.** [packaging/kong/scripts/kong-wrapper.sh [16-17]](https://github.com/carverauto/serviceradar/pull/1661/files#diff-f2be05ff9301bc7c31ed63663d444abaa92ee28ab76f98ab99d883c287fe307aR16-R17) ```diff -chown -R kong:kong "$KONG_PREFIX" || true -chown -R kong:kong /etc/kong || true +chown -R kong:kong "$KONG_PREFIX" +chown -R kong:kong /etc/kong ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: This suggestion correctly identifies that suppressing `chown` errors can lead to runtime failures. Removing `|| true` improves script robustness and ensures the service does not start in a misconfigured state. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Remove error suppression for mkdir</summary> ___ **In <code>kong-wrapper.sh</code>, remove the <code>|| true</code> from the <code>mkdir -p</code> command. This will <br>allow the script to fail on legitimate errors, improving its robustness, as the <br><code>-p</code> flag already prevents failure if the directory exists.** [packaging/kong/scripts/kong-wrapper.sh [12]](https://github.com/carverauto/serviceradar/pull/1661/files#diff-f2be05ff9301bc7c31ed63663d444abaa92ee28ab76f98ab99d883c287fe307aR12-R12) ```diff -mkdir -p "$KONG_PREFIX" || true +mkdir -p "$KONG_PREFIX" ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: The suggestion correctly points out that suppressing the exit code of `mkdir -p` can hide serious filesystem issues, and removing `|| true` improves the script's robustness by allowing it to fail fast. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update  </td><td></td></tr></tbody></table>