(feature): add statussubresource linter

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>

Author: everettraven

README.md

@@ -273,6 +273,20 @@ It will suggest to remove the pointer from the field, and update the `json` tag
 If you prefer not to suggest fixes for pointers in required fields, you can change the `pointerPolicy` to `Warn`.
 The linter will then only suggest to remove the `omitempty` value from the `json` tag.
 
+## StatusSubresource
+
+The `statussubresource` linter checks that the status subresource is configured correctly for
+structs marked with the `kubebuilder:object:root:=true` marker. Correct configuration is that
+when there is a status field the `kubebuilder:subresource:status` marker is present on the struct
+OR when the `kubebuilder:subresource:status` marker is present on the struct there is a status field.
+
+This linter is not enabled by default as it is only applicable to CustomResourceDefinitions.
+
+### Fixes (via standalone binary only)
+
+In the case where there is a status field present but no `kubebuilder:subresource:status` marker, the
+linter will suggest adding the comment `// +kubebuilder:subresource:status` above the struct.
+
 # Contributing
 
 New linters can be added by following the [New Linter][new-linter] guide.

pkg/analysis/registry.go

@@ -12,6 +12,7 @@ import (
 	"github.com/JoelSpeed/kal/pkg/analysis/nophase"
 	"github.com/JoelSpeed/kal/pkg/analysis/optionalorrequired"
 	"github.com/JoelSpeed/kal/pkg/analysis/requiredfields"
+	"github.com/JoelSpeed/kal/pkg/analysis/statussubresource"
 	"github.com/JoelSpeed/kal/pkg/config"
 	"golang.org/x/tools/go/analysis"
 
@@ -63,6 +64,7 @@ func NewRegistry() Registry {
 			nophase.Initializer(),
 			optionalorrequired.Initializer(),
 			requiredfields.Initializer(),
+			statussubresource.Initializer(),
 		},
 	}
 }

pkg/analysis/registry_test.go

@@ -40,6 +40,7 @@ var _ = Describe("Registry", func() {
 				"nophase",
 				"optionalorrequired",
 				"requiredfields",
+				"statussubresource",
 			))
 		})
 	})

pkg/analysis/statussubresource/analyzer.go

@@ -0,0 +1,155 @@
+package statussubresource
+
+import (
+	"errors"
+	"fmt"
+	"go/ast"
+	"go/token"
+
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/extractjsontags"
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/markers"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+const (
+	name = "statussubresource"
+
+	statusJSONTag = "status"
+
+	// kubebuilderRootMarker is the marker that indicates that a struct is the object root for code and CRD generation.
+	kubebuilderRootMarker = "kubebuilder:object:root:=true"
+
+	// kubebuilderStatusSubresourceMarker is the marker that indicates that the CRD generated for a struct should include the /status subresource.
+	kubebuilderStatusSubresourceMarker = "kubebuilder:subresource:status"
+)
+
+var (
+	errCouldNotGetInspector = errors.New("could not get inspector")
+	errCouldNotGetMarkers   = errors.New("could not get markers")
+	errCouldNotGetJSONTags  = errors.New("could not get json tags")
+)
+
+type analyzer struct{}
+
+// newAnalyzer creates a new analyzer with the given configuration.
+func newAnalyzer() *analysis.Analyzer {
+	a := &analyzer{}
+
+	return &analysis.Analyzer{
+		Name:     name,
+		Doc:      "Checks that a type marked with kubebuilder:object:root:=true and containing a status field is marked with kubebuilder:subresource:status",
+		Run:      a.run,
+		Requires: []*analysis.Analyzer{inspect.Analyzer, markers.Analyzer, extractjsontags.Analyzer},
+	}
+}
+
+func (a *analyzer) run(pass *analysis.Pass) (interface{}, error) {
+	inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	if !ok {
+		return nil, errCouldNotGetInspector
+	}
+
+	markersAccess, ok := pass.ResultOf[markers.Analyzer].(markers.Markers)
+	if !ok {
+		return nil, errCouldNotGetMarkers
+	}
+
+	jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags)
+	if !ok {
+		return nil, errCouldNotGetJSONTags
+	}
+
+	// Filter to type specs so we can get the names of types
+	nodeFilter := []ast.Node{
+		(*ast.TypeSpec)(nil),
+	}
+
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
+		typeSpec, ok := n.(*ast.TypeSpec)
+		if !ok {
+			return
+		}
+
+		// we only care about struct types
+		sTyp, ok := typeSpec.Type.(*ast.StructType)
+		if !ok {
+			return
+		}
+
+		// no identifier on the type
+		if typeSpec.Name == nil {
+			return
+		}
+
+		structMarkers := markersAccess.StructMarkers(sTyp)
+		a.checkStruct(pass, sTyp, typeSpec.Name.Name, structMarkers, jsonTags)
+	})
+
+	return nil, nil //nolint:nilnil
+}
+
+func (a *analyzer) checkStruct(pass *analysis.Pass, sTyp *ast.StructType, name string, structMarkers markers.MarkerSet, jsonTags extractjsontags.StructFieldTags) {
+	if sTyp == nil {
+		return
+	}
+
+	if !structMarkers.HasWithValue(kubebuilderRootMarker) {
+		return
+	}
+
+	hasStatusSubresourceMarker := structMarkers.Has(kubebuilderStatusSubresourceMarker)
+	hasStatusField := hasStatusField(sTyp, jsonTags)
+
+	switch {
+	case (hasStatusSubresourceMarker && hasStatusField), (!hasStatusSubresourceMarker && !hasStatusField):
+		// acceptable state
+	case hasStatusSubresourceMarker && !hasStatusField:
+		// Might be able to have some suggested fixes here, but it is likely much more complex
+		// so for now leave it with a descriptive failure message.
+		pass.Reportf(sTyp.Pos(), "root object type %q is marked to enable the status subresource with marker %q but has no status field", name, kubebuilderStatusSubresourceMarker)
+	case !hasStatusSubresourceMarker && hasStatusField:
+		// In this case we can suggest the autofix to add the status subresource marker
+		pass.Report(analysis.Diagnostic{
+			Pos:     sTyp.Pos(),
+			Message: fmt.Sprintf("root object type %q has a status field but does not have the marker %q to enable the status subresource", name, kubebuilderStatusSubresourceMarker),
+			SuggestedFixes: []analysis.SuggestedFix{
+				{
+					Message: "should add the kubebuilder:subresource:status marker",
+					TextEdits: []analysis.TextEdit{
+						// go one line above the struct and add the marker
+						{
+							// sTyp.Pos() is the beginning of the 'struct' keyword. Subtract
+							// the length of the struct name + 7 (2 for spaces surrounding type name, 4 for the 'type' keyword,
+							// and 1 for the newline) to position at the end of the line above the struct
+							// definition.
+							Pos: sTyp.Pos() - token.Pos(len(name)+7),
+							// prefix with a newline to ensure we aren't appending to a previous comment
+							NewText: []byte("\n// +kubebuilder:subresource:status"),
+						},
+					},
+				},
+			},
+		})
+	}
+}
+
+func hasStatusField(sTyp *ast.StructType, jsonTags extractjsontags.StructFieldTags) bool {
+	if sTyp == nil {
+		return false
+	}
+
+	if sTyp.Fields == nil || sTyp.Fields.List == nil {
+		return false
+	}
+
+	for _, field := range sTyp.Fields.List {
+		info := jsonTags.FieldTags(field)
+		if info.Name == statusJSONTag {
+			return true
+		}
+	}
+
+	return false
+}

pkg/analysis/statussubresource/analyzer_test.go

@@ -0,0 +1,21 @@
+package statussubresource_test
+
+import (
+	"testing"
+
+	"github.com/JoelSpeed/kal/pkg/analysis/statussubresource"
+	"github.com/JoelSpeed/kal/pkg/config"
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestStatusSubresourceAnalyzer(t *testing.T) {
+	testdata := analysistest.TestData()
+	initializer := statussubresource.Initializer()
+
+	analyzer, err := initializer.Init(config.LintersConfig{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "a")
+}

pkg/analysis/statussubresource/doc.go

@@ -0,0 +1,10 @@
+// statussubresource is a linter to check that the status subresource is configured correctly for
+// structs marked with the 'kubebuilder:object:root:=true' marker. Correct configuration is that
+// when there is a status field the 'kubebuilder:subresource:status' marker is present on the struct
+// OR when the 'kubebuilder:subresource:status' marker is present on the struct there is a status field.
+//
+// In the case where there is a status field present but no 'kubebuilder:subresource:status' marker, the
+// linter will suggest adding the comment '// +kubebuilder:subresource:status' above the struct.
+//
+// This linter is not enabled by default as it is only applicable to CustomResourceDefinitions.
+package statussubresource

pkg/analysis/statussubresource/initializer.go

@@ -0,0 +1,31 @@
+package statussubresource
+
+import (
+	"github.com/JoelSpeed/kal/pkg/config"
+	"golang.org/x/tools/go/analysis"
+)
+
+// Initializer returns the AnalyzerInitializer for this
+// Analyzer so that it can be added to the registry.
+func Initializer() initializer {
+	return initializer{}
+}
+
+// intializer implements the AnalyzerInitializer interface.
+type initializer struct{}
+
+// Name returns the name of the Analyzer.
+func (initializer) Name() string {
+	return name
+}
+
+// Init returns the intialized Analyzer.
+func (initializer) Init(cfg config.LintersConfig) (*analysis.Analyzer, error) {
+	return newAnalyzer(), nil
+}
+
+// Default determines whether this Analyzer is on by default, or not.
+func (initializer) Default() bool {
+	// This check only applies to CRDs so should not be on by default.
+	return false
+}

pkg/analysis/statussubresource/testdata/src/a/a.go

@@ -0,0 +1,60 @@
+package a
+
+// +kubebuilder:object:root:=true
+type Foo struct {
+	Spec FooSpec `json:"spec"`
+}
+
+type FooSpec struct {
+	Name string `json:"name"`
+}
+
+// +kubebuilder:object:root:=true
+// +kubebuilder:subresource:status
+type Bar struct { // want "root object type \"Bar\" is marked to enable the status subresource with marker \"kubebuilder:subresource:status\" but has no status field"
+	Spec BarSpec `json:"spec"`
+}
+
+type BarSpec struct {
+	Name string `json:"name"`
+}
+
+// +kubebuilder:object:root:=true
+type Baz struct { // want "root object type \"Baz\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
+	Spec   BazSpec   `json:"spec"`
+	Status BazStatus `json:"status,omitempty"`
+}
+
+type BazSpec struct {
+	Name string `json:"name"`
+}
+
+type BazStatus struct {
+	Name string `json:"name"`
+}
+
+// +kubebuilder:object:root:=true
+// +kubebuilder:subresource:status
+type FooBar struct {
+	Spec   FooBarSpec   `json:"spec"`
+	Status FooBarStatus `json:"status"`
+}
+
+type FooBarSpec struct {
+	Name string `json:"name"`
+}
+
+type FooBarStatus struct {
+	Name string `json:"name"`
+}
+
+// Test that it works with 'root=true' as well
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
+type FooBarBaz struct { // want "root object type \"FooBarBaz\" is marked to enable the status subresource with marker \"kubebuilder:subresource:status\" but has no status field"
+    Spec FooBarBazSpec `json:"spec"`
+}
+
+type FooBarBazSpec struct {
+    Name string `json:"name"`
+}

pkg/analysis/statussubresource/testdata/src/a/a.go.golden

@@ -0,0 +1,61 @@
+package a
+
+// +kubebuilder:object:root:=true
+type Foo struct {
+	Spec FooSpec `json:"spec"`
+}
+
+type FooSpec struct {
+	Name string `json:"name"`
+}
+
+// +kubebuilder:object:root:=true
+// +kubebuilder:subresource:status
+type Bar struct { // want "root object type \"Bar\" is marked to enable the status subresource with marker \"kubebuilder:subresource:status\" but has no status field"
+	Spec BarSpec `json:"spec"`
+}
+
+type BarSpec struct {
+	Name string `json:"name"`
+}
+
+// +kubebuilder:object:root:=true
+// +kubebuilder:subresource:status
+type Baz struct { // want "root object type \"Baz\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
+	Spec   BazSpec   `json:"spec"`
+	Status BazStatus `json:"status,omitempty"`
+}
+
+type BazSpec struct {
+	Name string `json:"name"`
+}
+
+type BazStatus struct {
+	Name string `json:"name"`
+}
+
+// +kubebuilder:object:root:=true
+// +kubebuilder:subresource:status
+type FooBar struct {
+	Spec   FooBarSpec   `json:"spec"`
+	Status FooBarStatus `json:"status"`
+}
+
+type FooBarSpec struct {
+	Name string `json:"name"`
+}
+
+type FooBarStatus struct {
+	Name string `json:"name"`
+}
+
+// Test that it works with 'root=true' as well
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
+type FooBarBaz struct { // want "root object type \"FooBarBaz\" is marked to enable the status subresource with marker \"kubebuilder:subresource:status\" but has no status field"
+    Spec FooBarBazSpec `json:"spec"`
+}
+
+type FooBarBazSpec struct {
+    Name string `json:"name"`
+}