From 86c813eec0516802ee492ff55c98133dee0dc983 Mon Sep 17 00:00:00 2001 From: mprahl Date: Wed, 6 Dec 2023 13:11:50 -0500 Subject: [PATCH] Use a generic type for simple Enabled/Disabled options Signed-off-by: mprahl --- api/v1alpha1/gatekeeper_types.go | 56 +++++++++++------------ api/v1alpha1/zz_generated.deepcopy.go | 12 ++--- controllers/gatekeeper_controller.go | 22 ++++----- controllers/gatekeeper_controller_test.go | 40 ++++++++-------- test/e2e/gatekeeper_controller_test.go | 10 ++-- 5 files changed, 66 insertions(+), 74 deletions(-) diff --git a/api/v1alpha1/gatekeeper_types.go b/api/v1alpha1/gatekeeper_types.go index dbc12a01..3cdc4085 100644 --- a/api/v1alpha1/gatekeeper_types.go +++ b/api/v1alpha1/gatekeeper_types.go @@ -25,6 +25,26 @@ import ( // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. +// +kubebuilder:validation:Enum:=Enabled;Disabled +type Mode string + +const ( + Enabled Mode = "Enabled" + Disabled Mode = "Disabled" +) + +func (m Mode) ToBool() bool { + return m == Enabled +} + +func (m Mode) ToBoolString() string { + if m.ToBool() { + return "true" + } + + return "false" +} + // GatekeeperSpec defines the desired state of Gatekeeper type GatekeeperSpec struct { // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster @@ -41,11 +61,11 @@ type GatekeeperSpec struct { // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Validating Webhook" // +optional - ValidatingWebhook *WebhookMode `json:"validatingWebhook,omitempty"` + ValidatingWebhook *Mode `json:"validatingWebhook,omitempty"` // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Mutating Webhook" // +optional - MutatingWebhook *WebhookMode `json:"mutatingWebhook,omitempty"` + MutatingWebhook *Mode `json:"mutatingWebhook,omitempty"` // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Webhook Config" // +optional @@ -101,21 +121,13 @@ type AuditConfig struct { // +optional LogLevel *LogLevelMode `json:"logLevel,omitempty"` // +optional - EmitAuditEvents *EmitEventsMode `json:"emitAuditEvents,omitempty"` + EmitAuditEvents *Mode `json:"emitAuditEvents,omitempty"` // +optional - AuditEventsInvolvedNamespace *EventsInvolvedNsMode `json:"auditEventsInvolvedNamespace,omitempty"` + AuditEventsInvolvedNamespace *Mode `json:"auditEventsInvolvedNamespace,omitempty"` // +optional Resources *corev1.ResourceRequirements `json:"resources,omitempty"` } -// +kubebuilder:validation:Enum:=Enabled;Disabled -type WebhookMode string - -const ( - WebhookEnabled WebhookMode = "Enabled" - WebhookDisabled WebhookMode = "Disabled" -) - type WebhookConfig struct { // +kubebuilder:validation:Minimum:=0 // +optional @@ -123,9 +135,9 @@ type WebhookConfig struct { // +optional LogLevel *LogLevelMode `json:"logLevel,omitempty"` // +optional - EmitAdmissionEvents *EmitEventsMode `json:"emitAdmissionEvents,omitempty"` + EmitAdmissionEvents *Mode `json:"emitAdmissionEvents,omitempty"` // +optional - AdmissionEventsInvolvedNamespace *EventsInvolvedNsMode `json:"admissionEventsInvolvedNamespace,omitempty"` + AdmissionEventsInvolvedNamespace *Mode `json:"admissionEventsInvolvedNamespace,omitempty"` // +optional FailurePolicy *admregv1.FailurePolicyType `json:"failurePolicy,omitempty"` // +optional @@ -155,22 +167,6 @@ const ( AuditFromCacheAutomatic AuditFromCacheMode = "Automatic" ) -// +kubebuilder:validation:Enum:=Enabled;Disabled -type EmitEventsMode string - -const ( - EmitEventsEnabled EmitEventsMode = "Enabled" - EmitEventsDisabled EmitEventsMode = "Disabled" -) - -// +kubebuilder:validation:Enum:=Enabled;Disabled -type EventsInvolvedNsMode string - -const ( - EventsInvolvedNsModeEnabled EventsInvolvedNsMode = "Enabled" - EventsInvolvedNsModeDisabled EventsInvolvedNsMode = "Disabled" -) - // GatekeeperStatus defines the observed state of Gatekeeper type GatekeeperStatus struct { // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b66df06a..99d5d947 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -63,12 +63,12 @@ func (in *AuditConfig) DeepCopyInto(out *AuditConfig) { } if in.EmitAuditEvents != nil { in, out := &in.EmitAuditEvents, &out.EmitAuditEvents - *out = new(EmitEventsMode) + *out = new(Mode) **out = **in } if in.AuditEventsInvolvedNamespace != nil { in, out := &in.AuditEventsInvolvedNamespace, &out.AuditEventsInvolvedNamespace - *out = new(EventsInvolvedNsMode) + *out = new(Mode) **out = **in } if in.Resources != nil { @@ -162,12 +162,12 @@ func (in *GatekeeperSpec) DeepCopyInto(out *GatekeeperSpec) { } if in.ValidatingWebhook != nil { in, out := &in.ValidatingWebhook, &out.ValidatingWebhook - *out = new(WebhookMode) + *out = new(Mode) **out = **in } if in.MutatingWebhook != nil { in, out := &in.MutatingWebhook, &out.MutatingWebhook - *out = new(WebhookMode) + *out = new(Mode) **out = **in } if in.Webhook != nil { @@ -299,12 +299,12 @@ func (in *WebhookConfig) DeepCopyInto(out *WebhookConfig) { } if in.EmitAdmissionEvents != nil { in, out := &in.EmitAdmissionEvents, &out.EmitAdmissionEvents - *out = new(EmitEventsMode) + *out = new(Mode) **out = **in } if in.AdmissionEventsInvolvedNamespace != nil { in, out := &in.AdmissionEventsInvolvedNamespace, &out.AdmissionEventsInvolvedNamespace - *out = new(EventsInvolvedNsMode) + *out = new(Mode) **out = **in } if in.FailurePolicy != nil { diff --git a/controllers/gatekeeper_controller.go b/controllers/gatekeeper_controller.go index 50852a43..f9ea0ff0 100644 --- a/controllers/gatekeeper_controller.go +++ b/controllers/gatekeeper_controller.go @@ -380,8 +380,9 @@ func (r *GatekeeperReconciler) validateWebhookDeployment() (error, bool) { } func getStaticAssets(gatekeeper *operatorv1alpha1.Gatekeeper) (deleteWebhookAssets, applyOrderedAssets, applyWebhookAssets, deleteCRDAssets []string) { - validatingWebhookEnabled := gatekeeper.Spec.ValidatingWebhook == nil || *gatekeeper.Spec.ValidatingWebhook == operatorv1alpha1.WebhookEnabled + validatingWebhookEnabled := gatekeeper.Spec.ValidatingWebhook == nil || gatekeeper.Spec.ValidatingWebhook.ToBool() mutatingWebhookEnabled := mutatingWebhookEnabled(gatekeeper.Spec.MutatingWebhook) + deleteWebhookAssets = make([]string, 0) applyOrderedAssets = make([]string, 0) applyWebhookAssets = make([]string, 0) @@ -407,8 +408,8 @@ func getStaticAssets(gatekeeper *operatorv1alpha1.Gatekeeper) (deleteWebhookAsse return } -func mutatingWebhookEnabled(mode *operatorv1alpha1.WebhookMode) bool { - return mode == nil || *mode == operatorv1alpha1.WebhookEnabled +func mutatingWebhookEnabled(mode *operatorv1alpha1.Mode) bool { + return mode == nil || mode.ToBool() } func getSubsetOfAssets(inputAssets []string, assetsToRemove ...string) []string { @@ -865,25 +866,20 @@ func setAuditChunkSize(obj *unstructured.Unstructured, auditChunkSize *uint64) e return nil } -func setEmitEvents(obj *unstructured.Unstructured, argName string, emitEvents *operatorv1alpha1.EmitEventsMode) error { +func setEmitEvents(obj *unstructured.Unstructured, argName string, emitEvents *operatorv1alpha1.Mode) error { if emitEvents != nil { - emitArgValue := "false" - if *emitEvents == operatorv1alpha1.EmitEventsEnabled { - emitArgValue = "true" - } + emitArgValue := emitEvents.ToBoolString() + return setContainerArg(obj, managerContainer, argName, emitArgValue, false) } return nil } func setEventsInvolvedNamespace(obj *unstructured.Unstructured, - argName string, eventsInvolvedNs *operatorv1alpha1.EventsInvolvedNsMode, + argName string, eventsInvolvedNs *operatorv1alpha1.Mode, ) error { if eventsInvolvedNs != nil { - emitEventsInvolvedNsArgValue := "false" - if *eventsInvolvedNs == operatorv1alpha1.EventsInvolvedNsModeEnabled { - emitEventsInvolvedNsArgValue = "true" - } + emitEventsInvolvedNsArgValue := eventsInvolvedNs.ToBoolString() return setContainerArg(obj, managerContainer, argName, emitEventsInvolvedNsArgValue, false) } diff --git a/controllers/gatekeeper_controller_test.go b/controllers/gatekeeper_controller_test.go index b58ed2a7..9c6573f3 100644 --- a/controllers/gatekeeper_controller_test.go +++ b/controllers/gatekeeper_controller_test.go @@ -55,8 +55,8 @@ func TestDeployWebhookConfigs(t *testing.T) { g.Expect(deleteWebhookAssets).NotTo(ContainElement(MutatingCRDs)) g.Expect(deleteCRDAssets).NotTo(ContainElements(MutatingCRDs)) - webhookEnabled := operatorv1alpha1.WebhookEnabled - webhookDisabled := operatorv1alpha1.WebhookDisabled + webhookEnabled := operatorv1alpha1.Enabled + webhookDisabled := operatorv1alpha1.Disabled // ValidatingWebhookConfiguration enabled // MutatingWebhookConfiguration enabled @@ -705,7 +705,7 @@ func TestFailurePolicy(t *testing.T) { webhook := operatorv1alpha1.WebhookConfig{ FailurePolicy: &failurePolicy, } - mutatingWebhook := operatorv1alpha1.WebhookEnabled + mutatingWebhook := operatorv1alpha1.Enabled gatekeeper := &operatorv1alpha1.Gatekeeper{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -796,7 +796,7 @@ func TestNamespaceSelector(t *testing.T) { webhook := operatorv1alpha1.WebhookConfig{ NamespaceSelector: &namespaceSelector, } - mutatingWebhook := operatorv1alpha1.WebhookEnabled + mutatingWebhook := operatorv1alpha1.Enabled gatekeeper := &operatorv1alpha1.Gatekeeper{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -1005,7 +1005,7 @@ func TestAuditFromCache(t *testing.T) { func TestEmitAuditEvents(t *testing.T) { g := NewWithT(t) - emitEvents := operatorv1alpha1.EmitEventsEnabled + emitEvents := operatorv1alpha1.Enabled auditOverride := operatorv1alpha1.AuditConfig{ EmitAuditEvents: &emitEvents, } @@ -1033,7 +1033,7 @@ func TestEmitAuditEvents(t *testing.T) { func TestAuditEventsInvolvedNamespace(t *testing.T) { g := NewWithT(t) - auditInvolvedNamespace := operatorv1alpha1.EventsInvolvedNsModeEnabled + auditInvolvedNamespace := operatorv1alpha1.Enabled auditOverride := operatorv1alpha1.AuditConfig{ AuditEventsInvolvedNamespace: &auditInvolvedNamespace, } @@ -1064,12 +1064,12 @@ func TestAllAuditArgs(t *testing.T) { auditChunkSize := uint64(10) auditFromCache := operatorv1alpha1.AuditFromCacheEnabled constraintViolationLimit := uint64(20) - emitEvents := operatorv1alpha1.EmitEventsEnabled + emitEvents := operatorv1alpha1.Enabled logLevel := operatorv1alpha1.LogLevelDEBUG auditInterval := metav1.Duration{ Duration: time.Hour, } - auditInvolvedNamespace := operatorv1alpha1.EventsInvolvedNsModeEnabled + auditInvolvedNamespace := operatorv1alpha1.Enabled auditOverride := operatorv1alpha1.AuditConfig{ AuditChunkSize: &auditChunkSize, @@ -1122,7 +1122,7 @@ func TestAllAuditArgs(t *testing.T) { expectObjContainerArgument(g, managerContainer, auditObj).To(HaveKeyWithValue(AuditEventsInvolvedNamespaceArg, "true")) expectObjContainerArgument(g, managerContainer, auditObj).To(HaveKeyWithValue(OperationArg, OperationMutationStatus)) // test override with mutation - mutatingWebhook := operatorv1alpha1.WebhookEnabled + mutatingWebhook := operatorv1alpha1.Enabled gatekeeper.Spec.MutatingWebhook = &mutatingWebhook err = crOverrides(gatekeeper, AuditFile, auditObj, namespace, false, false) g.Expect(err).ToNot(HaveOccurred()) @@ -1138,7 +1138,7 @@ func TestAllAuditArgs(t *testing.T) { func TestEmitAdmissionEvents(t *testing.T) { g := NewWithT(t) - emitEvents := operatorv1alpha1.EmitEventsEnabled + emitEvents := operatorv1alpha1.Enabled webhookOverride := operatorv1alpha1.WebhookConfig{ EmitAdmissionEvents: &emitEvents, } @@ -1166,7 +1166,7 @@ func TestEmitAdmissionEvents(t *testing.T) { func TestAdmissionEventsInvolvedNamespace(t *testing.T) { g := NewWithT(t) - admissionEventsInvolvedNamespace := operatorv1alpha1.EventsInvolvedNsModeEnabled + admissionEventsInvolvedNamespace := operatorv1alpha1.Enabled webhookOverride := operatorv1alpha1.WebhookConfig{ AdmissionEventsInvolvedNamespace: &admissionEventsInvolvedNamespace, } @@ -1246,7 +1246,7 @@ func TestMutationArg(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) expectObjContainerArgument(g, managerContainer, auditObj).To(HaveKeyWithValue(OperationArg, OperationMutationStatus)) // test disabled override - mutation := operatorv1alpha1.WebhookDisabled + mutation := operatorv1alpha1.Disabled gatekeeper.Spec.MutatingWebhook = &mutation err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, false, false) g.Expect(err).ToNot(HaveOccurred()) @@ -1255,7 +1255,7 @@ func TestMutationArg(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) expectObjContainerArgument(g, managerContainer, auditObj).NotTo(HaveKeyWithValue(OperationArg, OperationMutationStatus)) // test enabled override - mutation = operatorv1alpha1.WebhookEnabled + mutation = operatorv1alpha1.Enabled err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, false, false) g.Expect(err).ToNot(HaveOccurred()) expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(OperationArg, OperationMutationWebhook)) @@ -1299,9 +1299,9 @@ func TestDisabledBuiltins(t *testing.T) { func TestAllWebhookArgs(t *testing.T) { g := NewWithT(t) - emitEvents := operatorv1alpha1.EmitEventsEnabled + emitEvents := operatorv1alpha1.Enabled logLevel := operatorv1alpha1.LogLevelDEBUG - admissionEventsInvolvedNamespace := operatorv1alpha1.EventsInvolvedNsModeEnabled + admissionEventsInvolvedNamespace := operatorv1alpha1.Enabled webhookOverride := operatorv1alpha1.WebhookConfig{ EmitAdmissionEvents: &emitEvents, LogLevel: &logLevel, @@ -1337,7 +1337,7 @@ func TestAllWebhookArgs(t *testing.T) { expectObjContainerArgument(g, managerContainer, webhookObj).To(HaveKeyWithValue(LogLevelArg, "DEBUG")) expectObjContainerArgument(g, managerContainer, webhookObj).NotTo(HaveKey(EnableMutationArg)) // test override with mutation - mutatingWebhook := operatorv1alpha1.WebhookEnabled + mutatingWebhook := operatorv1alpha1.Enabled gatekeeper.Spec.MutatingWebhook = &mutatingWebhook err = crOverrides(gatekeeper, WebhookFile, webhookObj, namespace, false, false) g.Expect(err).ToNot(HaveOccurred()) @@ -1464,7 +1464,7 @@ func TestMutationRBACConfig(t *testing.T) { // Test RBAC config when mutating webhook mode is disabled obj = clusterRoleObj.DeepCopy() - mutation := operatorv1alpha1.WebhookDisabled + mutation := operatorv1alpha1.Disabled gatekeeper.Spec.MutatingWebhook = &mutation err = crOverrides(gatekeeper, ClusterRoleFile, obj, namespace, false, false) g.Expect(err).ToNot(HaveOccurred()) @@ -1483,7 +1483,7 @@ func TestMutationRBACConfig(t *testing.T) { // Test RBAC config when mutating webhook mode is enabled obj = clusterRoleObj.DeepCopy() - mutation = operatorv1alpha1.WebhookEnabled + mutation = operatorv1alpha1.Enabled gatekeeper.Spec.MutatingWebhook = &mutation err = crOverrides(gatekeeper, ClusterRoleFile, obj, namespace, false, false) g.Expect(err).ToNot(HaveOccurred()) @@ -1506,7 +1506,7 @@ func TestMutationRBACConfig(t *testing.T) { // Test RBAC config when mutating webhook mode is disabled obj = clusterRoleObj.DeepCopy() - mutation = operatorv1alpha1.WebhookDisabled + mutation = operatorv1alpha1.Disabled gatekeeper.Spec.MutatingWebhook = &mutation err = crOverrides(gatekeeper, ClusterRoleFile, obj, namespace, false, false) g.Expect(err).ToNot(HaveOccurred()) @@ -1525,7 +1525,7 @@ func TestMutationRBACConfig(t *testing.T) { // Test RBAC config when mutating webhook mode is enabled obj = clusterRoleObj.DeepCopy() - mutation = operatorv1alpha1.WebhookEnabled + mutation = operatorv1alpha1.Enabled gatekeeper.Spec.MutatingWebhook = &mutation err = crOverrides(gatekeeper, ClusterRoleFile, obj, namespace, false, false) g.Expect(err).ToNot(HaveOccurred()) diff --git a/test/e2e/gatekeeper_controller_test.go b/test/e2e/gatekeeper_controller_test.go index ac0c7cb1..54f8c7e1 100644 --- a/test/e2e/gatekeeper_controller_test.go +++ b/test/e2e/gatekeeper_controller_test.go @@ -418,7 +418,7 @@ var _ = Describe("Gatekeeper", func() { }) By("Updating Gatekeeper CR with validation disabled", func() { - webhookMode := v1alpha1.WebhookDisabled + webhookMode := v1alpha1.Disabled gatekeeper.Spec.ValidatingWebhook = &webhookMode Expect(K8sClient.Update(ctx, gatekeeper)).Should(Succeed()) }) @@ -429,7 +429,7 @@ var _ = Describe("Gatekeeper", func() { It("Enables Gatekeeper mutation with default values", func() { gatekeeper := emptyGatekeeper() - webhookMode := v1alpha1.WebhookEnabled + webhookMode := v1alpha1.Enabled gatekeeper.Spec.MutatingWebhook = &webhookMode Expect(K8sClient.Create(ctx, gatekeeper)).Should(Succeed()) auditDeployment, webhookDeployment := gatekeeperDeployments() @@ -451,7 +451,7 @@ var _ = Describe("Gatekeeper", func() { gatekeeper := emptyGatekeeper() err := loadGatekeeperFromFile(gatekeeper, gatekeeperWithAllValuesFile) Expect(err).ToNot(HaveOccurred()) - webhookMode := v1alpha1.WebhookEnabled + webhookMode := v1alpha1.Enabled gatekeeper.Spec.MutatingWebhook = &webhookMode Expect(K8sClient.Create(ctx, gatekeeper)).Should(Succeed()) auditDeployment, webhookDeployment := gatekeeperDeployments() @@ -472,7 +472,7 @@ var _ = Describe("Gatekeeper", func() { It("Enables then disables Gatekeeper mutation", func() { gatekeeper := emptyGatekeeper() By("First creating Gatekeeper CR with mutation enabled", func() { - webhookMode := v1alpha1.WebhookEnabled + webhookMode := v1alpha1.Enabled gatekeeper.Spec.MutatingWebhook = &webhookMode Expect(K8sClient.Create(ctx, gatekeeper)).Should(Succeed()) }) @@ -486,7 +486,7 @@ var _ = Describe("Gatekeeper", func() { }) By("Updating Gatekeeper CR with mutation disabled", func() { - webhookMode := v1alpha1.WebhookDisabled + webhookMode := v1alpha1.Disabled gatekeeper.Spec.MutatingWebhook = &webhookMode Expect(K8sClient.Update(ctx, gatekeeper)).Should(Succeed()) })