Merge pull request #4950 from camilamacedo86/fix-marker-skip

🐛 (go/v4): gracefully skip webhook test injection if file or markers are missing

Author: k8s-ci-robot

pkg/machinery/scaffold.go

@@ -25,6 +25,7 @@ import (
 	"strings"
 	"text/template"
 
+	log "github.com/sirupsen/logrus"
 	"github.com/spf13/afero"
 	"golang.org/x/tools/imports"
 
@@ -236,7 +237,15 @@ func doTemplate(t Template) ([]byte, error) {
 func (s Scaffold) updateFileModel(i Inserter, models map[string]*File) error {
 	m, err := s.loadPreviousModel(i, models)
 	if err != nil {
-		return fmt.Errorf("failed to load previous model: %w", err)
+		// TODO(kubebuilder/issues/4960): Create Machinery implementation to allow defining IfNotExistsAction
+		// If the file path starts with test/, warn and skip
+		// Workaround to allow projects be backwards compatible with previous versions
+		if strings.HasPrefix(i.GetPath(), "test/") {
+			log.Warnf("Skipping missing test file: %s", i.GetPath())
+			log.Warn("The code fragments will not be inserted.")
+			return nil
+		}
+		return fmt.Errorf("failed to load previous model for %s: %w", i.GetPath(), err)
 	}
 
 	// Get valid code fragments

pkg/plugins/golang/v4/scaffolds/internal/templates/test/e2e/test.go

@@ -17,9 +17,13 @@ limitations under the License.
 package e2e
 
 import (
+	"bytes"
 	"fmt"
+	"os"
 	"path/filepath"
 
+	log "github.com/sirupsen/logrus"
+
 	"sigs.k8s.io/kubebuilder/v4/pkg/machinery"
 )
 
@@ -77,41 +81,58 @@ func (f *WebhookTestUpdater) GetMarkers() []machinery.Marker {
 
 // GetCodeFragments implements file.Inserter
 func (f *WebhookTestUpdater) GetCodeFragments() machinery.CodeFragmentsMap {
-	codeFragments := machinery.CodeFragmentsMap{}
 	if !f.WireWebhook {
 		return nil
 	}
-	codeFragments[machinery.NewMarkerFor(f.GetPath(), webhookChecksMarker)] = append(
-		codeFragments[machinery.NewMarkerFor(f.GetPath(), webhookChecksMarker)],
-		webhookChecksFragment,
-	)
-
-	if f.Resource != nil && f.Resource.HasDefaultingWebhook() {
-		mutatingWebhookCode := fmt.Sprintf(mutatingWebhookChecksFragment, f.ProjectName)
-		codeFragments[machinery.NewMarkerFor(f.GetPath(), webhookChecksMarker)] = append(
-			codeFragments[machinery.NewMarkerFor(f.GetPath(), webhookChecksMarker)],
-			mutatingWebhookCode,
-		)
+
+	filePath := f.GetPath()
+
+	content, err := os.ReadFile(filePath)
+	if err != nil {
+		log.Warnf("Unable to read %q: %v", filePath, err)
+		log.Warnf("Webhook test code injection will be skipped for this file.")
+		log.Warnf("This typically occurs when the file was removed and is missing.")
+		log.Warnf("If you intend to scaffold webhook tests, ensure the file and its markers exist.")
+		return nil
 	}
 
-	if f.Resource.HasValidationWebhook() {
-		validatingWebhookCode := fmt.Sprintf(validatingWebhookChecksFragment, f.ProjectName)
-		codeFragments[machinery.NewMarkerFor(f.GetPath(), webhookChecksMarker)] = append(
-			codeFragments[machinery.NewMarkerFor(f.GetPath(), webhookChecksMarker)],
-			validatingWebhookCode,
-		)
+	codeFragments := machinery.CodeFragmentsMap{}
+	markers := f.GetMarkers()
+
+	for _, marker := range markers {
+		if !bytes.Contains(content, []byte(marker.String())) {
+			log.Warnf("Marker %q not found in %s; skipping injection of webhook test code",
+				marker.String(), filePath)
+			continue // skip this marker
+		}
+
+		var fragments []string
+		fragments = append(fragments, webhookChecksFragment)
+
+		if f.Resource != nil && f.Resource.HasDefaultingWebhook() {
+			mutatingWebhookCode := fmt.Sprintf(mutatingWebhookChecksFragment, f.ProjectName)
+			fragments = append(fragments, mutatingWebhookCode)
+		}
+
+		if f.Resource != nil && f.Resource.HasValidationWebhook() {
+			validatingWebhookCode := fmt.Sprintf(validatingWebhookChecksFragment, f.ProjectName)
+			fragments = append(fragments, validatingWebhookCode)
+		}
+
+		if f.Resource != nil && f.Resource.HasConversionWebhook() {
+			conversionWebhookCode := fmt.Sprintf(
+				conversionWebhookChecksFragment,
+				f.Resource.Kind,
+				f.Resource.Plural+"."+f.Resource.Group+"."+f.Resource.Domain,
+			)
+			fragments = append(fragments, conversionWebhookCode)
+		}
+
+		codeFragments[marker] = fragments
 	}
 
-	if f.Resource.HasConversionWebhook() {
-		conversionWebhookCode := fmt.Sprintf(
-			conversionWebhookChecksFragment,
-			f.Resource.Kind,
-			f.Resource.Plural+"."+f.Resource.Group+"."+f.Resource.Domain,
-		)
-		codeFragments[machinery.NewMarkerFor(f.GetPath(), webhookChecksMarker)] = append(
-			codeFragments[machinery.NewMarkerFor(f.GetPath(), webhookChecksMarker)],
-			conversionWebhookCode,
-		)
+	if len(codeFragments) == 0 {
+		return nil
 	}
 
 	return codeFragments