Merge branch 'master' into upgrade-golangci

apis/status/v1beta1/constraintpodstatus_types.go

@@ -80,7 +80,7 @@ func init() {
 // NewConstraintStatusForPod returns a constraint status object
 // that has been initialized with the bare minimum of fields to make it functional
 // with the constraint status controller.
-func NewConstraintStatusForPod(pod *corev1.Pod, constraint *unstructured.Unstructured, scheme *runtime.Scheme) (*ConstraintPodStatus, error) {
+func NewConstraintStatusForPod(pod *corev1.Pod, podOwnershipMode PodOwnershipMode, constraint *unstructured.Unstructured, scheme *runtime.Scheme) (*ConstraintPodStatus, error) {
 	obj := &ConstraintPodStatus{}
 	name, err := KeyForConstraint(pod.Name, constraint)
 	if err != nil {
@@ -97,7 +97,7 @@ func NewConstraintStatusForPod(pod *corev1.Pod, constraint *unstructured.Unstruc
 		// the template name is the lower-case of the constraint kind
 		ConstraintTemplateNameLabel: strings.ToLower(constraint.GetKind()),
 	})
-	if PodOwnershipEnabled() {
+	if podOwnershipMode == PodOwnershipEnabled {
 		if err := controllerutil.SetOwnerReference(pod, obj, scheme); err != nil {
 			return nil, err
 		}

apis/status/v1beta1/constraintpodstatus_types_test.go

@@ -49,7 +49,7 @@ func TestNewConstraintStatusForPod(t *testing.T) {
 		})
 	g.Expect(controllerutil.SetOwnerReference(pod, expectedStatus, scheme)).NotTo(HaveOccurred())
 
-	status, err := NewConstraintStatusForPod(pod, cstr, scheme)
+	status, err := NewConstraintStatusForPod(pod, PodOwnershipEnabled, cstr, scheme)
 	g.Expect(err).NotTo(HaveOccurred())
 	g.Expect(status).To(Equal(expectedStatus))
 	cmVal, err := KeyForConstraint(podName, cstr)

apis/status/v1beta1/constrainttemplatepodstatus_types.go

@@ -63,7 +63,7 @@ func init() {
 // NewConstraintTemplateStatusForPod returns a constraint template status object
 // that has been initialized with the bare minimum of fields to make it functional
 // with the constraint template status controller.
-func NewConstraintTemplateStatusForPod(pod *corev1.Pod, templateName string, scheme *runtime.Scheme) (*ConstraintTemplatePodStatus, error) {
+func NewConstraintTemplateStatusForPod(pod *corev1.Pod, podOwnershipMode PodOwnershipMode, templateName string, scheme *runtime.Scheme) (*ConstraintTemplatePodStatus, error) {
 	obj := &ConstraintTemplatePodStatus{}
 	name, err := KeyForConstraintTemplate(pod.Name, templateName)
 	if err != nil {
@@ -77,7 +77,7 @@ func NewConstraintTemplateStatusForPod(pod *corev1.Pod, templateName string, sch
 		ConstraintTemplateNameLabel: templateName,
 		PodLabel:                    pod.Name,
 	})
-	if PodOwnershipEnabled() {
+	if podOwnershipMode == PodOwnershipEnabled {
 		if err := controllerutil.SetOwnerReference(pod, obj, scheme); err != nil {
 			return nil, err
 		}

apis/status/v1beta1/constrainttemplatepodstatus_types_test.go

@@ -38,7 +38,7 @@ func TestNewConstraintTemplateStatusForPod(t *testing.T) {
 	})
 	g.Expect(controllerutil.SetOwnerReference(pod, expectedStatus, scheme)).NotTo(HaveOccurred())
 
-	status, err := NewConstraintTemplateStatusForPod(pod, templateName, scheme)
+	status, err := NewConstraintTemplateStatusForPod(pod, PodOwnershipEnabled, templateName, scheme)
 	g.Expect(err).NotTo(HaveOccurred())
 	g.Expect(status).To(Equal(expectedStatus))
 	n, err := KeyForConstraintTemplate(podName, templateName)

apis/status/v1beta1/mutatorpodstatus_types.go

@@ -48,6 +48,9 @@ type MutatorPodStatusStatus struct {
 
 // MutatorError represents a single error caught while adding a mutator to a system.
 type MutatorError struct {
+	// Type indicates a specific class of error for use by controller code.
+	// If not present, the error should be treated as not matching any known type.
+	Type    string `json:"type,omitempty"`
 	Message string `json:"message"`
 }
 
@@ -78,7 +81,7 @@ func init() {
 // NewMutatorStatusForPod returns a mutator status object
 // that has been initialized with the bare minimum of fields to make it functional
 // with the mutator status controller.
-func NewMutatorStatusForPod(pod *corev1.Pod, mutatorID mtypes.ID, scheme *runtime.Scheme) (*MutatorPodStatus, error) {
+func NewMutatorStatusForPod(pod *corev1.Pod, podOwnershipMode PodOwnershipMode, mutatorID mtypes.ID, scheme *runtime.Scheme) (*MutatorPodStatus, error) {
 	obj := &MutatorPodStatus{}
 	name, err := KeyForMutatorID(pod.Name, mutatorID)
 	if err != nil {
@@ -94,7 +97,7 @@ func NewMutatorStatusForPod(pod *corev1.Pod, mutatorID mtypes.ID, scheme *runtim
 		MutatorKindLabel: mutatorID.Kind,
 		PodLabel:         pod.Name,
 	})
-	if PodOwnershipEnabled() {
+	if podOwnershipMode == PodOwnershipEnabled {
 		if err := controllerutil.SetOwnerReference(pod, obj, scheme); err != nil {
 			return nil, err
 		}

apis/status/v1beta1/mutatorpodstatus_types_test.go

@@ -17,8 +17,17 @@ func TestNewMutatorStatusForPod(t *testing.T) {
 	podName := "some-gk-pod-m"
 	podNS := "a-gk-namespace-m"
 	mutator := testhelpers.NewDummyMutator("a-mutator", "spec.value", nil)
-	os.Setenv("POD_NAMESPACE", podNS)
-	defer os.Unsetenv("POD_NAMESPACE")
+	err := os.Setenv("POD_NAMESPACE", podNS)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Cleanup(func() {
+		err = os.Unsetenv("POD_NAMESPACE")
+		if err != nil {
+			t.Error(err)
+		}
+	})
 
 	scheme := runtime.NewScheme()
 	g.Expect(AddToScheme(scheme)).NotTo(HaveOccurred())
@@ -41,7 +50,7 @@ func TestNewMutatorStatusForPod(t *testing.T) {
 		})
 	g.Expect(controllerutil.SetOwnerReference(pod, expectedStatus, scheme)).NotTo(HaveOccurred())
 
-	status, err := NewMutatorStatusForPod(pod, mutator.ID(), scheme)
+	status, err := NewMutatorStatusForPod(pod, PodOwnershipEnabled, mutator.ID(), scheme)
 	g.Expect(err).NotTo(HaveOccurred())
 	g.Expect(status).To(Equal(expectedStatus))
 	cmVal, err := KeyForMutatorID(podName, mutator.ID())

apis/status/v1beta1/util.go

@@ -6,23 +6,34 @@ import (
 	"sync"
 )
 
+type PodOwnershipMode bool
+
+const (
+	// PodOwnershipDisabled indicates that owner references should not be put on
+	// dependent resources.
+	PodOwnershipDisabled PodOwnershipMode = false
+	// PodOwnershipEnabled indicates that owner references should be put on
+	// dependent resources.
+	PodOwnershipEnabled PodOwnershipMode = true
+)
+
 var (
-	podOwnershipEnabled = true
-	ownerMutex          = sync.RWMutex{}
+	podOwnershipMode = PodOwnershipEnabled
+	ownerMutex       = sync.RWMutex{}
 )
 
 // DisablePodOwnership disables setting the owner reference for Status resource.
 // This should only be used for testing, where a Pod resource may not be available.
 func DisablePodOwnership() {
 	ownerMutex.Lock()
 	defer ownerMutex.Unlock()
-	podOwnershipEnabled = false
+	podOwnershipMode = PodOwnershipDisabled
 }
 
-func PodOwnershipEnabled() bool {
+func GetPodOwnershipMode() PodOwnershipMode {
 	ownerMutex.RLock()
 	defer ownerMutex.RUnlock()
-	return podOwnershipEnabled
+	return podOwnershipMode == PodOwnershipEnabled
 }
 
 // dashExtractor unpacks the status resource name, unescaping `-`.

apis/status/v1beta1/util_test.go

@@ -9,11 +9,11 @@ import (
 func TestOwnership(t *testing.T) {
 	g := NewGomegaWithT(t)
 	t.Run("Ownership defaults to enabled", func(t *testing.T) {
-		g.Expect(PodOwnershipEnabled()).Should(BeTrue())
+		g.Expect(GetPodOwnershipMode()).Should(Equal(PodOwnershipEnabled))
 	})
 	t.Run("Disabling is honored", func(t *testing.T) {
 		DisablePodOwnership()
-		g.Expect(PodOwnershipEnabled()).Should(BeFalse())
+		g.Expect(GetPodOwnershipMode()).Should(Equal(PodOwnershipDisabled))
 	})
 }
 

config/crd/bases/mutations.gatekeeper.sh_assign.yaml

@@ -190,6 +190,9 @@ spec:
                         properties:
                           message:
                             type: string
+                          type:
+                            description: Type indicates a specific class of error for use by controller code. If not present, the error should be treated as not matching any known type.
+                            type: string
                         required:
                         - message
                         type: object

config/crd/bases/mutations.gatekeeper.sh_assignmetadata.yaml

@@ -156,6 +156,9 @@ spec:
                         properties:
                           message:
                             type: string
+                          type:
+                            description: Type indicates a specific class of error for use by controller code. If not present, the error should be treated as not matching any known type.
+                            type: string
                         required:
                         - message
                         type: object

config/crd/bases/mutations.gatekeeper.sh_modifyset.yaml

@@ -198,6 +198,9 @@ spec:
                         properties:
                           message:
                             type: string
+                          type:
+                            description: Type indicates a specific class of error for use by controller code. If not present, the error should be treated as not matching any known type.
+                            type: string
                         required:
                         - message
                         type: object

config/crd/bases/status.gatekeeper.sh_mutatorpodstatuses.yaml

@@ -40,6 +40,9 @@ spec:
                   properties:
                     message:
                       type: string
+                    type:
+                      description: Type indicates a specific class of error for use by controller code. If not present, the error should be treated as not matching any known type.
+                      type: string
                   required:
                   - message
                   type: object

manifest_staging/charts/gatekeeper/crds/assign-customresourcedefinition.yaml

@@ -194,6 +194,9 @@ spec:
                         properties:
                           message:
                             type: string
+                          type:
+                            description: Type indicates a specific class of error for use by controller code. If not present, the error should be treated as not matching any known type.
+                            type: string
                         required:
                         - message
                         type: object

manifest_staging/charts/gatekeeper/crds/assignmetadata-customresourcedefinition.yaml

@@ -160,6 +160,9 @@ spec:
                         properties:
                           message:
                             type: string
+                          type:
+                            description: Type indicates a specific class of error for use by controller code. If not present, the error should be treated as not matching any known type.
+                            type: string
                         required:
                         - message
                         type: object

manifest_staging/charts/gatekeeper/crds/modifyset-customresourcedefinition.yaml

@@ -202,6 +202,9 @@ spec:
                         properties:
                           message:
                             type: string
+                          type:
+                            description: Type indicates a specific class of error for use by controller code. If not present, the error should be treated as not matching any known type.
+                            type: string
                         required:
                         - message
                         type: object

manifest_staging/charts/gatekeeper/crds/mutatorpodstatus-customresourcedefinition.yaml

@@ -40,6 +40,9 @@ spec:
                   properties:
                     message:
                       type: string
+                    type:
+                      description: Type indicates a specific class of error for use by controller code. If not present, the error should be treated as not matching any known type.
+                      type: string
                   required:
                   - message
                   type: object

pkg/controller/constraint/constraint_controller.go

@@ -135,6 +135,8 @@ func newReconciler(
 		reporter:         reporter,
 		constraintsCache: constraintsCache,
 		tracker:          tracker,
+
+		podOwnershipMode: constraintstatusv1beta1.GetPodOwnershipMode(),
 	}
 	r.getPod = r.defaultGetPod
 	// default
@@ -192,6 +194,8 @@ type ReconcileConstraint struct {
 	// assumeDeleted allows us to short-circuit get requests
 	// that would otherwise trigger a watch
 	assumeDeleted func(schema.GroupVersionKind) bool
+
+	podOwnershipMode constraintstatusv1beta1.PodOwnershipMode
 }
 
 // +kubebuilder:rbac:groups=constraints.gatekeeper.sh,resources=*,verbs=get;list;watch;create;update;patch;delete
@@ -356,7 +360,7 @@ func (r *ReconcileConstraint) getOrCreatePodStatus(ctx context.Context, constrai
 	if err != nil {
 		return nil, err
 	}
-	statusObj, err = constraintstatusv1beta1.NewConstraintStatusForPod(pod, constraint, r.scheme)
+	statusObj, err = constraintstatusv1beta1.NewConstraintStatusForPod(pod, r.podOwnershipMode, constraint, r.scheme)
 	if err != nil {
 		return nil, err
 	}

pkg/controller/constrainttemplate/constrainttemplate_controller.go

@@ -166,15 +166,16 @@ func newReconciler(mgr manager.Manager, opa *opa.Client, wm *watch.Manager, cs *
 	}
 	r := newStatsReporter()
 	reconciler := &ReconcileConstraintTemplate{
-		Client:        mgr.GetClient(),
-		scheme:        mgr.GetScheme(),
-		opa:           opa,
-		watcher:       w,
-		statusWatcher: statusW,
-		cs:            cs,
-		metrics:       r,
-		tracker:       tracker,
-		getPod:        getPod,
+		Client:           mgr.GetClient(),
+		scheme:           mgr.GetScheme(),
+		opa:              opa,
+		watcher:          w,
+		statusWatcher:    statusW,
+		cs:               cs,
+		metrics:          r,
+		tracker:          tracker,
+		getPod:           getPod,
+		podOwnershipMode: statusv1beta1.GetPodOwnershipMode(),
 	}
 	if getPod == nil {
 		reconciler.getPod = reconciler.defaultGetPod
@@ -233,6 +234,8 @@ type ReconcileConstraintTemplate struct {
 	metrics       *reporter
 	tracker       *readiness.Tracker
 	getPod        func(context.Context) (*corev1.Pod, error)
+
+	podOwnershipMode statusv1beta1.PodOwnershipMode
 }
 
 // +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch;create;update;patch;delete
@@ -549,7 +552,7 @@ func (r *ReconcileConstraintTemplate) getOrCreatePodStatus(ctx context.Context,
 	if err != nil {
 		return nil, err
 	}
-	statusObj, err = statusv1beta1.NewConstraintTemplateStatusForPod(pod, ctName, r.scheme)
+	statusObj, err = statusv1beta1.NewConstraintTemplateStatusForPod(pod, r.podOwnershipMode, ctName, r.scheme)
 	if err != nil {
 		return nil, err
 	}

pkg/controller/constrainttemplate/constrainttemplate_controller_test.go

@@ -143,7 +143,7 @@ violation[{"msg": "denied!"}] {
 	if err != nil {
 		t.Fatalf("unable to set up OPA backend: %s", err)
 	}
-	opa, err := backend.NewClient(opa.Targets(&target.K8sValidationTarget{}))
+	opaClient, err := backend.NewClient(opa.Targets(&target.K8sValidationTarget{}))
 	if err != nil {
 		t.Fatalf("unable to set up OPA client: %s", err)
 	}
@@ -155,9 +155,10 @@ violation[{"msg": "denied!"}] {
 	g.Expect(err).NotTo(gomega.HaveOccurred())
 	pod := &corev1.Pod{}
 	pod.Name = "no-pod"
+	pod.Namespace = "gatekeeper-system"
 	// events will be used to receive events from dynamic watches registered
 	events := make(chan event.GenericEvent, 1024)
-	rec, _ := newReconciler(mgr, opa, wm, cs, tracker, events, events, func(context.Context) (*corev1.Pod, error) { return pod, nil })
+	rec, _ := newReconciler(mgr, opaClient, wm, cs, tracker, events, events, func(context.Context) (*corev1.Pod, error) { return pod, nil })
 	g.Expect(add(mgr, rec)).NotTo(gomega.HaveOccurred())
 
 	cstr := newDenyAllCstr()
@@ -231,11 +232,11 @@ violation[{"msg": "denied!"}] {
 			Name:      "FooNamespace",
 			Object:    runtime.RawExtension{Object: ns},
 		}
-		resp, err := opa.Review(ctx, req)
+		resp, err := opaClient.Review(ctx, req)
 		g.Expect(err).NotTo(gomega.HaveOccurred())
 		if len(resp.Results()) != 1 {
 			fmt.Println(resp.TraceDump())
-			fmt.Println(opa.Dump(ctx))
+			fmt.Println(opaClient.Dump(ctx))
 		}
 		g.Expect(len(resp.Results())).Should(gomega.Equal(1))
 	})
@@ -363,21 +364,21 @@ violation[{"msg": "denied!"}] {
 			Name:      "FooNamespace",
 			Object:    runtime.RawExtension{Object: ns},
 		}
-		resp, err := opa.Review(ctx, req)
+		resp, err := opaClient.Review(ctx, req)
 		g.Expect(err).NotTo(gomega.HaveOccurred())
 		if len(resp.Results()) != 1 {
 			fmt.Println(resp.TraceDump())
-			fmt.Println(opa.Dump(ctx))
+			fmt.Println(opaClient.Dump(ctx))
 		}
 		g.Expect(len(resp.Results())).Should(gomega.Equal(1))
 		g.Expect(c.Delete(ctx, instance.DeepCopy())).Should(gomega.BeNil())
 		g.Eventually(func() error {
-			resp, err := opa.Review(ctx, req)
+			resp, err := opaClient.Review(ctx, req)
 			if err != nil {
 				return err
 			}
 			if len(resp.Results()) != 0 {
-				dump, _ := opa.Dump(ctx)
+				dump, _ := opaClient.Dump(ctx)
 				return fmt.Errorf("Results not yet zero\nOPA DUMP:\n%s", dump)
 			}
 			return nil
@@ -468,7 +469,7 @@ violation[{"msg": "denied!"}] {
 	if err != nil {
 		t.Fatalf("unable to set up OPA backend: %s", err)
 	}
-	opa, err := backend.NewClient(opa.Targets(&target.K8sValidationTarget{}))
+	opaClient, err := backend.NewClient(opa.Targets(&target.K8sValidationTarget{}))
 	if err != nil {
 		t.Fatalf("unable to set up OPA client: %s", err)
 	}
@@ -478,9 +479,10 @@ violation[{"msg": "denied!"}] {
 	cs := watch.NewSwitch()
 	pod := &corev1.Pod{}
 	pod.Name = "no-pod"
+	pod.Namespace = "gatekeeper-system"
 	// events will be used to receive events from dynamic watches registered
 	events := make(chan event.GenericEvent, 1024)
-	rec, _ := newReconciler(mgr, opa, wm, cs, tracker, events, nil, func(context.Context) (*corev1.Pod, error) { return pod, nil })
+	rec, _ := newReconciler(mgr, opaClient, wm, cs, tracker, events, nil, func(context.Context) (*corev1.Pod, error) { return pod, nil })
 	g.Expect(add(mgr, rec)).NotTo(gomega.HaveOccurred())
 
 	// get the object tracker for the constraint