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

[probelabs]

User description

TT-16131

Summary	Gromit sync with tyk repo
Type	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, Tests, Bug fix

---

Description

• Add FIPS build and images to release

• Improve installer cleanup and permissions

• Adjust GoReleaser to skip docker when needed

• Make upgrade tests resilient to repo issues

---

Diagram Walkthrough

flowchart LR
  goreleaser["GoReleaser config updates"] -- "FIPS flags, boringcrypto" --> fipsbin["FIPS binaries"]
  fipsbin -- "BUILD_PACKAGE_NAME=tyk-gateway-fips" --> fipsimg["FIPS images (CI/prod)"]
  workflow["Release workflow"] -- "metadata + build-push" --> fipsimg
  workflow -- "skip docker on snapshot/tags" --> images["Standard/EE images"]
  postinstall["post_install.sh"] -- "safer cleanup, chmod if exists" --> installsafe["Safer installs"]
  tests["Upgrade test Dockerfiles"] -- "tolerant repo setup, fallback" --> resilient["Resilient upgrade tests"]
  dockerstd["ci/Dockerfile.std"] -- "copy deb, stricter cleanup" --> slimmer["Slimmer image"]

Loading

File Walkthrough

	Relevant files
Bug fix	post_install.sh Safer post-install cleanup and permissions                              ci/install/post_install.sh Swap cleanup paths for systemd/sysv Only chmod config if file exists +4/-4
Enhancement	release.yml Release workflow adds FIPS and hardens tests                          .github/workflows/release.yml Add FIPS image metadata and pushes Output FIPS tags for downstream jobs Skip docker builds in goreleaser when tagging/snapshot Make upgrade tests tolerate repo/old version absence +71/-3    Dockerfile.std Standard image installs from dist and cleans                          ci/Dockerfile.std Install package from dist with arch pattern Strengthen cleanup of apt/log caches Adjust deb naming/glob usage +5/-5      goreleaser.yml Enable boringcrypto for FIPS build                                              ci/goreleaser/goreleaser.yml Set GOEXPERIMENT=boringcrypto for FIPS Keep FIPS tags and ldflags intact +1/-1
Tests	upgrade-deb.yml (generated Dockerfile content) Debian upgrade test resilient to repo issues                          ci/tests/api-functionality/upgrade-deb.yml (generated Dockerfile content) Add fallback if repo setup fails Continue when previous version not found upgrade-rpm.yml (generated Dockerfile content) RPM upgrade test resilient to repo issues                                ci/tests/api-functionality/upgrade-rpm.yml (generated Dockerfile content) Add fallback if repo setup fails Continue when previous version not found

---

[buger]

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

[github-actions]

API Changes

