Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ (deployimage/v1alpha1): Improve error handling and pointer usage for value setting in controller #4399

Merged
merged 1 commit into from
Dec 2, 2024
Merged

✨ (deployimage/v1alpha1): Improve error handling and pointer usage for value setting in controller #4399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion docs/book/src/getting-started/testdata/project/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
k8s.io/api v0.31.0
k8s.io/apimachinery v0.31.0
k8s.io/client-go v0.31.0
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8
sigs.k8s.io/controller-runtime v0.19.1
)

Expand Down Expand Up @@ -90,7 +91,6 @@ require (
k8s.io/component-base v0.31.0 // indirect
k8s.io/klog/v2 v2.130.1 // indirect
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
Expand Down
9 changes: 5 additions & 4 deletions docs/book/src/getting-started/testdata/project/internal/controller/memcached_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
"time"

"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -229,7 +230,7 @@ func (r *MemcachedReconciler) deploymentForMemcached(
},
Spec: corev1.PodSpec{
SecurityContext: &corev1.PodSecurityContext{
RunAsNonRoot: &[]bool{true}[0],
RunAsNonRoot: ptr.To(true),
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
Expand All @@ -241,9 +242,9 @@ func (r *MemcachedReconciler) deploymentForMemcached(
// Ensure restrictive context for the container
// More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
SecurityContext: &corev1.SecurityContext{
RunAsNonRoot: &[]bool{true}[0],
RunAsUser: &[]int64{1001}[0],
AllowPrivilegeEscalation: &[]bool{false}[0],
RunAsNonRoot: ptr.To(true),
RunAsUser: ptr.To(int64(1001)),
AllowPrivilegeEscalation: ptr.To(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
Expand Down
9 changes: 5 additions & 4 deletions hack/docs/internal/getting-started/generate_getting_started.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ const controllerImports = `"context"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
`

const controllerStatusTypes = `
Expand Down Expand Up @@ -447,7 +448,7 @@ func (r *MemcachedReconciler) deploymentForMemcached(
},
Spec: corev1.PodSpec{
SecurityContext: &corev1.PodSecurityContext{
RunAsNonRoot: &[]bool{true}[0],
RunAsNonRoot: ptr.To(true),
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
Expand All @@ -459,9 +460,9 @@ func (r *MemcachedReconciler) deploymentForMemcached(
// Ensure restrictive context for the container
// More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
SecurityContext: &corev1.SecurityContext{
RunAsNonRoot: &[]bool{true}[0],
RunAsUser: &[]int64{1001}[0],
AllowPrivilegeEscalation: &[]bool{false}[0],
RunAsNonRoot: ptr.To(true),
RunAsUser: ptr.To(int64(1001)),
AllowPrivilegeEscalation: ptr.To(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
Expand Down
16 changes: 8 additions & 8 deletions pkg/plugins/golang/deploy-image/v1alpha1/scaffolds/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ func (s *apiScaffolder) updateControllerCode(controller controllers.Controller)
res = strings.TrimLeft(res, " ")

if err := util.InsertCode(controller.Path, `SecurityContext: &corev1.SecurityContext{
RunAsNonRoot: &[]bool{true}[0],
AllowPrivilegeEscalation: &[]bool{false}[0],
RunAsNonRoot: ptr.To(true),
AllowPrivilegeEscalation: ptr.To(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
Expand All @@ -234,8 +234,8 @@ func (s *apiScaffolder) updateControllerCode(controller controllers.Controller)
if err := util.InsertCode(
controller.Path,
`SecurityContext: &corev1.SecurityContext{
RunAsNonRoot: &[]bool{true}[0],
AllowPrivilegeEscalation: &[]bool{false}[0],
RunAsNonRoot: ptr.To(true),
AllowPrivilegeEscalation: ptr.To(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
Expand All @@ -256,7 +256,7 @@ func (s *apiScaffolder) updateControllerCode(controller controllers.Controller)
if len(s.runAsUser) > 0 {
if err := util.InsertCode(
controller.Path,
`RunAsNonRoot: &[]bool{true}[0],`,
`RunAsNonRoot: ptr.To(true),`,
fmt.Sprintf(runAsUserTemplate, s.runAsUser),
); err != nil {
return fmt.Errorf("error scaffolding user-id in the controller path (%s): %v",
Expand Down Expand Up @@ -297,8 +297,8 @@ const containerTemplate = `Containers: []corev1.Container{{
// Ensure restrictive context for the container
// More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
SecurityContext: &corev1.SecurityContext{
RunAsNonRoot: &[]bool{true}[0],
AllowPrivilegeEscalation: &[]bool{false}[0],
RunAsNonRoot: ptr.To(true),
AllowPrivilegeEscalation: ptr.To(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
Expand All @@ -308,7 +308,7 @@ const containerTemplate = `Containers: []corev1.Container{{
}}`

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

const commandTemplate = `
Command: []string{%s},`
Expand Down
16 changes: 10 additions & 6 deletions ...ugins/golang/deploy-image/v1alpha1/scaffolds/internal/templates/controllers/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/client-go/tools/record"
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
Expand Down Expand Up @@ -177,8 +178,9 @@ func (r *{{ .Resource.Kind }}Reconciler) Reconcile(ctx context.Context, req ctrl
if !controllerutil.ContainsFinalizer({{ lower .Resource.Kind }}, {{ lower .Resource.Kind }}Finalizer) {
log.Info("Adding Finalizer for {{ .Resource.Kind }}")
if ok := controllerutil.AddFinalizer({{ lower .Resource.Kind }}, {{ lower .Resource.Kind }}Finalizer); !ok {
log.Error(err, "Failed to add finalizer into the custom resource")
return ctrl.Result{Requeue: true}, nil
err = fmt.Errorf("finalizer for {{ .Resource.Kind }} was not added")
log.Error(err, "Failed to add finalizer for {{ .Resource.Kind }}")
return ctrl.Result{}, err
}

if err = r.Update(ctx, {{ lower .Resource.Kind }}); err != nil {
Expand Down Expand Up @@ -232,8 +234,9 @@ func (r *{{ .Resource.Kind }}Reconciler) Reconcile(ctx context.Context, req ctrl

log.Info("Removing Finalizer for {{ .Resource.Kind }} after successfully perform the operations")
if ok:= controllerutil.RemoveFinalizer({{ lower .Resource.Kind }}, {{ lower .Resource.Kind }}Finalizer); !ok{
err = fmt.Errorf("finalizer for {{ .Resource.Kind }} was not removed")
log.Error(err, "Failed to remove finalizer for {{ .Resource.Kind }}")
return ctrl.Result{Requeue: true}, nil
return ctrl.Result{}, err
}

if err := r.Update(ctx, {{ lower .Resource.Kind }}); err != nil {
Expand Down Expand Up @@ -280,7 +283,7 @@ func (r *{{ .Resource.Kind }}Reconciler) Reconcile(ctx context.Context, req ctrl
return ctrl.Result{RequeueAfter: time.Minute}, nil
} else if err != nil {
log.Error(err, "Failed to get Deployment")
// Let's return the error for the reconciliation be re-trigged again
// Let's return the error for the reconciliation be re-triggered again
return ctrl.Result{}, err
}

Expand Down Expand Up @@ -412,7 +415,7 @@ func (r *{{ .Resource.Kind }}Reconciler) deploymentFor{{ .Resource.Kind }}(
// },
// },
SecurityContext: &corev1.PodSecurityContext{
RunAsNonRoot: &[]bool{true}[0],
RunAsNonRoot: ptr.To(true),
// IMPORTANT: seccomProfile was introduced with Kubernetes 1.19
// If you are looking for to produce solutions to be supported
// on lower versions you must remove this option.
Expand Down Expand Up @@ -442,7 +445,8 @@ func labelsFor{{ .Resource.Kind }}() map[string]string {
if err == nil {
imageTag = strings.Split(image, ":")[1]
}
return map[string]string{"app.kubernetes.io/name": "{{ .ProjectName }}",
return map[string]string{
"app.kubernetes.io/name": "{{ .ProjectName }}",
"app.kubernetes.io/version": imageTag,
"app.kubernetes.io/managed-by": "{{ .Resource.Kind }}Controller",
}
Expand Down
2 changes: 1 addition & 1 deletion testdata/project-v4-multigroup/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
k8s.io/api v0.31.1
k8s.io/apimachinery v0.31.1
k8s.io/client-go v0.31.1
k8s.io/utils v0.0.0-20240921022957-49e7df575cb6
sigs.k8s.io/controller-runtime v0.19.1
)

Expand Down Expand Up @@ -92,7 +93,6 @@ require (
k8s.io/component-base v0.31.1 // indirect
k8s.io/klog/v2 v2.130.1 // indirect
k8s.io/kube-openapi v0.0.0-20240903163716-9e1beecbcb38 // indirect
k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 // indirect
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect
sigs.k8s.io/gateway-api v1.1.0 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
Expand Down
20 changes: 12 additions & 8 deletions testdata/project-v4-multigroup/internal/controller/example.com/busybox_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
Expand Down Expand Up @@ -123,8 +124,9 @@ func (r *BusyboxReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
if !controllerutil.ContainsFinalizer(busybox, busyboxFinalizer) {
log.Info("Adding Finalizer for Busybox")
if ok := controllerutil.AddFinalizer(busybox, busyboxFinalizer); !ok {
log.Error(err, "Failed to add finalizer into the custom resource")
return ctrl.Result{Requeue: true}, nil
err = fmt.Errorf("finalizer for Busybox was not added")
log.Error(err, "Failed to add finalizer for Busybox")
return ctrl.Result{}, err
}

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

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

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

Expand Down Expand Up @@ -358,7 +361,7 @@ func (r *BusyboxReconciler) deploymentForBusybox(
// },
// },
SecurityContext: &corev1.PodSecurityContext{
RunAsNonRoot: &[]bool{true}[0],
RunAsNonRoot: ptr.To(true),
// IMPORTANT: seccomProfile was introduced with Kubernetes 1.19
// If you are looking for to produce solutions to be supported
// on lower versions you must remove this option.
Expand All @@ -373,8 +376,8 @@ func (r *BusyboxReconciler) deploymentForBusybox(
// Ensure restrictive context for the container
// More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
SecurityContext: &corev1.SecurityContext{
RunAsNonRoot: &[]bool{true}[0],
AllowPrivilegeEscalation: &[]bool{false}[0],
RunAsNonRoot: ptr.To(true),
AllowPrivilegeEscalation: ptr.To(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
Expand Down Expand Up @@ -403,7 +406,8 @@ func labelsForBusybox() map[string]string {
if err == nil {
imageTag = strings.Split(image, ":")[1]
}
return map[string]string{"app.kubernetes.io/name": "project-v4-multigroup",
return map[string]string{
"app.kubernetes.io/name": "project-v4-multigroup",
"app.kubernetes.io/version": imageTag,
"app.kubernetes.io/managed-by": "BusyboxController",
}
Expand Down
22 changes: 13 additions & 9 deletions testdata/project-v4-multigroup/internal/controller/example.com/memcached_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
Expand Down Expand Up @@ -123,8 +124,9 @@ func (r *MemcachedReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
if !controllerutil.ContainsFinalizer(memcached, memcachedFinalizer) {
log.Info("Adding Finalizer for Memcached")
if ok := controllerutil.AddFinalizer(memcached, memcachedFinalizer); !ok {
log.Error(err, "Failed to add finalizer into the custom resource")
return ctrl.Result{Requeue: true}, nil
err = fmt.Errorf("finalizer for Memcached was not added")
log.Error(err, "Failed to add finalizer for Memcached")
return ctrl.Result{}, err
}

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

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

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

Expand Down Expand Up @@ -358,7 +361,7 @@ func (r *MemcachedReconciler) deploymentForMemcached(
// },
// },
SecurityContext: &corev1.PodSecurityContext{
RunAsNonRoot: &[]bool{true}[0],
RunAsNonRoot: ptr.To(true),
// IMPORTANT: seccomProfile was introduced with Kubernetes 1.19
// If you are looking for to produce solutions to be supported
// on lower versions you must remove this option.
Expand All @@ -373,9 +376,9 @@ func (r *MemcachedReconciler) deploymentForMemcached(
// Ensure restrictive context for the container
// More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
SecurityContext: &corev1.SecurityContext{
RunAsNonRoot: &[]bool{true}[0],
RunAsUser: &[]int64{1001}[0],
AllowPrivilegeEscalation: &[]bool{false}[0],
RunAsNonRoot: ptr.To(true),
RunAsUser: ptr.To(int64(1001)),
AllowPrivilegeEscalation: ptr.To(false),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{
"ALL",
Expand Down Expand Up @@ -409,7 +412,8 @@ func labelsForMemcached() map[string]string {
if err == nil {
imageTag = strings.Split(image, ":")[1]
}
return map[string]string{"app.kubernetes.io/name": "project-v4-multigroup",
return map[string]string{
"app.kubernetes.io/name": "project-v4-multigroup",
"app.kubernetes.io/version": imageTag,
"app.kubernetes.io/managed-by": "MemcachedController",
}
Expand Down
2 changes: 1 addition & 1 deletion testdata/project-v4-with-plugins/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
k8s.io/api v0.31.0
k8s.io/apimachinery v0.31.0
k8s.io/client-go v0.31.0
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8
sigs.k8s.io/controller-runtime v0.19.1
)

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