carverauto/serviceradar

Fork 0

fixing kong build for rpm #2239

Merged

mfreeman451 merged 1 commit from refs/pull/2239/head into main

2025-09-24 22:50:58 +00:00

mfreeman451 commented

2025-09-24 22:49:30 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1659
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1659
Original created: 2025-09-24T22:49:30Z
Original updated: 2025-09-24T22:51:01Z
Original head: carverauto/serviceradar:updates/kong_updates
Original base: main
Original merged: 2025-09-24T22:50:58Z by @mfreeman451

PR Type

Bug fix, Enhancement

Description

Fix Kong package channel derivation for RPM builds
Add systemd-rpm-macros dependency for RPM packaging
Support "none" build method in package setup
Add OCSF alignment documentation and BuildBuddy config

Diagram Walkthrough

flowchart LR
  A["Kong Version"] --> B["derive_channel()"]
  B --> C["Channel Format"]
  C --> D["RPM Package"]
  E["Build Method"] --> F["Package Setup"]
  F --> G["Binary Stage"]

File Walkthrough

Relevant files

Bug fix

fetch-kong-artifacts.sh `Fix Kong package channel derivation logic` scripts/fetch-kong-artifacts.sh Add `derive_channel()` function to properly format Kong package channels Replace simple string replacement with major.minor channel logic Support configurable channel overrides via environment variables	+15/-2
Dockerfile.rpm.proton `Fix RPM Docker build dependencies` docker/rpm/Dockerfile.rpm.proton Add `systemd-rpm-macros` package dependency for RPM builds Remove trailing whitespace from final CMD line	+3/-2

Enhancement

setup-package.sh

Support "none" build method in packaging

scripts/setup-package.sh

Add support for "none" build method to skip binary building
Handle empty build_method values gracefully

+4/-1

Documentation

SRQL_OCSF_ALIGNMENT_PLAN.md

Add OCSF alignment plan documentation

SRQL_OCSF_ALIGNMENT_PLAN.md

Add comprehensive 389-line documentation for OCSF alignment
Define migration phases, query language evolution, and federation
architecture
Include implementation roadmap and technical considerations

+389/-0

Configuration changes

buildbuddy.yaml

Add BuildBuddy CI/CD configuration

buildbuddy.yaml

Add BuildBuddy configuration for remote execution and caching
Configure build event streaming and execution parameters

+30/-0

Imported from GitHub pull request. Original GitHub pull request: #1659 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1659 Original created: 2025-09-24T22:49:30Z Original updated: 2025-09-24T22:51:01Z Original head: carverauto/serviceradar:updates/kong_updates Original base: main Original merged: 2025-09-24T22:50:58Z by @mfreeman451 --- ### **PR Type** Bug fix, Enhancement ___ ### **Description** - Fix Kong package channel derivation for RPM builds - Add systemd-rpm-macros dependency for RPM packaging - Support "none" build method in package setup - Add OCSF alignment documentation and BuildBuddy config ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Kong Version"] --> B["derive_channel()"] B --> C["Channel Format"] C --> D["RPM Package"] E["Build Method"] --> F["Package Setup"] F --> G["Binary Stage"] ``` <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>fetch-kong-artifacts.sh</strong><dd><code>Fix Kong package channel derivation logic</code>                                </dd></summary> <hr> scripts/fetch-kong-artifacts.sh <ul><li>Add <code>derive_channel()</code> function to properly format Kong package channels<br> <li> Replace simple string replacement with major.minor channel logic<br> <li> Support configurable channel overrides via environment variables</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1659/files#diff-448068ce58b8f5324251c7496e00a7955fe479b20c43a3111b4f9187b90150cd">+15/-2</a>    </td> </tr> <tr> <td> <details> <summary><strong>Dockerfile.rpm.proton</strong><dd><code>Fix RPM Docker build dependencies</code>                                                </dd></summary> <hr> docker/rpm/Dockerfile.rpm.proton <ul><li>Add <code>systemd-rpm-macros</code> package dependency for RPM builds<br> <li> Remove trailing whitespace from final CMD line</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1659/files#diff-46c394d0d6b38be50e9c5ebd2cf8eb5afc5e3dc5efcbf4245b21a091e8c02999">+3/-2</a>      </td> </tr> </table></td></tr><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>setup-package.sh</strong><dd><code>Support "none" build method in packaging</code>                                  </dd></summary> <hr> scripts/setup-package.sh <ul><li>Add support for "none" build method to skip binary building<br> <li> Handle empty build_method values gracefully</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1659/files#diff-388e4f6f99131c27ccade0a4d7f16112297ec05e4dda0a23e142f787498ac004">+4/-1</a>      </td> </tr> </table></td></tr><tr><td><strong>Documentation</strong></td><td><table> <tr> <td> <details> <summary><strong>SRQL_OCSF_ALIGNMENT_PLAN.md</strong><dd><code>Add OCSF alignment plan documentation</code>                                        </dd></summary> <hr> SRQL_OCSF_ALIGNMENT_PLAN.md <ul><li>Add comprehensive 389-line documentation for OCSF alignment<br> <li> Define migration phases, query language evolution, and federation <br>architecture<br> <li> Include implementation roadmap and technical considerations</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1659/files#diff-488bbb4a630bb43a568e9f984b9f4abc899bc1aeafbd1082094eb00ad95c26fd">+389/-0</a>  </td> </tr> </table></td></tr><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>buildbuddy.yaml</strong><dd><code>Add BuildBuddy CI/CD configuration</code>                                              </dd></summary> <hr> buildbuddy.yaml <ul><li>Add BuildBuddy configuration for remote execution and caching<br> <li> Configure build event streaming and execution parameters</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1659/files#diff-455c97ce748484a181e002949dbe70422aedc497a358e023dc162776ce940751">+30/-0</a>    </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-09-24 22:50:02 +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/1659#issuecomment-3330948615
Original created: 2025-09-24T22:50:02Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Possible Issue

The derived channel concatenates major and minor (e.g., 3.11 -> 311) but does not prepend the expected 'gateway-' or similar prefix used by Kong package channels; verify downstream usage expects just digits vs. full channel name.

# Kong publishes packages under major+minor channels (e.g. 3.11.x lives in gateway-311)
derive_channel() {
  local version="$1"
  local fallback="$2"

  IFS='.' read -r major minor _ <<<"$version"
  if [[ -n "${major:-}" && -n "${minor:-}" ]]; then
    printf '%s%s' "$major" "$minor"
  else
    printf '%s' "$fallback"
  fi
}

ENTERPRISE_CHANNEL=${KONG_ENTERPRISE_CHANNEL:-$(derive_channel "$ENTERPRISE_VERSION" "${ENTERPRISE_VERSION//./}" )}
FETCH_COMMUNITY=${KONG_FETCH_COMMUNITY:-0}
COMMUNITY_VERSION=${KONG_COMMUNITY_VERSION:-3.7.1}
COMMUNITY_CHANNEL=${KONG_COMMUNITY_CHANNEL:-$(derive_channel "$COMMUNITY_VERSION" "${COMMUNITY_VERSION//./}" )}

Logic Order

The new 'none' build_method branch short-circuits before 'external'; confirm no configs rely on empty or 'none' falling through to external logic and that required variables (e.g., output paths) are still handled when skipping.

elif [ "$build_method" = "none" ] || [ -z "$build_method" ]; then
    echo "No binary build required for $component (build_method '$build_method'), skipping binary stage."

elif [ "$build_method" = "external" ]; then
  local url output_path binary_is_archive extract_command
  url=$(echo "$config" | jq -r '.external_binary.source_url')
  output_path=$(echo "$config" | jq -r '.external_binary.output_path')
  binary_is_archive=$(echo "$config" | jq -r '.external_binary.binary_is_archive // "true"')

Build Reproducibility

Adding 'systemd-rpm-macros' and keeping 'policycoreutils-python-utils' without fixed versions may affect reproducibility; consider pinning or documenting required versions for Rocky 9.

RUN dnf clean all && \
    dnf update -y && \
    dnf install -y --nogpgcheck --skip-broken \
    rpm-build \
    rpmdevtools \
    curl \
    tar \
    systemd-devel \
    policycoreutils-python-utils \
    systemd-rpm-macros

# Set up RPM build environment
RUN rpmdev-setuptree

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1659#issuecomment-3330948615 Original created: 2025-09-24T22:50:02Z --- ## PR Reviewer Guide 🔍 Here are some key observations to aid the review process: <table> <tr><td>⏱️ <strong>Estimated effort to review</strong>: 2 🔵🔵⚪⚪⚪</td></tr> <tr><td>🧪 <strong>No relevant tests</strong></td></tr> <tr><td>🔒 <strong>No security concerns identified</strong></td></tr> <tr><td>⚡ <strong>Recommended focus areas for review</strong><br><br> <details><summary><a href='https://github.com/carverauto/serviceradar/pull/1659/files#diff-448068ce58b8f5324251c7496e00a7955fe479b20c43a3111b4f9187b90150cdR7-R24'><strong>Possible Issue</strong></a> The derived channel concatenates major and minor (e.g., 3.11 -> 311) but does not prepend the expected 'gateway-' or similar prefix used by Kong package channels; verify downstream usage expects just digits vs. full channel name. </summary> ```shell # Kong publishes packages under major+minor channels (e.g. 3.11.x lives in gateway-311) derive_channel() { local version="$1" local fallback="$2" IFS='.' read -r major minor _ <<<"$version" if [[ -n "${major:-}" && -n "${minor:-}" ]]; then printf '%s%s' "$major" "$minor" else printf '%s' "$fallback" fi } ENTERPRISE_CHANNEL=${KONG_ENTERPRISE_CHANNEL:-$(derive_channel "$ENTERPRISE_VERSION" "${ENTERPRISE_VERSION//./}" )} FETCH_COMMUNITY=${KONG_FETCH_COMMUNITY:-0} COMMUNITY_VERSION=${KONG_COMMUNITY_VERSION:-3.7.1} COMMUNITY_CHANNEL=${KONG_COMMUNITY_CHANNEL:-$(derive_channel "$COMMUNITY_VERSION" "${COMMUNITY_VERSION//./}" )} ``` </details> <details><summary><a href='https://github.com/carverauto/serviceradar/pull/1659/files#diff-388e4f6f99131c27ccade0a4d7f16112297ec05e4dda0a23e142f787498ac004R395-R402'><strong>Logic Order</strong></a> The new 'none' build_method branch short-circuits before 'external'; confirm no configs rely on empty or 'none' falling through to external logic and that required variables (e.g., output paths) are still handled when skipping. </summary> ```shell elif [ "$build_method" = "none" ] || [ -z "$build_method" ]; then echo "No binary build required for $component (build_method '$build_method'), skipping binary stage." elif [ "$build_method" = "external" ]; then local url output_path binary_is_archive extract_command url=$(echo "$config" | jq -r '.external_binary.source_url') output_path=$(echo "$config" | jq -r '.external_binary.output_path') binary_is_archive=$(echo "$config" | jq -r '.external_binary.binary_is_archive // "true"') ``` </details> <details><summary><a href='https://github.com/carverauto/serviceradar/pull/1659/files#diff-46c394d0d6b38be50e9c5ebd2cf8eb5afc5e3dc5efcbf4245b21a091e8c02999R19-R32'><strong>Build Reproducibility</strong></a> Adding 'systemd-rpm-macros' and keeping 'policycoreutils-python-utils' without fixed versions may affect reproducibility; consider pinning or documenting required versions for Rocky 9. </summary> ```txt RUN dnf clean all && \ dnf update -y && \ dnf install -y --nogpgcheck --skip-broken \ rpm-build \ rpmdevtools \ curl \ tar \ systemd-devel \ policycoreutils-python-utils \ systemd-rpm-macros # Set up RPM build environment RUN rpmdev-setuptree ``` </details> </td></tr> </table>

qodo-code-review[bot] commented

2025-09-24 22:51:00 +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/1659#issuecomment-3330950746
Original created: 2025-09-24T22:51:00Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Pin container image to a specific version In `buildbuddy.yaml`, pin the `container-image` to a specific version or digest instead of using the `:latest` tag to ensure build reproducibility. buildbuddy.yaml [20] `-container-image: "gcr.io/buildbuddy-io/executor:latest" +container-image: "gcr.io/buildbuddy-io/executor:v2.0.0" # Or a specific digest` Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion addresses a common anti-pattern in CI/CD configurations. Pinning the container image version instead of using `:latest` is a crucial best practice for ensuring reproducible and stable builds.	Medium
General	Handle version strings with 'v' prefix In the `derive_channel` function, make the script more robust by stripping any leading 'v' from the version string before parsing it. scripts/fetch-kong-artifacts.sh [12-17] `+version="${version#v}" IFS='.' read -r major minor _ <<<"$version" if [[ -n "${major:-}" && -n "${minor:-}" ]]; then printf '%s%s' "$major" "$minor" else printf '%s' "$fallback" fi` Apply / Chat Suggestion importance[1-10]: 6 __ Why: This suggestion correctly identifies a potential bug where version strings with a 'v' prefix would be parsed incorrectly and provides a simple, robust fix, improving the script's reliability.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1659#issuecomment-3330950746 Original created: 2025-09-24T22:51:00Z --- ## 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=2>General</td> <td> <details><summary>Pin container image to a specific version</summary> ___ **In <code>buildbuddy.yaml</code>, pin the <code>container-image</code> to a specific version or digest <br>instead of using the <code>:latest</code> tag to ensure build reproducibility.** [buildbuddy.yaml [20]](https://github.com/carverauto/serviceradar/pull/1659/files#diff-455c97ce748484a181e002949dbe70422aedc497a358e023dc162776ce940751R20-R20) ```diff -container-image: "gcr.io/buildbuddy-io/executor:latest" +container-image: "gcr.io/buildbuddy-io/executor:v2.0.0" # Or a specific digest ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion addresses a common anti-pattern in CI/CD configurations. Pinning the container image version instead of using `:latest` is a crucial best practice for ensuring reproducible and stable builds. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Handle version strings with 'v' prefix</summary> ___ **In the <code>derive_channel</code> function, make the script more robust by stripping any <br>leading 'v' from the version string before parsing it.** [scripts/fetch-kong-artifacts.sh [12-17]](https://github.com/carverauto/serviceradar/pull/1659/files#diff-448068ce58b8f5324251c7496e00a7955fe479b20c43a3111b4f9187b90150cdR12-R17) ```diff +version="${version#v}" IFS='.' read -r major minor _ <<<"$version" if [[ -n "${major:-}" && -n "${minor:-}" ]]; then printf '%s%s' "$major" "$minor" else printf '%s' "$fallback" fi ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 6</summary> __ Why: This suggestion correctly identifies a potential bug where version strings with a 'v' prefix would be parsed incorrectly and provides a simple, robust fix, improving the script's reliability. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>