Skip to content

Merging to release-5.10.1: Gromit sync with tyk repo TT-16131 (#7542)#7552

Merged
Razeen-Abdal-Rahman merged 2 commits intorelease-5.10.1from
merge/release-5.10.1/007b66aea6ff8cd1006aa3e2ac1666f01799bae0/TT-16131
Nov 19, 2025
Merged

Merging to release-5.10.1: Gromit sync with tyk repo TT-16131 (#7542)#7552
Razeen-Abdal-Rahman merged 2 commits intorelease-5.10.1from
merge/release-5.10.1/007b66aea6ff8cd1006aa3e2ac1666f01799bae0/TT-16131

Conversation

@probelabs
Copy link
Copy Markdown
Contributor

@probelabs probelabs Bot commented Nov 19, 2025

User description

TT-16131
Summary Gromit sync with tyk repo
Type Story Story
Status In Code Review
Points N/A
Labels -

Gromit sync with tyk repo TT-16131 (#7542)

Description

Carry over changes from gromit into the tyk repo

Related Issue

TT-16131

Motivation and Context

Ensure the tyk repo is in sync as the buildenv for this repo will
continue to be 1.24-bullseye with the addition of a manual pull of the
latest go version, this is done upstream when the image is built.
This PR carries over other changes from gromit ensuring everything is in
sync after this change.

Co-authored-by: Gromit policy@gromit
Co-authored-by: Leonid Bugaev leonsbox@gmail.com


PR Type

Enhancement, Bug fix


Description

  • Add FIPS build and images

  • Improve CI release flow robustness

  • Fix post-install service cleanup logic

  • Safeguard config chmod when absent


Diagram Walkthrough

flowchart LR
  ci["CI release workflow"] -- "add FIPS metadata/tags" --> fipsMeta["FIPS docker metadata"]
  ci -- "build+push FIPS images" --> fipsPushCI["Push FIPS CI image"]
  tags["Tag push"] -- "build+push FIPS prod" --> fipsPushProd["Push FIPS prod image"]
  goreleaser["Goreleaser config"] -- "boringcrypto experiment" --> fipsBuild["FIPS build target"]
  installer["post_install.sh"] -- "fix service cleanup, safe chmod" --> saferInstall["Safer post-install"]
  images["Distroless Dockerfile"] -- "install .deb earlier, clean logs" --> leanImage["Smaller attack surface"]
  tests["Package upgrade tests"] -- "tolerate repo failures" --> resilientTests["Resilient fallbacks"]
Loading

File Walkthrough

Relevant files
Bug fix
post_install.sh
Correct service cleanup and safe config chmod                       

ci/install/post_install.sh

  • Invert service cleanup paths correctly.
  • Guard chmod of tyk.conf with file check.
+4/-4     
Enhancement
release.yml
CI: FIPS images and more robust release flow                         

.github/workflows/release.yml

  • Add FIPS docker metadata, CI and prod pushes.
  • Expose fips_tags output from CI.
  • Skip docker in goreleaser when not tagging.
  • Make package upgrade tests resilient to repo issues.
+71/-3   
Dockerfile.std
Streamline std image install and cleanup                                 

ci/Dockerfile.std

  • Install package via dist/${BUILD_PACKAGE_NAME}... early.
  • Clean logs and apt dirs more precisely.
  • Adjust glob patterns for .deb install/removal.
+5/-5     
goreleaser.yml
Enable boringcrypto experiment for FIPS build                       

ci/goreleaser/goreleaser.yml

  • Set GOEXPERIMENT=boringcrypto for FIPS build.
  • Remove unused env placeholder.
+1/-1     

## Description

Carry over changes from gromit into the tyk repo

## Related Issue

[TT-16131](https://tyktech.atlassian.net/browse/TT-16131)

## Motivation and Context

Ensure the tyk repo is in sync as the buildenv for this repo will
continue to be `1.24-bullseye` with the addition of a manual pull of the
latest go version, this is done upstream when the image is built.
This PR carries over other changes from gromit ensuring everything is in
sync after this change.

[TT-16131]:
https://tyktech.atlassian.net/browse/TT-16131?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Co-authored-by: Gromit <policy@gromit>
Co-authored-by: Leonid Bugaev <leonsbox@gmail.com>
(cherry picked from commit 007b66a)
@buger
Copy link
Copy Markdown
Member

buger commented Nov 19, 2025

I'm a bot and I 👍 this PR title. 🤖

@github-actions
Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

7542 - Partially compliant

Compliant requirements:

  • Sync changes from Gromit into the tyk repo.
  • Carry over related CI/CD and packaging changes to keep repos in sync.

Non-compliant requirements:

  • Ensure build environment remains 1.24-bullseye with manual pull of latest Go version handled upstream.

Requires further human verification:

  • Verify upstream image build process actually pulls and pins the latest Go version as intended.
  • Validate CI pipelines produce expected artifacts (std, ee, fips images) and publish to correct registries.
  • Run end-to-end install/upgrade tests on supported distros for both .deb and .rpm flows.
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The cleanup logic swapped which file gets removed based on systemctl detection. Confirm that on systemd systems we do not remove the systemd unit inadvertently and that paths (/lib/systemd/system vs /etc/init.d) match actual install locations.

if [ "${use_systemctl}" = "True" ]; then
    rm -f /etc/init.d/tyk-gateway
else
    rm -f /lib/systemd/system/tyk-gateway.service
fi
CI Behavior Change

Goreleaser snapshot now skips docker builds; confirm this doesn't break expected snapshot image availability used by downstream jobs/tests.

echo '#!/bin/sh
ci/bin/unlock-agent.sh
git config --global url."https://${{ secrets.ORG_GH_TOKEN }}@github.com".insteadOf "https://github.com"
git config --global --add safe.directory /go/src/github.com/TykTechnologies/tyk
goreleaser release --clean -f ${{ matrix.goreleaser }} ${{ !startsWith(github.ref, 'refs/tags/') && ' --snapshot --skip=sign,docker' || '--skip=docker' }}' | tee /tmp/build.sh
chmod +x /tmp/build.sh
New FIPS Images

New FIPS image build/push paths added. Validate tags, labels, and build args produce correct images and don't collide with std/ee tags.

- name: Docker metadata for fips CI
  id: ci_metadata_fips
  if: ${{ matrix.golang_cross == '1.24-bullseye' }}
  uses: docker/metadata-action@v5
  with:
    images: |
      ${{ steps.ecr.outputs.registry }}/tyk
    flavor: |
      latest=false
    tags: |
      type=ref,event=branch
      type=ref,event=pr
      type=sha,format=long
      type=semver,pattern={{major}},prefix=v
      type=semver,pattern={{major}}.{{minor}},prefix=v
      type=semver,pattern={{version}},prefix=v
- name: push fips image to CI
  if: ${{ matrix.golang_cross == '1.24-bullseye' }}
  uses: docker/build-push-action@v6
  with:
    context: "dist"
    platforms: linux/amd64
    file: ci/Dockerfile.distroless
    provenance: mode=max
    sbom: true
    push: true
    cache-from: type=gha
    cache-to: type=gha,mode=max
    tags: ${{ steps.ci_metadata_fips.outputs.tags }}
    labels: ${{ steps.ci_metadata_fips.outputs.labels }}
    build-args: |
      BUILD_PACKAGE_NAME=tyk-gateway-fips
- name: Docker metadata for fips tag push
  id: tag_metadata_fips
  uses: docker/metadata-action@v5
  with:
    images: |
      tykio/tyk-gateway-fips
    flavor: |
      latest=false
      prefix=v
    tags: |
      type=semver,pattern={{major}}.{{minor}}
      type=semver,pattern={{version}}
    labels: |
      org.opencontainers.image.title=Tyk Gateway FIPS
      org.opencontainers.image.description=Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols Built with boringssl
      org.opencontainers.image.vendor=tyk.io
      org.opencontainers.image.version=${{ github.ref_name }}
- name: push fips image to prod
  if: ${{ matrix.golang_cross == '1.24-bullseye' }}
  uses: docker/build-push-action@v6
  with:
    context: "dist"
    platforms: linux/amd64
    file: ci/Dockerfile.distroless
    provenance: mode=max
    sbom: true
    cache-from: type=gha
    cache-to: type=gha,mode=max
    push: ${{ startsWith(github.ref, 'refs/tags') }}
    tags: ${{ steps.tag_metadata_fips.outputs.tags }}
    labels: ${{ steps.tag_metadata_fips.outputs.labels }}
    build-args: |
      BUILD_PACKAGE_NAME=tyk-gateway-fips
- name: Docker metadata for std CI

@github-actions
Copy link
Copy Markdown
Contributor

API Changes

no api changes detected

@github-actions
Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Preserve apt metadata directories

Removing /var/lib/apt breaks future apt-get operations during later layers or debug.
Keep apt metadata directories but clear caches to stay minimal without breaking
package manager functionality.

ci/Dockerfile.std [21-25]

 RUN rm -rf /root/.cache \
     && apt-get -y autoremove \
     && apt-get clean \
-    && rm -rf /usr/include/* /var/cache/apt/archives /var/lib/apt /var/lib/cache /var/log/* \
+    && rm -rf /usr/include/* /var/cache/apt/archives/* /var/log/* \
     && find /usr/lib -type f -name '*.a' -o -name '*.o' -delete
Suggestion importance[1-10]: 7

__

Why: The new Dockerfile removes /var/lib/apt entirely, which can break later apt operations; keeping metadata while clearing caches reduces image size without harming functionality.

Medium
Possible issue
Safely guard file removals

Guard deletions to avoid removing files still needed on the current init system.
Check for file existence before removal to prevent misleading failures and support
dry-run behaviors. This reduces the risk of breaking service management on the
target system.

ci/install/post_install.sh [18-24]

 cleanup() {
     # After installing, remove files that were not needed on this platform / system
     if [ "${use_systemctl}" = "True" ]; then
-        rm -f /etc/init.d/tyk-gateway
+        [ -e /etc/init.d/tyk-gateway ] && rm -f /etc/init.d/tyk-gateway
     else
-        rm -f /lib/systemd/system/tyk-gateway.service
+        [ -e /lib/systemd/system/tyk-gateway.service ] && rm -f /lib/systemd/system/tyk-gateway.service
     fi
 }
Suggestion importance[1-10]: 6

__

Why: Adding existence checks before rm is safe and reduces noisy errors; it's a modest robustness improvement and aligns with the PR's logic split between systemd/sysv without changing behavior.

Low
Fix conditional CLI flag spacing

The conditional expression injects leading spaces into CLI args and always passes a
value; this can produce parsing issues with GoReleaser flags. Remove leading spaces
and ensure the empty case really yields no extra token.

.github/workflows/release.yml [98-103]

-goreleaser release --clean -f ${{ matrix.goreleaser }} ${{ !startsWith(github.ref, 'refs/tags/') && ' --snapshot --skip=sign,docker' || '--skip=docker' }}'
+goreleaser release --clean -f ${{ matrix.goreleaser }}${{ !startsWith(github.ref, 'refs/tags/') && ' --snapshot --skip=sign,docker' || ' --skip=docker' }}'
Suggestion importance[1-10]: 4

__

Why: The concern about injected spaces is minor because shell parsing tolerates them; however, ensuring no stray tokens in GitHub expression output can improve reliability slightly.

Low

@probelabs
Copy link
Copy Markdown
Contributor Author

probelabs Bot commented Nov 19, 2025

🔍 Code Analysis Results

{
"text": "This PR introduces a FIPS-compliant build and release pipeline for Tyk Gateway, synchronized from an internal repository. The core change enables the creation of a new tyk-gateway-fips artifact by leveraging Go's boringcrypto experiment. The PR also enhances the CI/CD process by improving the robustness of package upgrade tests and fixing minor issues in the installation scripts.

Files Changed Analysis

  • .github/workflows/release.yml: The majority of changes are here, adding a new workflow to build and publish a tyk-gateway-fips Docker image. The package upgrade test steps are also made more resilient to repository failures.
  • ci/goreleaser/goreleaser.yml: Enables the GOEXPERIMENT=boringcrypto flag for builds. This is the key change for FIPS compliance and appears to affect all build variants.
  • ci/install/post_install.sh: Contains a bug fix for the service file cleanup logic, ensuring the correct init script is removed. It also adds a safety check before changing permissions on the configuration file.
  • ci/Dockerfile.std: Includes minor adjustments to the Docker build process, improving the order of operations and making cleanup commands safer.

Architecture & Impact Assessment

  • What this PR accomplishes: It adds the capability to produce FIPS-compliant Tyk Gateway builds, allowing deployment in regulated environments. It also hardens the CI and installation scripts.
  • Key technical changes introduced:
    • Go builds are configured with GOEXPERIMENT=boringcrypto to use the FIPS-validated BoringCrypto library.
    • The GitHub Actions release workflow is extended to build and push a new tykio/tyk-gateway-fips Docker image.
    • Post-installation script logic is corrected and made safer.
  • Affected system components: The primary impact is on the CI/CD pipeline and the final build artifacts. The change to the underlying cryptographic library may have performance implications and could affect TLS connections to downstream services if they are not configured with FIPS-compatible ciphers```mermaid
    graph TD
    subgraph "CI Release Workflow"
    A[Push/Tag Trigger] --> B[GoReleaser Build];
    B --|GOEXPERIMENT=boringcrypto|--> C[Generate FIPS-compliant Binary];
    C --> D[Create Packages .deb/.rpm];
    D --> E[Build Standard/EE Docker Images];
    D --> F[Build New FIPS Docker Image];
    F --> G[Push to tykio/tyk-gateway-fips];
    end
      end

Scope Discovery & Context Expansion

  • The most critical point for review is that enabling boringcrypto in the main goreleaser.yml likely forces FIPS-compliant cryptography on all builds, not just the new FIPS variant. This could be a significant breaking change for existing users and should be confirmed.
  • The new tykio/tyk-gateway-fips image will require updates in downstream deployment tools (e.g., Helm charts, Tyk Operator) to be usable.
  • The reliability improvements to the CI upgrade tests and installation scripts are valuable but isolated enhancements that reduce the brittleness of the pipeline.",
    "tags": {
    "review-effort": 3,
    "label": "enhancement"
    }
    }

Powered by Visor from Probelabs

Last updated: 2025-11-19T10:09:51.398Z | Triggered by: synchronize | Commit: 6d68ff9

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link
Copy Markdown
Contributor Author

probelabs Bot commented Nov 19, 2025

🔍 Code Analysis Results

Security Issues (3)

Severity Location Issue
🟡 Warning .github/workflows/release.yml:424-425
The Debian package upgrade test step is configured to silently fall back to a fresh install test if the previous version of Tyk Gateway cannot be installed. While this improves CI robustness against transient repository failures, it systematically masks potential issues with the package repository and causes the upgrade test path to be skipped. A consistently failing download could indicate a larger problem, and skipping the upgrade test reduces confidence in the release's stability for existing users.
💡 SuggestionInstead of silently continuing, the job should fail or at least create a visible warning/annotation if the target package for the upgrade test cannot be fetched. This ensures that failures in critical test prerequisites are not ignored.
🟡 Warning .github/workflows/release.yml:481-482
The RPM package upgrade test step is configured to silently fall back to a fresh install test if the previous version of Tyk Gateway cannot be installed. While this improves CI robustness against transient repository failures, it systematically masks potential issues with the package repository and causes the upgrade test path to be skipped. A consistently failing download could indicate a larger problem, and skipping the upgrade test reduces confidence in the release's stability for existing users.
💡 SuggestionInstead of silently continuing, the job should fail or at least create a visible warning/annotation if the target package for the upgrade test cannot be fetched. This ensures that failures in critical test prerequisites are not ignored.
🟡 Warning ci/Dockerfile.std:24
The Dockerfile aggressively removes `/var/lib/apt`, which contains essential metadata for the `apt` package manager. This breaks subsequent `apt` operations (like `apt-get update` or `apt-get install`), hindering debugging, security patching, or adding tools to the running container. While reducing image size is a goal, it should not come at the cost of breaking fundamental OS tooling.
💡 SuggestionTo clean package manager caches without breaking functionality, remove the contents of cache directories instead of the metadata directories themselves. Specifically, remove `/var/cache/apt/archives/*` but leave `/var/lib/apt` intact.
🔧 Suggested Fix
    && rm -rf /usr/include/* /var/cache/apt/archives/* /var/lib/cache /var/log/* \

Architecture Issues (2)

Severity Location Issue
🟢 Info .github/workflows/release.yml:201-266
The workflow configuration for building and pushing FIPS Docker images is largely duplicated from the existing `std` and `ee` image build steps. This repetition across the different image types increases maintenance complexity, as any change to the build process (e.g., updating actions, changing labels, adding security scanners) must be replicated in multiple places.
💡 SuggestionTo reduce duplication and improve maintainability, consider refactoring the Docker build-and-push logic into a reusable workflow (`workflow_call`). This would centralize the common steps, while allowing parameters for image-specific details like names, tags, and build arguments.
🟡 Warning ci/Dockerfile.std:24
The cleanup command `rm -rf /var/lib/apt` in the Dockerfile removes essential APT metadata, effectively breaking the package manager in the final image. This prevents developers from using `apt-get` to install debugging tools inside a running container, hindering troubleshooting.
💡 SuggestionModify the cleanup command to only remove package caches, not the entire APT state directory. Change `rm -rf ... /var/lib/apt ...` to `rm -rf /var/cache/apt/archives/*`. This will reduce image size without compromising the ability to use the package manager for debugging.

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (2)

Severity Location Issue
🟡 Warning ci/Dockerfile.std:24-28
The cleanup command removes `/var/lib/apt`, which contains essential metadata for the `apt` package manager. Deleting this directory breaks subsequent `apt-get` operations, making it difficult to debug the container or install additional tools. This harms the image's maintainability and goes against Docker best practices for creating reusable or debuggable layers.
💡 SuggestionModify the cleanup command to only remove package caches and lists, not the entire apt metadata directory. This keeps the image lean while preserving the functionality of the package manager.
🔧 Suggested Fix
RUN rm -rf /root/.cache \
    && apt-get -y autoremove \
    && apt-get clean \
    && rm -rf /usr/include/* /var/cache/apt/archives/* /var/lib/apt/lists/* /var/log/* \
    && find /usr/lib -type f -name '*.a' -o -name '*.o' -delete
🟡 Warning .github/workflows/release.yml:490-491
The package upgrade tests (`test-package-upgrade-deb` and `test-package-upgrade-rpm`) are configured to ignore failures when fetching the previous package version. This makes the CI pipeline more resilient but also means the upgrade path is not reliably tested, as the test silently proceeds with a fresh install instead. A regression in the upgrade logic could be missed if the old package is consistently unavailable during testing.
💡 SuggestionTo ensure upgrade paths are tested, consider failing the job if the prerequisite package cannot be installed. If CI flakiness is a major concern, a separate, non-blocking job could monitor the package repository's health. Alternatively, split the logic into two distinct jobs: one for fresh installs and one for upgrades, where the upgrade job is allowed to fail without blocking the entire pipeline.

Dependency Issues (4)

Severity Location Issue
🟠 Error ci/Dockerfile.std:23
Removing the `/var/lib/apt` directory breaks the `apt` package manager's state, which prevents future package management operations (e.g., installing debugging tools) within the container. While this reduces image size, it severely impacts the image's debuggability and extensibility.
💡 SuggestionPreserve the `apt` metadata directories and only clear caches. Replace the line with one that removes only cache contents, for example: `rm -rf /usr/include/* /var/cache/apt/archives/* /var/log/*`.
🟡 Warning .github/workflows/release.yml:201-266
The introduction of a new FIPS-compliant Docker image (`tykio/tyk-gateway-fips`) requires corresponding changes in downstream deployment tools like `tyk-operator` and `tyk-charts` to allow users to select and deploy this image variant. This pull request does not appear to include or reference such changes, making the new FIPS feature incomplete from a user's perspective.
💡 SuggestionEnsure that corresponding pull requests for `tyk-operator` and `tyk-charts` are created and linked, or at least that issues are filed to track the necessary work to support the new FIPS image.
🟡 Warning .github/workflows/release.yml:424-425
The upgrade tests for .deb packages now suppress failures during the installation of the old version using `|| echo "..."`. While this makes the CI pipeline more resilient to package repository issues, it can also mask persistent problems. The silent continuation might lead to the upgrade test path being consistently skipped, reducing test coverage without clear visibility.
💡 SuggestionTo improve visibility without failing the job, consider emitting a GitHub Actions workflow command to highlight the event. For example: `... || echo "::warning::Repository setup failed, but continuing"`.
🟡 Warning .github/workflows/release.yml:482-483
The upgrade tests for .rpm packages now suppress failures during the installation of the old version using `|| echo "..."`. While this makes the CI pipeline more resilient to package repository issues, it can also mask persistent problems. The silent continuation might lead to the upgrade test path being consistently skipped, reducing test coverage without clear visibility.
💡 SuggestionTo improve visibility without failing the job, consider emitting a GitHub Actions workflow command to highlight the event. For example: `... || echo "::warning::Repository setup failed, but continuing"`.

✅ Connectivity Check Passed

No connectivity issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2025-11-19T10:09:52.693Z | Triggered by: synchronize | Commit: 6d68ff9

💡 TIP: You can chat with Visor using /visor ask <your question>

@Razeen-Abdal-Rahman Razeen-Abdal-Rahman merged commit bdb57b8 into release-5.10.1 Nov 19, 2025
45 of 50 checks passed
@Razeen-Abdal-Rahman Razeen-Abdal-Rahman deleted the merge/release-5.10.1/007b66aea6ff8cd1006aa3e2ac1666f01799bae0/TT-16131 branch November 19, 2025 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants