Skip to content

Commit 88b3a4c

Browse files
mandremandre
authored
Merge pull request #758 from shiftstack/fix-fip-adoption
Tighten adoption filters
2 parents bfbfe11 + 8fb26f7 commit 88b3a4c

12 files changed

Lines changed: 314 additions & 65 deletions

File tree

‎internal/controllers/addressscope/actuator.go‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,25 @@ func (actuator addressscopeActuator) ListOSResourcesForAdoption(ctx context.Cont
7171
return nil, false
7272
}
7373

74+
// Resolve the project ID from ProjectRef if set.
75+
var projectID string
76+
if resourceSpec.ProjectRef != nil {
77+
project, rs := dependency.FetchDependency(
78+
ctx, actuator.k8sClient, orcObject.Namespace, resourceSpec.ProjectRef, "Project",
79+
func(dep *orcv1alpha1.Project) bool {
80+
return orcv1alpha1.IsAvailable(dep) && dep.Status.ID != nil
81+
},
82+
)
83+
if needsReschedule, _ := rs.NeedsReschedule(); needsReschedule {
84+
return nil, false
85+
}
86+
projectID = ptr.Deref(project.Status.ID, "")
87+
}
88+
7489
listOpts := addressscopes.ListOpts{
75-
Name: getResourceName(orcObject),
90+
Name: getResourceName(orcObject),
91+
IPVersion: int(resourceSpec.IPVersion),
92+
ProjectID: projectID,
7693
}
7794

7895
return actuator.osClient.ListAddressScopes(ctx, listOpts), true

‎internal/controllers/floatingip/actuator.go‎

Lines changed: 58 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,11 @@ type (
4646
)
4747

4848
type floatingipActuator struct {
49-
osClient osclients.NetworkClient
50-
}
51-
52-
type floatingipCreateActuator struct {
53-
floatingipActuator
49+
osClient osclients.NetworkClient
5450
k8sClient client.Client
5551
}
5652

57-
var _ createResourceActuator = floatingipCreateActuator{}
53+
var _ createResourceActuator = floatingipActuator{}
5854
var _ deleteResourceActuator = floatingipActuator{}
5955

