Skip to content

Commit fa31e00

Browse files
yaacovmnecas
yaacov
authored and
mnecas
committed
MTV-4766 | Permissive source validation for network and storage mappings
Source references not found in the provider inventory now produce a Warn condition instead of Critical in both NetworkMap and StorageMap. This allows shared mappings to stay Ready even when some sources are temporarily unavailable, so plans that only use a subset of the mapped sources can proceed independently. Plan-level validation still enforces that every VM's networks and storage exist in the inventory via Status.Refs, so missing sources are caught at migration time. Ref: https://issues.redhat.com/browse/MTV-4766 Resolves: MTV-4766 Signed-off-by: yaacov <yzamir@redhat.com>
1 parent 992da0f commit fa31e00

File tree

5 files changed

+360
-2
lines changed

5 files changed

+360
-2
lines changed

‎docs/enhancements/allow-missing-sources.md‎

Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
---
2+
title: allow-missing-sources
3+
authors:
4+
- "@yaacov"
5+
reviewers:
6+
- TBD
7+
approvers:
8+
- TBD
9+
creation-date: 2026-03-22
10+
last-updated: 2026-03-25
11+
status: implemented
12+
---
13+
14+
# Permissive Source Validation in Network and Storage Mappings
15+
16+
## Release Signoff Checklist
17+
18+
- [x] Enhancement is `implementable`
19+
- [x] Design details are appropriately documented from clear requirements
20+
- [x] Test plan is defined
21+
- [ ] User-facing documentation is created
22+
23+
## Summary
24+
25+
Source references not found in the provider inventory produce a `Warn`
26+
condition instead of a `Critical` one in both `NetworkMap` and `StorageMap`
27+
custom resources. This allows a single shared mapping to remain `Ready` even
28+
when some of its source entries are temporarily or permanently unavailable,
29+
enabling multiple migration plans that each use only a subset of the mapped
30+
sources to proceed independently.
31+
32+
Plan-level validation independently verifies that every VM selected for
33+
migration has its networks and storage present in the provider inventory
34+
(via `Status.Refs`), so missing sources are still caught at migration time.
35+
36+
## Motivation
37+
38+
Organizations managing large virtualization environments commonly have dozens
39+
of networks and datastores. Rather than creating a dedicated mapping for every
40+
migration plan, administrators prefer to define a single NetworkMap and a single
41+
StorageMap that cover all known sources. Individual plans each reference the
42+
same shared mappings but only migrate VMs that use a subset of those sources.
43+
44+
If every source listed in a mapping had to exist in the provider inventory for
45+
the mapping to be marked `Ready`, even one temporarily unavailable source --
46+
because the provider inventory has not yet synced it, or because a datastore
47+
was decommissioned that no current plan needs -- would block the entire mapping
48+
with a `Critical` condition. This would cascade to every plan that references
49+
the mapping, preventing migrations that have nothing to do with the missing
50+
source.
51+
52+
### Goals
53+
54+
* Allow a single NetworkMap or StorageMap to be shared across many migration
55+
plans without requiring every mapped source to be present in inventory at all
56+
times.
57+
* Report source `NotFound` validation errors as warnings so the mapping stays
58+
`Ready` and referencing plans are not blocked.
59+
* Rely on plan-level per-VM validation to catch any missing source that a
60+
migration actually needs.
61+
62+
### Non-Goals
63+
64+
* Relaxing validation for destination references -- the destination cluster must
65+
be fully configured before migration.
66+
* Relaxing validation for configuration errors (`NotSet`, `Ambiguous`) -- these
67+
indicate broken mapping entries, not temporarily missing sources.
68+
* Changing plan-level per-VM validation -- plans independently verify that each
69+
selected VM has its networks and storage covered by the mapping's resolved
70+
references (`Status.Refs`).
71+
72+
## Proposal
73+
74+
### User Stories
75+
76+
#### Story 1
77+
78+
As a migration administrator, I create one NetworkMap that covers all 20 source
79+
port groups in my vSphere environment and one StorageMap that covers all 8
80+
datastores. I then create several migration plans, each targeting a different
81+
set of VMs. Some port groups and datastores are used by only one plan. I want
82+
all plans to proceed even if a source that no running plan needs has been
83+
removed from vSphere or is not yet synced into the Forklift inventory.
84+
85+
#### Story 2
86+
87+
As a migration administrator, I am rolling out migrations incrementally. I
88+
pre-configure a mapping with all sources I will eventually need. Early plans
89+
only use a fraction of those sources; the rest will be added to the provider
90+
later. I do not want to wait until every source exists in inventory before I can
91+
start my first migration.
92+
93+
### Implementation Details
94+
95+
The `validateSource` functions in the network and storage map controllers set
96+
`Category: Warn` instead of `Category: Critical` for source refs that produce
97+
a `NotFound` error. Since `Warn` conditions do not block the `Ready` state
98+
(`HasBlockerCondition` only checks for `Critical` and `Error`), the mapping
99+
remains usable.
100+
101+
The following validations remain `Critical`:
102+
103+
| Validation | Reason |
104+
|---|---|
105+
| Source ref with no ID or Name (`NotSet`) | Configuration error -- the entry is incomplete |
106+
| Source ref matching multiple inventory objects (`Ambiguous`) | Configuration error -- the ref must be unique |
107+
| Destination network/storage class not found | Destination must be configured before migration |
108+
109+
Plan-level validation independently verifies that every VM selected for
110+
migration has its networks and storage covered by the mapping's resolved
111+
references in `Status.Refs` (`NetworksMapped` / `StorageMapped`). Only sources
112+
that are successfully resolved against the provider inventory are added to
113+
`Status.Refs`. A VM that needs a source missing from inventory will still
114+
produce a `Critical` condition on the plan.
115+
116+
### Security, Risks, and Mitigations
117+
118+
Permissive mapping validation does not bypass any plan-level safety checks.
119+
The only change is the severity of a mapping condition, not whether the
120+
condition is reported -- missing sources are still visible as warnings in the
121+
mapping status.
122+
123+
## Design Details
124+
125+
### Controller Changes
126+
127+
In `pkg/controller/map/network/validation.go` and
128+
`pkg/controller/map/storage/validation.go`, the `validateSource` method uses
129+
`Warn` category for `NotFound` sources:
130+
131+
```go
132+
mp.Status.SetCondition(libcnd.Condition{
133+
Type: SourceStorageNotValid,
134+
Status: True,
135+
Reason: NotFound,
136+
Category: Warn,
137+
Message: "Source storage not found.",
138+
Items: notValid,
139+
})
140+
```
141+
142+
### Examples
143+
144+
#### Shared NetworkMap
145+
146+
```yaml
147+
apiVersion: forklift.konveyor.io/v1beta1
148+
kind: NetworkMap
149+
metadata:
150+
name: shared-network-map
151+
namespace: openshift-mtv
152+
spec:
153+
provider:
154+
source:
155+
name: vsphere-provider
156+
namespace: openshift-mtv
157+
destination:
158+
name: host
159+
namespace: openshift-mtv
160+
map:
161+
- source:
162+
id: dvportgroup-10
163+
destination:
164+
type: pod
165+
- source:
166+
id: dvportgroup-20
167+
destination:
168+
type: multus
169+
namespace: openshift-mtv
170+
name: vlan20-nad
171+
- source:
172+
id: dvportgroup-30
173+
destination:
174+
type: multus
175+
namespace: openshift-mtv
176+
name: vlan30-nad
177+
```
178+
179+
If `dvportgroup-30` is not yet present in the provider inventory, the mapping
180+
will show a warning condition but will still be `Ready`. A plan that only
181+
migrates VMs on `dvportgroup-10` and `dvportgroup-20` will proceed without
182+
issue.
183+
184+
#### Shared StorageMap
185+
186+
```yaml
187+
apiVersion: forklift.konveyor.io/v1beta1
188+
kind: StorageMap
189+
metadata:
190+
name: shared-storage-map
191+
namespace: openshift-mtv
192+
spec:
193+
provider:
194+
source:
195+
name: vsphere-provider
196+
namespace: openshift-mtv
197+
destination:
198+
name: host
199+
namespace: openshift-mtv
200+
map:
201+
- source:
202+
id: datastore-100
203+
destination:
204+
storageClass: ocs-storagecluster-ceph-rbd
205+
- source:
206+
id: datastore-200
207+
destination:
208+
storageClass: ocs-storagecluster-ceph-rbd
209+
- source:
210+
id: datastore-300
211+
destination:
212+
storageClass: nfs-csi
213+
```
214+
215+
If `datastore-300` has been removed from vSphere, the mapping stays `Ready`.
216+
Plans migrating VMs that only reside on `datastore-100` or `datastore-200` are
217+
unaffected.
218+
219+
### Test Plan
220+
221+
Unit tests verify:
222+
- `NotFound` sources produce `Warn` and the mapping remains `Ready`.
223+
- `NotSet` and `Ambiguous` remain `Critical` and block `Ready`.
224+
225+
### Upgrade / Downgrade Strategy
226+
227+
No migration of existing resources is required. Plan-level validation continues
228+
to enforce that all sources needed by selected VMs exist in inventory, so no
229+
migration safety is lost.
230+
231+
## Implementation History
232+
233+
* 2026-03-25 - Enhancement implemented.
234+
235+
## Drawbacks
236+
237+
Users who do not review mapping warnings may not notice that sources have
238+
disappeared from their environment. This is mitigated by the fact that warnings
239+
are still reported in the mapping status and that plan-level validation catches
240+
any missing source a migration actually needs.
241+
242+
## Alternatives
243+
244+
1. **Per-entry opt-out**: Allow each mapping entry to be marked as optional.
245+
More granular but adds complexity to the API.
246+
2. **Plan-level override**: Put the flag on the Plan instead of the mapping.
247+
This would require plan validation to ignore mapping `Ready` state, which
248+
couples plan and mapping validation more tightly.

