Skip to content

Commit ba2825d

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#4399 from mateusoliveira43/fix/deploy-image-plugin-refactor
✨ (deployimage/v1alpha1): Improve error handling and pointer usage for value setting in controller
2 parents a8b23ca + 48eaf6b commit ba2825d

File tree

11 files changed

+81
-59
lines changed

11 files changed

+81
-59
lines changed

‎docs/book/src/getting-started/testdata/project/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
k8s.io/api v0.31.0
99
k8s.io/apimachinery v0.31.0
1010
k8s.io/client-go v0.31.0
11+
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8
1112
sigs.k8s.io/controller-runtime v0.19.1
1213
)
1314

@@ -90,7 +91,6 @@ require (
9091
k8s.io/component-base v0.31.0 // indirect
9192
k8s.io/klog/v2 v2.130.1 // indirect
9293
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
93-
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
9494
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect
9595
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
9696
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect

‎docs/book/src/getting-started/testdata/project/internal/controller/memcached_controller.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"k8s.io/apimachinery/pkg/api/meta"
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2727
"k8s.io/apimachinery/pkg/types"
28+
"k8s.io/utils/ptr"
2829
"time"
2930

3031
"k8s.io/apimachinery/pkg/runtime"
@@ -229,7 +230,7 @@ func (r *MemcachedReconciler) deploymentForMemcached(
229230
},
230231
Spec: corev1.PodSpec{
231232
SecurityContext: &corev1.PodSecurityContext{
232-
RunAsNonRoot: &[]bool{true}[0],
233+
RunAsNonRoot: ptr.To(true),
233234
SeccompProfile: &corev1.SeccompProfile{
234235
Type: corev1.SeccompProfileTypeRuntimeDefault,
235236
},
@@ -241,9 +242,9 @@ func (r *MemcachedReconciler) deploymentForMemcached(
241242
// Ensure restrictive context for the container
242243
// More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
243244
SecurityContext: &corev1.SecurityContext{
244-
RunAsNonRoot: &[]bool{true}[0],
245-
RunAsUser: &[]int64{1001}[0],
246-
AllowPrivilegeEscalation: &[]bool{false}[0],
245+
RunAsNonRoot: ptr.To(true),
246+
RunAsUser: ptr.To(int64(1001)),
247+
AllowPrivilegeEscalation: ptr.To(false),
247248
Capabilities: &corev1.Capabilities{
248249
Drop: []corev1.Capability{
249250
"ALL",

‎hack/docs/internal/getting-started/generate_getting_started.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ const controllerImports = `"context"
278278
"k8s.io/apimachinery/pkg/api/meta"
279279
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
280280
"k8s.io/apimachinery/pkg/types"
281+
"k8s.io/utils/ptr"
281282
`
282283

283284
const controllerStatusTypes = `
@@ -447,7 +448,7 @@ func (r *MemcachedReconciler) deploymentForMemcached(
447448
},
448449
Spec: corev1.PodSpec{
449450
SecurityContext: &corev1.PodSecurityContext{
450-
RunAsNonRoot: &[]bool{true}[0],
451+
RunAsNonRoot: ptr.To(true),
451452
SeccompProfile: &corev1.SeccompProfile{
452453
Type: corev1.SeccompProfileTypeRuntimeDefault,
453454
},
@@ -459,9 +460,9 @@ func (r *MemcachedReconciler) deploymentForMemcached(
459460
// Ensure restrictive context for the container
460461
// More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
461462
SecurityContext: &corev1.SecurityContext{
462-
RunAsNonRoot: &[]bool{true}[0],
463-
RunAsUser: &[]int64{1001}[0],
464-
AllowPrivilegeEscalation: &[]bool{false}[0],
463+
RunAsNonRoot: ptr.To(true),
464+
RunAsUser: ptr.To(int64(1001)),
465+
AllowPrivilegeEscalation: ptr.To(false),
465466
Capabilities: &corev1.Capabilities{
466467
Drop: []corev1.Capability{
467468
"ALL",

‎pkg/plugins/golang/deploy-image/v1alpha1/scaffolds/api.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,8 @@ func (s *apiScaffolder) updateControllerCode(controller controllers.Controller)
216216
res = strings.TrimLeft(res, " ")
217217

218218
if err := util.InsertCode(controller.Path, `SecurityContext: &corev1.SecurityContext{
219-
RunAsNonRoot: &[]bool{true}[0],
220-
AllowPrivilegeEscalation: &[]bool{false}[0],
219+
RunAsNonRoot: ptr.To(true),
220+
AllowPrivilegeEscalation: ptr.To(false),
221221
Capabilities: &corev1.Capabilities{
222222
Drop: []corev1.Capability{
223223
"ALL",
@@ -234,8 +234,8 @@ func (s *apiScaffolder) updateControllerCode(controller controllers.Controller)
234234
if err := util.InsertCode(
235235
controller.Path,
236236
`SecurityContext: &corev1.SecurityContext{
237-
RunAsNonRoot: &[]bool{true}[0],
238-
AllowPrivilegeEscalation: &[]bool{false}[0],
237+
RunAsNonRoot: ptr.To(true),
238+
AllowPrivilegeEscalation: ptr.To(false),
239239
Capabilities: &corev1.Capabilities{
240240
Drop: []corev1.Capability{
241241
"ALL",
@@ -256,7 +256,7 @@ func (s *apiScaffolder) updateControllerCode(controller controllers.Controller)
256256
if len(s.runAsUser) > 0 {
257257
if err := util.InsertCode(
258258
controller.Path,
259-
`RunAsNonRoot: &[]bool{true}[0],`,
259+
`RunAsNonRoot: ptr.To(true),`,
260260
fmt.Sprintf(runAsUserTemplate, s.runAsUser),
261261
); err != nil {
262262
return fmt.Errorf("error scaffolding user-id in the controller path (%s): %v",
@@ -297,8 +297,8 @@ const containerTemplate = `Containers: []corev1.Container{{
297297
// Ensure restrictive context for the container
298298
// More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
299299
SecurityContext: &corev1.SecurityContext{
300-
RunAsNonRoot: &[]bool{true}[0],
301-
AllowPrivilegeEscalation: &[]bool{false}[0],
300+
RunAsNonRoot: ptr.To(true),
301+
AllowPrivilegeEscalation: ptr.To(false),
302302
Capabilities: &corev1.Capabilities{
303303
Drop: []corev1.Capability{
304304
"ALL",
@@ -308,7 +308,7 @@ const containerTemplate = `Containers: []corev1.Container{{
308308
}}`
309309

310310
const runAsUserTemplate = `
311-
RunAsUser: &[]int64{%s}[0],`
311+
RunAsUser: ptr.To(int64(%s)),`
312312

313313
const commandTemplate = `
314314
Command: []string{%s},`

‎pkg/plugins/golang/deploy-image/v1alpha1/scaffolds/internal/templates/controllers/controller.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ import (
8383
"k8s.io/apimachinery/pkg/runtime"
8484
"k8s.io/apimachinery/pkg/api/meta"
8585
"k8s.io/client-go/tools/record"
86+
"k8s.io/utils/ptr"
8687
ctrl "sigs.k8s.io/controller-runtime"
8788
"sigs.k8s.io/controller-runtime/pkg/client"
8889
"sigs.k8s.io/controller-runtime/pkg/log"
@@ -177,8 +178,9 @@ func (r *{{ .Resource.Kind }}Reconciler) Reconcile(ctx context.Context, req ctrl
177178
if !controllerutil.ContainsFinalizer({{ lower .Resource.Kind }}, {{ lower .Resource.Kind }}Finalizer) {
178179
log.Info("Adding Finalizer for {{ .Resource.Kind }}")
179180
if ok := controllerutil.AddFinalizer({{ lower .Resource.Kind }}, {{ lower .Resource.Kind }}Finalizer); !ok {
180-
log.Error(err, "Failed to add finalizer into the custom resource")
181-
return ctrl.Result{Requeue: true}, nil
181+
err = fmt.Errorf("finalizer for {{ .Resource.Kind }} was not added")
182+
log.Error(err, "Failed to add finalizer for {{ .Resource.Kind }}")
183+
return ctrl.Result{}, err
182184
}
183185
184186
if err = r.Update(ctx, {{ lower .Resource.Kind }}); err != nil {
@@ -232,8 +234,9 @@ func (r *{{ .Resource.Kind }}Reconciler) Reconcile(ctx context.Context, req ctrl
232234
233235
log.Info("Removing Finalizer for {{ .Resource.Kind }} after successfully perform the operations")
234236
if ok:= controllerutil.RemoveFinalizer({{ lower .Resource.Kind }}, {{ lower .Resource.Kind }}Finalizer); !ok{
237+
err = fmt.Errorf("finalizer for {{ .Resource.Kind }} was not removed")
235238
log.Error(err, "Failed to remove finalizer for {{ .Resource.Kind }}")
236-
return ctrl.Result{Requeue: true}, nil
239+
return ctrl.Result{}, err
237240
}
238241
239242
if err := r.Update(ctx, {{ lower .Resource.Kind }}); err != nil {
@@ -280,7 +283,7 @@ func (r *{{ .Resource.Kind }}Reconciler) Reconcile(ctx context.Context, req ctrl
280283
return ctrl.Result{RequeueAfter: time.Minute}, nil
281284
} else if err != nil {
282285
log.Error(err, "Failed to get Deployment")
283-
// Let's return the error for the reconciliation be re-trigged again
286+
// Let's return the error for the reconciliation be re-triggered again
284287
return ctrl.Result{}, err
285288
}
286289
@@ -412,7 +415,7 @@ func (r *{{ .Resource.Kind }}Reconciler) deploymentFor{{ .Resource.Kind }}(
412415
// },
413416
// },
414417
SecurityContext: &corev1.PodSecurityContext{
415-
RunAsNonRoot: &[]bool{true}[0],
418+
RunAsNonRoot: ptr.To(true),
416419
// IMPORTANT: seccomProfile was introduced with Kubernetes 1.19
417420
// If you are looking for to produce solutions to be supported
418421
// on lower versions you must remove this option.
@@ -442,7 +445,8 @@ func labelsFor{{ .Resource.Kind }}() map[string]string {
442445
if err == nil {
443446
imageTag = strings.Split(image, ":")[1]
444447
}
445-
return map[string]string{"app.kubernetes.io/name": "{{ .ProjectName }}",
448+
return map[string]string{
449+
"app.kubernetes.io/name": "{{ .ProjectName }}",
446450
"app.kubernetes.io/version": imageTag,
447451
"app.kubernetes.io/managed-by": "{{ .Resource.Kind }}Controller",
448452
}

‎testdata/project-v4-multigroup/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ require (
99
k8s.io/api v0.31.1
1010
k8s.io/apimachinery v0.31.1
1111
k8s.io/client-go v0.31.1
12+
k8s.io/utils v0.0.0-20240921022957-49e7df575cb6
1213
sigs.k8s.io/controller-runtime v0.19.1
1314
)
1415

@@ -92,7 +93,6 @@ require (
9293
k8s.io/component-base v0.31.1 // indirect
9394
k8s.io/klog/v2 v2.130.1 // indirect
9495
k8s.io/kube-openapi v0.0.0-20240903163716-9e1beecbcb38 // indirect
95-
k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 // indirect
9696
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect
9797
sigs.k8s.io/gateway-api v1.1.0 // indirect
9898
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect

‎testdata/project-v4-multigroup/internal/controller/example.com/busybox_controller.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"k8s.io/apimachinery/pkg/runtime"
3232
"k8s.io/apimachinery/pkg/types"
3333
"k8s.io/client-go/tools/record"
34+
"k8s.io/utils/ptr"
3435
ctrl "sigs.k8s.io/controller-runtime"
3536
"sigs.k8s.io/controller-runtime/pkg/client"
3637
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -123,8 +124,9 @@ func (r *BusyboxReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
123124
if !controllerutil.ContainsFinalizer(busybox, busyboxFinalizer) {
124125
log.Info("Adding Finalizer for Busybox")
125126
if ok := controllerutil.AddFinalizer(busybox, busyboxFinalizer); !ok {
126-
log.Error(err, "Failed to add finalizer into the custom resource")
127-
return ctrl.Result{Requeue: true}, nil
127+
err = fmt.Errorf("finalizer for Busybox was not added")
128+
log.Error(err, "Failed to add finalizer for Busybox")
129+
return ctrl.Result{}, err
128130
}
129131

130132
if err = r.Update(ctx, busybox); err != nil {
@@ -178,8 +180,9 @@ func (r *BusyboxReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
178180

179181
log.Info("Removing Finalizer for Busybox after successfully perform the operations")
180182
if ok := controllerutil.RemoveFinalizer(busybox, busyboxFinalizer); !ok {
183+
err = fmt.Errorf("finalizer for Busybox was not removed")
181184
log.Error(err, "Failed to remove finalizer for Busybox")
182-
return ctrl.Result{Requeue: true}, nil
185+
return ctrl.Result{}, err
183186
}
184187

185188
if err := r.Update(ctx, busybox); err != nil {
@@ -226,7 +229,7 @@ func (r *BusyboxReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
226229
return ctrl.Result{RequeueAfter: time.Minute}, nil
227230
} else if err != nil {
228231
log.Error(err, "Failed to get Deployment")
229-
// Let's return the error for the reconciliation be re-trigged again
232+
// Let's return the error for the reconciliation be re-triggered again
230233
return ctrl.Result{}, err
231234
}
232235

@@ -358,7 +361,7 @@ func (r *BusyboxReconciler) deploymentForBusybox(
358361
// },
359362
// },
360363
SecurityContext: &corev1.PodSecurityContext{
361-
RunAsNonRoot: &[]bool{true}[0],
364+
RunAsNonRoot: ptr.To(true),
362365
// IMPORTANT: seccomProfile was introduced with Kubernetes 1.19
363366
// If you are looking for to produce solutions to be supported
364367
// on lower versions you must remove this option.
@@ -373,8 +376,8 @@ func (r *BusyboxReconciler) deploymentForBusybox(
373376
// Ensure restrictive context for the container
374377
// More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
375378
SecurityContext: &corev1.SecurityContext{
376-
RunAsNonRoot: &[]bool{true}[0],
377-
AllowPrivilegeEscalation: &[]bool{false}[0],
379+
RunAsNonRoot: ptr.To(true),
380+
AllowPrivilegeEscalation: ptr.To(false),
378381
Capabilities: &corev1.Capabilities{
379382
Drop: []corev1.Capability{
380383
"ALL",
@@ -403,7 +406,8 @@ func labelsForBusybox() map[string]string {
403406
if err == nil {
404407
imageTag = strings.Split(image, ":")[1]
405408
}
406-
return map[string]string{"app.kubernetes.io/name": "project-v4-multigroup",
409+
return map[string]string{
410+
"app.kubernetes.io/name": "project-v4-multigroup",
407411
"app.kubernetes.io/version": imageTag,
408412
"app.kubernetes.io/managed-by": "BusyboxController",
409413
}

‎testdata/project-v4-multigroup/internal/controller/example.com/memcached_controller.go‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"k8s.io/apimachinery/pkg/runtime"
3232
"k8s.io/apimachinery/pkg/types"
3333
"k8s.io/client-go/tools/record"
34+
"k8s.io/utils/ptr"
3435
ctrl "sigs.k8s.io/controller-runtime"
3536
"sigs.k8s.io/controller-runtime/pkg/client"
3637
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -123,8 +124,9 @@ func (r *MemcachedReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
123124
if !controllerutil.ContainsFinalizer(memcached, memcachedFinalizer) {
124125
log.Info("Adding Finalizer for Memcached")
125126
if ok := controllerutil.AddFinalizer(memcached, memcachedFinalizer); !ok {
126-
log.Error(err, "Failed to add finalizer into the custom resource")
127-
return ctrl.Result{Requeue: true}, nil
127+
err = fmt.Errorf("finalizer for Memcached was not added")
128+
log.Error(err, "Failed to add finalizer for Memcached")
129+
return ctrl.Result{}, err
128130
}
129131

130132
if err = r.Update(ctx, memcached); err != nil {
@@ -178,8 +180,9 @@ func (r *MemcachedReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
178180

179181
log.Info("Removing Finalizer for Memcached after successfully perform the operations")
180182
if ok := controllerutil.RemoveFinalizer(memcached, memcachedFinalizer); !ok {
183+
err = fmt.Errorf("finalizer for Memcached was not removed")
181184
log.Error(err, "Failed to remove finalizer for Memcached")
182-
return ctrl.Result{Requeue: true}, nil
185+
return ctrl.Result{}, err
183186
}
184187

185188
if err := r.Update(ctx, memcached); err != nil {
@@ -226,7 +229,7 @@ func (r *MemcachedReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
226229
return ctrl.Result{RequeueAfter: time.Minute}, nil
227230
} else if err != nil {
228231
log.Error(err, "Failed to get Deployment")
229-
// Let's return the error for the reconciliation be re-trigged again
232+
// Let's return the error for the reconciliation be re-triggered again
230233
return ctrl.Result{}, err
231234
}
232235

@@ -358,7 +361,7 @@ func (r *MemcachedReconciler) deploymentForMemcached(
358361
// },
359362
// },
360363
SecurityContext: &corev1.PodSecurityContext{
361-
RunAsNonRoot: &[]bool{true}[0],
364+
RunAsNonRoot: ptr.To(true),
362365
// IMPORTANT: seccomProfile was introduced with Kubernetes 1.19
363366
// If you are looking for to produce solutions to be supported
364367
// on lower versions you must remove this option.
@@ -373,9 +376,9 @@ func (r *MemcachedReconciler) deploymentForMemcached(
373376
// Ensure restrictive context for the container
374377
// More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
375378
SecurityContext: &corev1.SecurityContext{
376-
RunAsNonRoot: &[]bool{true}[0],
377-
RunAsUser: &[]int64{1001}[0],
378-
AllowPrivilegeEscalation: &[]bool{false}[0],
379+
RunAsNonRoot: ptr.To(true),
380+
RunAsUser: ptr.To(int64(1001)),
381+
AllowPrivilegeEscalation: ptr.To(false),
379382
Capabilities: &corev1.Capabilities{
380383
Drop: []corev1.Capability{
381384
"ALL",
@@ -409,7 +412,8 @@ func labelsForMemcached() map[string]string {
409412
if err == nil {
410413
imageTag = strings.Split(image, ":")[1]
411414
}
412-
return map[string]string{"app.kubernetes.io/name": "project-v4-multigroup",
415+
return map[string]string{
416+
"app.kubernetes.io/name": "project-v4-multigroup",
413417
"app.kubernetes.io/version": imageTag,
414418
"app.kubernetes.io/managed-by": "MemcachedController",
415419
}

‎testdata/project-v4-with-plugins/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
k8s.io/api v0.31.0
99
k8s.io/apimachinery v0.31.0
1010
k8s.io/client-go v0.31.0
11+
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8
1112
sigs.k8s.io/controller-runtime v0.19.1
1213
)
1314

@@ -90,7 +91,6 @@ require (
9091
k8s.io/component-base v0.31.0 // indirect
9192
k8s.io/klog/v2 v2.130.1 // indirect
9293
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
93-
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
9494
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect
9595
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
9696
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect

0 commit comments

Comments
 (0)