6056
func (floatingipActuator) GetResourceID(osResource *osResourceT) string {
@@ -70,22 +66,69 @@ func (actuator floatingipActuator) GetOSResourceByID(ctx context.Context, id str
7066
}
7167

7268
func (actuator floatingipActuator) ListOSResourcesForAdoption(ctx context.Context, obj *orcv1alpha1.FloatingIP) (floatingipIterator, bool) {
73-
if obj.Spec.Resource == nil {
69+
resource := obj.Spec.Resource
70+
if resource == nil {
7471
return nil, false
7572
}
7673
// we only support adoption of floatingips by IP as they don't have name
77-
if obj.Spec.Resource.FloatingIP == nil {
74+
if resource.FloatingIP == nil {
7875
return nil, false
7976
}
8077

78+
// Resolve the floating network ID from either FloatingNetworkRef or
79+
// FloatingSubnetRef. Exactly one of these must be set per API
80+
// validation. Without the network ID, adoption could match a floating
81+
// IP on the wrong network.
82+
var floatingNetworkID string
83+
if resource.FloatingNetworkRef != nil {
84+
network, rs := dependency.FetchDependency(
85+
ctx, actuator.k8sClient, obj.Namespace, resource.FloatingNetworkRef, "Network",
86+
func(dep *orcv1alpha1.Network) bool {
87+
return orcv1alpha1.IsAvailable(dep) && dep.Status.ID != nil
88+
},
89+
)
90+
if needsReschedule, _ := rs.NeedsReschedule(); needsReschedule {
91+
return nil, false
92+
}
93+
floatingNetworkID = ptr.Deref(network.Status.ID, "")
94+
} else if resource.FloatingSubnetRef != nil {
95+
subnet, rs := dependency.FetchDependency(
96+
ctx, actuator.k8sClient, obj.Namespace, resource.FloatingSubnetRef, "Subnet",
97+
func(dep *orcv1alpha1.Subnet) bool {
98+
return orcv1alpha1.IsAvailable(dep) && dep.Status.ID != nil && dep.Status.Resource != nil
99+
},
100+
)
101+
if needsReschedule, _ := rs.NeedsReschedule(); needsReschedule {
102+
return nil, false
103+
}
104+
floatingNetworkID = subnet.Status.Resource.NetworkID
105+
}
106+
107+
// Resolve the project ID from ProjectRef if set.
108+
var projectID string
109+
if resource.ProjectRef != nil {
110+
project, rs := dependency.FetchDependency(
111+
ctx, actuator.k8sClient, obj.Namespace, resource.ProjectRef, "Project",
112+
func(dep *orcv1alpha1.Project) bool {
113+
return orcv1alpha1.IsAvailable(dep) && dep.Status.ID != nil
114+
},
115+
)
116+
if needsReschedule, _ := rs.NeedsReschedule(); needsReschedule {
117+
return nil, false
118+
}
119+
projectID = ptr.Deref(project.Status.ID, "")
120+
}
121+
81122
listOpts := floatingips.ListOpts{
82-
FloatingIP: string(ptr.Deref(obj.Spec.Resource.FloatingIP, "")),
83-
Tags: tags.Join(obj.Spec.Resource.Tags),
123+
FloatingIP: string(ptr.Deref(resource.FloatingIP, "")),
124+
FloatingNetworkID: floatingNetworkID,
125+
ProjectID: projectID,
126+
Tags: tags.Join(resource.Tags),
84127
}
85128
return actuator.osClient.ListFloatingIP(ctx, listOpts), true
86129
}
87130

88-
func (actuator floatingipCreateActuator) ListOSResourcesForImport(ctx context.Context, obj orcObjectPT, filter filterT) (iter.Seq2[*osResourceT, error], progress.ReconcileStatus) {
131+
func (actuator floatingipActuator) ListOSResourcesForImport(ctx context.Context, obj orcObjectPT, filter filterT) (iter.Seq2[*osResourceT, error], progress.ReconcileStatus) {
89132
var reconcileStatus progress.ReconcileStatus
90133

91134
network, rs := dependency.FetchDependency[*orcv1alpha1.Network](
@@ -126,7 +169,7 @@ func (actuator floatingipCreateActuator) ListOSResourcesForImport(ctx context.Co
126169
return actuator.osClient.ListFloatingIP(ctx, listOpts), nil
127170
}
128171

129-
func (actuator floatingipCreateActuator) CreateResource(ctx context.Context, obj *orcv1alpha1.FloatingIP) (*osResourceT, progress.ReconcileStatus) {
172+
func (actuator floatingipActuator) CreateResource(ctx context.Context, obj *orcv1alpha1.FloatingIP) (*osResourceT, progress.ReconcileStatus) {
130173
resource := obj.Spec.Resource
131174
if resource == nil {
132175
// Should have been caught by API validation
@@ -290,7 +333,7 @@ func (floatingipHelperFactory) NewAPIObjectAdapter(obj orcObjectPT) adapterI {
290333
}
291334

292335
func (floatingipHelperFactory) NewCreateActuator(ctx context.Context, orcObject orcObjectPT, controller interfaces.ResourceController) (createResourceActuator, progress.ReconcileStatus) {
293-
return newCreateActuator(ctx, orcObject, controller)
336+
return newActuator(ctx, orcObject, controller)
294337
}
295338

296339
func (floatingipHelperFactory) NewDeleteActuator(ctx context.Context, orcObject orcObjectPT, controller interfaces.ResourceController) (deleteResourceActuator, progress.ReconcileStatus) {
@@ -316,18 +359,7 @@ func newActuator(ctx context.Context, orcObject *orcv1alpha1.FloatingIP, control
316359
}
317360

318361
return floatingipActuator{
319-
osClient: osClient,
320-
}, nil
321-
}
322-
323-
func newCreateActuator(ctx context.Context, orcObject *orcv1alpha1.FloatingIP, controller interfaces.ResourceController) (floatingipCreateActuator, progress.ReconcileStatus) {
324-
floatingipActuator, reconcileStatus := newActuator(ctx, orcObject, controller)
325-
if needsReschedule, _ := reconcileStatus.NeedsReschedule(); needsReschedule {
326-
return floatingipCreateActuator{}, reconcileStatus
327-
}
328-
329-
return floatingipCreateActuator{
330-
floatingipActuator: floatingipActuator,
331-
k8sClient: controller.GetK8sClient(),
362+
osClient: osClient,
363+
k8sClient: controller.GetK8sClient(),
332364
}, nil
333365
}

‎internal/controllers/group/actuator.go‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,25 @@ func (actuator groupActuator) ListOSResourcesForAdoption(ctx context.Context, or
7171
return nil, false
7272
}
7373

74+
// Resolve the domain ID from DomainRef if set. Without the domain
75+
// ID, adoption could match a group in the wrong domain.
76+
var domainID string
77+
if resourceSpec.DomainRef != nil {
78+
domain, rs := dependency.FetchDependency(
79+
ctx, actuator.k8sClient, orcObject.Namespace, resourceSpec.DomainRef, "Domain",
80+
func(dep *orcv1alpha1.Domain) bool {
81+
return orcv1alpha1.IsAvailable(dep) && dep.Status.ID != nil
82+
},
83+
)
84+
if needsReschedule, _ := rs.NeedsReschedule(); needsReschedule {
85+
return nil, false
86+
}
87+
domainID = ptr.Deref(domain.Status.ID, "")
88+
}
89+
7490
listOpts := groups.ListOpts{
75-
Name: getResourceName(orcObject),
91+
Name: getResourceName(orcObject),
92+
DomainID: domainID,
7693
}
7794

7895
return actuator.osClient.ListGroups(ctx, listOpts), true

‎internal/controllers/network/actuator.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,32 @@ func (actuator networkActuator) GetOSResourceByID(ctx context.Context, id string
7272
}
7373

7474
func (actuator networkActuator) ListOSResourcesForAdoption(ctx context.Context, obj orcObjectPT) (iter.Seq2[*osResourceT, error], bool) {
75-
if obj.Spec.Resource == nil {
75+
resource := obj.Spec.Resource
76+
if resource == nil {
7677
return nil, false
7778
}
7879

79-
listOpts := networks.ListOpts{Name: getResourceName(obj)}
80+
// Resolve the project ID from ProjectRef if set. Without the project
81+
// ID, adoption with admin-scoped credentials could match a network
82+
// in the wrong project.
83+
var projectID string
84+
if resource.ProjectRef != nil {
85+
project, rs := dependency.FetchDependency(
86+
ctx, actuator.k8sClient, obj.Namespace, resource.ProjectRef, "Project",
87+
func(dep *orcv1alpha1.Project) bool {
88+
return orcv1alpha1.IsAvailable(dep) && dep.Status.ID != nil
89+
},
90+
)
91+
if needsReschedule, _ := rs.NeedsReschedule(); needsReschedule {
92+
return nil, false
93+
}
94+
projectID = ptr.Deref(project.Status.ID, "")
95+
}
96+
97+
listOpts := networks.ListOpts{
98+
Name: getResourceName(obj),
99+
ProjectID: projectID,
100+
}
80101
return actuator.osClient.ListNetwork(ctx, listOpts), true
81102
}
82103

‎internal/controllers/port/actuator.go‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,44 @@ func (actuator portActuator) GetOSResourceByID(ctx context.Context, id string) (
117117
}
118118

119119
func (actuator portActuator) ListOSResourcesForAdoption(ctx context.Context, obj *orcv1alpha1.Port) (portIterator, bool) {
120-
if obj.Spec.Resource == nil {
120+
resource := obj.Spec.Resource
121+
if resource == nil {
122+
return nil, false
123+
}
124+
125+
// Resolve the network ID from NetworkRef. Without the network ID,
126+
// adoption could match a port on the wrong network.
127+
network, rs := dependency.FetchDependency(
128+
ctx, actuator.k8sClient, obj.Namespace, &resource.NetworkRef, "Network",
129+
func(dep *orcv1alpha1.Network) bool {
130+
return orcv1alpha1.IsAvailable(dep) && dep.Status.ID != nil
131+
},
132+
)
133+
if needsReschedule, _ := rs.NeedsReschedule(); needsReschedule {
121134
return nil, false
122135
}
123136

124-
listOpts := ports.ListOpts{Name: getResourceName(obj)}
137+
// Resolve the project ID from ProjectRef if set.
138+
var projectID string
139+
if resource.ProjectRef != nil {
140+
project, rs := dependency.FetchDependency(
141+
ctx, actuator.k8sClient, obj.Namespace, resource.ProjectRef, "Project",
142+
func(dep *orcv1alpha1.Project) bool {
143+
return orcv1alpha1.IsAvailable(dep) && dep.Status.ID != nil
144+
},
145+
)
146+
if needsReschedule, _ := rs.NeedsReschedule(); needsReschedule {
147+
return nil, false
148+
}
149+
projectID = ptr.Deref(project.Status.ID, "")
150+
}
151+
152+
listOpts := ports.ListOpts{
153+
Name: getResourceName(obj),
154+
NetworkID: ptr.Deref(network.Status.ID, ""),
155+
MACAddress: resource.MACAddress,
156+
ProjectID: projectID,
157+
}
125158
return actuator.osClient.ListPort(ctx, listOpts), true
126159
}
127160

‎internal/controllers/project/actuator.go‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,31 @@ func (actuator projectActuator) GetOSResourceByID(ctx context.Context, id string
7575
}
7676

7777
func (actuator projectActuator) ListOSResourcesForAdoption(ctx context.Context, obj orcObjectPT) (iter.Seq2[*osResourceT, error], bool) {
78-
if obj.Spec.Resource == nil {
78+
resource := obj.Spec.Resource
79+
if resource == nil {
7980
return nil, false
8081
}
8182

83+
// Resolve the domain ID from DomainRef if set. Without the domain
84+
// ID, adoption could match a project in the wrong domain.
85+
var domainID string
86+
if resource.DomainRef != nil {
87+
domain, rs := dependency.FetchDependency(
88+
ctx, actuator.k8sClient, obj.Namespace, resource.DomainRef, "Domain",
89+
func(dep *orcv1alpha1.Domain) bool {
90+
return orcv1alpha1.IsAvailable(dep) && dep.Status.ID != nil
91+
},
92+
)
93+
if needsReschedule, _ := rs.NeedsReschedule(); needsReschedule {
94+
return nil, false
95+
}
96+
domainID = ptr.Deref(domain.Status.ID, "")
97+
}
98+
8299
listOpts := projects.ListOpts{
83-
Name: getResourceName(obj),
84-
Tags: tags.Join(obj.Spec.Resource.Tags),
100+
Name: getResourceName(obj),
101+
DomainID: domainID,
102+
Tags: tags.Join(resource.Tags),
85103
}
86104

87105
return actuator.osClient.ListProjects(ctx, listOpts), true

0 commit comments

Comments
 (0)