From c7a5cc8f016cbb0d4b20d98d8b2374bbbdb7b6ff Mon Sep 17 00:00:00 2001 From: Roman Berezkin Date: Mon, 22 Jun 2026 11:15:56 +0300 Subject: [PATCH 1/2] Fetch plugin contracts from the manifest annotation - `Client.GetManifest` fetches the raw image manifest over `//manifests/` - one manifest fetch, no layer pull. The binary is pulled only on install. - The CLI reads the base64 `contract` annotation itself; the proxy stays generic and returns the manifest verbatim. - Multi-platform images: when the index carries no contract, the first child manifest is followed by digest, so the contract is found wherever it lives. - A missing annotation yields name+version (contract-less plugin); a malformed one is an error. `selectCompatible` hard-stops on any contract error and never silently demotes to an older installed version. Signed-off-by: Roman Berezkin --- internal/plugins/README.md | 14 ++-- internal/plugins/doc.go | 6 +- internal/plugins/install_test.go | 15 ++-- internal/plugins/rpp_source.go | 115 ++++++++++++++++++++-------- internal/plugins/rpp_source_test.go | 102 +++++++++++++++++++++--- internal/plugins/select.go | 12 +-- internal/rpp/client.go | 45 +++++++++++ internal/rpp/client_http_test.go | 21 +++++ internal/rpp/doc.go | 1 + internal/rpp/image.go | 27 +++++++ internal/rpp/image_test.go | 16 ++++ 11 files changed, 308 insertions(+), 66 deletions(-) diff --git a/internal/plugins/README.md b/internal/plugins/README.md index dc73c099..c010b11d 100644 --- a/internal/plugins/README.md +++ b/internal/plugins/README.md @@ -40,11 +40,15 @@ the plugin routes are `/v1/images/deckhouse-cli/plugins//...`. ## What a plugin image contains -- `plugin` - the executable; -- `contract.yaml` - the contract: name, version, description, requested env - vars, flags, and `requirements` (Kubernetes / Deckhouse / modules / plugins). - -The RPP source reads the `contract.yaml` file from the image tar. +- `plugin` - the executable, in the image layers; +- the contract - name, version, description, requested env vars, flags, and + `requirements` (Kubernetes / Deckhouse / modules / plugins) - published as a + base64-JSON `contract` annotation on the image manifest. + +The RPP source fetches the raw image manifest over the proxy `manifests/` +route (a single manifest fetch, no layer pull) and reads the contract from its +base64-JSON `contract` annotation itself. The binary is pulled separately (full +image, over the `images/` route) only when a plugin is installed. ## On-disk layout diff --git a/internal/plugins/doc.go b/internal/plugins/doc.go index 60bc4725..7f89c115 100644 --- a/internal/plugins/doc.go +++ b/internal/plugins/doc.go @@ -29,14 +29,14 @@ limitations under the License. // # What d8-cli can do with plugins // // - Download them. -// - Validate their dependencies (requirements declared in contract.yaml that plugins declare). +// - Validate their dependencies (requirements the plugin declares in its contract). // - Run them as if they were native subcommands. // // # Where a plugin lives // // A plugin lives in the cluster's OCI registry, reached exclusively through the -// in-cluster registry-packages-proxy. The image carries the plugin binary and a -// contract.yaml file that describes the plugin: +// in-cluster registry-packages-proxy. The image carries the plugin binary in its +// layers and a contract, published as a manifest annotation, that describes the plugin: // // - name; // - version; diff --git a/internal/plugins/install_test.go b/internal/plugins/install_test.go index 0946a912..86706bcc 100644 --- a/internal/plugins/install_test.go +++ b/internal/plugins/install_test.go @@ -306,7 +306,7 @@ func TestInstallPluginSmokeFailureRollsBack(t *testing.T) { assert.True(t, os.IsNotExist(binErr), "the broken binary is removed on rollback (fresh install)") } -func TestInstallPluginDoesNotDowngradeOnContractError(t *testing.T) { +func TestInstallPluginHardStopsOnContractError(t *testing.T) { root := t.TempDir() m := testManager() m.pluginDirectory = root @@ -315,8 +315,9 @@ func TestInstallPluginDoesNotDowngradeOnContractError(t *testing.T) { src := &fakeInstallSource{ tags: []string{"v1.2.0", "v1.3.0"}, contractByTag: map[string]*internal.Plugin{ - // v1.3.0 is deliberately absent: its contract fetch fails transiently, - // so selection falls back to v1.2.0. + // v1.3.0 is deliberately absent: its contract fetch fails (operational + // error). That must stop selection, not silently demote to v1.2.0 - a + // missing contract is signalled separately (without an error). "v1.2.0": {Name: "p", Version: "v1.2.0"}, }, extract: func(dest string) error { @@ -336,14 +337,14 @@ func TestInstallPluginDoesNotDowngradeOnContractError(t *testing.T) { require.NoError(t, err) require.NoError(t, os.Symlink(absV1, layout.CurrentLinkPath(root, "p"))) - require.NoError(t, m.InstallPlugin(context.Background(), "p"), - "a transient contract error on the newest tag is not an install failure") + require.Error(t, m.InstallPlugin(context.Background(), "p"), + "an operational contract error must stop selection, not silently demote") - assert.False(t, extractCalled, "the older selection must not be installed over a newer installed version") + assert.False(t, extractCalled, "nothing is installed when contract resolution fails") version, err := pluginBinaryVersion(context.Background(), v1) require.NoError(t, err) - assert.Equal(t, "1.3.0", version.String(), "the installed newer version is kept, not silently downgraded") + assert.Equal(t, "1.3.0", version.String(), "the installed version is left untouched") } func TestValidateInstalledRequirementsFailsOnUnsatisfiedDep(t *testing.T) { diff --git a/internal/plugins/rpp_source.go b/internal/plugins/rpp_source.go index b18ff98a..a5c336ab 100644 --- a/internal/plugins/rpp_source.go +++ b/internal/plugins/rpp_source.go @@ -18,6 +18,8 @@ package plugins import ( "context" + "encoding/base64" + "encoding/json" "fmt" "log/slog" @@ -30,22 +32,25 @@ import ( "github.com/deckhouse/deckhouse-cli/pkg/registry/service" ) -const ( - // pluginBinaryEntryName is the file inside a plugin image that holds the - // executable. - pluginBinaryEntryName = "plugin" - - // pluginContractEntryName is the file inside a plugin image that holds the - // contract. The proxy serves the image as a tar (not via OCI annotations), so - // the contract travels as a file. Confirmed against the plugin build CI - // (d8-package-plugin: a scratch image with /plugin and /contract.yaml). A - // missing contract is still tolerated for older / contract-less images. - pluginContractEntryName = "contract.yaml" - - // maxContractBytes caps the contract read so a malformed image cannot exhaust - // memory; real contracts are a few KiB. - maxContractBytes = 1 << 20 -) +// pluginBinaryEntryName is the file inside a plugin image that holds the executable. +const pluginBinaryEntryName = "plugin" + +// contractAnnotation is the manifest annotation key that carries the plugin +// contract as base64-encoded JSON. +const contractAnnotation = "contract" + +// imageManifest is the subset of an OCI/Docker image manifest or index the CLI +// reads: the top-level annotations (where the plugin contract lives) and, for an +// index, the child descriptors so it can follow one when the index itself carries +// no contract. +type imageManifest struct { + Annotations map[string]string `json:"annotations,omitempty"` + Manifests []manifestDescriptor `json:"manifests,omitempty"` +} + +type manifestDescriptor struct { + Digest string `json:"digest"` +} // rppPluginSource adapts the registry-packages-proxy client to pluginSource. type rppPluginSource struct { @@ -74,31 +79,74 @@ func (s *rppPluginSource) GetPluginContract(ctx context.Context, pluginName, tag return nil, err } - // The proxy serves no manifest/annotations, so reading the contract needs a - // full image pull; an install therefore pulls the image twice (here and in - // ExtractPlugin). Plugin images are small, so this is acceptable for now. - body, err := s.client.PullImage(ctx, ref, tag) + // The contract is a manifest annotation, so a manifest fetch is enough - no + // image/layer pull. The proxy returns the raw manifest; the contract key and its + // base64 encoding are CLI knowledge, decoded here. The binary still needs a full + // pull in ExtractPlugin. + encoded, err := s.contractAnnotation(ctx, ref, tag) if err != nil { return nil, err } - defer func() { _ = body.Close() }() + if encoded == "" { + // No contract annotation anywhere: surface just name+version. Install can + // proceed and the cluster/plugin requirement checks have nothing to enforce. + s.logger.Debug("plugin image has no contract annotation", slog.String("plugin", pluginName), slog.String("tag", tag)) + + return &internal.Plugin{Name: pluginName, Version: tag}, nil + } - raw, found, err := rpp.ReadFile(body, pluginContractEntryName, maxContractBytes) + contract, err := base64.StdEncoding.DecodeString(encoded) if err != nil { - return nil, fmt.Errorf("read contract for plugin %q: %w", pluginName, err) + // A present but malformed annotation is a publishing bug, not a contract-less + // plugin - surface it rather than silently degrading to name+version. + return nil, fmt.Errorf("decode contract annotation for plugin %q: %w", pluginName, err) } - if !found { - // Plugin images do not carry a contract yet, so surface just name+version: - // install can proceed and the cluster/plugin requirement checks have nothing - // to enforce. - s.logger.Debug("plugin image has no contract file", slog.String("plugin", pluginName), slog.String("tag", tag)) + return contractFromBytes(contract, pluginName, tag) +} - return &internal.Plugin{Name: pluginName, Version: tag}, nil +// contractAnnotation returns the base64 "contract" annotation for ref:tag, or "" +// when absent. A multi-platform tag points at an index; the contract is platform- +// independent, so it may sit on the index or on its (identical) child manifests. +// When the index carries none, the first child is followed - one extra manifest +// fetch, still no layer pull. +func (s *rppPluginSource) contractAnnotation(ctx context.Context, ref rpp.ImageRef, tag string) (string, error) { + man, err := s.fetchManifest(ctx, ref, tag) + if err != nil { + return "", err + } + + if encoded := man.Annotations[contractAnnotation]; encoded != "" { + return encoded, nil + } + + if len(man.Manifests) == 0 { + return "", nil + } + + child, err := s.fetchManifest(ctx, ref, man.Manifests[0].Digest) + if err != nil { + return "", err + } + + return child.Annotations[contractAnnotation], nil +} + +// fetchManifest fetches and decodes the raw manifest for ref addressed by refOrDigest +// (a tag or a child digest). +func (s *rppPluginSource) fetchManifest(ctx context.Context, ref rpp.ImageRef, refOrDigest string) (imageManifest, error) { + raw, err := s.client.GetManifest(ctx, ref, refOrDigest) + if err != nil { + return imageManifest{}, fmt.Errorf("get manifest for plugin %q: %w", ref.String(), err) + } + + var man imageManifest + if err := json.Unmarshal(raw, &man); err != nil { + return imageManifest{}, fmt.Errorf("decode manifest for plugin %q: %w", ref.String(), err) } - return contractFromBytes(raw, pluginName, tag) + return man, nil } func (s *rppPluginSource) ExtractPlugin(ctx context.Context, pluginName, tag, destination string) error { @@ -121,10 +169,11 @@ func (s *rppPluginSource) ExtractPlugin(ctx context.Context, pluginName, tag, de return nil } -// contractFromBytes decodes a contract file, backfilling identity from the request -// so a present-but-degenerate contract still yields a usable name/version. +// contractFromBytes decodes a contract, backfilling identity from the request so a +// present-but-degenerate contract still yields a usable name/version. func contractFromBytes(raw []byte, pluginName, tag string) (*internal.Plugin, error) { - // The contract file is YAML (contract.yaml); the shared decoder expects JSON. + // The decoded annotation is JSON; YAMLToJSON is a tolerant pass-through (valid + // JSON is valid YAML) and the shared decoder expects JSON. jsonRaw, err := yaml.YAMLToJSON(raw) if err != nil { return nil, fmt.Errorf("decode contract for plugin %q: %w", pluginName, err) diff --git a/internal/plugins/rpp_source_test.go b/internal/plugins/rpp_source_test.go index 4acb4af1..938673cb 100644 --- a/internal/plugins/rpp_source_test.go +++ b/internal/plugins/rpp_source_test.go @@ -21,6 +21,9 @@ import ( "bytes" "compress/gzip" "context" + "encoding/base64" + "encoding/json" + "io" "net/http" "net/http/httptest" "os" @@ -96,10 +99,45 @@ func TestRppSourceExtractPlugin(t *testing.T) { assert.NotZero(t, info.Mode()&0o100, "extracted binary must be executable") } -func TestRppSourceGetPluginContractBackfillsIdentity(t *testing.T) { - src := newTestRppSource(t, gzipTar(t, map[string]tarFile{ - pluginContractEntryName: {content: `{}`, mode: 0o644}, +// newTestRppManifestSource serves the /manifests route, returning the given status +// and (for 200) a raw manifest body. The contract tests read the contract from the +// manifest annotation rather than pulling the image. +func newTestRppManifestSource(t *testing.T, status int, manifestJSON string) *rppPluginSource { + t.Helper() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/v1/images/deckhouse-cli/plugins/stronghold/manifests/v1.0.0", r.URL.Path) + + if status != http.StatusOK { + w.WriteHeader(status) + + return + } + + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + _, _ = io.WriteString(w, manifestJSON) })) + t.Cleanup(srv.Close) + + client := rpp.NewWithHTTPClient(srv.URL, srv.Client(), dkplog.NewNop()) + + return newRppPluginSource(client, dkplog.NewNop()) +} + +// manifestWithContract renders a minimal manifest carrying contractJSON as the +// base64-encoded "contract" annotation - the exact shape the CLI decodes. +func manifestWithContract(t *testing.T, contractJSON string) string { + t.Helper() + + encoded := base64.StdEncoding.EncodeToString([]byte(contractJSON)) + raw, err := json.Marshal(imageManifest{Annotations: map[string]string{contractAnnotation: encoded}}) + require.NoError(t, err) + + return string(raw) +} + +func TestRppSourceGetPluginContractBackfillsIdentity(t *testing.T) { + src := newTestRppManifestSource(t, http.StatusOK, manifestWithContract(t, `{}`)) plugin, err := src.GetPluginContract(context.Background(), "stronghold", "v1.0.0") require.NoError(t, err) @@ -108,7 +146,8 @@ func TestRppSourceGetPluginContractBackfillsIdentity(t *testing.T) { } func TestRppSourceGetPluginContractTolerantWhenAbsent(t *testing.T) { - src := newTestRppSource(t, gzipTar(t, map[string]tarFile{"plugin": {content: "BINARY", mode: 0o755}})) + // A manifest with no contract annotation is tolerated as name+version only. + src := newTestRppManifestSource(t, http.StatusOK, `{"annotations":{}}`) plugin, err := src.GetPluginContract(context.Background(), "stronghold", "v1.0.0") require.NoError(t, err) @@ -116,15 +155,58 @@ func TestRppSourceGetPluginContractTolerantWhenAbsent(t *testing.T) { assert.Equal(t, "v1.0.0", plugin.Version) } -func TestRppSourceGetPluginContractParsesFile(t *testing.T) { - // The real contract ships as YAML (contract.yaml), as produced by the plugin CI. - contract := "name: stronghold\nversion: v1.0.0\ndescription: d\n" - src := newTestRppSource(t, gzipTar(t, map[string]tarFile{ - pluginContractEntryName: {content: contract, mode: 0o644}, - })) +func TestRppSourceGetPluginContractParsesContract(t *testing.T) { + src := newTestRppManifestSource(t, http.StatusOK, + manifestWithContract(t, `{"name":"stronghold","version":"v1.0.0","description":"d"}`)) plugin, err := src.GetPluginContract(context.Background(), "stronghold", "v1.0.0") require.NoError(t, err) assert.Equal(t, "stronghold", plugin.Name) assert.Equal(t, "d", plugin.Description) } + +func TestRppSourceGetPluginContractFollowsIndexToChild(t *testing.T) { + // Multi-platform plugin: the tag points at an index with no top-level contract; + // the contract lives on the child manifest. The CLI follows the first child by + // digest and reads it there. + const childDigest = "sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + const indexJSON = `{"mediaType":"application/vnd.oci.image.index.v1+json","manifests":[{"digest":"` + childDigest + `"}]}` + child := manifestWithContract(t, `{"name":"stronghold","version":"v1.0.0","description":"from-child"}`) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v1/images/deckhouse-cli/plugins/stronghold/manifests/v1.0.0": + _, _ = io.WriteString(w, indexJSON) + case "/v1/images/deckhouse-cli/plugins/stronghold/manifests/" + childDigest: + _, _ = io.WriteString(w, child) + default: + t.Errorf("unexpected request path %q", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + + src := newRppPluginSource(rpp.NewWithHTTPClient(srv.URL, srv.Client(), dkplog.NewNop()), dkplog.NewNop()) + + plugin, err := src.GetPluginContract(context.Background(), "stronghold", "v1.0.0") + require.NoError(t, err) + assert.Equal(t, "stronghold", plugin.Name) + assert.Equal(t, "from-child", plugin.Description) +} + +func TestRppSourceGetPluginContractRejectsMalformedAnnotation(t *testing.T) { + // A present but non-base64 contract annotation is a publishing bug, not a + // contract-less plugin - it must surface as an error. + src := newTestRppManifestSource(t, http.StatusOK, `{"annotations":{"contract":"!!not-base64!!"}}`) + + _, err := src.GetPluginContract(context.Background(), "stronghold", "v1.0.0") + require.Error(t, err) +} + +func TestRppSourceGetPluginContractPropagatesError(t *testing.T) { + // An operational failure (5xx) must surface as an error, not a tolerated absence. + src := newTestRppManifestSource(t, http.StatusInternalServerError, "") + + _, err := src.GetPluginContract(context.Background(), "stronghold", "v1.0.0") + require.Error(t, err) +} diff --git a/internal/plugins/select.go b/internal/plugins/select.go index 6bc50b5d..d88c94ab 100644 --- a/internal/plugins/select.go +++ b/internal/plugins/select.go @@ -19,7 +19,6 @@ package plugins import ( "context" "fmt" - "log/slog" "sort" "strings" @@ -119,13 +118,10 @@ func (m *Manager) selectCompatible( contract, err := m.PluginContract(ctx, pluginName, version.Original()) if err != nil { - // GetPluginContract has no typed not-found, so a transient registry error - // is indistinguishable from "no contract"; both demote to an older version. - m.logger.Warn("skipping version: contract unavailable", - slog.String("plugin", pluginName), slog.String("version", version.Original()), slog.String("error", err.Error())) - rejected = append(rejected, rejectedCandidate{version.Original(), &unsatisfiableReason{kind: reasonContractUnavailable, detail: "contract unavailable"}}) - - continue + // A missing contract is not an error - GetPluginContract returns name+version + // for a contract-less image. So any error here is operational (registry/proxy + // unreachable): hard stop, never silently demote to an older version. + return nil, nil, err } compatible, reason, err := m.clusterCompatible(ctx, contract) diff --git a/internal/rpp/client.go b/internal/rpp/client.go index d25aac41..caea1c7d 100644 --- a/internal/rpp/client.go +++ b/internal/rpp/client.go @@ -37,12 +37,24 @@ const ( headerAccept = "Accept" mediaTypeJSON = "application/json" + // acceptManifest lists the OCI and Docker manifest/index media types a raw + // manifest fetch may return. The proxy passes the upstream manifest through + // verbatim, so either an image manifest or a multi-platform index can arrive. + acceptManifest = "application/vnd.oci.image.manifest.v1+json," + + "application/vnd.oci.image.index.v1+json," + + "application/vnd.docker.distribution.manifest.v2+json," + + "application/vnd.docker.distribution.manifest.list.v2+json" + loggerName = "rpp" // maxTagsResponseBytes caps the tags JSON read so a misbehaving endpoint cannot // make the client buffer an unbounded response; real tag lists are a few KiB. maxTagsResponseBytes int64 = 4 << 20 + // maxManifestResponseBytes caps the manifest read; real manifests (even with an + // embedded base64 contract annotation) are a few KiB. + maxManifestResponseBytes int64 = 1 << 20 + // maxBodySnippetBytes bounds how much of an error response body is echoed // into the error message. maxBodySnippetBytes = 256 @@ -240,6 +252,39 @@ func (c *Client) PullImage(ctx context.Context, ref ImageRef, version string) (i return resp.Body, nil } +// GetManifest fetches the raw image manifest for a version without pulling layers. +// The plugin contract lives as a base64-JSON annotation inside it; the caller reads +// it from the returned bytes. The manifest is platform-independent (it is the index +// for a multi-platform image), so no platform is sent. +func (c *Client) GetManifest(ctx context.Context, image ImageRef, ref string) ([]byte, error) { + if err := validateRef(ref); err != nil { + return nil, err + } + + c.logger.Debug("getting manifest", slog.String("image", image.String()), slog.String("ref", ref)) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.baseURL+image.manifestPath(ref), nil) + if err != nil { + return nil, fmt.Errorf("build manifest request: %w", err) + } + + req.Header.Set(headerAccept, acceptManifest) + + resp, err := c.do(req) + if err != nil { + return nil, err + } + + defer func() { _ = resp.Body.Close() }() + + raw, err := io.ReadAll(io.LimitReader(resp.Body, maxManifestResponseBytes)) + if err != nil { + return nil, fmt.Errorf("read manifest for %q: %w", image.String(), err) + } + + return raw, nil +} + // do executes the request and, on a non-2xx status, closes the body and maps the // status to a sentinel error. On success the response is returned with its body // still open for the caller to consume. diff --git a/internal/rpp/client_http_test.go b/internal/rpp/client_http_test.go index a9880b94..5a48e5e7 100644 --- a/internal/rpp/client_http_test.go +++ b/internal/rpp/client_http_test.go @@ -90,6 +90,27 @@ func TestClientPullImage(t *testing.T) { assert.Equal(t, payload, string(got)) } +func TestClientGetManifest(t *testing.T) { + const manifestJSON = `{"annotations":{"contract":"e30="}}` + + client := testClient(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/v1/images/deckhouse-cli/plugins/foo/manifests/v1.0.0", r.URL.Path) + assert.Empty(t, r.URL.Query().Get("platform"), "manifest is platform-independent") + assert.Equal(t, acceptManifest, r.Header.Get(headerAccept)) + + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + _, _ = io.WriteString(w, manifestJSON) + }) + + ref, err := PluginImage("foo") + require.NoError(t, err) + + raw, err := client.GetManifest(context.Background(), ref, "v1.0.0") + require.NoError(t, err) + assert.Equal(t, manifestJSON, string(raw)) +} + func TestClientStatusErrorMapping(t *testing.T) { tests := []struct { name string diff --git a/internal/rpp/doc.go b/internal/rpp/doc.go index df5419e0..a4f911a1 100644 --- a/internal/rpp/doc.go +++ b/internal/rpp/doc.go @@ -19,6 +19,7 @@ limitations under the License. // // - GET /v1/images//tags - list available versions // - GET /v1/images//images/ - download a version's image +// - GET /v1/images//manifests/ - fetch an image's raw manifest // // It lets deckhouse-cli list available versions of itself and its plugins and // download their images. All traffic goes to the in-cluster proxy and is diff --git a/internal/rpp/image.go b/internal/rpp/image.go index fb6f048b..b12aa8f0 100644 --- a/internal/rpp/image.go +++ b/internal/rpp/image.go @@ -40,6 +40,11 @@ const ( // imagesPathSegment is the route segment that addresses a single version's // image for download (/v1/images//images/). imagesPathSegment = "images" + + // manifestsPathSegment is the route segment that returns an image's raw + // manifest (/v1/images//manifests/). The plugin contract is a + // base64-JSON annotation inside that manifest; the CLI reads it from there. + manifestsPathSegment = "manifests" ) // pluginNamePattern is the OCI repository path-component grammar (lowercase @@ -53,6 +58,11 @@ var pluginNamePattern = regexp.MustCompile(`^[a-z0-9]+(?:[._-][a-z0-9]+)*$`) // dot) cannot be a real registry tag and must not reach the request URL. var tagPattern = regexp.MustCompile(`^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$`) +// digestPattern is the OCI digest grammar (algorithm:hex), e.g. sha256:<64 hex>. +// The manifest route also accepts a digest ref (used to follow an index to a +// child manifest), so it must pass validation alongside a tag. +var digestPattern = regexp.MustCompile(`^[a-z0-9]+:[a-fA-F0-9]{32,}$`) + // ImageRef identifies an image the proxy is allowed to serve over /v1/images: // either the deckhouse-cli binary or a single plugin. Construct it through // CLIImage or PluginImage so the value always stays within the allow-list. @@ -97,6 +107,13 @@ func (r ImageRef) imagePath(version string) string { return imagesPathPrefix + r.path + "/" + imagesPathSegment + "/" + url.PathEscape(version) } +// manifestPath is the route that returns an image's raw manifest. The ref is +// path-escaped as defense in depth; after validateRef this is a no-op, but it +// keeps URL metacharacters out of the route even if validation ever loosens. +func (r ImageRef) manifestPath(ref string) string { + return imagesPathPrefix + r.path + "/" + manifestsPathSegment + "/" + url.PathEscape(ref) +} + // validateTag rejects strings that cannot be a registry tag, so the proxy route // (the final version/ref path segment) cannot be altered by a crafted // --version value (slashes, ?, #, leading dots). @@ -111,3 +128,13 @@ func validateTag(tag string) error { return nil } + +// validateRef accepts a registry tag or a digest. The manifest route takes either: +// a version tag from the caller, or a child digest when following an index. +func validateRef(ref string) error { + if tagPattern.MatchString(ref) || digestPattern.MatchString(ref) { + return nil + } + + return fmt.Errorf("%w: %q is not a valid tag or digest", ErrInvalidImage, ref) +} diff --git a/internal/rpp/image_test.go b/internal/rpp/image_test.go index 537929f9..94d2264d 100644 --- a/internal/rpp/image_test.go +++ b/internal/rpp/image_test.go @@ -86,3 +86,19 @@ func TestValidateTag(t *testing.T) { } }) } + +func TestValidateRef(t *testing.T) { + t.Run("a tag is accepted", func(t *testing.T) { + require.NoError(t, validateRef("v1.2.3")) + }) + + t.Run("a digest is accepted", func(t *testing.T) { + require.NoError(t, validateRef("sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef")) + }) + + t.Run("metacharacters are rejected", func(t *testing.T) { + for _, ref := range []string{"", "with/slash", "sha256:zzzz", "v1?x=y", "sha256:"} { + assert.ErrorIs(t, validateRef(ref), ErrInvalidImage, "ref %q must be rejected", ref) + } + }) +} From 2177ee88a6602d78b9ebcd9184721abc77717e6e Mon Sep 17 00:00:00 2001 From: Roman Berezkin Date: Wed, 24 Jun 2026 16:30:15 +0300 Subject: [PATCH 2/2] Let built-in commands satisfy plugin dependencies - A plugin dependency named after a built-in command (e.g. `delivery-kit`) is satisfied by the command's presence - no install, no registry lookup. - Version constraints on such deps are ignored: a built-in can't be upgraded, so presence alone is enough. - The planner and the pre-switch validators short-circuit on built-in deps, for both mandatory and conditional requirements. - `d8` registers `delivery-kit` as a built-in via `SetBuiltinCommands` when it builds the `plugins` command. Signed-off-by: Roman Berezkin --- cmd/commands/delivery.go | 7 +- cmd/d8/root.go | 2 +- internal/plugins/builtins.go | 44 ++++++++ internal/plugins/builtins_test.go | 151 +++++++++++++++++++++++++++ internal/plugins/cmd/plugins.go | 6 +- internal/plugins/cmd/plugins_test.go | 2 +- internal/plugins/planner.go | 17 +++ internal/plugins/plugins.go | 6 ++ internal/plugins/validators.go | 12 +++ internal/selfupdate/README.md | 2 +- 10 files changed, 244 insertions(+), 5 deletions(-) create mode 100644 internal/plugins/builtins.go create mode 100644 internal/plugins/builtins_test.go diff --git a/cmd/commands/delivery.go b/cmd/commands/delivery.go index 5b8153ca..9d776c1c 100644 --- a/cmd/commands/delivery.go +++ b/cmd/commands/delivery.go @@ -28,6 +28,11 @@ import ( "github.com/werf/werf/v2/pkg/storage/synchronization/server" ) +// DeliveryKitCommandName is the top-level command name for the built-in werf +// re-skin. A plugin contract uses this same name to depend on delivery-kit +// while it ships as a built-in command rather than a standalone plugin. +const DeliveryKitCommandName = "delivery-kit" + func NewDeliveryCommand() (*cobra.Command, context.Context) { server.DefaultAddress = "https://delivery-sync.deckhouse.ru" @@ -49,7 +54,7 @@ func NewDeliveryCommand() (*cobra.Command, context.Context) { return nil, ctx } - werfRootCmd.Use = "delivery-kit" + werfRootCmd.Use = DeliveryKitCommandName werfRootCmd.Aliases = []string{werfAlias} werfRootCmd = ReplaceCommandName("werf", fmt.Sprintf("d8 %s", werfAlias), werfRootCmd) werfRootCmd.Short = "A set of tools for building, distributing, and deploying containerized applications" diff --git a/cmd/d8/root.go b/cmd/d8/root.go index 7ec4b5de..e5291ec3 100644 --- a/cmd/d8/root.go +++ b/cmd/d8/root.go @@ -129,7 +129,7 @@ func (r *RootCommand) registerCommands() { r.cmd.AddCommand(packagecmd.NewCommand()) - r.cmd.AddCommand(pluginscmd.NewCommand(r.logger.Named("plugins-command"))) + r.cmd.AddCommand(pluginscmd.NewCommand(r.logger.Named("plugins-command"), []string{commands.DeliveryKitCommandName})) r.cmd.AddCommand(selfupdatecmd.NewCommand(r.logger.Named("cli-command"))) } diff --git a/internal/plugins/builtins.go b/internal/plugins/builtins.go new file mode 100644 index 00000000..62ad16e4 --- /dev/null +++ b/internal/plugins/builtins.go @@ -0,0 +1,44 @@ +/* +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 plugins + +// SetBuiltinCommands records the d8 built-in command names that satisfy a plugin +// dependency of the same name. It bridges capabilities that ship as a built-in +// command but are not yet published as standalone plugins (e.g. delivery-kit): +// such a dependency counts as satisfied by the command's mere presence, with no +// on-disk install and no registry lookup. +func (m *Manager) SetBuiltinCommands(names []string) { + if len(names) == 0 { + m.builtins = nil + + return + } + + m.builtins = make(map[string]struct{}, len(names)) + for _, name := range names { + m.builtins[name] = struct{}{} + } +} + +// isBuiltinCommand reports whether name is provided by a built-in command and so +// satisfies a plugin dependency on it. Version constraints are not enforced: a +// built-in cannot be upgraded, so its presence alone satisfies the dependency. +func (m *Manager) isBuiltinCommand(name string) bool { + _, ok := m.builtins[name] + + return ok +} diff --git a/internal/plugins/builtins_test.go b/internal/plugins/builtins_test.go new file mode 100644 index 00000000..536a37d7 --- /dev/null +++ b/internal/plugins/builtins_test.go @@ -0,0 +1,151 @@ +/* +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 plugins + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/deckhouse/deckhouse-cli/internal" +) + +// A built-in command satisfies a mandatory plugin dependency by presence. The +// dependency name is marked unpublished, so if resolution ever reached the +// registry it would fail with reasonDepNotPublished; the built-in short-circuit +// must avoid that lookup entirely. This is the delivery-kit bridge: it ships as a +// built-in command, not a plugin. +func TestPlannerBuiltinSatisfiesMandatoryDep(t *testing.T) { + src := &multiPluginSource{ + unpublished: map[string]bool{"delivery-kit": true}, + contracts: map[string]map[string]*internal.Plugin{}, + } + m := plannerManager(t, src) + m.SetBuiltinCommands([]string{"delivery-kit"}) + + top := requires("package", "v1.0.0", "delivery-kit", ">= 1.0.0") + + plan, reason, err := m.planFor(context.Background(), top, false) + require.NoError(t, err) + require.Nil(t, reason, "a built-in command satisfies the dependency") + require.NotNil(t, plan) + assert.NotContains(t, planStepVersions(plan), "delivery-kit", + "a built-in is not installed, so it is never planned") +} + +// presence-only: a version constraint on a built-in dependency is not enforced, +// since a built-in cannot be upgraded. +func TestPlannerBuiltinIgnoresVersionConstraint(t *testing.T) { + src := &multiPluginSource{ + unpublished: map[string]bool{"delivery-kit": true}, + contracts: map[string]map[string]*internal.Plugin{}, + } + m := plannerManager(t, src) + m.SetBuiltinCommands([]string{"delivery-kit"}) + + // An unsatisfiable constraint still passes - presence is enough. + top := requires("package", "v1.0.0", "delivery-kit", ">= 999.0.0") + + _, reason, err := m.planFor(context.Background(), top, false) + require.NoError(t, err) + assert.Nil(t, reason, "presence satisfies the dependency regardless of the constraint") +} + +// Without the built-in registered, the same contract is unsatisfiable: the +// dependency is looked up in the registry and reported unpublished. This is the +// contrast that proves the built-in short-circuit is what changes the outcome. +func TestPlannerUnregisteredBuiltinStillUnresolved(t *testing.T) { + src := &multiPluginSource{ + unpublished: map[string]bool{"delivery-kit": true}, + contracts: map[string]map[string]*internal.Plugin{}, + } + m := plannerManager(t, src) // no SetBuiltinCommands + + top := requires("package", "v1.0.0", "delivery-kit", "") + + plan, reason, err := m.planFor(context.Background(), top, false) + require.NoError(t, err) + require.Nil(t, plan) + require.NotNil(t, reason, "an unregistered, unpublished dependency is unsatisfiable") + assert.Equal(t, reasonDepNotPublished, reason.kind) +} + +// The final pre-switch guard also treats a built-in as satisfying a mandatory +// requirement: no soft failure is recorded, so the install is not rejected after +// the binary swap. +func TestValidateMandatoryRequirementSatisfiedByBuiltin(t *testing.T) { + m := testManager() + m.pluginDirectory = t.TempDir() + m.SetBuiltinCommands([]string{"delivery-kit"}) + + plugin := &internal.Plugin{ + Name: "package", + Version: "v1.0.0", + Requirements: internal.Requirements{ + Plugins: internal.PluginRequirementsGroup{ + Mandatory: []internal.PluginRequirement{{Name: "delivery-kit", Constraint: ">= 1.0.0"}}, + }, + }, + } + + failed, err := m.validatePluginRequirementMandatory(plugin) + require.NoError(t, err) + assert.Empty(t, failed, "a built-in command satisfies the mandatory requirement") +} + +// Without the built-in registered, the same mandatory requirement is recorded as +// missing - the regression guard for the validator change. +func TestValidateMandatoryRequirementMissingWithoutBuiltin(t *testing.T) { + m := testManager() + m.pluginDirectory = t.TempDir() // empty install root: nothing installed + + plugin := &internal.Plugin{ + Name: "package", + Version: "v1.0.0", + Requirements: internal.Requirements{ + Plugins: internal.PluginRequirementsGroup{ + Mandatory: []internal.PluginRequirement{{Name: "delivery-kit", Constraint: ""}}, + }, + }, + } + + failed, err := m.validatePluginRequirementMandatory(plugin) + require.NoError(t, err) + assert.Contains(t, failed, "delivery-kit", "an unregistered, uninstalled dependency is missing") +} + +// A conditional requirement on a built-in is satisfied (a no-op), never a hard +// error - even with a version constraint that is not enforced for a built-in. +func TestValidateConditionalRequirementSatisfiedByBuiltin(t *testing.T) { + m := testManager() + m.pluginDirectory = t.TempDir() + m.SetBuiltinCommands([]string{"delivery-kit"}) + + plugin := &internal.Plugin{ + Name: "package", + Version: "v1.0.0", + Requirements: internal.Requirements{ + Plugins: internal.PluginRequirementsGroup{ + Conditional: []internal.PluginRequirement{{Name: "delivery-kit", Constraint: ">= 1.0.0"}}, + }, + }, + } + + require.NoError(t, m.validatePluginRequirementConditional(plugin)) +} diff --git a/internal/plugins/cmd/plugins.go b/internal/plugins/cmd/plugins.go index caabd86c..ca8ccc84 100644 --- a/internal/plugins/cmd/plugins.go +++ b/internal/plugins/cmd/plugins.go @@ -32,8 +32,12 @@ import ( ) // NewCommand returns the `d8 plugins` command tree for managing plugins. -func NewCommand(logger *dkplog.Logger) *cobra.Command { +// builtinCommands are built-in command names that satisfy a plugin dependency of +// the same name (e.g. delivery-kit) until such capabilities ship as standalone +// plugins. +func NewCommand(logger *dkplog.Logger, builtinCommands []string) *cobra.Command { manager := plugins.NewManager(logger) + manager.SetBuiltinCommands(builtinCommands) cmd := &cobra.Command{ Use: "plugins", diff --git a/internal/plugins/cmd/plugins_test.go b/internal/plugins/cmd/plugins_test.go index ad0c7dcb..50630532 100644 --- a/internal/plugins/cmd/plugins_test.go +++ b/internal/plugins/cmd/plugins_test.go @@ -71,7 +71,7 @@ func TestPluginsDirFlagIsHonored(t *testing.T) { dir := t.TempDir() + "/custom-root" - cmd := NewCommand(dkplog.NewNop()) + cmd := NewCommand(dkplog.NewNop(), nil) cmd.SetContext(context.Background()) cmd.SetArgs([]string{"list", "--plugins-dir", dir}) cmd.SetOut(io.Discard) diff --git a/internal/plugins/planner.go b/internal/plugins/planner.go index 4c81bdd0..b54f546e 100644 --- a/internal/plugins/planner.go +++ b/internal/plugins/planner.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "log/slog" "maps" "os" "slices" @@ -242,6 +243,12 @@ func (m *Manager) reverseConflictReason(plugin *internal.Plugin) (*unsatisfiable // out-of-constraint conditional dependency makes the candidate unsatisfiable (a // conditional dependency is not auto-upgraded). func (m *Manager) conditionalReason(req internal.PluginRequirement, plan *resolutionPlan) (*unsatisfiableReason, error) { + if m.isBuiltinCommand(req.Name) { + // Provided by a built-in command: the conditional dependency is satisfied; + // its version constraint is not enforced for a built-in. + return nil, nil + } + version, present, err := m.effectiveVersion(req.Name, plan) if err != nil { return nil, err @@ -278,6 +285,16 @@ func (m *Manager) resolveMandatoryDep( depth int, path []string, ) (*unsatisfiableReason, error) { + if m.isBuiltinCommand(req.Name) { + // A built-in command of this name provides the capability: the dependency + // is satisfied by presence. A built-in cannot be upgraded, so any version + // constraint is not enforced; no plan step, no registry lookup. + m.logger.Debug("mandatory dependency satisfied by built-in command", + slog.String("plugin_dependency", req.Name)) + + return nil, nil + } + if visited[req.Name] { return &unsatisfiableReason{kind: reasonDepCycle, pluginName: req.Name, path: append(slices.Clone(path), req.Name), detail: "dependency cycle"}, nil } diff --git a/internal/plugins/plugins.go b/internal/plugins/plugins.go index 34c06984..de26ba46 100644 --- a/internal/plugins/plugins.go +++ b/internal/plugins/plugins.go @@ -51,6 +51,12 @@ type Manager struct { // listing to one registry call per plugin within a command run. tagsCache map[string][]string + // builtins are d8 built-in command names that satisfy a plugin dependency of + // the same name by their mere presence - a bridge for capabilities not yet + // shipped as standalone plugins (e.g. delivery-kit). Set via + // SetBuiltinCommands; nil for Managers that never resolve dependencies. + builtins map[string]struct{} + logger *dkplog.Logger } diff --git a/internal/plugins/validators.go b/internal/plugins/validators.go index df77ad00..5cc91d00 100644 --- a/internal/plugins/validators.go +++ b/internal/plugins/validators.go @@ -238,6 +238,14 @@ func (m *Manager) validatePluginRequirementMandatory(plugin *internal.Plugin) (f result := make(failedConstraints) for _, pluginRequirement := range plugin.Requirements.Plugins.Mandatory { + if m.isBuiltinCommand(pluginRequirement.Name) { + m.logger.Debug("mandatory plugin requirement satisfied by built-in command", + slog.String("plugin", plugin.Name), + slog.String("requirement", pluginRequirement.Name)) + + continue + } + installed, err := m.checkInstalled(pluginRequirement.Name) if err != nil { return nil, fmt.Errorf("failed to check if plugin is installed: %w", err) @@ -284,6 +292,10 @@ func (m *Manager) validatePluginRequirementMandatory(plugin *internal.Plugin) (f // - if the dependency is installed but fails the constraint, return a hard error func (m *Manager) validatePluginRequirementConditional(plugin *internal.Plugin) error { for _, pluginRequirement := range plugin.Requirements.Plugins.Conditional { + if m.isBuiltinCommand(pluginRequirement.Name) { + continue + } + installed, err := m.checkInstalled(pluginRequirement.Name) if err != nil { return fmt.Errorf("failed to check if plugin is installed: %w", err) diff --git a/internal/selfupdate/README.md b/internal/selfupdate/README.md index 439ecca5..b929d61c 100644 --- a/internal/selfupdate/README.md +++ b/internal/selfupdate/README.md @@ -112,7 +112,7 @@ Platforms (`rpp_source.go`): - releases are published as multi-platform OCI image indexes under plain version tags (`v1.2.3`); - `ListTags` returns those plain version tags as-is; -- `ExtractBinary` pulls the plain tag; the client attaches `?platform=/` +- `ExtractBinary` pulls the plain tag; the client attaches `?platform=-` (see `internal/rpp`), so the proxy resolves the matching per-platform child manifest from the index. The proxy must be recent enough to honor the query - see [internal/rpp](../rpp).