✨ Markers can now indicate their priority when applying

pkg/crd/markers/doc.go

@@ -25,8 +25,8 @@ limitations under the License.
 // (crd.SchemaMarker).  Any marker implementing this will automatically
 // be run after the rest of a given schema node has been generated.
 // Markers that need to be run before any other markers can also
-// implement ApplyFirst, but this is discouraged and may change
-// in the future.
+// implement ApplyFirst or markers.ApplyPriority, but this is discouraged
+// and may change in the future.
 //
 // All validation markers start with "+kubebuilder:validation", and
 // have the same name as their type name.

pkg/crd/markers/priority.go

@@ -0,0 +1,37 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package markers
+
+import "math"
+
+type ApplyPriority int64
+
+const (
+	// ApplyPriorityDefault is the default priority for markers
+	// that don't implement ApplyPriorityMarker
+	ApplyPriorityDefault ApplyPriority = math.MaxInt64 >> 2
+
+	// ApplyPriorityFirst is the priority value assigned to markers
+	// that implement the ApplyFirst() method
+	ApplyPriorityFirst ApplyPriority = ApplyPriorityDefault - 10
+)
+
+// ApplyPriorityMarker designates the order markers should be applied.
+// Lower priority indicates it should be applied first
+type ApplyPriorityMarker interface {
+	ApplyPriority() ApplyPriority
+}

pkg/crd/markers/topology.go

@@ -111,7 +111,9 @@ func (l ListType) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	return nil
 }
 
-func (l ListType) ApplyFirst() {}
+func (l ListType) ApplyPriority() ApplyPriority {
+	return ApplyPriorityDefault - 1
+}
 
 func (l ListMapKey) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	if schema.Type != "array" {

pkg/crd/markers/validation.go

@@ -448,7 +448,9 @@ func (m Type) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	return nil
 }
 
-func (m Type) ApplyFirst() {}
+func (m Type) ApplyPriority() ApplyPriority {
+	return ApplyPriorityDefault - 1
+}
 
 func (m Nullable) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	schema.Nullable = true
@@ -484,7 +486,9 @@ func (m XIntOrString) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	return nil
 }
 
-func (m XIntOrString) ApplyFirst() {}
+func (m XIntOrString) ApplyPriority() ApplyPriority {
+	return ApplyPriorityDefault - 1
+}
 
 func (m XValidation) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	schema.XValidations = append(schema.XValidations, apiext.ValidationRule{

pkg/crd/schema.go

@@ -22,6 +22,7 @@ import (
 	"go/ast"
 	"go/token"
 	"go/types"
+	"sort"
 	"strings"
 
 	apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -126,39 +127,44 @@ func infoToSchema(ctx *schemaContext) *apiext.JSONSchemaProps {
 
 // applyMarkers applies schema markers to the given schema, respecting "apply first" markers.
 func applyMarkers(ctx *schemaContext, markerSet markers.MarkerValues, props *apiext.JSONSchemaProps, node ast.Node) {
+	markers := make([]SchemaMarker, 0, len(markerSet))
+
 	// apply "apply first" markers first...
 	for _, markerValues := range markerSet {
 		for _, markerValue := range markerValues {
-			if _, isApplyFirst := markerValue.(applyFirstMarker); !isApplyFirst {
-				continue
+			if schemaMarker, isSchemaMarker := markerValue.(SchemaMarker); isSchemaMarker {
+				markers = append(markers, schemaMarker)
 			}
+		}
+	}
 
-			schemaMarker, isSchemaMarker := markerValue.(SchemaMarker)
-			if !isSchemaMarker {
-				continue
-			}
+	sort.Slice(markers, func(i, j int) bool {
+		var iPriority, jPriority crdmarkers.ApplyPriority
 
-			if err := schemaMarker.ApplyToSchema(props); err != nil {
-				ctx.pkg.AddError(loader.ErrFromNode(err /* an okay guess */, node))
-			}
+		switch m := markers[i].(type) {
+		case crdmarkers.ApplyPriorityMarker:
+			iPriority = m.ApplyPriority()
+		case applyFirstMarker:
+			iPriority = crdmarkers.ApplyPriorityFirst
+		default:
+			iPriority = crdmarkers.ApplyPriorityDefault
 		}
-	}
 
-	// ...then the rest of the markers
-	for _, markerValues := range markerSet {
-		for _, markerValue := range markerValues {
-			if _, isApplyFirst := markerValue.(applyFirstMarker); isApplyFirst {
-				// skip apply-first markers, which were already applied
-				continue
-			}
+		switch m := markers[j].(type) {
+		case crdmarkers.ApplyPriorityMarker:
+			jPriority = m.ApplyPriority()
+		case applyFirstMarker:
+			jPriority = crdmarkers.ApplyPriorityFirst
+		default:
+			jPriority = crdmarkers.ApplyPriorityDefault
+		}
 
-			schemaMarker, isSchemaMarker := markerValue.(SchemaMarker)
-			if !isSchemaMarker {
-				continue
-			}
-			if err := schemaMarker.ApplyToSchema(props); err != nil {
-				ctx.pkg.AddError(loader.ErrFromNode(err /* an okay guess */, node))
-			}
+		return iPriority < jPriority
+	})
+
+	for _, schemaMarker := range markers {
+		if err := schemaMarker.ApplyToSchema(props); err != nil {
+			ctx.pkg.AddError(loader.ErrFromNode(err /* an okay guess */, node))
 		}
 	}
 }

pkg/crd/schema_test.go

@@ -25,6 +25,7 @@ import (
 	"golang.org/x/tools/go/packages"
 	pkgstest "golang.org/x/tools/go/packages/packagestest"
 	apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
 	testloader "sigs.k8s.io/controller-tools/pkg/loader/testutils"
 	"sigs.k8s.io/controller-tools/pkg/markers"
 )
@@ -110,3 +111,59 @@ func Test_Schema_MapOfStringToArrayOfFloat32(t *testing.T) {
 		},
 	}))
 }
+
+func Test_Schema_ApplyMarkers(t *testing.T) {
+	g := gomega.NewWithT(t)
+
+	props := &apiext.JSONSchemaProps{}
+	ctx := &schemaContext{}
+
+	var invocations []string
+
+	applyMarkers(ctx, markers.MarkerValues{
+		"blah": []interface{}{
+			&testPriorityMarker{
+				priority: 0, callback: func() {
+					invocations = append(invocations, "0")
+				},
+			},
+			&testPriorityMarker{priority: 1, callback: func() {
+				invocations = append(invocations, "1")
+
+			}},
+			&testapplyFirstMarker{callback: func() {
+				invocations = append(invocations, "applyFirst")
+			}},
+			&testPriorityMarker{priority: crdmarkers.ApplyPriorityDefault, callback: func() {
+				invocations = append(invocations, "default")
+
+			}},
+		}}, props, nil)
+
+	g.Expect(invocations).To(gomega.Equal([]string{"0", "1", "applyFirst", "default"}))
+}
+
+type testPriorityMarker struct {
+	priority crdmarkers.ApplyPriority
+	callback func()
+}
+
+func (m *testPriorityMarker) ApplyPriority() crdmarkers.ApplyPriority {
+	return m.priority
+}
+
+func (m *testPriorityMarker) ApplyToSchema(*apiext.JSONSchemaProps) error {
+	m.callback()
+	return nil
+}
+
+type testapplyFirstMarker struct {
+	priority crdmarkers.ApplyPriority
+	callback func()
+}
+
+func (m *testapplyFirstMarker) ApplyFirst() {}
+func (m *testapplyFirstMarker) ApplyToSchema(*apiext.JSONSchemaProps) error {
+	m.callback()
+	return nil
+}