Clean up and add more docs

.github/workflows/checks.yaml

@@ -7,8 +7,8 @@ defaults:
     shell: bash
 
 jobs:
-  makechecks:
-    name: Make Checks
+  cleanliness:
+    name: Run Cleanliness Checks
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
@@ -25,6 +25,7 @@ jobs:
 
       - run: |
           make manifests
+          ( cd test/fakepolicy && make manifests )
           make generate
           make fmt
           make vet
@@ -33,7 +34,7 @@ jobs:
 
   # A separate job so that it can annotate the code
   golangci:
-    name: golangci-lint
+    name: Run golangci-lint
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
@@ -49,7 +50,7 @@ jobs:
           # Automatically uses ./.golangci.yml for configuration
 
   tests:
-    name: Tests
+    name: Run Tests
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4

.github/workflows/code_cov_comment.yaml

@@ -6,6 +6,7 @@ on:
 
 jobs:
   comment:
+    name: Add PR Code Coverage Report
     runs-on: ubuntu-latest
     steps:
       - uses: actions/github-script@v7

.github/workflows/pr_agent.yaml

@@ -5,13 +5,13 @@ on:
     types: [created]
 jobs:
   pr_agent_job:
+    name: Run Codium-ai/pr-agent
     if: ${{ github.event.sender.type != 'Bot' && !contains(github.event.pull_request.labels.*.name, 'skip_ai') }}
     runs-on: ubuntu-latest
     permissions:
       issues: write
       pull-requests: write
       contents: write
-    name: Run pr agent on every pull request, respond to user comments
     steps:
       - name: PR Agent action step
         id: pr-agent

Makefile

@@ -17,16 +17,16 @@ export GOBIN ?= $(GOBIN_DEFAULT)
 export PATH=$(LOCAL_BIN):$(GOBIN):$(shell echo $$PATH)
 
 # Setting SHELL to bash allows bash commands to be executed by recipes.
-# This is a requirement for 'setup-envtest.sh' in the test target.
+# This might be a requirement for envtest, used by the test suites.
 # Options are set to exit when a recipe line exits non-zero or a piped command fails.
 SHELL = /usr/bin/env bash -o pipefail
 .SHELLFLAGS = -ec
 
 # go-install will 'go install' any package $1 to LOCAL_BIN
-# Note: this replaces `go-get-tool`.
+# Note: this replaces the `go-get-tool` scaffolded by kubebuilder.
 go-install = @set -e ; mkdir -p $(LOCAL_BIN) ; GOBIN=$(LOCAL_BIN) go install $(1)
 
-# Define local utilities before other targets so they work correctly
+# Local utilities must be defined before other targets so they work correctly.
 # Note: this pattern of variables, paths, and target names allows users to
 #  override the version used, and helps Make by not using PHONY targets.
 # To 'refresh' versions, remove the local bin directory.
@@ -80,10 +80,11 @@ manifests: $(CONTROLLER_GEN) $(KUSTOMIZE) ## Generate WebhookConfiguration, Clus
 	  output:crd:artifacts:config=test/fakepolicy/config/crd/bases \
 	  output:rbac:artifacts:config=test/fakepolicy/config/rbac
 	$(KUSTOMIZE) build ./test/fakepolicy/config/default > ./test/fakepolicy/config/deploy.yaml