‎pkg/controller/map/network/network_test.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,61 @@ var _ = Describe("NetworkMap Struct", func() {
6262
Expect(nm.Status.HasAnyCondition(SourceNetworkNotValid, DestinationNetworkNotValid)).To(BeTrue())
6363
})
6464
})
65+
66+
Describe("Permissive source validation", func() {
67+
It("should use Warn for NotFound (non-blocking)", func() {
68+
nm := &api.NetworkMap{
69+
ObjectMeta: meta.ObjectMeta{
70+
Name: "test-network-map",
71+
Namespace: "default",
72+
},
73+
}
74+
nm.Status.SetCondition(libcnd.Condition{
75+
Type: SourceNetworkNotValid,
76+
Status: True,
77+
Reason: NotFound,
78+
Category: Warn,
79+
Message: "Source network not found.",
80+
Items: []string{"net-1"},
81+
})
82+
Expect(nm.Status.HasBlockerCondition()).To(BeFalse())
83+
})
84+
85+
It("should keep Critical for NotSet (blocking)", func() {
86+
nm := &api.NetworkMap{
87+
ObjectMeta: meta.ObjectMeta{
88+
Name: "test-network-map",
89+
Namespace: "default",
90+
},
91+
}
92+
nm.Status.SetCondition(libcnd.Condition{
93+
Type: SourceNetworkNotValid,
94+
Status: True,
95+
Reason: NotSet,
96+
Category: Critical,
97+
Message: "Source network: either `ID` or `Name` required.",
98+
})
99+
Expect(nm.Status.HasBlockerCondition()).To(BeTrue())
100+
})
101+
102+
It("should keep Critical for Ambiguous (blocking)", func() {
103+
nm := &api.NetworkMap{
104+
ObjectMeta: meta.ObjectMeta{
105+
Name: "test-network-map",
106+
Namespace: "default",
107+
},
108+
}
109+
nm.Status.SetCondition(libcnd.Condition{
110+
Type: SourceNetworkNotValid,
111+
Status: True,
112+
Reason: Ambiguous,
113+
Category: Critical,
114+
Message: "Source network has ambiguous ref.",
115+
Items: []string{"net-1"},
116+
})
117+
Expect(nm.Status.HasBlockerCondition()).To(BeTrue())
118+
})
119+
})
65120
})
66121

