Skip to content

fix(helm-repository): strip trailing slash from OCI repository and registry URL - #2964

Merged
rmnbrd merged 3 commits into
stagingfrom
claude/helm-oci-trailing-slash-66c3ef
Sep 15, 2026
Merged

rmnbrd merged 3 commits into
stagingfrom
claude/helm-oci-trailing-slash-66c3ef

Conversation

@acarranoqovery

@acarranoqovery acarranoqovery commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Strip trailing slash(es) from the Helm repository URL before submitting the create/edit form, so a URL like oci://docker.io/ no longer gets rejected by the backend (which requires an empty URL path for OCI repositories)
  • Applies to both create and edit flows, for any repository kind
  • Added a regression test covering the trailing-slash case

Screenshots / Recordings

N/A (form submission logic only, no visual change)

Testing

  • Changes tested locally in the relevant Console's pages and Storybooks
  • yarn test or yarn test -u (if you need to regenerate snapshots)
  • yarn format
  • yarn lint

PR Checklist

  • I followed naming, styling, and TypeScript rules (see .cursor/rules)
  • I performed a self-review (diff inspected, dead code removed)
  • I titled the PR using Conventional Commits with a scope when possible (e.g. feat(service): add new Terraform service) - required for semantic-release
  • I only kept necessary comments, written in English (watch for useless AI comments)
  • I involved a designer to validate UI changes if I am not a designer
  • I covered new business logic with tests (unit)
  • I confirmed CI is green (Codecov red can be accepted)
  • I reviewed and executed locally any AI-assisted code

🤖 Generated with Claude Code


Summary by cubic

Normalizes Helm repository and container registry URLs before submission so values like oci://docker.io/ and https://my-registry.example.com/ are accepted instead of rejected for having a non-empty path. Applies to create and edit flows, preserves query strings and fragments, and leaves omitted URLs unchanged.

  • Reuses a shared stripUrlTrailingSlash helper for both modals.
  • Adds regression coverage for the helper and both create/edit flows.

Written for commit 95dcf03. Summary will update on new commits.

Review in cubic

Submitting an OCI helm repository URL with a trailing slash (e.g. `oci://docker.io/`) is rejected by the backend, which requires an empty URL path. Strip trailing slash(es) client-side before submit so users don't hit that error for a purely cosmetic typo.
@nx-cloud

nx-cloud Bot commented Sep 14, 2026

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 95dcf03

Command Status Duration Result
nx run console:build --parallel=3 --configurati... ✅ Succeeded 1m View ↗
nx affected --target=test --parallel=3 --config... ✅ Succeeded 2m 34s View ↗
nx affected --target=lint --parallel=3 ✅ Succeeded 2m 31s View ↗
nx-cloud record -- yarn nx format:check ✅ Succeeded 7s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-09-15 15:28:49 UTC

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Extract the trailing-slash stripping into a shared stripUrlTrailingSlash
util (query string/fragment aware, per review feedback on the helm
repository fix), reuse it for container registries, and cover both the
create and edit flow for each modal.
@acarranoqovery

Copy link
Copy Markdown
Contributor Author

Addressed both points from the cubic review:

  1. The trailing-slash strip now only touches the path segment (splitting on the first ?/#), leaving any query string or fragment untouched. Extracted into a shared, tested util: stripUrlTrailingSlash in libs/shared/util-js/src/lib/strip-url-trailing-slash.ts.
  2. Added an edit-flow regression test (should strip trailing slash from OCI URL when editing a repository) alongside the existing create-flow one.

Also extended the same fix to container registries (container-registry-create-edit-modal.tsx), with create/edit regression tests there too, since the backend has the same "no path allowed" validation for registry URLs.

HelmRepositoryRequest.url is typed as optional (string | undefined)
in the generated API client, so passing it directly to
stripUrlTrailingSlash(url: string) failed the build's type check.
Guard for the undefined case, same as already done in the container
registry modal.
@acarranoqovery

Copy link
Copy Markdown
Contributor Author

Fixed the CI build failure: `HelmRepositoryRequest.url` is typed as optional (`string | undefined`) in the generated API client, so passing it straight into `stripUrlTrailingSlash(url: string)` failed `nx run console:build`'s type check (the container registry modal already guarded for this correctly, the helm one didn't). Guarded it the same way. Verified locally with `nx run console:build:development` and the full `domains-organizations-feature` test suite (318 tests) — both pass now.

This also explains why testing on the PR preview showed no trailing-slash stripping for HTTPS repos: the build never succeeded, so no new preview was ever deployed from this branch. Should be fixed once this commit's CI run goes green.

@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.29%. Comparing base (f39f461) to head (95dcf03).
⚠️ Report is 3 commits behind head on staging.

Files with missing lines Patch % Lines
...dit-modal/container-registry-create-edit-modal.tsx 0.00% 0 Missing and 1 partial ⚠️
...e-edit-modal/helm-repository-create-edit-modal.tsx 0.00% 0 Missing and 1 partial ⚠️
libs/shared/util-js/src/index.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           staging    #2964      +/-   ##
===========================================
- Coverage    49.52%   49.29%   -0.23%     
===========================================
  Files         1286     1289       +3     
  Lines        27799    27978     +179     
  Branches      8300     8277      -23     
===========================================
+ Hits         13767    13792      +25     
- Misses       11740    11893     +153     
- Partials      2292     2293       +1     
Flag Coverage Δ
unittests 49.29% <62.50%> (-0.23%) ⬇️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@acarranoqovery acarranoqovery changed the title fix(helm-repository): strip trailing slash from OCI repository URL fix(helm-repository): strip trailing slash from OCI repository and registry URL Sep 15, 2026
@rmnbrd
rmnbrd merged commit 332338c into staging Sep 15, 2026
13 checks passed
@rmnbrd
rmnbrd deleted the claude/helm-oci-trailing-slash-66c3ef branch September 15, 2026 15:36
@RemiBonnet

Copy link
Copy Markdown
Member

🎉 This PR is included in version 1.354.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants