Add deb repository and remote pages by warisshaikh1 · Pull Request #347 · pulp/pulp-ui

warisshaikh1 · 2026-08-28T18:16:13Z

Addresses part of #277 (deb in the side menu).

Draft on purpose — I asked three scoping questions in #277 and haven't heard back yet. This is here so there's something concrete to react to rather than to pre-empt the answers. Happy to cut it down, extend it, or restructure it entirely.

What this adds

A Pulp deb menu section gated on hasPlugin('deb'), with Repositories and Remotes, mirroring Pulp file:

	list	detail	edit	tabs
`remotes/deb/apt`	yes	yes	yes	details
`repositories/deb/apt`	yes	yes	yes	details, versions, distributions

What it deliberately leaves out

Publications. deb has two publication endpoints rather than a field: publications/deb/apt generates fresh metadata and needs a signing service to be consumable, while publications/deb/verbatim republishes upstream's Release/InRelease byte for byte so an ordinary ubuntu-archive-keyring verifies it. A publications tab has to either show both or ask which to create, which is a design decision rather than wiring, so it seemed better to agree it first.
Content browsing. content/deb/packages is the useful one, but there are a dozen deb content endpoints where rpm has one. Easy follow-up once the shape here is agreed.

The one place deb genuinely differs from file

RemoteForm needed the APT fields, which have no equivalent elsewhere: distributions (suites), components, architectures, gpgkey, and the sync_sources / sync_udebs / sync_installer switches.

distributions is required, for deb only. Every other plugin's remote needs just a URL; pulp_deb rejects one without suites:

POST /pulp/api/v3/remotes/deb/apt/
{"name":"x","url":"http://deb.debian.org/debian/","tls_validation":true,"download_concurrency":10}
-> 400 {"distributions": ["This field is required."]}

So requiredFields becomes conditional. gpgkey reuses the FileUpload treatment ca_cert and client_cert already get, since it's an armoured key file rather than a one-line value.

Everything else is registry entries — plugin2api, the plugin unions on LazyRepositories / LazyDistributions / RepositoryForm, and deb-only optional fields on the shared RemoteType. All additive; ansible, container and file behaviour is unchanged (the RemoteForm diff is worth a look on that point specifically).

Testing

Verified against a live pulpcore 3.116.0 / pulp_deb 3.10.0 mirroring Ubuntu noble, with the dev server proxied at it (API_PROXY):

every query the new pages issue returns 200 — remote and repository lists with ordering / name__icontains / pulp_label_select / remote filters, …/versions/, …/versions/?number=N, and distributions/deb/apt/?repository=<href>
the exact create payload DebRemoteEdit sends → 201; the smartUpdate PUT with all APT fields → 202, and the fields read back verbatim
the repository create plus auto-created distribution path → 201 / 202, and pluginRepositoryBasePath('deb', …) resolves the base_path the Repository URL field shows
test objects cleaned up afterwards, existing content untouched

npm run lint:js, lint:ts, lint:ls, the this-without-class check and npm run build are all clean (build warnings unchanged at 18, all pre-existing bundle-size ones). No CHANGES.md entry, since that's generated from PR titles at release.

Two things I'd flag for review

A third copy of SyncModal. ansible-repository-sync.tsx and file-repository-sync.tsx already carry identical copies and I've followed suit rather than refactoring inside a feature PR. Happy to extract all three into a shared component, here or separately, if you'd prefer.
mirror defaults to true in the sync modal, matching the other plugins, though pulp_deb's own API default is false. I kept UI consistency, but say the word and I'll flip it — mirroring is the destructive direction.

@bmbouter @himdel @dkliban @ipanova

Adds a "Pulp deb" menu section with Repositories and Remotes, mirroring the structure of Pulp file: list, detail and edit for each, plus the versions and distributions tabs on a repository. Deliberately scoped to repositories and remotes. Publications are left out because deb has two publication endpoints rather than a field -- publications/deb/apt generates fresh metadata and needs a signing service, publications/deb/verbatim republishes upstream's Release byte for byte -- so a publications tab needs a design decision rather than just wiring. Content browsing is left out for the same reason: deb has a dozen content endpoints where rpm has one. RemoteForm gains the APT fields, which have no equivalent in the other plugins: distributions (suites), components, architectures, gpgkey, and the sync_sources/sync_udebs/sync_installer switches. `distributions` is added to requiredFields for deb only -- pulp_deb answers a remote without it with "This field is required.", unlike every other plugin where url alone is enough. gpgkey reuses the FileUpload treatment the certificate fields already use, since it is an armoured key file. The remaining changes are registry entries: plugin2api, the plugin unions on LazyRepositories/LazyDistributions/RepositoryForm, and the deb-only fields on the shared RemoteType. All are additive; ansible, container and file behaviour is unchanged. Refs pulp#277

Redtigercod4

Thanks for this - solid work overall, and I've left comments on the specific issues, tagged using MoSCoW. A couple of things I'd suggest:

Commit size: This is a large PR, 1,942 lines across 32 files submitted as a single commit. That makes it much harder for reviewers to follow, especially when trying to understand how the implementation came together. Smaller, sequential PRs would be preferred going forward.
Cypress Testing: As this is new UI functionality within PulpUI, could you please add corresponding test cases to cypress/e2e/smoke.js.
AI-assisted contributions: No issue either way, but if any AI tooling was used, please make sure the appropriate attributions is included, in line with Pulp's AI policy.

Ref:
https://github.com/pulp/.github/blob/cf562eff2ec6fe0f49bdfd693c3627453eef5203/profile/README.md

Redtigercod4 · 2026-09-13T13:49:17Z

+  { name, pulp_href, pulpId },
+  { addAlert, setState, listQuery },
+) {
+  // TODO: handle more pages


MUST: Bare // TODO isn't enough here - if the repository being deleted has more than 100 distributions, the excess are silently orphaned. I'm aware this same pattern exists in both ansible and file repository actions, but I don't want to carry it over into deb as well.

Please either link this TODO to a tracked Github issue, or implement pagination using the offset value returned by the API.

Redtigercod4 · 2026-09-13T14:01:27Z

@@ -0,0 +1,149 @@
+import { msg, t } from '@lingui/core/macro';


MUST (mirror default): Good to see this flagged on the PR already - I'd agree it's worth resolving to match what pulp_deb defaults to, since that's the safer, non-destruction option. Aware ansible and file hardcode true too, but I'll raise a separate issue to track.

SHOULD (SyncModal duplication): Also flagged by you already - agreed this is worth doing, but I'd suggest pulling SyncModal out into a shared component as a precursor PR rather than here, since it's identical to file and ansible.

Redtigercod4 · 2026-09-13T14:09:06Z

+  listUrl: formatPath(Paths.deb.remote.list),
+  query: ({ name }) =>
+    DebRemoteAPI.list({ name })
+      .then(({ data: { results } }) => results[0])


MUST: results[0] assumes an array with no local .catch - if the response shape is ever wrong, it throws past page-with-tabs.tsx catch handler, and the page would hang silently. Also, the comment here is a little misleading, worth rewording so it's clear we're synthesizing a not-found rejection, not describing actual API behaviour.

Redtigercod4 · 2026-09-13T14:12:51Z

-      : plugin === 'file'
-        ? FileRemoteAPI.list({ ...(name ? { name__icontains: name } : {}) })
-        : Promise.reject(plugin)
+      : plugin === 'deb'


SHOULD: Nested ternary here is easy to misread and easy to break silently if a fourth plugin is added. Suggest breaking it out into a helper function with a switch statement instead.

Redtigercod4 · 2026-09-13T14:44:21Z

+        addAlert,
+        query,
+        hasPermission,
+        hasObjectPermission: (_p: string): boolean => true,


SHOULD: hasObjectPermission here always returns true, unlike the ansible implementation which conducts a real check against item.my_permissions. It's most likely harmless today, but it's a potential landmine for whoever wires up a permission-gated action later and copies in this stub.

Redtigercod4 · 2026-09-13T14:48:09Z

@@ -0,0 +1,71 @@
+import { PulpAPI } from './pulp';
+
+export class DebRemoteType {


WOULD: This class only holds property definitions - no constructor or methods, never instantiated. Per TypeScript's own docs, this fits an interface better.

Redtigercod4 · 2026-09-13T14:48:23Z

@@ -0,0 +1,42 @@
+import { PulpAPI } from './pulp';
+
+export class DebRepositoryType {


WOULD: This class only holds property definitions - no constructor or methods, never instantiated. Per TypeScript's own docs, this fits an interface better.

Redtigercod4 · 2026-09-13T14:49:36Z

+      remote[field] = null;
+    }
+
+    // API returns headers:null bull doesn't accept it .. and we don't edit headers


COULD: Typo - bull should be but.

Redtigercod4 · 2026-09-13T14:52:15Z

+const DebRepositoryDetail = PageWithTabs<
+  DebRepositoryType & { remote?: DebRemoteType }
+>({
+  breadcrumbs: ({ name, tab, params: { repositoryVersion } }) =>


WOULD: The three tab === 'repositoru-versions' tenaries repeat the same guard to build what's really one decision - harder to read than it needs to be. Suggest collapsing into a single guarded branch instead.

warisshaikh1 marked this pull request as ready for review September 1, 2026 09:51

Redtigercod4 suggested changes Sep 13, 2026

View reviewed changes

Navigation Menu

Uh oh!

Add deb repository and remote pages - #347

Add deb repository and remote pages#347
warisshaikh1 wants to merge 1 commit into
pulp:mainfrom
warisshaikh1:deb-repositories-and-remotes

warisshaikh1 commented Aug 28, 2026 •

edited

Loading

Uh oh!

Redtigercod4 left a comment •

edited

Loading

Uh oh!

Redtigercod4 Sep 13, 2026

Uh oh!

Redtigercod4 Sep 13, 2026

Uh oh!

Redtigercod4 Sep 13, 2026

Uh oh!

Redtigercod4 Sep 13, 2026

Uh oh!

Redtigercod4 Sep 13, 2026

Uh oh!

Redtigercod4 Sep 13, 2026

Uh oh!

Redtigercod4 Sep 13, 2026

Uh oh!

Redtigercod4 Sep 13, 2026

Uh oh!

Redtigercod4 Sep 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,149 @@
		import { msg, t } from '@lingui/core/macro';

		@@ -0,0 +1,71 @@
		import { PulpAPI } from './pulp';

		export class DebRemoteType {

		@@ -0,0 +1,42 @@
		import { PulpAPI } from './pulp';

		export class DebRepositoryType {

Uh oh!

Conversation

warisshaikh1 commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this adds

What it deliberately leaves out

The one place deb genuinely differs from file

Testing

Two things I'd flag for review

Uh oh!

Redtigercod4 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

warisshaikh1 commented Aug 28, 2026 •

edited

Loading

Redtigercod4 left a comment •

edited

Loading