67122
var _ = Describe("NetworkPair", func() {

‎pkg/controller/map/network/validation.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func (r *Reconciler) validateSource(mp *api.NetworkMap) (err error) {
120120
Type: SourceNetworkNotValid,
121121
Status: True,
122122
Reason: NotFound,
123-
Category: Critical,
123+
Category: Warn,
124124
Message: "Source network not found.",
125125
Items: notValid,
126126
})

‎pkg/controller/map/storage/storage_test.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,61 @@ var _ = Describe("StorageMap Struct", func() {
6363
Expect(sm.Status.HasAnyCondition(SourceStorageNotValid, DestinationStorageNotValid)).To(BeTrue())
6464
})
6565
})
66+
67+
Describe("Permissive source validation", func() {
68+
It("should use Warn for NotFound (non-blocking)", func() {
69+
sm := &api.StorageMap{
70+
ObjectMeta: meta.ObjectMeta{
71+
Name: "test-storage-map",
72+
Namespace: "default",
73+
},
74+
}
75+
sm.Status.SetCondition(libcnd.Condition{
76+
Type: SourceStorageNotValid,
77+
Status: True,
78+
Reason: NotFound,
79+
Category: Warn,
80+
Message: "Source storage not found.",
81+
Items: []string{"ds-1"},
82+
})
83+
Expect(sm.Status.HasBlockerCondition()).To(BeFalse())
84+
})
85+
86+
It("should keep Critical for NotSet (blocking)", func() {
87+
sm := &api.StorageMap{
88+
ObjectMeta: meta.ObjectMeta{
89+
Name: "test-storage-map",
90+
Namespace: "default",
91+
},
92+
}
93+
sm.Status.SetCondition(libcnd.Condition{
94+
Type: SourceStorageNotValid,
95+
Status: True,
96+
Reason: NotSet,
97+
Category: Critical,
98+
Message: "Source storage: either `ID` or `Name` required.",
99+
})
100+
Expect(sm.Status.HasBlockerCondition()).To(BeTrue())
101+
})
102+
103+
It("should keep Critical for Ambiguous (blocking)", func() {
104+
sm := &api.StorageMap{
105+
ObjectMeta: meta.ObjectMeta{
106+
Name: "test-storage-map",
107+
Namespace: "default",
108+
},
109+
}
110+
sm.Status.SetCondition(libcnd.Condition{
111+
Type: SourceStorageNotValid,
112+
Status: True,
113+
Reason: Ambiguous,
114+
Category: Critical,
115+
Message: "Source storage has ambiguous ref.",
116+
Items: []string{"ds-1"},
117+
})
118+
Expect(sm.Status.HasBlockerCondition()).To(BeTrue())
119+
})
120+
})
66121
})
67122

68123
var _ = Describe("StoragePair", func() {

‎pkg/controller/map/storage/validation.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func (r *Reconciler) validateSource(mp *api.StorageMap) (err error) {
111111
Type: SourceStorageNotValid,
112112
Status: True,
113113
Reason: NotFound,
114-
Category: Critical,
114+
Category: Warn,
115115
Message: "Source storage not found.",
116116
Items: notValid,
117117
})

0 commit comments

Comments
 (0)