Merge pull request #4548 from wazery/lint-doc-samples

🌱 : Improve samples linting & fix samples lint issues

.github/workflows/lint-sample.yml

@@ -16,7 +16,10 @@ jobs:
         folder: [
           "testdata/project-v4",
           "testdata/project-v4-with-plugins",
-          "testdata/project-v4-multigroup"
+          "testdata/project-v4-multigroup",
+          "docs/book/src/cronjob-tutorial/testdata/project",
+          "docs/book/src/getting-started/testdata/project",
+          "docs/book/src/multiversion-tutorial/testdata/project"
         ]
     if: (github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository)
     steps:

docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go

@@ -96,6 +96,7 @@ var (
 //
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
+// nolint:gocyclo
 func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	log := log.FromContext(ctx)
 

docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller_test.go

@@ -129,7 +129,7 @@ var _ = Describe("CronJob controller", func() {
 			By("By checking the CronJob has zero active Jobs")
 			Consistently(func(g Gomega) {
 				g.Expect(k8sClient.Get(ctx, cronjobLookupKey, createdCronjob)).To(Succeed())
-				g.Expect(createdCronjob.Status.Active).To(HaveLen(0))
+				g.Expect(createdCronjob.Status.Active).To(BeEmpty())
 			}, duration, interval).Should(Succeed())
 			/*
 				Next, we actually create a stubbed Job that will belong to our CronJob, as well as its downstream template specs.

docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go

@@ -20,6 +20,7 @@ package v1
 import (
 	"context"
 	"fmt"
+
 	"github.com/robfig/cron"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"

docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go

@@ -32,10 +32,13 @@ var _ = Describe("CronJob Webhook", func() {
 		defaulter CronJobCustomDefaulter
 	)
 
+	const validCronJobName = "valid-cronjob-name"
+	const schedule = "*/5 * * * *"
+
 	BeforeEach(func() {
 		obj = &batchv1.CronJob{
 			Spec: batchv1.CronJobSpec{
-				Schedule:                   "*/5 * * * *",
+				Schedule:                   schedule,
 				ConcurrencyPolicy:          batchv1.AllowConcurrent,
 				SuccessfulJobsHistoryLimit: new(int32),
 				FailedJobsHistoryLimit:     new(int32),
@@ -46,7 +49,7 @@ var _ = Describe("CronJob Webhook", func() {
 
 		oldObj = &batchv1.CronJob{
 			Spec: batchv1.CronJobSpec{
-				Schedule:                   "*/5 * * * *",
+				Schedule:                   schedule,
 				ConcurrencyPolicy:          batchv1.AllowConcurrent,
 				SuccessfulJobsHistoryLimit: new(int32),
 				FailedJobsHistoryLimit:     new(int32),
@@ -80,7 +83,7 @@ var _ = Describe("CronJob Webhook", func() {
 			obj.Spec.FailedJobsHistoryLimit = nil     // This should default to 1
 
 			By("calling the Default method to apply defaults")
-			defaulter.Default(ctx, obj)
+			_ = defaulter.Default(ctx, obj)
 
 			By("checking that the default values are set")
 			Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent")
@@ -100,7 +103,7 @@ var _ = Describe("CronJob Webhook", func() {
 			*obj.Spec.FailedJobsHistoryLimit = 2
 
 			By("calling the Default method to apply defaults")
-			defaulter.Default(ctx, obj)
+			_ = defaulter.Default(ctx, obj)
 
 			By("checking that the fields were not overwritten")
 			Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value")
@@ -119,7 +122,7 @@ var _ = Describe("CronJob Webhook", func() {
 		})
 
 		It("Should admit creation if the name is valid", func() {
-			obj.ObjectMeta.Name = "valid-cronjob-name"
+			obj.ObjectMeta.Name = validCronJobName
 			Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
 				"Expected name validation to pass for a valid name")
 		})
@@ -132,14 +135,14 @@ var _ = Describe("CronJob Webhook", func() {
 		})
 
 		It("Should admit creation if the schedule is valid", func() {
-			obj.Spec.Schedule = "*/5 * * * *"
+			obj.Spec.Schedule = schedule
 			Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
 				"Expected spec validation to pass for a valid schedule")
 		})
 
 		It("Should deny update if both name and spec are invalid", func() {
-			oldObj.ObjectMeta.Name = "valid-cronjob-name"
-			oldObj.Spec.Schedule = "*/5 * * * *"
+			oldObj.ObjectMeta.Name = validCronJobName
+			oldObj.Spec.Schedule = schedule
 
 			By("simulating an update")
 			obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long"
@@ -151,8 +154,8 @@ var _ = Describe("CronJob Webhook", func() {
 		})
 
 		It("Should admit update if both name and spec are valid", func() {
-			oldObj.ObjectMeta.Name = "valid-cronjob-name"
-			oldObj.Spec.Schedule = "*/5 * * * *"
+			oldObj.ObjectMeta.Name = validCronJobName
+			oldObj.Spec.Schedule = schedule
 
 			By("simulating an update")
 			obj.ObjectMeta.Name = "valid-cronjob-name-updated"

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

@@ -19,14 +19,16 @@ package controller
 import (
 	"context"
 	"fmt"
+
+	"time"
+
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"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"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -89,7 +91,7 @@ func (r *MemcachedReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	}
 
 	// Let's just set the status as Unknown when no status is available
-	if memcached.Status.Conditions == nil || len(memcached.Status.Conditions) == 0 {
+	if len(memcached.Status.Conditions) == 0 {
 		meta.SetStatusCondition(&memcached.Status.Conditions, metav1.Condition{Type: typeAvailableMemcached, Status: metav1.ConditionUnknown, Reason: "Reconciling", Message: "Starting reconciliation"})
 		if err = r.Status().Update(ctx, memcached); err != nil {
 			log.Error(err, "Failed to update Memcached status")

docs/book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller.go

@@ -96,6 +96,7 @@ var (
 //
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
+// nolint:gocyclo
 func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	log := log.FromContext(ctx)
 

docs/book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller_test.go

@@ -129,7 +129,7 @@ var _ = Describe("CronJob controller", func() {
 			By("By checking the CronJob has zero active Jobs")
 			Consistently(func(g Gomega) {
 				g.Expect(k8sClient.Get(ctx, cronjobLookupKey, createdCronjob)).To(Succeed())
-				g.Expect(createdCronjob.Status.Active).To(HaveLen(0))
+				g.Expect(createdCronjob.Status.Active).To(BeEmpty())
 			}, duration, interval).Should(Succeed())
 			/*
 				Next, we actually create a stubbed Job that will belong to our CronJob, as well as its downstream template specs.

docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go

@@ -20,6 +20,7 @@ package v1
 import (
 	"context"
 	"fmt"
+
 	"github.com/robfig/cron"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"

docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go

@@ -31,11 +31,13 @@ var _ = Describe("CronJob Webhook", func() {
 		validator CronJobCustomValidator
 		defaulter CronJobCustomDefaulter
 	)
+	const validCronJobName = "valid-cronjob-name"
+	const schedule = "*/5 * * * *"
 
 	BeforeEach(func() {
 		obj = &batchv1.CronJob{
 			Spec: batchv1.CronJobSpec{
-				Schedule:                   "*/5 * * * *",
+				Schedule:                   schedule,
 				ConcurrencyPolicy:          batchv1.AllowConcurrent,
 				SuccessfulJobsHistoryLimit: new(int32),
 				FailedJobsHistoryLimit:     new(int32),
@@ -46,7 +48,7 @@ var _ = Describe("CronJob Webhook", func() {
 
 		oldObj = &batchv1.CronJob{
 			Spec: batchv1.CronJobSpec{
-				Schedule:                   "*/5 * * * *",
+				Schedule:                   schedule,
 				ConcurrencyPolicy:          batchv1.AllowConcurrent,
 				SuccessfulJobsHistoryLimit: new(int32),
 				FailedJobsHistoryLimit:     new(int32),
@@ -80,7 +82,7 @@ var _ = Describe("CronJob Webhook", func() {
 			obj.Spec.FailedJobsHistoryLimit = nil     // This should default to 1
 
 			By("calling the Default method to apply defaults")
-			defaulter.Default(ctx, obj)
+			_ = defaulter.Default(ctx, obj)
 
 			By("checking that the default values are set")
 			Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent")
@@ -100,7 +102,7 @@ var _ = Describe("CronJob Webhook", func() {
 			*obj.Spec.FailedJobsHistoryLimit = 2
 
 			By("calling the Default method to apply defaults")
-			defaulter.Default(ctx, obj)
+			_ = defaulter.Default(ctx, obj)
 
 			By("checking that the fields were not overwritten")
 			Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value")
@@ -119,7 +121,7 @@ var _ = Describe("CronJob Webhook", func() {
 		})
 
 		It("Should admit creation if the name is valid", func() {
-			obj.ObjectMeta.Name = "valid-cronjob-name"
+			obj.ObjectMeta.Name = validCronJobName
 			Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
 				"Expected name validation to pass for a valid name")
 		})
@@ -132,14 +134,14 @@ var _ = Describe("CronJob Webhook", func() {
 		})
 
 		It("Should admit creation if the schedule is valid", func() {
-			obj.Spec.Schedule = "*/5 * * * *"
+			obj.Spec.Schedule = schedule
 			Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
 				"Expected spec validation to pass for a valid schedule")
 		})
 
 		It("Should deny update if both name and spec are invalid", func() {
-			oldObj.ObjectMeta.Name = "valid-cronjob-name"
-			oldObj.Spec.Schedule = "*/5 * * * *"
+			oldObj.ObjectMeta.Name = validCronJobName
+			oldObj.Spec.Schedule = schedule
 
 			By("simulating an update")
 			obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long"
@@ -151,8 +153,8 @@ var _ = Describe("CronJob Webhook", func() {
 		})
 
 		It("Should admit update if both name and spec are valid", func() {
-			oldObj.ObjectMeta.Name = "valid-cronjob-name"
-			oldObj.Spec.Schedule = "*/5 * * * *"
+			oldObj.ObjectMeta.Name = validCronJobName
+			oldObj.Spec.Schedule = schedule
 
 			By("simulating an update")
 			obj.ObjectMeta.Name = "valid-cronjob-name-updated"

hack/docs/internal/cronjob-tutorial/controller_implementation.go

@@ -84,6 +84,9 @@ var (
 )
 `
 
+const skipGoCycloLint = `
+// nolint:gocyclo`
+
 const controllerReconcileLogic = `log := log.FromContext(ctx)
 
 	/*

hack/docs/internal/cronjob-tutorial/generate_cronjob.go

@@ -286,6 +286,11 @@ func (sp *Sample) updateController() {
 		`// +kubebuilder:rbac:groups=batch.tutorial.kubebuilder.io,resources=cronjobs/finalizers,verbs=update`, controllerReconcile)
 	hackutils.CheckError("fixing cronjob_controller.go", err)
 
+	err = pluginutil.InsertCode(
+		filepath.Join(sp.ctx.Dir, "internal/controller/cronjob_controller.go"),
+		`// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile`, skipGoCycloLint)
+	hackutils.CheckError("fixing cronjob_controller.go", err)
+
 	err = pluginutil.ReplaceInFile(
 		filepath.Join(sp.ctx.Dir, "internal/controller/cronjob_controller.go"),
 		`	_ = log.FromContext(ctx)
@@ -398,6 +403,11 @@ func (sp *Sample) updateWebhookTests() {
 		webhookTestingValidatingExampleFragment)
 	hackutils.CheckError("replace validating defaulting test", err)
 
+	err = pluginutil.ReplaceInFile(file,
+		webhookTestsVars,
+		webhookTestsConstants)
+	hackutils.CheckError("replace before each webhook test ", err)
+
 	err = pluginutil.ReplaceInFile(file,
 		webhookTestsBeforeEachOriginal,
 		webhookTestsBeforeEachChanged)
@@ -420,6 +430,7 @@ func (sp *Sample) updateWebhook() {
 	"context"
 	"fmt"`,
 		`
+	
 	"github.com/robfig/cron"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"

hack/docs/internal/cronjob-tutorial/webhook_implementation.go

@@ -205,7 +205,7 @@ const webhookTestCreateDefaultingReplaceFragment = `It("Should apply defaults wh
 			obj.Spec.FailedJobsHistoryLimit = nil     // This should default to 1
 
 			By("calling the Default method to apply defaults")
-			defaulter.Default(ctx, obj)
+			_ = defaulter.Default(ctx, obj)
 
 			By("checking that the default values are set")
 			Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent")
@@ -225,7 +225,7 @@ const webhookTestCreateDefaultingReplaceFragment = `It("Should apply defaults wh
 			*obj.Spec.FailedJobsHistoryLimit = 2
 
 			By("calling the Default method to apply defaults")
-			defaulter.Default(ctx, obj)
+			_ = defaulter.Default(ctx, obj)
 			
 			By("checking that the fields were not overwritten")
 			Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value")
@@ -263,7 +263,7 @@ const webhookTestingValidatingExampleFragment = `It("Should deny creation if the
 		})
 
 		It("Should admit creation if the name is valid", func() {
-			obj.ObjectMeta.Name = "valid-cronjob-name"
+			obj.ObjectMeta.Name = validCronJobName
 			Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
 				"Expected name validation to pass for a valid name")
 		})
@@ -276,14 +276,14 @@ const webhookTestingValidatingExampleFragment = `It("Should deny creation if the
 		})
 
 		It("Should admit creation if the schedule is valid", func() {
-			obj.Spec.Schedule = "*/5 * * * *"
+			obj.Spec.Schedule = schedule
 			Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
 				"Expected spec validation to pass for a valid schedule")
 		})
 
 		It("Should deny update if both name and spec are invalid", func() {
-			oldObj.ObjectMeta.Name = "valid-cronjob-name"
-			oldObj.Spec.Schedule = "*/5 * * * *"
+			oldObj.ObjectMeta.Name = validCronJobName
+			oldObj.Spec.Schedule = schedule
 
 			By("simulating an update")
 			obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long"
@@ -295,8 +295,8 @@ const webhookTestingValidatingExampleFragment = `It("Should deny creation if the
 		})
 
 		It("Should admit update if both name and spec are valid", func() {
-			oldObj.ObjectMeta.Name = "valid-cronjob-name"
-			oldObj.Spec.Schedule = "*/5 * * * *"
+			oldObj.ObjectMeta.Name = validCronJobName
+			oldObj.Spec.Schedule = schedule
 
 			By("simulating an update")
 			obj.ObjectMeta.Name = "valid-cronjob-name-updated"
@@ -307,6 +307,21 @@ const webhookTestingValidatingExampleFragment = `It("Should deny creation if the
 				"Expected validation to pass for a valid update")
 		})`
 
+const webhookTestsVars = `var (
+		obj       *batchv1.CronJob
+		oldObj    *batchv1.CronJob
+		validator CronJobCustomValidator
+		defaulter CronJobCustomDefaulter
+	)`
+const webhookTestsConstants = `	var (
+		obj       *batchv1.CronJob
+		oldObj    *batchv1.CronJob
+		validator CronJobCustomValidator
+		defaulter CronJobCustomDefaulter
+	)
+
+	const validCronJobName = "valid-cronjob-name"
+	const schedule = "*/5 * * * *"`
 const webhookTestsBeforeEachOriginal = `obj = &batchv1.CronJob{}
 		oldObj = &batchv1.CronJob{}
 		validator = CronJobCustomValidator{}
@@ -319,7 +334,7 @@ const webhookTestsBeforeEachOriginal = `obj = &batchv1.CronJob{}
 
 const webhookTestsBeforeEachChanged = `obj = &batchv1.CronJob{
 			Spec: batchv1.CronJobSpec{
-				Schedule:                   "*/5 * * * *",
+				Schedule:                   schedule,
 				ConcurrencyPolicy:          batchv1.AllowConcurrent,
 				SuccessfulJobsHistoryLimit: new(int32),
 				FailedJobsHistoryLimit:     new(int32),
@@ -330,7 +345,7 @@ const webhookTestsBeforeEachChanged = `obj = &batchv1.CronJob{
 
 		oldObj = &batchv1.CronJob{
 			Spec: batchv1.CronJobSpec{
-				Schedule:                   "*/5 * * * *",
+				Schedule:                   schedule,
 				ConcurrencyPolicy:          batchv1.AllowConcurrent,
 				SuccessfulJobsHistoryLimit: new(int32),
 				FailedJobsHistoryLimit:     new(int32),

hack/docs/internal/cronjob-tutorial/writing_tests_controller.go

@@ -148,7 +148,7 @@ var _ = Describe("CronJob controller", func() {
 			By("By checking the CronJob has zero active Jobs")
 			Consistently(func(g Gomega) {
 				g.Expect(k8sClient.Get(ctx, cronjobLookupKey, createdCronjob)).To(Succeed())
-				g.Expect(createdCronjob.Status.Active).To(HaveLen(0))
+				g.Expect(createdCronjob.Status.Active).To(BeEmpty())
 			}, duration, interval).Should(Succeed())
 			/*
 				Next, we actually create a stubbed Job that will belong to our CronJob, as well as its downstream template specs.