feat(KFLUXVNGD-1174): enable Cargo config.json auth-required rewrite

[kelchen123]

Configure nginx.subFilters on staging artifact-registry proxies to rewrite "auth-required":true to false on config.json responses.

Depends on caching #1056 merge before subFilters render in nginx.

Jira-Url: https://redhat.atlassian.net/browse/KFLUXVNGD-1174

[github-actions]

Kustomize Render Diff

Comparing 5e24b04f4 → 1570c533e

Component	Environment	Changes
components/squid/development	development	+41 -41
components/squid/staging	staging	+41 -41

Total: 2 components, +82 -82 lines

📋 Full diff available in the workflow summary and as a downloadable artifact.

[qodo-for-redhat-appstudio]

PR Summary by Qodo

Enable NGINX sub_filter to rewrite Cargo config.json auth-required flag

✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Bump staging caching-helm chart to a version that supports nginx subFilters.
• Add an NGINX subFilter rewrite for Cargo config.json to flip auth-required to false.
• Scope the rewrite to the cargo-proxy config.json path and JSON responses only.

Diagram

graph TD
A["caching-helm-generator.yaml"] --> B["caching-helm chart"] --> C["Staging squid/nginx proxy"] --> D{{"Cargo client"}}
C --> E["Nexus cargo-proxy"]

subgraph Legend
  direction LR
  _cfg["Config file"] ~~~ _svc["Service"] ~~~ _ext{{"External"}}
end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Fix upstream config.json at the source (Nexus/repo config)

• ➕ Avoids brittle string-based response rewriting
• ➕ Behavior is consistent for all consumers (not just via this proxy)
• ➖ May not be feasible if upstream is vendor-managed or shared across environments
• ➖ Could require broader policy/security sign-off

2. Serve a dedicated static config.json for Cargo via an explicit location block

• ➕ Deterministic response, no string-match pitfalls
• ➕ Clear ownership of the Cargo metadata contract at the proxy layer
• ➖ Adds content management overhead
• ➖ Risk of drift if upstream config schema changes

3. Use a more structured rewrite (e.g., Lua/JS filter) instead of sub_filter

• ➕ JSON-aware mutation avoids false positives/formatting sensitivity
• ➕ Easier to extend for future config tweaks
• ➖ Higher operational complexity and runtime footprint
• ➖ May be unavailable/unsupported in the current chart/image

Recommendation: The current sub_filter approach is a pragmatic, low-effort staging mitigation as long as it remains tightly scoped (exact path + JSON content-type + exact string). If this becomes long-lived or needs to handle formatting variants, prefer either an upstream fix or a dedicated static config.json location to avoid brittle string replacement.

Files changed (1) +8 / -1

Other (1) +8 / -1

caching-helm-generator.yamlBump caching-helm and add Cargo config.json auth-required rewrite +8/-1

Bump caching-helm and add Cargo config.json auth-required rewrite

• Updates the staging caching-helm chart version and configures nginx.subFilters on the artifact-registry proxy. The new rule targets the Cargo config.json endpoint and rewrites the JSON field from "auth-required":true to false for application/json responses.

components/squid/staging/caching-helm-generator.yaml

[qodo-for-redhat-appstudio]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. Fragile auth-required rewrite ⊘ Outdated 🐞 Bug ≡ Correctness

Description

The new subFilters configuration replaces only the exact literal substring "auth-required":true,
so the rewrite will not happen if the upstream JSON contains whitespace variants (e.g.,
"auth-required": true) and Cargo will still see auth-required as true.

Code

components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[R33-34]

+          - string: '"auth-required":true'
+            replacement: '"auth-required":false'

Relevance

●●● Strong

Concrete formatting-sensitive correctness issue; accepted bug precedents favor guarding exact
matching assumptions.

PR-#13345

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Both updated Helm generator configs introduce exactly one replacement pattern
("auth-required":true) and provide no alternative patterns that would match whitespace-formatted
JSON, making the rewrite dependent on upstream JSON formatting.

components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[30-34]
components/squid/staging/caching-helm-generator.yaml[32-36]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`subFilters` currently contains a single literal-match replacement string (`"auth-required":true`). If the upstream `config.json` is not minified exactly the same way, the literal match won’t trigger and the intended rewrite won’t be applied.

### Issue Context
The PR goal is to rewrite `auth-required` to `false` in `/repository/<repo>/config.json` responses. The config currently provides only one exact string variant.