no api changes detected

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix inverted cleanup paths The removal targets look inverted: when systemctl is used, you should delete the systemd unit, not the sysv init script. Swap the rm targets to prevent deleting the wrong service file and leaving stale entries behind. This avoids broken service management on both systemd and sysv systems. ci/install/post_install.sh [20-24] if [ "${use_systemctl}" = "True" ]; then + rm -f /lib/systemd/system/tyk-gateway.service +else rm -f /etc/init.d/tyk-gateway -else - rm -f /lib/systemd/system/tyk-gateway.service fi Suggestion importance[1-10]: 8 __ Why: The PR inverted the cleanup logic; when use_systemctl is True it should remove the systemd unit, not the sysv script. The proposed swap matches expected behavior and prevents leaving stale service files.	Medium
	Fix expression quoting in step In GitHub Actions, single-quoted strings are not evaluated; the inner conditional will be treated literally, likely breaking the command. Use the expression entirely within the workflow context without single quotes so the condition resolves at runtime. .github/workflows/release.yml [102] -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]: 7 __ Why: Removing single quotes around the GitHub expression helps ensure the conditional is evaluated, aligning with Actions expression parsing. Moderate impact as quoting nuances can be tricky but this likely improves correctness.	Medium
	Avoid deleting apt state dir Removing /var/lib/apt at this stage breaks subsequent apt-get operations (including during multi-stage builds or image consumers). Clean caches without deleting apt state directories to avoid failures when layers are reused. 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/* \ - && find /usr/lib -type f -name '*.a' -o -name '*.o' -delete + && 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]: 5 __ Why: The suggestion warns against removing /var/lib/apt, which can hinder later apt operations, but the PR already changed the cleanup to avoid deleting /var/lib/apt and moved install earlier. Impact is minor since the new code no longer deletes it.	Low

[probelabs]

🔍 Code Analysis Results

{
"text": "This PR introduces a FIPS-compliant build pipeline for Tyk Gateway, synchronizing changes from an upstream repository. The core of the change is the integration of BoringCrypto to create a new tyk-gateway-fips Docker image. The release workflow, installer scripts, and upgrade tests have been updated to support this new build variant and improve overall robustness.

Files Changed Analysis

The changes are focused on the CI/CD and packaging scripts:

• .github/workflows/release.yml: The majority of changes are here. A new workflow is added to build, tag, and push a FIPS-compliant Docker image (tyk-gateway-fips). The goreleaser step is modified to skip Docker builds, which are now handled by dedicated docker/build-push-action steps. Additionally, the Debian and RPM upgrade tests are made more resilient by suppressing errors if a previous version cannot be installed, allowing the test to proceed as a fresh install.
• ci/goreleaser/goreleaser.yml: Enables the FIPS-compliant build by setting the GOEXPERIMENT=boringcrypto environment variable for the Go compiler.
• ci/Dockerfile.std: Minor adjustments to the package installation order and cleanup commands for standardization and a slightly smaller image.
• ci/install/post_install.sh: Fixes a logic bug in the post-install cleanup script and adds a check to ensure the config file exists before changing its permissions, preventing errors on clean installs.

Architecture & Impact Assessment

• What this PR accomplishes: It introduces a new, separate build and release process for a FIPS-compliant version of Tyk Gateway, resulting in a new tyk-gateway-fips Docker image artifact.

• Key technical changes introduced:

  • Integration of BoringCrypto via the GOEXPERIMENT=boringcrypto build flag.
  • A new, parallel Docker build process within the GitHub Actions release workflow for FIPS images.
  • Decoupling of the Docker image build from the main goreleaser process.
  • Increased fault tolerance in upgrade tests, which now fall back to fresh-install tests on failure.

• Affected system components: The primary impact is on the CI/CD and build system. While the application's core logic is unchanged, the underlying cryptographic library is replaced in the FIPS build. This introduces a new release artifact that consumers requiring FIPS compliance will use.

• **R```mermaid
  graph TD
  subgraph "Release Job"
  A[Start] --> B(Run GoReleaser --skip=docker);
  B --> C[Binaries & Packages Created];
  C --> D{Build Docker Images};
  D --> D_STD[Build/Push Standard Image];
  D --> D_EE[Build/Push EE Image];
  D --> D_FIPS[Build/Push FIPS Image];
  D_FIPS --|Uses boringcrypto build|--> E[End];
  D_STD --> E;
  D_EE --> E;
  end

        D_EE --> E;
    end

Scope Discovery & Context Expansion

• The changes are well-contained within the build and deployment pipeline (.github/workflows, ci/). The use of GOEXPERIMENT=boringcrypto is a build-time flag that swaps the standard crypto library for a FIPS-compliant one without altering the Go source code.
• The broader impact is the introduction of a new product artifact (tyk-gateway-fips) which will require its own lifecycle management, including documentation, testing, and support.
• The modifications to make upgrade tests more resilient represent a trade-off, prioritizing pipeline stability over strict test enforcement. This could potentially mask underlying issues in the package repositories or the upgrade process itself.
• No further code exploration is immediately necessary, as the changes are confined to the CI/CD configuration and related scripts.",
  "tags": {
  "review-effort": 3,
  "label": "enhancement"
  }
  }

---

Powered by Visor from Probelabs

Last updated: 2025-11-19T10:06:55.370Z | Triggered by: synchronize | Commit: ea4f697

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

[probelabs]

🔍 Code Analysis Results

Security Issues (2)

Severity	Location	Issue
🟡 Warning	.github/workflows/release.yml:491-492	The upgrade test workflow pipes a script from `packagecloud.io` directly to `bash`. This `curl | bash` pattern introduces a supply chain risk. If the `packagecloud.io` repository or the connection to it is compromised, arbitrary code could be executed within the CI runner. While this is in a test environment, it's a practice that should be avoided. 💡 Suggestion Download the script, verify its integrity (e.g., via a checksum), and then execute it. This prevents execution of a tampered script. For example: `curl -fsSL -o script.deb.sh ... && sha256sum -c script.sha256 && bash script.deb.sh`.
🟡 Warning	ci/Dockerfile.std:24-25	The Dockerfile cleanup step deletes the `/var/lib/apt` directory. This directory contains state essential for the `apt` package manager. Removing it breaks `apt` functionality, preventing package installation or updates in any downstream Docker images or layers that build upon this one. 💡 Suggestion To ensure the image remains extensible, modify the cleanup step to only remove cache files. Instead of deleting the entire `/var/lib/apt` directory, remove only its cache contents, for example: `rm -rf /var/cache/apt/archives/* /var/lib/apt/lists/*`.

Architecture Issues (2)

Severity	Location	Issue
🟠 Error	ci/Dockerfile.std:24	The `rm -rf ... /var/lib/apt ...` command in the cleanup step removes the `apt` package manager's state directory. This breaks `apt-get` functionality in the final image, making it difficult to debug or extend the image by installing additional packages. While this reduces image size, it severely impacts usability and violates common Docker best practices for creating extensible images. 💡 Suggestion Modify the cleanup command to preserve the `apt` state. Remove only package caches (`/var/cache/apt/archives/*`) and lists (`/var/lib/apt/lists/*`), but not the entire `/var/lib/apt` directory. For creating minimal production images, a multi-stage build that copies only the necessary application binaries and configuration into a minimal base image (like `distroless` or `scratch`) is the recommended pattern.
🟡 Warning	.github/workflows/release.yml:198-269	The workflow contains significant duplication for building and pushing Docker images for different variants (std, ee, and the newly added fips). The `docker/metadata-action` and `docker/build-push-action` steps are repeated three times with only minor variations in parameters like `if` conditions and `build-args`. This makes the workflow harder to maintain and prone to inconsistencies when updates are needed. 💡 Suggestion Refactor the duplicated steps to improve maintainability. Consider using a matrix strategy on the job or step level to iterate over the image variants (`std`, `ee`, `fips`). This would centralize the build logic into a single set of steps that are parameterized by the matrix variables, adhering to the DRY (Don't Repeat Yourself) principle.

Performance Issues (1)

Severity	Location	Issue
🟡 Warning	ci/goreleaser/goreleaser.yml:66	Enabling BoringCrypto for FIPS-compliant builds (`GOEXPERIMENT=boringcrypto`) introduces a different cryptographic backend. This can impact the performance of TLS handshakes, JWT validation, and other cryptographic operations due to FIPS-mandated self-tests and different algorithm implementations. This performance trade-off for compliance should be quantified. 💡 Suggestion Benchmark the FIPS-enabled build against the standard build, focusing on crypto-heavy API flows (e.g., high volume of new TLS connections, JWT-secured endpoints). Publish the results to inform users about potential performance differences when choosing the FIPS-compliant gateway.

Quality Issues (2)

Severity	Location	Issue
🟠 Error	.github/workflows/release.yml:102	The conditional expression for goreleaser flags is wrapped in single quotes (`'...'`). In GitHub Actions workflow syntax, single-quoted strings are treated as literals and expressions within them are not evaluated. This will cause the literal string `${{ !startsWith(github.ref, 'refs/tags/') && ' --snapshot --skip=sign,docker' || '--skip=docker' }}` to be passed to the command, instead of the evaluated result, likely causing the step to fail. 💡 Suggestion Remove the outer single quotes from the expression to allow GitHub Actions to correctly evaluate the conditional logic at runtime. 🔧 Suggested Fix goreleaser release --clean -f ${{ matrix.goreleaser }} ${{ !startsWith(github.ref, 'refs/tags/') && '--snapshot --skip=sign,docker' || '--skip=docker' }} | tee /tmp/build.sh
🟠 Error	ci/Dockerfile.std:24	The image cleanup step removes `/var/lib/apt`, which contains essential state for the `apt` package manager. Deleting this directory will cause any subsequent `apt-get` commands to fail, which breaks the ability to reuse this Docker image layer in multi-stage builds or by downstream users. 💡 Suggestion Modify the `rm` command to only remove package caches and lists, not the entire `apt` state directory. The directory `/var/lib/apt/lists` should be cleaned, but `/var/lib/apt` itself should be preserved. 🔧 Suggested Fix && rm -rf /usr/include/* /var/cache/apt/archives/* /var/lib/apt/lists/* /var/lib/cache /var/log/* \

✅ Dependency Check Passed

No dependency issues found – changes LGTM.

✅ Connectivity Check Passed

No connectivity issues found – changes LGTM.

---

Powered by Visor from Probelabs

Last updated: 2025-11-19T10:06:56.641Z | Triggered by: synchronize | Commit: ea4f697

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

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud