From ddcff89c3e655c258fe0a105b1207c08505471ac Mon Sep 17 00:00:00 2001 From: Roman Berezkin Date: Fri, 21 Aug 2026 11:19:32 +0300 Subject: [PATCH 1/2] [mirror] Push CLI plugins to the registry root above the edition d8 CLI artifacts are published once for all editions, at the registry root above the edition segment. Pull already reads plugins from there, and the in-cluster registry-packages-proxy looks them up there by cutting the edition off the cluster's imagesRepo. Push was the only side gluing every bundle segment onto the target as typed, so a push to REGISTRY/dkp/ee left plugins at REGISTRY/dkp/ee/deckhouse-cli/plugins while the cluster looked one level up: d8 plugins install could not find them in an air-gapped registry. Push now writes the deckhouse-cli/ part of the bundle, images and the catalog discovery tags alike, to the target minus its trailing edition segment. A target without an edition is used as is. Platform, modules, packages, installer and security databases stay under the target as before. The push log names the plugins path before the first write there, and a denied write above the target no longer surfaces as a bare 401/403: the diagnostic says which repository was refused, which target it sits above and which edition was cut off. Signed-off-by: Roman Berezkin --- internal/mirror/README.MD | 19 +- .../mirror/cmd/push/errdetect/diagnose.go | 53 +++ .../cmd/push/errdetect/diagnose_test.go | 59 +++ internal/mirror/cmd/push/push.go | 16 +- internal/mirror/plugins_path.go | 121 ++++++ internal/mirror/plugins_path_test.go | 66 ++++ internal/mirror/pull_plugins_e2e_test.go | 32 +- internal/mirror/push.go | 139 +++++-- internal/mirror/push_test.go | 349 +++++++++++++++++- 9 files changed, 791 insertions(+), 63 deletions(-) create mode 100644 internal/mirror/plugins_path.go create mode 100644 internal/mirror/plugins_path_test.go diff --git a/internal/mirror/README.MD b/internal/mirror/README.MD index 2f1da3591..5be3db805 100644 --- a/internal/mirror/README.MD +++ b/internal/mirror/README.MD @@ -411,6 +411,8 @@ Registry layout (at the **bare root**, outside the edition segment - like the in | `/deckhouse-cli/plugins` | Plugin catalog; its tags are plugin names | | `/deckhouse-cli/plugins/:` | One plugin version - a multi-platform OCI index (linux/darwin/windows) | +`` is the registry path **without** the edition: `registry.deckhouse.io/deckhouse/deckhouse-cli/plugins`, never `registry.deckhouse.io/deckhouse/ee/deckhouse-cli/plugins`. Pull derives it by cutting a trailing edition segment (`ce`, `be`, `se`, `se-plus`, `ee`, `fe`) off `--source`, and the in-cluster registry-packages-proxy derives it the same way from the cluster's `imagesRepo`. Push applies the same rule to its target, so the two sides agree: `d8 mirror push ./bundle REGISTRY/dkp/ee` writes the platform, modules, packages and security databases under `REGISTRY/dkp/ee` and the plugins to `REGISTRY/dkp/deckhouse-cli/plugins`. A target without an edition segment (`registry.company.com/deckhouse`, `dev-registry.deckhouse.io/sys/deckhouse-oss`) is used as is. The push log says where plugins go before the first plugin upload. + **Bundle output:** one `plugin-.tar` per plugin. Multi-platform indexes are stored whole, so every platform binary and the contract annotation reach the target registry exactly as published. What is checked at mirror time vs install time: @@ -419,7 +421,9 @@ What is checked at mirror time vs install time: - `kubernetes` and `noneOf` requirements are cluster-side: `d8 plugins install` enforces them on the target cluster as usual. - Conditional requirements never gate mirroring; conflicts show up as warnings. -After `d8 mirror push`, `d8 plugins install ` works in the air-gapped cluster through the registry-packages-proxy. Note the proxy serves plugins **by exact name** - listing the plugin catalog through it is not supported (a registry-packages-proxy limitation, not a bundle one; `crane ls /deckhouse-cli/plugins` shows the names). +After `d8 mirror push`, `d8 plugins install ` works in the air-gapped cluster through the registry-packages-proxy. Note the proxy serves plugins **by exact name** - listing the plugin catalog through it is not supported (a registry-packages-proxy limitation, not a bundle one; `crane ls /deckhouse-cli/plugins` shows the names, e.g. `crane ls REGISTRY/dkp/deckhouse-cli/plugins` after a push to `REGISTRY/dkp/ee`). + +Because plugins are written one level above the target, the push credentials must be allowed to write there too. When the registry answers HTTP 401/403 for `/deckhouse-cli/...` while the target itself is writable, the push fails at the first plugin write - the pre-push access check covers the target only, so the components under it are uploaded first - with a diagnostic that names the refused repository, the target it sits above and the edition that was cut off; grant the account push access to `/deckhouse-cli` (or create that repository, on registries that need repositories created up front), or push the bundle without `plugin-.tar` archives via `--file`. If the source registry has no `deckhouse-cli/plugins` catalog, or denies access to it (HTTP 401/403 - token-auth registries answer this way for any path outside the license's scope, published or not), the automatic selection is skipped quietly. Explicit `--include-plugin` entries still resolve against their own repositories, and a denied explicit include fails the pull. @@ -491,7 +495,9 @@ When `--modules-path-suffix` moves modules off the default `modules/` and module Uploads a previously downloaded Deckhouse Kubernetes Platform distribution bundle to a third-party container registry. This is typically used to populate an air-gapped registry with Deckhouse images. -Before uploading, push runs a write-access check against the target repository (15-second timeout, or `D8_MIRROR_TIMEOUT`); set `MIRROR_BYPASS_ACCESS_CHECKS=1` to proceed even if that check fails. Chunked packages (`.tar.NNNN.chunk`) are reassembled transparently, and each image upload is retried up to 4 times. After the layouts are pushed, small discovery-index tags are created at `/modules:` and `/packages:` so the mirrored modules and packages can be enumerated by tag listing. +Before uploading, push runs a write-access check against the target repository (15-second timeout, or `D8_MIRROR_TIMEOUT`); set `MIRROR_BYPASS_ACCESS_CHECKS=1` to proceed even if that check fails. Chunked packages (`.tar.NNNN.chunk`) are reassembled transparently, and each image upload is retried up to 4 times. After the layouts are pushed, small discovery-index tags are created at `/modules:`, `/packages:` and `/deckhouse-cli/plugins:` so the mirrored modules, packages and plugins can be enumerated by tag listing. + +Everything in the bundle is written under the target as is, with one exception: d8 CLI plugins go to the registry root **above** the target's edition segment, `REGISTRY/dkp/ee` -> `REGISTRY/dkp/deckhouse-cli/plugins`, because that is where pull reads them from and where the in-cluster registry-packages-proxy looks for them (it cuts the edition off the cluster's `imagesRepo` the same way). A target without an edition segment is used as is. See [Plugin Mirroring](#plugin-mirroring) for the rule and the access rights it needs. ### Synopsis @@ -553,11 +559,18 @@ d8 mirror push /tmp/d8-bundle registry.company.com/deckhouse \ --registry-login admin \ --registry-password secretpassword \ --tmp-dir /mnt/large-disk/tmp + +# Push to an edition-suffixed target (the DKP docs layout): the platform, +# modules, packages and security databases go under registry.company.com/dkp/ee, +# the CLI plugins to registry.company.com/dkp/deckhouse-cli/plugins +d8 mirror push /tmp/d8-bundle registry.company.com/dkp/ee \ + --registry-login admin \ + --registry-password secretpassword ``` ### Summary -After a push, `d8 mirror` prints a framed summary of what was written to the target registry (platform, installer, security databases, module and package counts). +After a push, `d8 mirror` prints a framed summary of what was written to the target registry (platform, installer, security databases, module, package and plugin counts). When `--modules-path-suffix` moves modules off the default `modules/` and modules were actually pushed, the summary warns about the non-default modules path, highlighting it with a hint of the standard path. A default path, or a moved path that no module went through, produces no warning. diff --git a/internal/mirror/cmd/push/errdetect/diagnose.go b/internal/mirror/cmd/push/errdetect/diagnose.go index 9662df7ba..347bb0f0b 100644 --- a/internal/mirror/cmd/push/errdetect/diagnose.go +++ b/internal/mirror/cmd/push/errdetect/diagnose.go @@ -27,10 +27,13 @@ import ( "net" "net/http" "os" + "path" "syscall" "github.com/google/go-containerregistry/pkg/v1/remote/transport" + "github.com/deckhouse/deckhouse-cli/internal" + "github.com/deckhouse/deckhouse-cli/internal/mirror" "github.com/deckhouse/deckhouse-cli/internal/mirror/errmatch" "github.com/deckhouse/deckhouse-cli/pkg/diagnostic" ) @@ -41,6 +44,7 @@ const ( categoryAuth = "Authentication failed" categoryAuth401 = "Authentication failed (HTTP 401 Unauthorized)" categoryAuth403 = "Access denied (HTTP 403 Forbidden)" + categoryPluginsRoot = "Access denied to the CLI plugins path above the target" categoryRateLimit = "Rate limited by registry (HTTP 429 Too Many Requests)" categoryServerError = "Registry server error" categoryDNS = "DNS resolution failed" @@ -123,6 +127,11 @@ func Diagnose(err error) *diagnostic.HelpfulError { }, } + // A denied write to the plugins root names a path the user never typed: + // spell that path out instead of giving the generic credentials advice. + case isPluginsRootAuthError(err): + return diagnosePluginsRootDenied(err) + case isAuthenticationError(err): category := categoryAuth if code := authStatusCode(err); code == http.StatusUnauthorized { @@ -346,8 +355,52 @@ func Diagnose(err error) *diagnostic.HelpfulError { return nil } +// diagnosePluginsRootDenied explains a 401/403 on the CLI plugins root: where +// the write went, why that path is above the target, and how to unblock it. +func diagnosePluginsRootDenied(err error) *diagnostic.HelpfulError { + var rootErr *mirror.PluginsRootError + errors.As(err, &rootErr) + + report := rootErr.Report + + category := categoryPluginsRoot + if code := authStatusCode(err); code != 0 { + category = fmt.Sprintf("%s (HTTP %d)", categoryPluginsRoot, code) + } + + pluginsRepo := path.Join(report.Root, internal.D8CLISegment) + + return &diagnostic.HelpfulError{ + Category: category, + OriginalErr: err, + Suggestions: []diagnostic.Suggestion{ + { + Cause: fmt.Sprintf( + "d8 CLI plugins are written to %s, the registry root above the %q edition of the target %s "+ + "(registry-packages-proxy in the cluster looks for them there). "+ + "The registry refused the write to %s: the credentials cover only the target path, or the registry has no repository at that level", + report.Path, report.Edition, report.Target, rootErr.Repo, + ), + Solutions: []string{ + fmt.Sprintf("Grant the account push access to %s and its sub-repositories", pluginsRepo), + fmt.Sprintf("If the registry needs repositories (projects) created up front, create %s next to %s", pluginsRepo, report.Target), + "To push the bundle without plugins, pass the archives with --file and leave out plugin-.tar", + }, + }, + }, + } +} + // --- detection functions --- +// isPluginsRootAuthError reports a 401/403 on a write to the CLI plugins +// root above the target (see mirror.PluginsRootError). +func isPluginsRootAuthError(err error) bool { + var rootErr *mirror.PluginsRootError + + return errors.As(err, &rootErr) && isAuthenticationError(err) +} + func isEOF(err error) bool { return errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) } diff --git a/internal/mirror/cmd/push/errdetect/diagnose_test.go b/internal/mirror/cmd/push/errdetect/diagnose_test.go index e87cc9f4c..de39b4667 100644 --- a/internal/mirror/cmd/push/errdetect/diagnose_test.go +++ b/internal/mirror/cmd/push/errdetect/diagnose_test.go @@ -31,6 +31,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/deckhouse/deckhouse-cli/internal/mirror" "github.com/deckhouse/deckhouse-cli/pkg/diagnostic" ) @@ -86,6 +87,64 @@ func TestDiagnose_PushSpecificAuth(t *testing.T) { assert.NotContains(t, solutions, "--source-login") } +// pluginsRootErr wraps a registry status the way push does for a failed write +// above the edition: the status sits inside the typed error's chain. +func pluginsRootErr(status int) error { + return fmt.Errorf("push to registry: %w", &mirror.PluginsRootError{ + Repo: "registry.example.com/dkp/deckhouse-cli/plugins/postgresql-mgr", + Report: mirror.PluginsPathReport{ + Edition: "ee", + Target: "registry.example.com/dkp/ee", + Root: "registry.example.com/dkp", + Path: "registry.example.com/dkp/deckhouse-cli/plugins", + }, + Err: fmt.Errorf("push layout: %w", &transport.Error{StatusCode: status}), + }) +} + +// TestDiagnose_PluginsRootDenied checks that a 401/403 on the plugins root +// above the edition is explained in terms of the paths: where the write went, +// which target it sits above, and what to grant. The generic auth advice +// (check --registry-login) is wrong here: the credentials do work for the +// target. +func TestDiagnose_PluginsRootDenied(t *testing.T) { + for _, status := range []int{http.StatusUnauthorized, http.StatusForbidden} { + t.Run(http.StatusText(status), func(t *testing.T) { + diag := Diagnose(pluginsRootErr(status)) + require.NotNil(t, diag) + + assert.Contains(t, diag.Category, categoryPluginsRoot) + assert.Contains(t, diag.Category, fmt.Sprintf("HTTP %d", status)) + assert.NotContains(t, diag.Category, categoryAuth) + + require.NotEmpty(t, diag.Suggestions) + cause := diag.Suggestions[0].Cause + assert.Contains(t, cause, "registry.example.com/dkp/deckhouse-cli/plugins") + assert.Contains(t, cause, `"ee" edition`) + assert.Contains(t, cause, "registry.example.com/dkp/ee") + assert.Contains(t, cause, "registry.example.com/dkp/deckhouse-cli/plugins/postgresql-mgr") + + solutions := allSolutions(diag) + assert.Contains(t, solutions, "registry.example.com/dkp/deckhouse-cli") + assert.Contains(t, solutions, "--file") + assert.NotContains(t, solutions, "--registry-login") + + var transportErr *transport.Error + assert.True(t, errors.As(diag, &transportErr), "the registry status must stay reachable") + }) + } +} + +// TestDiagnose_PluginsRootNonAuthFallsThrough checks that the plugins-root +// wrapper changes nothing for errors that are not about access: a 500 on the +// plugins path is still a server error. +func TestDiagnose_PluginsRootNonAuthFallsThrough(t *testing.T) { + diag := Diagnose(pluginsRootErr(http.StatusInternalServerError)) + require.NotNil(t, diag) + assert.Contains(t, diag.Category, categoryServerError) + assert.NotContains(t, diag.Category, categoryPluginsRoot) +} + func TestDiagnose_DiskFull(t *testing.T) { diag := Diagnose(fmt.Errorf("write temp: %w", syscall.ENOSPC)) require.NotNil(t, diag) diff --git a/internal/mirror/cmd/push/push.go b/internal/mirror/cmd/push/push.go index cc3261d43..ca3a64ca5 100644 --- a/internal/mirror/cmd/push/push.go +++ b/internal/mirror/cmd/push/push.go @@ -244,8 +244,11 @@ func (p *Pusher) warnNonDefaultModulesPath() { // executeNewPush runs the push using the push service. // This service expects the bundle to have the exact same structure as the registry: -// - Each OCI layout's relative path becomes its registry segment -// - Works with unified bundles where pull saved the structure as-is +// - Each OCI layout's relative path becomes its registry segment +// - Works with unified bundles where pull saved the structure as-is +// - CLI plugins (deckhouse-cli/) are the exception: they go to the registry +// root above the target's edition segment, e.g. REGISTRY/dkp/ee pushes them +// to REGISTRY/dkp/deckhouse-cli/plugins func (p *Pusher) executeNewPush() error { // Set up graceful cancellation on Ctrl+C ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) @@ -268,16 +271,15 @@ func (p *Pusher) executeNewPush() error { clientOpts = append(clientOpts, regclient.WithTimeout(MirrorTimeout)) } + // The client is scoped to the host only. The service scopes the target + // path itself: it needs the path to place CLI plugins at the registry + // root above the target's edition segment. client := pkgclient.NewFromOptions(p.pushParams.RegistryHost, clientOpts...) - // Scope to the registry path - if p.pushParams.RegistryPath != "" { - client = client.WithSegment(p.pushParams.RegistryPath) - } - svc := mirror.NewPushService( client, &mirror.PushServiceOptions{ + TargetPath: p.pushParams.RegistryPath, Packages: Packages, WorkingDir: p.pushParams.WorkingDir, ModulesPathSuffix: p.pushParams.ModulesPathSuffix, diff --git a/internal/mirror/plugins_path.go b/internal/mirror/plugins_path.go new file mode 100644 index 000000000..27027ae3a --- /dev/null +++ b/internal/mirror/plugins_path.go @@ -0,0 +1,121 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mirror + +import ( + "fmt" + "path" + + "github.com/deckhouse/deckhouse-cli/internal" + "github.com/deckhouse/deckhouse-cli/pkg" + pkgclient "github.com/deckhouse/deckhouse-cli/pkg/registry/client" + registryservice "github.com/deckhouse/deckhouse-cli/pkg/registry/service" +) + +// PluginsPathReport describes where d8 CLI plugins are written by push. +// +// CLI artifacts live at the registry root above the edition segment: +// registry.example.com/deckhouse/deckhouse-cli/plugins, never +// registry.example.com/deckhouse/ee/deckhouse-cli/plugins. Pull reads them +// from there, and the in-cluster registry-packages-proxy looks them up there +// by cutting the edition off the cluster's imagesRepo. Push applies the same +// rule to its target, so the other components (platform, modules, packages, +// installer, security) stay under the target while plugins go one level up. +type PluginsPathReport struct { + // Edition is the edition segment cut from the end of the push target, + // e.g. "ee" for a target ".../deckhouse/ee". Empty when the target has + // no edition segment, in which case Root equals Target. + Edition string + // Target is the push target as given by the user, e.g. + // "registry.example.com/deckhouse/ee". + Target string + // Root is the registry root plugins are written under: Target without + // Edition, e.g. "registry.example.com/deckhouse". + Root string + // Path is the plugins catalog, "/deckhouse-cli/plugins". Plugin + // images go to "/", discovery tags to Path itself. + Path string +} + +// pluginsRootPath returns the target path CLI artifacts are written under: +// targetPath without its trailing edition segment, plus that segment. +// +// The root always keeps at least one path segment. A target like +// "registry.example.com/ee" names a project that happens to be called "ee", +// not an edition of a Deckhouse repository, so it is left alone. The +// registry-packages-proxy applies the same rule when it looks the artifacts +// up, and both sides must agree on the result. +func pluginsRootPath(targetPath string) (string, string) { + root, edition := registryservice.GetEditionFromRegistryPath(targetPath) + if edition == pkg.NoEdition { + return targetPath, "" + } + + if len(pkgclient.PathToSegments(root)) == 0 { + return targetPath, "" + } + + return root, edition.String() +} + +func newPluginsPathReport(target, root, edition string) PluginsPathReport { + return PluginsPathReport{ + Edition: edition, + Target: target, + Root: root, + Path: path.Join(root, internal.D8CLISegment, internal.D8PluginsSegment), + } +} + +// Moved reports whether plugins go above the push target, i.e. an edition +// segment was cut off it. +func (p PluginsPathReport) Moved() bool { + return p.Edition != "" +} + +// Notice is the single-line form for the push log, printed before the first +// plugin write so the user sees where plugins go and why it is not the target. +func (p PluginsPathReport) Notice() string { + if !p.Moved() { + return fmt.Sprintf("CLI plugins go to %s", p.Path) + } + + return fmt.Sprintf("CLI plugins go to %s (registry root above the %q edition of %s; registry-packages-proxy looks for them there)", + p.Path, p.Edition, p.Target) +} + +// PluginsRootError is a failed write to the CLI plugins root when that root +// sits above the push target (see PluginsPathReport). It carries the paths so +// the diagnostic can say where the write went and why, instead of leaving the +// user with a bare registry error for a path they never typed. +type PluginsRootError struct { + // Repo is the repository the write went to, e.g. + // "registry.example.com/deckhouse/deckhouse-cli/plugins/postgresql-mgr". + Repo string + // Report describes the plugins root relative to the push target. + Report PluginsPathReport + // Err is the underlying push error. + Err error +} + +func (e *PluginsRootError) Error() string { + return e.Err.Error() +} + +func (e *PluginsRootError) Unwrap() error { + return e.Err +} diff --git a/internal/mirror/plugins_path_test.go b/internal/mirror/plugins_path_test.go new file mode 100644 index 000000000..045817b27 --- /dev/null +++ b/internal/mirror/plugins_path_test.go @@ -0,0 +1,66 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mirror + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestPluginsRootPath: the CLI artifacts root is the push target without its +// trailing edition segment. The rule must match the one the +// registry-packages-proxy applies to the cluster's imagesRepo, including the +// cases where nothing is cut. +func TestPluginsRootPath(t *testing.T) { + t.Parallel() + + cases := []struct { + target string + wantRoot string + wantEdition string + }{ + // An edition segment is cut off. + {"deckhouse/ee", "deckhouse", "ee"}, + {"deckhouse/ce", "deckhouse", "ce"}, + {"deckhouse/se-plus", "deckhouse", "se-plus"}, + {"dkp/ee", "dkp", "ee"}, + {"mirror/deckhouse/fe", "mirror/deckhouse", "fe"}, + // No edition segment: the target is the root already. + {"deckhouse", "deckhouse", ""}, + {"sys/deckhouse-oss", "sys/deckhouse-oss", ""}, + {"deckhouse/ee-mirror", "deckhouse/ee-mirror", ""}, + {"", "", ""}, + // CSE keeps its editionless artifacts under deckhouse/cse, so that + // path is a root of its own. + {"deckhouse/cse", "deckhouse/cse", ""}, + // The root keeps at least one path segment: "ee" alone names a + // project, and cutting it would push plugins to the registry host. + {"ee", "ee", ""}, + {"/se/", "/se/", ""}, + } + + for _, tc := range cases { + t.Run(tc.target, func(t *testing.T) { + t.Parallel() + + root, edition := pluginsRootPath(tc.target) + assert.Equal(t, tc.wantRoot, root, "root") + assert.Equal(t, tc.wantEdition, edition, "edition") + }) + } +} diff --git a/internal/mirror/pull_plugins_e2e_test.go b/internal/mirror/pull_plugins_e2e_test.go index 172726f87..00f235f1a 100644 --- a/internal/mirror/pull_plugins_e2e_test.go +++ b/internal/mirror/pull_plugins_e2e_test.go @@ -450,9 +450,10 @@ func TestPullE2E_OnlyExtraImages_PluginsSkipped(t *testing.T) { // TestPullE2E_RoundTrip_PullThenPushPlugins carries plugins through the whole // mirror path: pull from the source registry, pack into bundle tars, push the -// tars into a target registry. The plugin repository must land verbatim at -// deckhouse-cli/plugins with its discovery tag, and --modules-path-suffix -// must move modules only. +// tars into a target registry with an edition segment. The plugin repository +// must land at deckhouse-cli/plugins under the root above that edition, with +// its discovery tag, while --modules-path-suffix moves modules only and they +// stay under the edition. func TestPullE2E_RoundTrip_PullThenPushPlugins(t *testing.T) { reg := upfake.NewRegistry(pullStubRootURL) addModule(reg, "postgresql", map[string]string{"stable": "v1.5.0", "alpha": "v1.10.0"}) @@ -478,13 +479,14 @@ func TestPullE2E_RoundTrip_PullThenPushPlugins(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, tars) - destReg := upfake.NewRegistry("registry.example.com/deckhouse/ee") - destClient := pkgclient.Adapt(upfake.NewClient(destReg)) + destReg := upfake.NewRegistry("registry.example.com") + hostClient := pkgclient.Adapt(upfake.NewClient(destReg)) logger := dkplog.NewLogger(dkplog.WithLevel(slog.LevelWarn)) userLogger := log.NewSLogger(slog.LevelWarn) - pushSvc := NewPushService(destClient, &PushServiceOptions{ + pushSvc := NewPushService(hostClient, &PushServiceOptions{ + TargetPath: "/deckhouse/ee", Packages: tars, WorkingDir: t.TempDir(), // A moved modules path must not touch plugins. @@ -496,18 +498,28 @@ func TestPullE2E_RoundTrip_PullThenPushPlugins(t *testing.T) { assert.Equal(t, 1, pushSummary.Plugins, "one plugin repository pushed") assert.Equal(t, 1, pushSummary.Modules) + assert.Equal(t, "registry.example.com/deckhouse/deckhouse-cli/plugins", pushSvc.pluginsPath.Path) - // Both resolved plugin versions land verbatim at deckhouse-cli/plugins. - pluginClient := destClient.WithSegment("deckhouse-cli", "plugins", "postgresql-mgr") + // Plugins are rooted above the edition: registry.example.com/deckhouse, + // not .../deckhouse/ee. Both resolved plugin versions land there. + rootClient := hostClient.WithSegment("deckhouse") + destClient := rootClient.WithSegment("ee") + + pluginClient := rootClient.WithSegment("deckhouse-cli", "plugins", "postgresql-mgr") assert.NoError(t, pluginClient.CheckImageExists(ctx, "v1.1.0")) assert.NoError(t, pluginClient.CheckImageExists(ctx, "v1.2.0")) // The discovery tag makes the plugin visible to catalog listing. - catalogTags, err := destClient.WithSegment("deckhouse-cli", "plugins").ListTags(ctx) + catalogTags, err := rootClient.WithSegment("deckhouse-cli", "plugins").ListTags(ctx) require.NoError(t, err) assert.Contains(t, catalogTags, "postgresql-mgr") - // The module moved with the suffix; the plugin stayed put. + // Nothing plugin-related is left under the edition. + underEdition := destClient.WithSegment("deckhouse-cli", "plugins", "postgresql-mgr") + assert.Error(t, underEdition.CheckImageExists(ctx, "v1.1.0"), + "plugins must not be written under the edition segment") + + // The module moved with the suffix and stayed under the edition. movedModule := destClient.WithSegment("my", "mods", "postgresql") assert.NoError(t, movedModule.CheckImageExists(ctx, "v1.5.0")) assert.NoError(t, movedModule.CheckImageExists(ctx, "v1.10.0")) diff --git a/internal/mirror/push.go b/internal/mirror/push.go index 29638a997..87baa916c 100644 --- a/internal/mirror/push.go +++ b/internal/mirror/push.go @@ -49,6 +49,12 @@ const ( // PushServiceOptions contains configuration options for PushService type PushServiceOptions struct { + // TargetPath is the repository path under the registry host the bundle is + // pushed to, e.g. "/deckhouse/ee". Leading and trailing slashes are + // ignored; empty pushes to the host root. A trailing edition segment + // ("ee", "se-plus", ...) is cut off to get the root CLI plugins are + // written under (see PluginsPathReport). + TargetPath string // Packages is the list of tar/chunked package archive paths to push. Packages []string // WorkingDir is the temporary directory for unpacking bundles @@ -94,16 +100,27 @@ type PushServiceOptions struct { // ├── index.json // └── blobs/ type PushService struct { - client client.Client + // client is scoped to the push target: host + TargetPath. + client client.Client + // targetRef is the push target as a registry reference, + // e.g. "registry.example.com/deckhouse/ee". + targetRef string + // pluginsRoot is scoped to the target minus its trailing edition segment. + // The deckhouse-cli/ part of the bundle is written under it, everything + // else under client. Equals client when the target has no edition. + pluginsRoot client.Client + pluginsPath PluginsPathReport + options *PushServiceOptions pusher *pusher.Service logger *dkplog.Logger userLogger *log.SLogger } -// NewPushService creates a new PushService +// NewPushService creates a new PushService. hostClient is scoped to the +// registry host only; the service scopes it to options.TargetPath itself. func NewPushService( - client client.Client, + hostClient client.Client, options *PushServiceOptions, logger *dkplog.Logger, userLogger *log.SLogger, @@ -112,12 +129,25 @@ func NewPushService( options = &PushServiceOptions{} } + host := hostClient.GetRegistry() + target := hostClient.WithSegment(pkgclient.PathToSegments(options.TargetPath)...) + + // CLI plugins live at the registry root above the edition, the same rule + // pull reads them by and registry-packages-proxy looks them up by. + rootPath, edition := pluginsRootPath(options.TargetPath) + pluginsRoot := hostClient.WithSegment(pkgclient.PathToSegments(rootPath)...) + + targetRef := path.Join(host, options.TargetPath) + return &PushService{ - client: client, - options: options, - pusher: pusher.NewService(logger, userLogger), - logger: logger, - userLogger: userLogger, + client: target, + targetRef: targetRef, + pluginsRoot: pluginsRoot, + pluginsPath: newPluginsPathReport(targetRef, path.Join(host, rootPath), edition), + options: options, + pusher: pusher.NewService(logger, userLogger), + logger: logger, + userLogger: userLogger, } } @@ -126,7 +156,9 @@ func NewPushService( // using its relative path as the registry segment. // // The key principle: no path transformations. Whatever path the layout has -// in the unpacked directory becomes its path in the registry. +// in the unpacked directory becomes its path in the registry. The two +// exceptions are the modules path (--modules-path-suffix) and the +// deckhouse-cli/ segment, which is rooted above the target's edition. func (svc *PushService) Push(ctx context.Context) (*PushSummary, error) { // The modules path is known up front, so it is on the summary even on error. summary := &PushSummary{ModulesPath: svc.modulesPathReport()} @@ -184,7 +216,7 @@ func (svc *PushService) Push(ctx context.Context) (*PushSummary, error) { // modulesPathReport resolves the target modules path for the push summary, // honoring --modules-path-suffix. func (svc *PushService) modulesPathReport() ModulesPathReport { - return BuildModulesPathReport(svc.client.GetRegistry(), svc.options.ModulesPathSuffix) + return BuildModulesPathReport(svc.targetRef, svc.options.ModulesPathSuffix) } // modulesPath returns the registry path for module repositories, relative to @@ -296,6 +328,14 @@ func (svc *PushService) pushAllLayouts(ctx context.Context, rootDir string, summ svc.userLogger.Infof("Found %d layouts to push", len(layouts)) + // Say where plugins go before the first write there: it is the one path + // that is not under the target the user typed. + if slices.ContainsFunc(layouts, func(layoutDir string) bool { + return isPluginsSegment(layoutSegment(rootDir, layoutDir)) + }) { + svc.userLogger.InfoLn(svc.pluginsPath.Notice()) + } + for _, layoutDir := range layouts { if err := ctx.Err(); err != nil { return err @@ -309,6 +349,26 @@ func (svc *PushService) pushAllLayouts(ctx context.Context, rootDir string, summ return nil } +// layoutSegment is the bundle segment of a layout: its path relative to the +// unpacked root in slash form, so it is registry-native and OS-independent +// (filepath.Rel yields OS separators on Windows). Empty for the root layout. +func layoutSegment(rootDir, layoutDir string) string { + relPath, _ := filepath.Rel(rootDir, layoutDir) + if relPath == "." { + return "" + } + + return filepath.ToSlash(relPath) +} + +// isPluginsSegment reports whether a bundle segment belongs to the +// deckhouse-cli/ part of the bundle, which is rooted above the edition. +func isPluginsSegment(segment string) bool { + first, _, _ := strings.Cut(segment, "/") + + return first == internal.D8CLISegment +} + // findLayouts finds all OCI layouts in the directory by looking for index.json files. func (svc *PushService) findLayouts(rootDir string) ([]string, error) { var layouts []string @@ -351,15 +411,7 @@ func (svc *PushService) pushSingleLayout(ctx context.Context, rootDir, layoutDir return nil } - // Build registry segment from relative path. Use slash form so the - // segment is registry-native and OS-independent (filepath.Rel yields - // OS separators on Windows). - relPath, _ := filepath.Rel(rootDir, layoutDir) - - segment := "" - if relPath != "." { - segment = filepath.ToSlash(relPath) - } + segment := layoutSegment(rootDir, layoutDir) // support old behavior when modules stored as "module-.tar" if strings.HasPrefix(layoutDir, "module-") { segment = internal.ModulesSegment @@ -371,12 +423,14 @@ func (svc *PushService) pushSingleLayout(ctx context.Context, rootDir, layoutDir // Rewrite the leading "modules" component to honor --modules-path-suffix. segment = svc.remapModulesSegment(segment) - targetClient := svc.client.WithSegment(pkgclient.PathToSegments(segment)...) + targetClient := svc.layoutRoot(origSegment).WithSegment(pkgclient.PathToSegments(segment)...) svc.userLogger.Infof("Pushing %s", targetClient.GetRegistry()) if err := svc.pusher.PushLayout(ctx, layout.Path(layoutDir), targetClient); err != nil { - return fmt.Errorf("push layout %q to registry %s: %w", relPath, targetClient.GetRegistry(), err) + err = fmt.Errorf("push layout %q to registry %s: %w", origSegment, targetClient.GetRegistry(), err) + + return svc.wrapPluginsRootError(origSegment, err) } recordPushedComponent(summary, origSegment) @@ -384,6 +438,35 @@ func (svc *PushService) pushSingleLayout(ctx context.Context, rootDir, layoutDir return nil } +// layoutRoot returns the registry root a bundle segment is written under: the +// deckhouse-cli/ segment goes above the target's edition, everything else to +// the target. It takes the bundle segment, not the registry path the layout +// ends up at, so a --modules-path-suffix naming the deckhouse-cli segment +// still moves modules within the target. +func (svc *PushService) layoutRoot(bundleSegment string) client.Client { + if isPluginsSegment(bundleSegment) { + return svc.pluginsRoot + } + + return svc.client +} + +// wrapPluginsRootError marks a failed write to the plugins root with the +// paths involved, but only when that root differs from the target: that is +// the case where the user did not type the path the registry refused and +// needs it spelled out. Other errors pass through unchanged. +func (svc *PushService) wrapPluginsRootError(segment string, err error) error { + if !isPluginsSegment(segment) || !svc.pluginsPath.Moved() { + return err + } + + return &PluginsRootError{ + Repo: path.Join(svc.pluginsPath.Root, segment), + Report: svc.pluginsPath, + Err: err, + } +} + // recordPushedComponent tallies a pushed layout into the summary by its bundle // segment. Modules, packages, and plugins are counted from their index steps, // so their layouts are ignored here. @@ -548,9 +631,10 @@ func (svc *PushService) createPackagesIndex(ctx context.Context, rootDir string, // createPluginsIndex creates the CLI plugins index in the registry: a small // random image per plugin with tag = plugin name on the deckhouse-cli/plugins -// path. The same directory-as-tags convention modules and packages use; the -// registry-bundle server synthesizes an identical index for bundle-served -// registries, so both air-gapped delivery shapes look the same. +// path under the plugins root (above the target's edition, see +// PluginsPathReport). The same directory-as-tags convention modules and +// packages use; the registry-bundle server synthesizes an identical index for +// bundle-served registries, so both air-gapped delivery shapes look the same. func (svc *PushService) createPluginsIndex(ctx context.Context, rootDir string, summary *PushSummary) error { pluginsDir := filepath.Join(rootDir, internal.D8CLISegment, internal.D8PluginsSegment) @@ -581,7 +665,8 @@ func (svc *PushService) createPluginsIndex(ctx context.Context, rootDir string, summary.Plugins = len(pluginNames) svc.userLogger.Infof("Creating plugins index with %d plugins", len(pluginNames)) - pluginsClient := svc.client.WithSegment(internal.D8CLISegment, internal.D8PluginsSegment) + pluginsSegment := path.Join(internal.D8CLISegment, internal.D8PluginsSegment) + pluginsClient := svc.layoutRoot(pluginsSegment).WithSegment(internal.D8CLISegment, internal.D8PluginsSegment) for _, pluginName := range pluginNames { if err := ctx.Err(); err != nil { @@ -596,7 +681,9 @@ func (svc *PushService) createPluginsIndex(ctx context.Context, rootDir string, } if err := pluginsClient.PushImage(ctx, pluginName, img); err != nil { - return fmt.Errorf("push plugin index tag %s to registry %s: %w", pluginName, pluginsClient.GetRegistry(), err) + err = fmt.Errorf("push plugin index tag %s to registry %s: %w", pluginName, pluginsClient.GetRegistry(), err) + + return svc.wrapPluginsRootError(pluginsSegment, err) } } diff --git a/internal/mirror/push_test.go b/internal/mirror/push_test.go index 6f8ee4b8e..f5099dbd7 100644 --- a/internal/mirror/push_test.go +++ b/internal/mirror/push_test.go @@ -18,17 +18,23 @@ package mirror import ( "context" + "errors" "log/slog" + "net/http" "os" "path" "path/filepath" + "strings" "testing" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/layout" + "github.com/google/go-containerregistry/pkg/v1/remote/transport" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" dkplog "github.com/deckhouse/deckhouse/pkg/log" + dkpreg "github.com/deckhouse/deckhouse/pkg/registry" upfake "github.com/deckhouse/deckhouse/pkg/registry/fake" "github.com/deckhouse/deckhouse-cli/pkg/libmirror/bundle" @@ -99,13 +105,122 @@ func buildLayoutBundle(t *testing.T, dir, tarName, prefix, shortTag string) stri return tarPath } -// TestPushService_PluginsLayout verifies the plugin leg of push: a bundle tar -// prefixed deckhouse-cli/plugins/ lands verbatim at that registry path, -// the discovery tag appears on the plugins index, the summary counts it, and -// --modules-path-suffix never moves plugin paths. -func TestPushService_PluginsLayout(t *testing.T) { +// TestPushService_PluginsGoToRootAboveEdition verifies the plugin leg of push: +// a bundle tar prefixed deckhouse-cli/plugins/ lands under the registry +// root above the target's edition segment (the path pull reads plugins from +// and registry-packages-proxy looks them up by), the discovery tag appears on +// that plugins catalog, the summary counts it, the service's path report +// names it, and nothing else moves: a platform layout stays under the target +// and --modules-path-suffix never touches plugin paths. +func TestPushService_PluginsGoToRootAboveEdition(t *testing.T) { const ( - repoHost = "registry.example.com/deckhouse/ee" + host = "registry.example.com" + pluginName = "postgresql-mgr" + pluginTag = "v1.2.0" + installTag = "v1.76.2" + ) + + tests := []struct { + name string + // target is the push target path under host, as the CLI parses it + // (leading slash from the URL path). + target string + // wantRoot is the path under host plugins are rooted at. + wantRoot string + wantEdition string + }{ + {name: "edition target", target: "/deckhouse/ee", wantRoot: "deckhouse", wantEdition: "ee"}, + {name: "hyphenated edition", target: "/deckhouse/se-plus", wantRoot: "deckhouse", wantEdition: "se-plus"}, + {name: "DKP docs layout", target: "/dkp/ee", wantRoot: "dkp", wantEdition: "ee"}, + {name: "trailing slash", target: "/deckhouse/ee/", wantRoot: "deckhouse", wantEdition: "ee"}, + {name: "no edition", target: "/deckhouse", wantRoot: "deckhouse"}, + {name: "no edition, nested path", target: "/sys/deckhouse-oss", wantRoot: "sys/deckhouse-oss"}, + {name: "edition name not at the end is not an edition", target: "/ee/deckhouse", wantRoot: "ee/deckhouse"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bundleDir := t.TempDir() + pluginPkg := buildLayoutBundle(t, bundleDir, "plugin-"+pluginName+".tar", + path.Join("deckhouse-cli", "plugins", pluginName), pluginTag) + // A platform layout: it must stay under the target. + installPkg := buildLayoutBundle(t, bundleDir, "platform.tar", "install", installTag) + + reg := upfake.NewRegistry(host) + hostClient := pkgclient.Adapt(upfake.NewClient(reg)) + + logger := dkplog.NewLogger(dkplog.WithLevel(slog.LevelWarn)) + userLogger := log.NewSLogger(slog.LevelWarn) + + svc := NewPushService(hostClient, &PushServiceOptions{ + TargetPath: tt.target, + Packages: []string{pluginPkg, installPkg}, + WorkingDir: t.TempDir(), + // A moved modules path must not touch plugins. + ModulesPathSuffix: "/my/mods", + }, logger, userLogger) + + summary, err := svc.Push(context.Background()) + require.NoError(t, err, "push must succeed") + + assert.Equal(t, 1, summary.Plugins, "one plugin repository pushed") + assert.True(t, summary.PlatformPushed, "install layout counts as platform") + + ctx := context.Background() + + targetClient := hostClient.WithSegment(pkgclient.PathToSegments(tt.target)...) + rootClient := hostClient.WithSegment(pkgclient.PathToSegments(tt.wantRoot)...) + + // The service's report (what the log notice and the denied-write + // error are built from) names the real paths. Expected values are + // spelled out as strings: the fake client's GetRegistry reports the + // host only, so it cannot serve as an oracle. + wantTarget := host + "/" + strings.Trim(tt.target, "/") + wantRoot := host + "/" + tt.wantRoot + assert.Equal(t, tt.wantEdition, svc.pluginsPath.Edition) + assert.Equal(t, wantTarget, svc.pluginsPath.Target) + assert.Equal(t, wantRoot, svc.pluginsPath.Root) + assert.Equal(t, wantRoot+"/deckhouse-cli/plugins", svc.pluginsPath.Path) + assert.Equal(t, tt.wantEdition != "", svc.pluginsPath.Moved()) + + // Plugin image and discovery tag live at the root. + pluginClient := rootClient.WithSegment("deckhouse-cli", "plugins", pluginName) + assert.NoErrorf(t, pluginClient.CheckImageExists(ctx, pluginTag), + "plugin image must exist at %s/deckhouse-cli/plugins/%s:%s", wantRoot, pluginName, pluginTag) + + catalogClient := rootClient.WithSegment("deckhouse-cli", "plugins") + tags, err := catalogClient.ListTags(ctx) + require.NoError(t, err) + assert.Containsf(t, tags, pluginName, "discovery tag must exist at %s/deckhouse-cli/plugins", wantRoot) + + // The platform layout is unaffected. + installClient := targetClient.WithSegment("install") + assert.NoErrorf(t, installClient.CheckImageExists(ctx, installTag), + "install layout must stay at %s/install", wantTarget) + + if tt.wantEdition == "" { + return + } + + // An edition was cut off: plugins must MOVE above it, not be copied, + // so nothing is left under /deckhouse-cli. + underEdition := targetClient.WithSegment("deckhouse-cli", "plugins", pluginName) + assert.Errorf(t, underEdition.CheckImageExists(ctx, pluginTag), + "plugin must not remain under the edition at %s/deckhouse-cli/plugins/%s", wantTarget, pluginName) + + editionTags, err := targetClient.WithSegment("deckhouse-cli", "plugins").ListTags(ctx) + require.NoError(t, err) + assert.Empty(t, editionTags, "no discovery tag may remain under the edition") + }) + } +} + +// TestPushService_PluginLayoutIsNotPlatform pins the summary accounting for a +// plugin-only bundle: the plugin layout is counted by the plugins index step, +// never as a platform layout (which is what an unclassified segment would be). +func TestPushService_PluginLayoutIsNotPlatform(t *testing.T) { + const ( + host = "registry.example.com" pluginName = "postgresql-mgr" pluginTag = "v1.2.0" ) @@ -114,17 +229,15 @@ func TestPushService_PluginsLayout(t *testing.T) { pluginPkg := buildLayoutBundle(t, bundleDir, "plugin-"+pluginName+".tar", path.Join("deckhouse-cli", "plugins", pluginName), pluginTag) - reg := upfake.NewRegistry(repoHost) - destClient := pkgclient.Adapt(upfake.NewClient(reg)) + hostClient := pkgclient.Adapt(upfake.NewClient(upfake.NewRegistry(host))) logger := dkplog.NewLogger(dkplog.WithLevel(slog.LevelWarn)) userLogger := log.NewSLogger(slog.LevelWarn) - svc := NewPushService(destClient, &PushServiceOptions{ + svc := NewPushService(hostClient, &PushServiceOptions{ + TargetPath: "/deckhouse/ee", Packages: []string{pluginPkg}, WorkingDir: t.TempDir(), - // A moved modules path must not touch plugins. - ModulesPathSuffix: "/my/mods", }, logger, userLogger) summary, err := svc.Push(context.Background()) @@ -132,17 +245,219 @@ func TestPushService_PluginsLayout(t *testing.T) { assert.Equal(t, 1, summary.Plugins, "one plugin repository pushed") assert.False(t, summary.PlatformPushed, "a plugin layout must not be classified as platform") + assert.False(t, summary.InstallerPushed) + assert.Zero(t, summary.Modules) + assert.Zero(t, summary.Packages) + assert.Zero(t, summary.SecurityDatabases) +} + +// TestPushService_ModulesPathSuffixIntoPluginsNamespace pins what the routing +// keys on: layouts go to the plugins root by their bundle segment, not by the +// path they end up at. A --modules-path-suffix that names the CLI plugins +// segment therefore moves modules inside the target, never above its edition, +// and their images and discovery tag stay together. +func TestPushService_ModulesPathSuffixIntoPluginsNamespace(t *testing.T) { + const ( + host = "registry.example.com" + moduleName = "postgresql" + moduleTag = "v1.5.0" + ) + + bundleDir := t.TempDir() + modulePkg := buildLayoutBundle(t, bundleDir, "module-"+moduleName+".tar", + path.Join("modules", moduleName), moduleTag) + + hostClient := pkgclient.Adapt(upfake.NewClient(upfake.NewRegistry(host))) + + logger := dkplog.NewLogger(dkplog.WithLevel(slog.LevelWarn)) + userLogger := log.NewSLogger(slog.LevelWarn) + + svc := NewPushService(hostClient, &PushServiceOptions{ + TargetPath: "/deckhouse/ee", + Packages: []string{modulePkg}, + WorkingDir: t.TempDir(), + ModulesPathSuffix: "deckhouse-cli", + }, logger, userLogger) + + summary, err := svc.Push(context.Background()) + require.NoError(t, err, "push must succeed") + assert.Equal(t, 1, summary.Modules, "one module pushed") ctx := context.Background() - pluginClient := destClient.WithSegment("deckhouse-cli", "plugins", pluginName) - assert.NoErrorf(t, pluginClient.CheckImageExists(ctx, pluginTag), - "plugin image must exist at %s:%s", pluginClient.GetRegistry(), pluginTag) + underEdition := hostClient.WithSegment("deckhouse", "ee", "deckhouse-cli") + assert.NoError(t, underEdition.WithSegment(moduleName).CheckImageExists(ctx, moduleTag), + "module images must stay under the target") - indexClient := destClient.WithSegment("deckhouse-cli", "plugins") - tags, err := indexClient.ListTags(ctx) + tags, err := underEdition.ListTags(ctx) require.NoError(t, err) - assert.Contains(t, tags, pluginName, "discovery tag must exist on the plugins index path") + assert.Contains(t, tags, moduleName, "the discovery tag must sit next to the images") + + aboveEdition := hostClient.WithSegment("deckhouse", "deckhouse-cli", moduleName) + assert.Error(t, aboveEdition.CheckImageExists(ctx, moduleTag), + "a module must never be written above the edition") +} + +// denyWritesClient refuses writes to the repositories denies() selects with +// HTTP 403, the way a registry scopes credentials to one path. Reads and +// other writes go to the wrapped fake. +type denyWritesClient struct { + dkpreg.Client + // repo is the full repository reference of this scope. Tracked here + // because the fake client's GetRegistry reports the host only. + repo string + denies func(repo string) bool +} + +func newDenyWritesClient(host string, denies func(repo string) bool) *denyWritesClient { + return &denyWritesClient{ + Client: pkgclient.Adapt(upfake.NewClient(upfake.NewRegistry(host))), + repo: host, + denies: denies, + } +} + +func (c *denyWritesClient) WithSegment(segments ...string) dkpreg.Client { + return &denyWritesClient{ + Client: c.Client.WithSegment(segments...), + repo: path.Join(append([]string{c.repo}, segments...)...), + denies: c.denies, + } +} + +func (c *denyWritesClient) denied() error { + if c.denies(c.repo) { + return &transport.Error{StatusCode: http.StatusForbidden} + } + + return nil +} + +func (c *denyWritesClient) PushImage(ctx context.Context, tag string, img v1.Image, opts ...dkpreg.ImagePushOption) error { + if err := c.denied(); err != nil { + return err + } + + return c.Client.PushImage(ctx, tag, img, opts...) +} + +func (c *denyWritesClient) PushIndex(ctx context.Context, tag string, idx v1.ImageIndex, opts ...dkpreg.ImagePushOption) error { + if err := c.denied(); err != nil { + return err + } + + return c.Client.PushIndex(ctx, tag, idx, opts...) +} + +// TestPushService_PluginsRootDenied verifies that a registry refusing the +// write above the edition (credentials scoped to the target path) surfaces as +// a PluginsRootError naming the refused repository and the target it sits +// above, with the HTTP status still reachable through the chain. Without an +// edition to cut off, the plugins path is under the target like everything +// else and the plain error passes through. +// +// The denial is placed on the plugins catalog (the discovery-tag step), which +// fails at once; the plugin layout push goes through the pusher's retries and +// is covered by TestPushService_PluginsRootDenied_LayoutPush. +func TestPushService_PluginsRootDenied(t *testing.T) { + const ( + host = "registry.example.com" + pluginName = "postgresql-mgr" + pluginTag = "v1.2.0" + ) + + tests := []struct { + name string + target string + root string + wantWrapped bool + }{ + {name: "edition target: error names the root and the edition", target: "/deckhouse/ee", root: "deckhouse", wantWrapped: true}, + {name: "no edition: plain error", target: "/deckhouse", root: "deckhouse", wantWrapped: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bundleDir := t.TempDir() + pluginPkg := buildLayoutBundle(t, bundleDir, "plugin-"+pluginName+".tar", + path.Join("deckhouse-cli", "plugins", pluginName), pluginTag) + + catalog := path.Join(host, tt.root, "deckhouse-cli", "plugins") + // Deny the catalog repo itself, not its sub-repositories. + hostClient := newDenyWritesClient(host, func(repo string) bool { return repo == catalog }) + + logger := dkplog.NewLogger(dkplog.WithLevel(slog.LevelWarn)) + userLogger := log.NewSLogger(slog.LevelWarn) + + svc := NewPushService(hostClient, &PushServiceOptions{ + TargetPath: tt.target, + Packages: []string{pluginPkg}, + WorkingDir: t.TempDir(), + }, logger, userLogger) + + _, err := svc.Push(context.Background()) + require.Error(t, err) + + var transportErr *transport.Error + require.ErrorAs(t, err, &transportErr, "the registry status must stay reachable") + assert.Equal(t, http.StatusForbidden, transportErr.StatusCode) + + var rootErr *PluginsRootError + if !tt.wantWrapped { + assert.False(t, errors.As(err, &rootErr), "no edition cut off: nothing to explain") + return + } + + require.ErrorAs(t, err, &rootErr) + assert.Equal(t, catalog, rootErr.Repo) + assert.Equal(t, "ee", rootErr.Report.Edition) + assert.Equal(t, path.Join(host, "deckhouse", "ee"), rootErr.Report.Target) + assert.Equal(t, path.Join(host, "deckhouse"), rootErr.Report.Root) + }) + } +} + +// TestPushService_PluginsRootDenied_LayoutPush is the same denial on the +// plugin image repository, the first write above the edition a push makes. +// It goes through the pusher's retry schedule, so it is skipped in -short. +func TestPushService_PluginsRootDenied_LayoutPush(t *testing.T) { + if testing.Short() { + t.Skip("walks the pusher's retry schedule") + } + + const ( + host = "registry.example.com" + pluginName = "postgresql-mgr" + pluginTag = "v1.2.0" + ) + + bundleDir := t.TempDir() + pluginPkg := buildLayoutBundle(t, bundleDir, "plugin-"+pluginName+".tar", + path.Join("deckhouse-cli", "plugins", pluginName), pluginTag) + + deniedPrefix := path.Join(host, "deckhouse", "deckhouse-cli") + hostClient := newDenyWritesClient(host, func(repo string) bool { return strings.HasPrefix(repo, deniedPrefix) }) + + logger := dkplog.NewLogger(dkplog.WithLevel(slog.LevelWarn)) + userLogger := log.NewSLogger(slog.LevelWarn) + + svc := NewPushService(hostClient, &PushServiceOptions{ + TargetPath: "/deckhouse/ee", + Packages: []string{pluginPkg}, + WorkingDir: t.TempDir(), + }, logger, userLogger) + + _, err := svc.Push(context.Background()) + require.Error(t, err) + + var rootErr *PluginsRootError + require.ErrorAs(t, err, &rootErr) + assert.Equal(t, path.Join(host, "deckhouse", "deckhouse-cli", "plugins", pluginName), rootErr.Repo) + assert.Equal(t, "ee", rootErr.Report.Edition) + + var transportErr *transport.Error + require.ErrorAs(t, err, &transportErr) + assert.Equal(t, http.StatusForbidden, transportErr.StatusCode) } // TestPushService_ModulesPathSuffix verifies that --modules-path-suffix moves From 87a2790de1241edcb3a69d870d3eae05cf5145c1 Mon Sep 17 00:00:00 2001 From: Roman Berezkin Date: Fri, 21 Aug 2026 14:03:23 +0300 Subject: [PATCH 2/2] [plugins] Advise the cli-download role on a 403 from registry-packages-proxy Plugins are downloaded over /v1/images/, and kube-rbac-proxy authorizes that whole prefix through the deployments/cli-binary subresource, granted by the ClusterRole d8:registry-packages-proxy:cli-download. The 403 diagnostic named packages-download instead. That role covers /v1/packages/, which the CLI never calls, so a user who followed the advice bound the role and still got 403. The same suggestion told the user to wait about 5 minutes and retry with a fresh token. kube-rbac-proxy caches a denial for about 30 seconds, and the existing token keeps working, so both halves of that hint were wrong. The d8 dist copy of the diagnostic carried the same wrong hint and is fixed too. Signed-off-by: Roman Berezkin --- internal/dist/cmd/errdetect/diagnose.go | 2 +- internal/plugins/cmd/errdetect/diagnose.go | 7 +++++-- internal/plugins/cmd/errdetect/diagnose_test.go | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/dist/cmd/errdetect/diagnose.go b/internal/dist/cmd/errdetect/diagnose.go index 785939654..3a3f53080 100644 --- a/internal/dist/cmd/errdetect/diagnose.go +++ b/internal/dist/cmd/errdetect/diagnose.go @@ -42,7 +42,7 @@ func Diagnose(err error) *diagnostic.HelpfulError { return help(err, "registry-packages-proxy: forbidden (403)", "the identity may not download the CLI", "bind the ClusterRole 'd8:registry-packages-proxy:cli-download' to the user/group", - "authorization is cached ~5 min - after binding, retry with a fresh token") + "a denied check is cached ~30s - after binding the role, wait half a minute and retry with the same token") case errors.Is(err, rpp.ErrNotFound): return help(err, "registry-packages-proxy: version not found (404)", "this deckhouse-cli version is not published", diff --git a/internal/plugins/cmd/errdetect/diagnose.go b/internal/plugins/cmd/errdetect/diagnose.go index a80e14b34..fcba38d43 100644 --- a/internal/plugins/cmd/errdetect/diagnose.go +++ b/internal/plugins/cmd/errdetect/diagnose.go @@ -38,11 +38,14 @@ func Diagnose(err error) *diagnostic.HelpfulError { return help(err, "registry-packages-proxy: unauthorized (401)", "no accepted Bearer token (a client-certificate kubeconfig is not enough)", "use a kubeconfig with an OIDC token (Kubeconfig Generator or 'd8 login')") + // Plugins are downloaded over /v1/images/, which kube-rbac-proxy authorizes + // through the deployments/cli-binary subresource: that is the cli-download + // role, not packages-download (it covers /v1/packages/). case errors.Is(err, rpp.ErrForbidden): return help(err, "registry-packages-proxy: forbidden (403)", "the identity may not download plugins", - "bind the ClusterRole 'd8:registry-packages-proxy:packages-download' to the user/group", - "authorization is cached ~5 min - after binding, retry with a fresh token") + "bind the ClusterRole 'd8:registry-packages-proxy:cli-download' to the user/group", + "a denied check is cached ~30s - after binding the role, wait half a minute and retry with the same token") case errors.Is(err, rpp.ErrNotFound): return help(err, "registry-packages-proxy: plugin or version not found (404)", "this plugin or version is not published", diff --git a/internal/plugins/cmd/errdetect/diagnose_test.go b/internal/plugins/cmd/errdetect/diagnose_test.go index 655e46837..89576c12c 100644 --- a/internal/plugins/cmd/errdetect/diagnose_test.go +++ b/internal/plugins/cmd/errdetect/diagnose_test.go @@ -37,7 +37,7 @@ func TestDiagnose(t *testing.T) { wantSol string }{ {"401", rpp.ErrUnauthorized, "unauthorized (401)", "OIDC"}, - {"403", rpp.ErrForbidden, "forbidden (403)", "packages-download"}, + {"403", rpp.ErrForbidden, "forbidden (403)", "cli-download"}, {"404", rpp.ErrNotFound, "plugin or version not found (404)", "deckhouse-cli/plugins"}, {"5xx", rpp.ErrUpstream, "upstream error (5xx)", "registry-packages-proxy pods"}, {"discovery", rpp.ErrEndpointDiscovery, "endpoint discovery via the Kubernetes API failed", "--rpp-endpoint"},