+	cp ./config/samples/* ./test/fakepolicy/test/utils/testdata/
 
 .PHONY: generate
 generate: $(CONTROLLER_GEN) ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
-	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."
+	$(CONTROLLER_GEN) object:headerFile="config/boilerplate.go.txt" paths="./..."
 
 .PHONY: fmt
 fmt: $(GOFUMPT) $(GCI)
@@ -99,25 +100,29 @@ vet: $(GOSEC)
 
 # Note: this target is not used by Github Actions. Instead, each linter is run 
 # separately to automatically decorate the code with the linting errors.
-# Note: this target will fail if yamllint is not installed.
+# Note: this target will fail if yamllint is not installed. Installing yamllint
+# is outside the scope of this Makefile.
 .PHONY: lint
 lint: $(GOLANGCI)
 	$(GOLANGCI) run
 	yamllint -c $(ROOTDIR)/.yamllint.yaml .
 
 ENVTEST_K8S_VERSION ?= 1.29
 .PHONY: test
-test: manifests generate $(GINKGO) $(ENVTEST) ## Run all the tests
+test: manifests generate $(GINKGO) $(ENVTEST) ## Run all the tests, except for fuzz tests
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" $(GINKGO) \
 	  --coverpkg=./... --covermode=count --coverprofile=raw-cover.out ./...
 
+.PHONY: test-unit
 test-unit: ## Run only the unit tests
 	go test --coverpkg=./... --covermode=count --coverprofile=cover-unit.out ./api/... ./pkg/...
 
+.PHONY: test-basicsuite
 test-basicsuite: manifests generate $(GINKGO) $(ENVTEST) ## Run just the basic suite of tests
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" $(GINKGO) \
 	  --coverpkg=./... --covermode=count --coverprofile=cover-basic.out ./test/fakepolicy/test/basic
 
+.PHONY: test-compsuite
 test-compsuite: manifests generate $(GINKGO) $(ENVTEST) ## Run just the compliance event tests
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" $(GINKGO) \
 	  --coverpkg=./... --covermode=count --coverprofile=cover-comp.out ./test/fakepolicy/test/compliance

api/v1alpha1/clientObjectFakes_test.go

@@ -13,7 +13,7 @@ import (
 // fakeObjList minimally implements client.ObjectList. It is only to be used for tests.
 type fakeObjList string
 
-// ensure fakeObjList implements client.ObjectList.
+// Run a compile-time check to ensure fakeObjList implements client.ObjectList.
 var _ client.ObjectList = (*fakeObjList)(nil)
 
 func (l fakeObjList) GetResourceVersion() string {
@@ -55,7 +55,7 @@ func (l fakeObjList) DeepCopyObject() runtime.Object {
 // fakeObjList minimally implements client.Object. It is only to be used for tests.
 type fakeObj string
 
-// ensure fakeObj implements client.Object.
+// Run a compile-time check to ensure fakeObj implements client.Object.
 var _ client.Object = (*fakeObj)(nil)
 
 func (o fakeObj) GetNamespace() string {

api/v1alpha1/doc.go

@@ -0,0 +1,7 @@
+// Copyright Contributors to the Open Cluster Management project
+
+// Package v1alpha1 contains experimental types which may be useful for policy "templates"
+// interacting with the ocm-io policy framework addon, but are not recommended for "real" use yet.
+// They can and will be changed or completely removed with no additional notice; be aware of this
+// when importing and using them.
+package v1alpha1

api/v1alpha1/reflectiveResourceList.go

@@ -13,16 +13,16 @@ import (
 
 //+kubebuilder:object:generate=false
 
-// ReflectiveResourceList implements ResourceList for the wrapped client.ObjectList, by using
-// reflection. The wrapped list must have an Items field, with a slice of items which satisfy the
-// client.Object interface - most types which satisfy client.ObjectList seem to follow this
-// convention. Using this type is not recommended: implementing ResourceList yourself will generally
+// ReflectiveResourceList implements v1beta1.ResourceList for the wrapped client.ObjectList, by
+// using reflection. The wrapped list must have an Items field, with a slice of items which satisfy
+// the client.Object interface - most types which satisfy client.ObjectList seem to follow this
+// convention. Using this type is not recommended: implementing ResourceList by hand will likely
 // lead to better performance.
 type ReflectiveResourceList struct {
 	ClientList client.ObjectList
 }
 
-// ensure ReflectiveResourceList implements ResourceList.
+// Run a compile-time check to ensure ReflectiveResourceList implements ResourceList.
 var _ v1beta1.ResourceList = (*ReflectiveResourceList)(nil)
 
 // Items returns the list of items in the list. Since this implementation uses reflection, it may

api/v1beta1/doc.go

@@ -0,0 +1,12 @@
+// Copyright Contributors to the Open Cluster Management project
+
+// Package v1beta1 contains types which are useful or necessary for implementing a policy "template"
+// which will interact with the ocm-io policy framework addon. The types here are meant to be
+// embedded in a CRD, allowing utilities (defined elsewhere) to work with all templates more
+// generally.
+//
+// Code here can be changed over time, but the intent is to do so in a backwards-compatible way, and
+// not remove things without a deprecation notice. Code here is meant to be usable by templates,
+// even before it graduates to `v1`; based on historical trends, many types may begin here, but
+// should try to advance to something more stable once they've proven their worth.
+package v1beta1

api/v1beta1/policycore_types.go

@@ -131,7 +131,7 @@ type namespaceResList struct {
 	corev1.NamespaceList
 }
 
-// ensure namespaceResList implements ResourceList.
+// Run a compile-time check to ensure namespaceResList implements ResourceList.
 var _ ResourceList = (*namespaceResList)(nil)
 
 func (l *namespaceResList) Items() ([]client.Object, error) {
@@ -186,7 +186,9 @@ const (
 //+kubebuilder:object:root=true
 //+kubebuilder:subresource:status
 
-// PolicyCore is the Schema for the policycores API.
+// PolicyCore is the Schema for the policycores API. This is not a real API, but
+// is included so that an example CRD can be generated showing the validated
+// fields and types.
 type PolicyCore struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
@@ -198,7 +200,41 @@ type PolicyCore struct {
 //+kubebuilder:object:generate=false
 
 // PolicyLike is an interface that policies should implement so that they can
-// benefit from some of the general tools in the nucleus.
+// benefit from some of the general tools in the nucleus. Here is a simple
+// example implementation, which utilizes the core types of the nucleus:
+//
+//	import  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+//	import nucleusv1beta1 "open-cluster-management.io/governance-policy-nucleus/api/v1beta1"
+//
+//	type FakePolicy struct {
+//		metav1.TypeMeta   `json:",inline"`
+//		metav1.ObjectMeta `json:"metadata,omitempty"`
+//		Spec   nucleusv1beta1.PolicyCoreSpec   `json:"spec,omitempty"`
+//		Status nucleusv1beta1.PolicyCoreStatus `json:"status,omitempty"`
+//	}
+//
+//	func (f FakePolicy) ComplianceState() nucleusv1beta1.ComplianceState {
+//		return f.Status.ComplianceState
+//	}
+//
+//	func (f FakePolicy) ComplianceMessage() string {
+//		idx, compCond := f.Status.GetCondition("Compliant")
+//		if idx == -1 {
+//			return ""
+//		}
+//		return compCond.Message
+//	}
+//
+//	func (f FakePolicy) Parent() metav1.OwnerReference {
+//		if len(f.OwnerReferences) == 0 {
+//			return metav1.OwnerReference{}
+//		}
+//		return f.OwnerReferences[0]
+//	}
+//
+//	func (f FakePolicy) ParentNamespace() string {
+//		return f.Namespace
+//	}
 type PolicyLike interface {
 	client.Object
 

config/README.md

@@ -0,0 +1,8 @@
+# /config
+
+This directory is a remnant of the kubebuilder scaffold. Since the PolicyCore is not meant to be a
+real CRD, many details have been removed here. The CRD and a sample instance remain as examples.
+
+The samples scaffolded by kubebuilder for projects are often out of date, *however* in this 
+repository, the sample is tested in the fakepolicy "basic" suite to ensure that it is not missing
+any required fields, or any values that would be defaulted.

config/crd/bases/policy.open-cluster-management.io_policycores.yaml

@@ -17,7 +17,10 @@ spec:
   - name: v1beta1
     schema:
       openAPIV3Schema:
-        description: PolicyCore is the Schema for the policycores API.
+        description: |-
+          PolicyCore is the Schema for the policycores API. This is not a real API, but
+          is included so that an example CRD can be generated showing the validated
+          fields and types.
         properties:
           apiVersion:
             description: |-

config/default/kustomization.yaml

@@ -14,4 +14,3 @@ namePrefix: governance-policy-nucleus-
 
 bases:
 - ../crd
-- ../rbac

config/samples/policy_v1beta1_policycore.yaml

@@ -2,6 +2,7 @@ apiVersion: policy.open-cluster-management.io/v1beta1
 kind: PolicyCore
 metadata:
   name: policycore-sample
+  namespace: default
 spec:
   severity: low
   remediationAction: inform

test/fakepolicy/Makefile

@@ -6,3 +6,4 @@ include ../../Makefile
 manifests: $(CONTROLLER_GEN) $(KUSTOMIZE) ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
 	$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
 	$(KUSTOMIZE) build ./config/default > config/deploy.yaml
+	cp ./config/samples/* ./test/utils/testdata/

test/fakepolicy/README.md

@@ -0,0 +1,10 @@
+# FakePolicy
+
+This directory contains a controller for a FakePolicy CRD in the kubebuilder style (although with
+some changed details), which is meant to be used for testing features in the nucleus. Because of its
+purpose, the way it uses features is highly contrived and it might not always be a good example.
+
+## Test Suites
+
+The "basic" suite requires very specific namespaces and configmaps in the cluster, so it was
+separated from the other tests, which may add namespaces and other resources.

test/fakepolicy/api/v1beta1/fakepolicy_types.go

@@ -48,7 +48,7 @@ type FakePolicy struct {
 	Status FakePolicyStatus `json:"status,omitempty"`
 }
 
-// ensure FakePolicy implements PolicyLike.
+// Run a compile-time check to ensure FakePolicy implements PolicyLike.
 var _ nucleusv1beta1.PolicyLike = (*FakePolicy)(nil)
 
 func (f FakePolicy) ComplianceState() nucleusv1beta1.ComplianceState {

test/fakepolicy/config/samples/policy_v1beta1_fakepolicy.yaml

@@ -2,9 +2,14 @@ apiVersion: policy.open-cluster-management.io/v1beta1
 kind: FakePolicy
 metadata:
   name: fakepolicy-sample
+  namespace: default
 spec:
   severity: low
   remediationAction: inform
   namespaceSelector:
     include: ["*"]
     exclude: ["kube-*"]
+  targetConfigMaps:
+    matchExpressions:
+    - key: sample
+      operator: Exists

test/fakepolicy/controllers/fakepolicy_controller.go

@@ -235,7 +235,7 @@ type configMapResList struct {
 	corev1.ConfigMapList
 }
 
-// ensure configMapResList implements ResourceList.
+// Run a compile-time check to ensure configMapResList implements ResourceList.
 var _ nucleusv1beta1.ResourceList = (*configMapResList)(nil)
 
 func (l *configMapResList) Items() ([]client.Object, error) {

test/fakepolicy/test/basic/suite_test.go

@@ -49,7 +49,10 @@ var _ = BeforeSuite(func() {
 
 	By("bootstrapping test environment")
 	testEnv = &envtest.Environment{
-		CRDDirectoryPaths:     []string{filepath.Join("..", "..", "config", "crd", "bases")},
+		CRDDirectoryPaths: []string{
+			filepath.Join("..", "..", "config", "crd", "bases"),
+			filepath.Join("..", "..", "..", "..", "config", "crd", "bases"),
+		},
 		ErrorIfCRDPathMissing: true,
 	}
 

test/fakepolicy/test/basic/validation_test.go

@@ -14,7 +14,7 @@ import (
 var _ = Describe("FakePolicy CRD Validation", func() {
 	DescribeTable("Validating spec inputs",
 		func(ctx SpecContext, severity, remediationAction string, include, exclude []string, isValid bool) {
-			policy := FromTestdata("fakepolicy-sample.yaml")
+			policy := FromTestdata("policy_v1beta1_fakepolicy.yaml")
 
 			Expect(unstructured.SetNestedField(policy.Object,
 				severity, "spec", "severity")).To(Succeed())