### Fix Focus Areas
- Add additional filter entries for common whitespace variants (or switch to a regex-capable mechanism if the chart supports it).
- components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[30-34]
- components/squid/staging/caching-helm-generator.yaml[32-36]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Staging chart version drift 🐞 Bug ⚙ Maintainability

Description

Staging is bumped to caching chart 0.1.1889 while other environments in this repo remain pinned to
older caching chart versions, increasing the risk of staging-only behavior changes and making
promotion/debugging harder. If subFilters depends on the newer chart, the mismatch also increases
the chance of later rollout friction when other environments need the same feature.

Code

components/squid/staging/caching-helm-generator.yaml[7]

+version: 0.1.1889+ca96506

Relevance

●● Moderate

Chart bumps historically align development and staging; this feature explicitly requires the newer
chart, but broader environment drift remains subjective.

PR-#12890
PR-#11925

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The staging generator now pins 0.1.1889, while other repo generators still pin older caching chart
versions (e.g., squid development at 0.1.1775 and artifact-registry-proxy ring configs at 0.1.1871).

components/squid/staging/caching-helm-generator.yaml[5-8]
components/squid/development/caching-helm-generator.yaml[5-8]
components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[5-8]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Only the staging `caching-helm-generator.yaml` was updated to a newer caching chart version; multiple other environments still use older versions. This introduces avoidable version skew.

### Issue Context
Version skew can cause staging to behave differently from development/production even with identical values, and can complicate later rollouts of `subFilters`.

### Fix Focus Areas
- components/squid/staging/caching-helm-generator.yaml[7-7]

### Suggested fix
Either:
1) Bump the caching chart version consistently across the other `components/squid/**/caching-helm-generator.yaml` (and/or other consumers) that should mirror staging, or
2) Add an explicit comment in the staging generator explaining why staging must be ahead (and what follow-up will align versions).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Ambiguous chart bump TODO 🐞 Bug ⚙ Maintainability

Description

A new TODO comment about bumping “after caching #1056 chart is on quay” is unclear given the pinned
version, which can confuse future version updates and what version is actually required for this
feature.

Code

components/squid/staging/caching-helm-generator.yaml[R7-8]

+# TODO([KFLUXVNGD-1174](https://redhat.atlassian.net/browse/KFLUXVNGD-1174)): bump after caching #1056 chart is on quay
+version: 0.1.1836+6dabb5a

Relevance

●● Moderate

Comment clarity is subjective; similar explanatory-comment requests have been rejected without a
close chart-TODO precedent.

PR-#13458
PR-#12351

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR adds the same TODO line immediately above the pinned chart version in both updated generator
files, making the intent ambiguous without further clarification.

components/squid/staging/caching-helm-generator.yaml[7-8]
components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[7-8]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The TODO about bumping after caching #1056 is published on Quay is ambiguous next to the already-pinned version. It’s unclear whether the version already includes the required change or whether a further bump is intended.

### Issue Context
This PR depends on caching #1056 for `subFilters` rendering, and also pins chart version `0.1.1836+6dabb5a`.

