(feature): add statussubresource linter

Signed-off-by: everettraven <everettraven@gmail.com>

Author: everettraven

README.md

@@ -201,6 +201,31 @@ 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.
+
+The status field name used to check for presence of a status field can be configured with
+the `statusFieldName` option. By default `statusFieldName` is set to `"Status"`.
+
+This linter is not enabled by default as it is only applicable to CustomResourceDefinitions.
+
+### Configuration
+
+```yaml
+lintersConfig:
+  statussubresource:
+    statusFieldName: "Status"
+```
+
+### 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

@@ -7,6 +7,7 @@ import (
 	"github.com/JoelSpeed/kal/pkg/analysis/jsontags"
 	"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"
 
@@ -53,6 +54,7 @@ func NewRegistry() Registry {
 			jsontags.Initializer(),
 			optionalorrequired.Initializer(),
 			requiredfields.Initializer(),
+			statussubresource.Initializer(),
 		},
 	}
 }

pkg/analysis/registry_test.go

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

pkg/analysis/statussubresource/analyzer.go

@@ -0,0 +1,159 @@
+package statussubresource
+
+import (
+	"errors"
+	"fmt"
+	"go/ast"
+	"go/token"
+
+	"github.com/JoelSpeed/kal/pkg/analysis/helpers/markers"
+	"github.com/JoelSpeed/kal/pkg/config"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+const (
+	name = "statussubresource"
+
+	DefaultStatusFieldName = "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")
+)
+
+type analyzer struct {
+	statusFieldName string
+}
+
+// newAnalyzer creates a new analyzer with the given configuration.
+func newAnalyzer(cfg config.StatusSubresourceConfig) *analysis.Analyzer {
+	defaultConfig(&cfg)
+
+	a := &analyzer{
+		statusFieldName: cfg.StatusFieldName,
+	}
+
+	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},
+	}
+}
+
+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
+	}
+
+	// Filter to structs so that we can iterate over fields in a struct.
+	nodeFilter := []ast.Node{
+		(*ast.TypeSpec)(nil),
+	}
+
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
+		typeSpec, ok := n.(*ast.TypeSpec)
+		if !ok {
+			return
+		}
+
+		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)
+	})
+
+	return nil, nil
+}
+
+func (a *analyzer) checkStruct(pass *analysis.Pass, sTyp *ast.StructType, name string, structMarkers markers.MarkerSet) {
+	if sTyp == nil {
+		return
+	}
+
+	if !structMarkers.Has(KubebuilderRootMarker) {
+		return
+	}
+
+	hasStatusSubresourceMarker := structMarkers.Has(KubebuilderStatusSubresourceMarker)
+	hasStatusField := hasStatusField(sTyp.Fields, a.statusFieldName)
+
+	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
+						{
+							Pos: sTyp.Pos() - token.Pos(len(name)) - token.Pos(2) - token.Pos(len("type")) - token.Pos(1),
+							// prefix with a newline to ensure we aren't appending to a previous comment
+							NewText: []byte("\n// +kubebuilder:subresource:status"),
+						},
+					},
+				},
+			},
+		})
+	}
+}
+
+func hasStatusField(fieldList *ast.FieldList, statusFieldName string) bool {
+	if fieldList == nil {
+		return false
+	}
+
+	for _, field := range fieldList.List {
+		if field == nil || len(field.Names) == 0 {
+			continue
+		}
+
+		if field.Names[0] == nil {
+			continue
+		}
+
+		if field.Names[0].Name == statusFieldName {
+			return true
+		}
+	}
+
+	return false
+}
+
+func defaultConfig(cfg *config.StatusSubresourceConfig) {
+	if cfg.StatusFieldName == "" {
+		cfg.StatusFieldName = DefaultStatusFieldName
+	}
+}

pkg/analysis/statussubresource/analyzer_test.go

@@ -0,0 +1,35 @@
+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 TestStatusSubresourceAnalyzerWithDefaultConfig(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")
+}
+
+func TestStatusSubresourceAnalyzerWithCustomStatusFieldName(t *testing.T) {
+	testdata := analysistest.TestData()
+	initializer := statussubresource.Initializer()
+	analyzer, err := initializer.Init(config.LintersConfig{
+		StatusSubresource: config.StatusSubresourceConfig{
+			StatusFieldName: "MyCustomStatus",
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "b")
+}

pkg/analysis/statussubresource/doc.go

@@ -0,0 +1,13 @@
+// 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.
+//
+// The status field name used to check for presence of a status field can be configured with
+// the 'statusFieldName' option. By default 'statusFieldName' is set to 'Status'.
+//
+// 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(cfg.StatusSubresource), 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,49 @@
+package a
+
+// +kubebuilder:object:root:=true
+type Foo struct {
+	Spec FooSpec
+}
+
+type FooSpec struct {
+	Name string
+}
+
+// +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
+}
+
+type BarSpec struct {
+	Name string
+}
+
+// +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
+	Status BazStatus
+}
+
+type BazSpec struct {
+	Name string
+}
+
+type BazStatus struct {
+	Name string
+}
+
+// +kubebuilder:object:root:=true
+// +kubebuilder:subresource:status
+type FooBar struct {
+	Spec   FooBarSpec
+	Status FooBarStatus
+}
+
+type FooBarSpec struct {
+	Name string
+}
+
+type FooBarStatus struct {
+	Name string
+}

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

@@ -0,0 +1,50 @@
+package a
+
+// +kubebuilder:object:root:=true
+type Foo struct {
+	Spec FooSpec
+}
+
+type FooSpec struct {
+	Name string
+}
+
+// +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
+}
+
+type BarSpec struct {
+	Name string
+}
+
+// +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
+	Status BazStatus
+}
+
+type BazSpec struct {
+	Name string
+}
+
+type BazStatus struct {
+	Name string
+}
+
+// +kubebuilder:object:root:=true
+// +kubebuilder:subresource:status
+type FooBar struct {
+	Spec   FooBarSpec
+	Status FooBarStatus
+}
+
+type FooBarSpec struct {
+	Name string
+}
+
+type FooBarStatus struct {
+	Name string
+}