Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion api/aicr/v1/server.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,8 @@ paths:
version: v25.3.3
source: https://helm.ngc.nvidia.com/nvidia
chart: gpu-operator
deploymentOrder:
- gpu-operator
responses:
"200":
description: Zip archive containing generated bundles
Expand Down Expand Up @@ -1495,7 +1497,26 @@ components:
path:
type: string
description: Path within the source to the kustomization (Kustomize)
description: List of component references with deployment order
patches:
type: array
items:
type: string
description: >-
Unsupported / not applied. No deployer applies patch files, so
an enabled ref that sets `patches` is rejected with 400 rather
than silently producing an unpatched bundle. Do not set. See
issue #1588.
description: >-
List of component references. Deployment order is not per-ref; it is
the top-level `deploymentOrder` array.
deploymentOrder:
type: array
items:
type: string
description: >-
Topologically sorted component names giving the order to deploy in.
Preserve this on a GET→POST round trip (POST /v1/bundle accepts this
same shape) — dropping it changes deployment sequencing.
constraints:
type: object
description: Deployment constraints (driver versions, etc.)
Expand Down
5 changes: 5 additions & 0 deletions docs/integrator/recipe-development.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,11 @@ componentRefs:

A component must have either `helm` OR `kustomize` configuration, not both.

> **`patches` is not supported.** The `componentRefs[].patches` field is not
> applied by any deployer. An enabled ref that sets `patches` is rejected at
> recipe resolution (rather than silently producing an unpatched bundle), so do
> not use it. See [#1588](https://github.com/NVIDIA/aicr/issues/1588).

## Component Configuration

### Configuration Patterns
Expand Down
1 change: 1 addition & 0 deletions docs/user/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ componentRefs:
type: Helm
version: vXX.Y.Z # illustrative; see Component Catalog for current pins
source: https://helm.ngc.nvidia.com/nvidia
chart: gpu-operator
deploymentOrder:
- gpu-operator
constraints:
Expand Down
8 changes: 7 additions & 1 deletion pkg/client/v1/aicr_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,13 @@ func (b *blockingReadFileProvider) ReadFile(ctx context.Context, path string) ([
default:
close(b.readStarted)
}
<-b.readUnblock
// Honor the DataProvider contract: return the context error if canceled
// rather than parking forever.
select {
case <-b.readUnblock:
case <-ctx.Done():
return nil, ctx.Err()
}
}
return b.underlying.ReadFile(ctx, path)
}
Expand Down
15 changes: 14 additions & 1 deletion pkg/recipe/componentref_coherence_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ func TestComponentRefCoherenceProblem(t *testing.T) {
wantBad: true,
},
{
// The REST wire format / OpenAPI example uses lowercase; it must be
// Lowercase is accepted as backward-compatible input (hand-authored
// recipes / older clients); the canonical form is Helm. It must be
// accepted case-insensitively, not rejected as an unsupported type.
name: "lowercase helm is accepted",
ref: ComponentRef{Name: "a", Type: ComponentType("helm"), Version: "v1"},
Expand All @@ -98,6 +99,18 @@ func TestComponentRefCoherenceProblem(t *testing.T) {
ref: ComponentRef{Name: "a", Type: ComponentType("helm"), Version: "v1", Tag: "v2"},
wantBad: true,
},
{
// Patches are carried through resolution but applied by no deployer;
// a ref that declares them is rejected rather than silently dropped.
name: "helm with patches rejected",
ref: ComponentRef{Name: "a", Type: ComponentTypeHelm, Version: "v1", Patches: []string{"p.yaml"}},
wantBad: true,
},
{
name: "kustomize with patches rejected",
ref: ComponentRef{Name: "a", Type: ComponentTypeKustomize, Path: "deploy", Patches: []string{"p.yaml"}},
wantBad: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
58 changes: 41 additions & 17 deletions pkg/recipe/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,11 @@ type ComponentRef struct {
// Merge order: base values → ValuesFile → Overrides (highest precedence).
Overrides map[string]any `json:"overrides,omitempty" yaml:"overrides,omitempty"`

// Patches is a list of patch files to apply (for Kustomize).
// Patches is a list of patch files (intended for Kustomize). NOT CURRENTLY
// APPLIED by any deployer — an enabled ref that declares patches is rejected
// by ValidateCoherence (disabled refs are skipped) rather than silently
// producing an unpatched bundle. See #1588 (implement application or drop
// the field).
Patches []string `json:"patches,omitempty" yaml:"patches,omitempty"`

// DependencyRefs is a list of component names this component depends on.
Expand Down Expand Up @@ -223,27 +227,46 @@ func (ref *ComponentRef) ApplyRegistryDefaults(config *ComponentConfig) {
// resolution rather than silently deploying as a different type (or producing
// a signed attestation whose metadata does not match what deploys):
//
// - the deployers do not trust the declared Type — Helm/Helmfile/ArgoCD drop
// it, and localformat classifies any ref carrying a Tag or Path as
// Kustomize (see pkg/bundler/deployer/localformat classify/write) — so a
// Helm ref must not carry Kustomize fields;
// - a Kustomize ref needs a Path to build from; and
// - a Tag is only meaningful with a Source (git repo / OCI ref).
// - the field-classifying deployers (localformat, and the Helm/Helmfile/ArgoCD
// generators built on it) do not trust the declared Type — localformat
// classifies any ref carrying a Tag or Path as Kustomize (see
// pkg/bundler/deployer/localformat classify/write). The Flux generator
// differs: it switches on the declared Type. So a Helm ref carrying a
// tag/path builds as Kustomize under the field-classifiers but as Helm
// under Flux — the same ref deploys differently by deployer — which is why
// a Helm ref must not carry Kustomize fields. (An explicitly Kustomize ref,
// conversely, is rejected outright by the Helm-only Flux generator — see
// #1588.);
// - a Kustomize ref needs a Path to build from;
// - a Tag is only meaningful with a Source (git repo / OCI ref); and
// - no deployer applies ComponentRef.Patches, so a ref that declares patch
// files is rejected rather than silently producing an unpatched bundle
// (see #1588).
//
// Keep these in lockstep with the localformat rules.
func (ref *ComponentRef) coherenceProblem() string {
// Patches are unsupported for every deployment type: the field is carried
// through resolution but no deployer applies it (localformat's Component has
// no patches field). Fail closed on any type rather than drop it silently.
if len(ref.Patches) > 0 {
return fmt.Sprintf("component %q declares patches, but no deployer applies patch files; "+
"remove `patches` (removing it does not change the generated bundle). See #1588.", ref.Name)
}

hasTag, hasPath := ref.Tag != "", ref.Path != ""
// Match the type case-insensitively: the resolver emits the canonical
// ComponentType ("Helm"/"Kustomize"), but the REST wire format and
// hand-authored recipes may use lowercase (see the /v1/bundle OpenAPI
// example), and the deployers classify by tag/path rather than by this
// field — so "helm" and "Helm" must be treated the same here.
// Match the type case-insensitively: the resolver and the OpenAPI examples
// use the canonical ComponentType ("Helm"/"Kustomize"), but lowercase is
// accepted as backward-compatible input from hand-authored recipes or older
// clients, and the field-classifying deployers key off tag/path (while Flux
// switches on Type) — so "helm" and "Helm" must be treated the same.
switch {
case strings.EqualFold(string(ref.Type), string(ComponentTypeHelm)):
if hasTag || hasPath {
return fmt.Sprintf("component %q is Helm but carries Kustomize field(s) (tag=%q, path=%q); "+
"the deployers classify any ref with a tag or path as Kustomize, so it would deploy as a "+
"different type than declared", ref.Name, ref.Tag, ref.Path)
"the field-classifying deployers would treat it as Kustomize while the Type-switching Flux "+
"deployer would treat it as Helm, so the same ref deploys differently — remove the tag/path, "+
"or convert it into a coherent Kustomize ref (which needs a path, and a source if it sets a tag)",
ref.Name, ref.Tag, ref.Path)
}
case strings.EqualFold(string(ref.Type), string(ComponentTypeKustomize)):
if !hasPath {
Expand All @@ -263,9 +286,10 @@ func (ref *ComponentRef) coherenceProblem() string {
default:
// After ApplyRegistryDefaults every registry-backed ref has a supported
// Type; an empty or unknown Type here means an externally-supplied ref
// the registry did not populate. The deployers classify by tag/path, not
// by this field, so an unsupported Type would deploy ambiguously — fail
// closed rather than silently accept it.
// the registry did not populate. The field-classifying deployers key off
// tag/path (ignoring this field) while Flux switches on it, so an
// unsupported Type would deploy ambiguously or be rejected — fail closed
// rather than silently accept it.
return fmt.Sprintf("component %q has unsupported type %q; expected %q or %q",
ref.Name, ref.Type, ComponentTypeHelm, ComponentTypeKustomize)
}
Expand Down
Loading