Skip to content

Commit 40bd06d

Browse files
davidcassanydavidcassany
committed
Small refactor to centralize registration config checks
Signed-off-by: David Cassany <dcassany@suse.com> (cherry picked from commit 3eec04e)
1 parent 6c9564e commit 40bd06d

File tree

5 files changed

+121
-30
lines changed

5 files changed

+121
-30
lines changed

‎api/v1beta1/machineregistration_types.go‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20+
"fmt"
21+
2022
corev1 "k8s.io/api/core/v1"
23+
"k8s.io/apimachinery/pkg/api/meta"
2124
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2225
)
2326

@@ -76,7 +79,7 @@ type MachineRegistrationList struct {
7679

7780
// GetClientRegistrationConfig returns the configuration required by the elemental-register
7881
// to register itself against this MachineRegistration instance.
79-
func (m MachineRegistration) GetClientRegistrationConfig(cacert string) *Config {
82+
func (m MachineRegistration) GetClientRegistrationConfig(cacert string) (*Config, error) {
8083
conf := m.Spec.Config
8184

8285
if conf == nil {
@@ -85,6 +88,14 @@ func (m MachineRegistration) GetClientRegistrationConfig(cacert string) *Config
8588

8689
mRegistration := conf.Elemental.Registration
8790

91+
if !meta.IsStatusConditionTrue(m.Status.Conditions, ReadyCondition) {
92+
return nil, fmt.Errorf("machine registration is not ready")
93+
}
94+
95+
if m.Status.RegistrationURL == "" {
96+
return nil, fmt.Errorf("registration URL is not set")
97+
}
98+
8899
return &Config{
89100
Elemental: Elemental{
90101
Registration: Registration{
@@ -96,7 +107,7 @@ func (m MachineRegistration) GetClientRegistrationConfig(cacert string) *Config
96107
Auth: mRegistration.Auth,
97108
},
98109
},
99-
}
110+
}, nil
100111
}
101112

102113
func init() {

‎controllers/seedimage_controller.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,11 @@ func (r *SeedImageReconciler) reconcileConfigMapObject(ctx context.Context, seed
287287

288288
podConfigMap := &corev1.ConfigMap{}
289289

290-
regClientConf := mRegistration.GetClientRegistrationConfig(util.GetRancherCACert(ctx, r))
290+
regClientConf, err := mRegistration.GetClientRegistrationConfig(util.GetRancherCACert(ctx, r))
291+
if err != nil {
292+
return fmt.Errorf("failed processing registration config: %w", err)
293+
}
294+
291295
regData, err := yaml.Marshal(regClientConf)
292296
if err != nil {
293297
return fmt.Errorf("failed marshalling registration config: %w", err)

‎controllers/seedimage_controller_test.go‎

Lines changed: 78 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,18 @@ var _ = Describe("reconcile seed image", func() {
5656
Name: "test-name",
5757
Namespace: "default",
5858
},
59+
Status: elementalv1.MachineRegistrationStatus{
60+
RegistrationURL: "https://example.com/token",
61+
RegistrationToken: "token",
62+
Conditions: []metav1.Condition{
63+
{
64+
Type: elementalv1.ReadyCondition,
65+
Reason: elementalv1.SuccessfullyCreatedReason,
66+
Status: metav1.ConditionTrue,
67+
LastTransitionTime: metav1.Now(),
68+
},
69+
},
70+
},
5971
}
6072

6173
seedImg = &elementalv1.SeedImage{
@@ -93,9 +105,15 @@ var _ = Describe("reconcile seed image", func() {
93105
},
94106
}
95107

108+
statusCopy := mRegistration.Status.DeepCopy()
109+
96110
Expect(cl.Create(ctx, mRegistration)).To(Succeed())
97111
Expect(cl.Create(ctx, seedImg)).To(Succeed())
98112
Expect(cl.Create(ctx, setting)).To(Succeed())
113+
114+
patchBase := client.MergeFrom(mRegistration.DeepCopy())
115+
mRegistration.Status = *statusCopy
116+
Expect(cl.Status().Patch(ctx, mRegistration, patchBase)).To(Succeed())
99117
})
100118

101119
AfterEach(func() {
@@ -160,6 +178,7 @@ var _ = Describe("reconcile seed image", func() {
160178

161179
var _ = Describe("reconcileBuildImagePod", func() {
162180
var r *SeedImageReconciler
181+
var setting *managementv3.Setting
163182
var mRegistration *elementalv1.MachineRegistration
164183
var seedImg *elementalv1.SeedImage
165184
var pod *corev1.Pod
@@ -177,6 +196,18 @@ var _ = Describe("reconcileBuildImagePod", func() {
177196
Name: "test-name",
178197
Namespace: "default",
179198
},
199+
Status: elementalv1.MachineRegistrationStatus{
200+
RegistrationURL: "https://example.com/token",
201+
RegistrationToken: "token",
202+
Conditions: []metav1.Condition{
203+
{
204+
Type: elementalv1.ReadyCondition,
205+
Reason: elementalv1.SuccessfullyCreatedReason,
206+
Status: metav1.ConditionTrue,
207+
LastTransitionTime: metav1.Now(),
208+
},
209+
},
210+
},
180211
}
181212

182213
seedImg = &elementalv1.SeedImage{
@@ -207,12 +238,26 @@ var _ = Describe("reconcileBuildImagePod", func() {
207238
},
208239
}
209240

241+
setting = &managementv3.Setting{
242+
ObjectMeta: metav1.ObjectMeta{
243+
Name: "server-url",
244+
},
245+
Value: "https://example.com",
246+
}
247+
248+
statusCopy := mRegistration.Status.DeepCopy()
249+
210250
Expect(cl.Create(ctx, mRegistration)).To(Succeed())
211251
Expect(cl.Create(ctx, seedImg)).To(Succeed())
252+
Expect(cl.Create(ctx, setting)).To(Succeed())
253+
254+
patchBase := client.MergeFrom(mRegistration.DeepCopy())
255+
mRegistration.Status = *statusCopy
256+
Expect(cl.Status().Patch(ctx, mRegistration, patchBase)).To(Succeed())
212257
})
213258

214259
AfterEach(func() {
215-
Expect(test.CleanupAndWait(ctx, cl, mRegistration, seedImg, pod, svc)).To(Succeed())
260+
Expect(test.CleanupAndWait(ctx, cl, mRegistration, setting, seedImg, pod, svc)).To(Succeed())
216261
})
217262

218263
It("should return error when a pod with the same name but different owner is there", func() {
@@ -225,7 +270,7 @@ var _ = Describe("reconcileBuildImagePod", func() {
225270

226271
Expect(cl.Create(ctx, pod)).To(Succeed())
227272

228-
err := r.reconcileBuildImagePod(ctx, seedImg, &elementalv1.MachineRegistration{})
273+
err := r.reconcileBuildImagePod(ctx, seedImg, mRegistration)
229274

230275
// Pod already there and not owned by the SeedImage obj
231276
Expect(err).To(HaveOccurred())
@@ -276,7 +321,8 @@ var _ = Describe("reconcileBuildImagePod", func() {
276321
Expect(foundPod.Annotations["elemental.cattle.io/base-image"]).To(Equal(seedImg.Spec.BaseImage))
277322

278323
seedImg.Spec.BaseImage = "https://example.com/new-base.iso"
279-
err = r.reconcileBuildImagePod(ctx, seedImg, &elementalv1.MachineRegistration{})
324+
325+
err = r.reconcileBuildImagePod(ctx, seedImg, mRegistration)
280326
Expect(err).ToNot(HaveOccurred())
281327
Expect(cl.Get(ctx, client.ObjectKey{
282328
Name: seedImg.Name,
@@ -309,7 +355,7 @@ var _ = Describe("reconcileBuildImagePod", func() {
309355
"write_files": {},
310356
}
311357

312-
err = r.reconcileBuildImagePod(ctx, seedImg, &elementalv1.MachineRegistration{})
358+
err = r.reconcileBuildImagePod(ctx, seedImg, mRegistration)
313359
Expect(err).ToNot(HaveOccurred())
314360
Expect(cl.Get(ctx, client.ObjectKey{
315361
Name: seedImg.Name,
@@ -320,6 +366,7 @@ var _ = Describe("reconcileBuildImagePod", func() {
320366

321367
var _ = Describe("createConfigMapObject", func() {
322368
var r *SeedImageReconciler
369+
var setting *managementv3.Setting
323370
var mRegistration *elementalv1.MachineRegistration
324371
var seedImg *elementalv1.SeedImage
325372
var configMap *corev1.ConfigMap
@@ -338,6 +385,18 @@ var _ = Describe("createConfigMapObject", func() {
338385
Name: "test-name",
339386
Namespace: "default",
340387
},
388+
Status: elementalv1.MachineRegistrationStatus{
389+
RegistrationURL: "https://example.com/token",
390+
RegistrationToken: "token",
391+
Conditions: []metav1.Condition{
392+
{
393+
Type: elementalv1.ReadyCondition,
394+
Reason: elementalv1.SuccessfullyCreatedReason,
395+
Status: metav1.ConditionTrue,
396+
LastTransitionTime: metav1.Now(),
397+
},
398+
},
399+
},
341400
}
342401

343402
seedImg = &elementalv1.SeedImage{
@@ -373,12 +432,26 @@ var _ = Describe("createConfigMapObject", func() {
373432
},
374433
}
375434

435+
setting = &managementv3.Setting{
436+
ObjectMeta: metav1.ObjectMeta{
437+
Name: "server-url",
438+
},
439+
Value: "https://example.com",
440+
}
441+
442+
statusCopy := mRegistration.Status.DeepCopy()
443+
376444
Expect(cl.Create(ctx, mRegistration)).To(Succeed())
377445
Expect(cl.Create(ctx, seedImg)).To(Succeed())
446+
Expect(cl.Create(ctx, setting)).To(Succeed())
447+
448+
patchBase := client.MergeFrom(mRegistration.DeepCopy())
449+
mRegistration.Status = *statusCopy
450+
Expect(cl.Status().Patch(ctx, mRegistration, patchBase)).To(Succeed())
378451
})
379452

380453
AfterEach(func() {
381-
Expect(test.CleanupAndWait(ctx, cl, mRegistration, seedImg, configMap, pod, svc)).To(Succeed())
454+
Expect(test.CleanupAndWait(ctx, cl, mRegistration, seedImg, setting, configMap, pod, svc)).To(Succeed())
382455
})
383456

384457
It("should create a configmap with empty cloud-config data", func() {

‎pkg/server/api_registration.go‎

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,12 @@ func (i *InventoryServer) apiRegistration(resp http.ResponseWriter, req *http.Re
123123
}
124124

125125
func (i *InventoryServer) unauthenticatedResponse(registration *elementalv1.MachineRegistration, writer io.Writer) error {
126+
config, err := registration.GetClientRegistrationConfig(i.getRancherCACert())
127+
if err != nil {
128+
return err
129+
}
126130
return yaml.NewEncoder(writer).
127-
Encode(registration.GetClientRegistrationConfig(i.getRancherCACert()))
131+
Encode(config)
128132
}
129133

130134
func (i *InventoryServer) writeMachineInventoryCloudConfig(conn *websocket.Conn, protoVersion register.MessageType, inventory *elementalv1.MachineInventory, registration *elementalv1.MachineRegistration) error {
@@ -159,37 +163,28 @@ func (i *InventoryServer) writeMachineInventoryCloudConfig(conn *websocket.Conn,
159163
registration.Spec.Config = &elementalv1.Config{}
160164
}
161165

162-
if registration.Status.RegistrationURL == "" {
163-
return fmt.Errorf("registration URL is not set")
166+
config, err := registration.GetClientRegistrationConfig(i.getRancherCACert())
167+
if err != nil {
168+
return err
164169
}
165-
166-
elementalConf := elementalv1.Elemental{
167-
Registration: elementalv1.Registration{
168-
URL: registration.Status.RegistrationURL,
169-
CACert: i.getRancherCACert(),
170-
},
171-
SystemAgent: elementalv1.SystemAgent{
172-
URL: fmt.Sprintf("%s/k8s/clusters/local", serverURL),
173-
Token: string(secret.Data["token"]),
174-
SecretName: inventory.Name,
175-
SecretNamespace: inventory.Namespace,
176-
},
177-
Install: registration.Spec.Config.Elemental.Install,
178-
Reset: registration.Spec.Config.Elemental.Reset,
170+
config.Elemental.SystemAgent = elementalv1.SystemAgent{
171+
URL: fmt.Sprintf("%s/k8s/clusters/local", serverURL),
172+
Token: string(secret.Data["token"]),
173+
SecretName: inventory.Name,
174+
SecretNamespace: inventory.Namespace,
179175
}
176+
config.Elemental.Install = registration.Spec.Config.Elemental.Install
177+
config.Elemental.Reset = registration.Spec.Config.Elemental.Reset
180178

181179
cloudConf := registration.Spec.Config.CloudConfig
182180

183181
// legacy register client (old ISO): we should send serialization of legacy (pre-kubebuilder) config.
184182
if protoVersion == register.MsgUndefined {
185183
log.Debugf("Detected old register client: sending legacy CloudConfig serialization.")
186-
return sendLegacyConfig(conn, elementalConf, cloudConf)
184+
return sendLegacyConfig(conn, config.Elemental, cloudConf)
187185
}
188186

189-
config := elementalv1.Config{
190-
Elemental: elementalConf,
191-
CloudConfig: cloudConf,
192-
}
187+
config.CloudConfig = cloudConf
193188

194189
// If client does not support MsgConfig we send back the config as a
195190
// byte-stream without a message-type to keep backwards-compatibility.

‎pkg/server/api_registration_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,14 @@ func TestUnauthenticatedResponse(t *testing.T) {
8484
registration := elementalv1.MachineRegistration{}
8585
registration.Spec.Config = test.config
8686
registration.Status.RegistrationURL = test.regUrl
87+
registration.Status.Conditions = []metav1.Condition{
88+
{
89+
Type: elementalv1.ReadyCondition,
90+
Reason: elementalv1.SuccessfullyCreatedReason,
91+
Status: metav1.ConditionTrue,
92+
LastTransitionTime: metav1.Now(),
93+
},
94+
}
8795

8896
buffer := new(bytes.Buffer)
8997

0 commit comments

Comments
 (0)