### Fix Focus Areas
- Either remove the TODO if the pinned version already contains the needed change, or rewrite it to explicitly state what version/condition you are waiting for (e.g., “once version X.Y.Z is published”).
- components/squid/staging/caching-helm-generator.yaml[7-8]
- components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[7-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 3 rules

Tip of the day

💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 4627c5d ⚖️ Balanced

Results up to commit d9c9039 ⚖️ Balanced

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Fragile auth-required rewrite ⊘ Outdated 🐞 Bug ≡ Correctness

Description

The new subFilters configuration replaces only the exact literal substring "auth-required":true,
so the rewrite will not happen if the upstream JSON contains whitespace variants (e.g.,
"auth-required": true) and Cargo will still see auth-required as true.

Code

components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[R33-34]

+          - string: '"auth-required":true'
+            replacement: '"auth-required":false'

Relevance

●●● Strong

Concrete formatting-sensitive correctness issue; accepted bug precedents favor guarding exact
matching assumptions.

PR-#13345

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Both updated Helm generator configs introduce exactly one replacement pattern
("auth-required":true) and provide no alternative patterns that would match whitespace-formatted
JSON, making the rewrite dependent on upstream JSON formatting.

components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[30-34]
components/squid/staging/caching-helm-generator.yaml[32-36]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`subFilters` currently contains a single literal-match replacement string (`"auth-required":true`). If the upstream `config.json` is not minified exactly the same way, the literal match won’t trigger and the intended rewrite won’t be applied.

### Issue Context
The PR goal is to rewrite `auth-required` to `false` in `/repository/<repo>/config.json` responses. The config currently provides only one exact string variant.

### Fix Focus Areas
- Add additional filter entries for common whitespace variants (or switch to a regex-capable mechanism if the chart supports it).
- components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[30-34]
- components/squid/staging/caching-helm-generator.yaml[32-36]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Ambiguous chart bump TODO 🐞 Bug ⚙ Maintainability

Description

A new TODO comment about bumping “after caching #1056 chart is on quay” is unclear given the pinned
version, which can confuse future version updates and what version is actually required for this
feature.

Code

components/squid/staging/caching-helm-generator.yaml[R7-8]

+# TODO([KFLUXVNGD-1174](https://redhat.atlassian.net/browse/KFLUXVNGD-1174)): bump after caching #1056 chart is on quay
+version: 0.1.1836+6dabb5a

Relevance

●● Moderate

Comment clarity is subjective; similar explanatory-comment requests have been rejected without a
close chart-TODO precedent.

PR-#13458
PR-#12351

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR adds the same TODO line immediately above the pinned chart version in both updated generator
files, making the intent ambiguous without further clarification.

components/squid/staging/caching-helm-generator.yaml[7-8]
components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[7-8]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The TODO about bumping after caching #1056 is published on Quay is ambiguous next to the already-pinned version. It’s unclear whether the version already includes the required change or whether a further bump is intended.

### Issue Context
This PR depends on caching #1056 for `subFilters` rendering, and also pins chart version `0.1.1836+6dabb5a`.

### Fix Focus Areas
- Either remove the TODO if the pinned version already contains the needed change, or rewrite it to explicitly state what version/condition you are waiting for (e.g., “once version X.Y.Z is published”).
- components/squid/staging/caching-helm-generator.yaml[7-8]
- components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[7-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit f24a081 ⚖️ Balanced

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Staging chart version drift 🐞 Bug ⚙ Maintainability

Description

Staging is bumped to caching chart 0.1.1889 while other environments in this repo remain pinned to
older caching chart versions, increasing the risk of staging-only behavior changes and making
promotion/debugging harder. If subFilters depends on the newer chart, the mismatch also increases
the chance of later rollout friction when other environments need the same feature.

Code

components/squid/staging/caching-helm-generator.yaml[7]

+version: 0.1.1889+ca96506

Relevance

●● Moderate

Chart bumps historically align development and staging; this feature explicitly requires the newer
chart, but broader environment drift remains subjective.

PR-#12890
PR-#11925

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The staging generator now pins 0.1.1889, while other repo generators still pin older caching chart
versions (e.g., squid development at 0.1.1775 and artifact-registry-proxy ring configs at 0.1.1871).

components/squid/staging/caching-helm-generator.yaml[5-8]
components/squid/development/caching-helm-generator.yaml[5-8]
components/artifact-registry-proxy/rings/ring-1/base/caching-helm-generator.yaml[5-8]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Only the staging `caching-helm-generator.yaml` was updated to a newer caching chart version; multiple other environments still use older versions. This introduces avoidable version skew.

### Issue Context
Version skew can cause staging to behave differently from development/production even with identical values, and can complicate later rollouts of `subFilters`.

### Fix Focus Areas
- components/squid/staging/caching-helm-generator.yaml[7-7]

### Suggested fix
Either:
1) Bump the caching chart version consistently across the other `components/squid/**/caching-helm-generator.yaml` (and/or other consumers) that should mirror staging, or
2) Add an explicit comment in the staging generator explaining why staging must be ahead (and what follow-up will align versions).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit 82eeaa3 ⚖️ Balanced

No changes from previous review

Results up to commit 129bec3 ⚖️ Balanced

No changes from previous review

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.38%. Comparing base (5e24b04) to head (4627c5d).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #13561   +/-   ##
=======================================
  Coverage   60.38%   60.38%           
=======================================
  Files          24       24           
  Lines        1628     1628           
=======================================
  Hits          983      983           
  Misses        563      563           
  Partials       82       82           

Flag	Coverage Δ
go	60.38% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[qodo-for-redhat-appstudio]

Code review by qodo was updated up to the latest commit f24a081

[qodo-for-redhat-appstudio]

Code review by qodo was updated up to the latest commit 82eeaa3

[amisstea]

We should bump the chart version in the dev overlay too.

[qodo-for-redhat-appstudio]

Code review by qodo was updated up to the latest commit 129bec3

[qodo-for-redhat-appstudio]

Code review by qodo was updated up to the latest commit 4627c5d

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisstea, kelchen123

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• components/squid/OWNERS [amisstea,kelchen123]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment