refactor: make toggles easier to define and use (#4456)

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
Co-authored-by: shuting <shuting@nirmata.com>

cmd/cli/kubectl-kyverno/utils/common/common_test.go

@@ -104,7 +104,7 @@ func Test_NamespaceSelector(t *testing.T) {
 		assert.Equal(t, int64(rc.Pass), int64(tc.result.Pass))
 		assert.Equal(t, int64(rc.Fail), int64(tc.result.Fail))
 		// TODO: autogen rules seem to not be present when autogen internals is disabled
-		if toggle.AutogenInternals() {
+		if toggle.AutogenInternals.Enabled() {
 			assert.Equal(t, int64(rc.Skip), int64(tc.result.Skip))
 		} else {
 			assert.Equal(t, int64(rc.Skip), int64(0))

cmd/kyverno/main.go

@@ -108,10 +108,10 @@ func main() {
 	flag.BoolVar(&autoUpdateWebhooks, "autoUpdateWebhooks", true, "Set this flag to 'false' to disable auto-configuration of the webhook.")
 	flag.Float64Var(&clientRateLimitQPS, "clientRateLimitQPS", 0, "Configure the maximum QPS to the Kubernetes API server from Kyverno. Uses the client default if zero.")
 	flag.IntVar(&clientRateLimitBurst, "clientRateLimitBurst", 0, "Configure the maximum burst for throttle. Uses the client default if zero.")
-	flag.Func(toggle.AutogenInternalsFlagName, toggle.AutogenInternalsDescription, toggle.AutogenInternalsFlag)
+	flag.Func(toggle.AutogenInternalsFlagName, toggle.AutogenInternalsDescription, toggle.AutogenInternals.Parse)
 	flag.DurationVar(&webhookRegistrationTimeout, "webhookRegistrationTimeout", 120*time.Second, "Timeout for webhook registration, e.g., 30s, 1m, 5m.")
 	flag.IntVar(&changeRequestLimit, "maxReportChangeRequests", 1000, "Maximum pending report change requests per namespace or for the cluster-wide policy report.")
-	flag.Func(toggle.SplitPolicyReportFlagName, "Set the flag to 'true', to enable the split-up PolicyReports per policy.", toggle.SplitPolicyReportFlag)
+	flag.Func(toggle.SplitPolicyReportFlagName, toggle.SplitPolicyReportDescription, toggle.SplitPolicyReport.Parse)
 	if err := flag.Set("v", "2"); err != nil {
 		setupLog.Error(err, "failed to set log level")
 		os.Exit(1)

pkg/autogen/autogen.go

@@ -267,7 +267,7 @@ func convertRule(rule kyvernoRule, kind string) (*kyvernov1.Rule, error) {
 }
 
 func ComputeRules(p kyvernov1.PolicyInterface) []kyvernov1.Rule {
-	if !toggle.AutogenInternals() {
+	if !toggle.AutogenInternals.Enabled() {
 		spec := p.GetSpec()
 		return spec.Rules
 	}

pkg/policy/policy_controller.go

@@ -178,7 +178,7 @@ func (pc *PolicyController) addPolicy(obj interface{}) {
 	// register kyverno_policy_changes_total metric concurrently
 	go pc.registerPolicyChangesMetricAddPolicy(logger, p)
 
-	if !toggle.AutogenInternals() {
+	if !toggle.AutogenInternals.Enabled() {
 		if p.Spec.Background == nil || p.Spec.ValidationFailureAction == "" || missingAutoGenRules(p, logger) {
 			pol, _ := utilscommon.MutatePolicy(p, logger)
 			_, err := pc.kyvernoClient.KyvernoV1().ClusterPolicies().Update(context.TODO(), pol.(*kyvernov1.ClusterPolicy), metav1.UpdateOptions{})
@@ -206,7 +206,7 @@ func (pc *PolicyController) updatePolicy(old, cur interface{}) {
 	// register kyverno_policy_changes_total metric concurrently
 	go pc.registerPolicyChangesMetricUpdatePolicy(logger, oldP, curP)
 
-	if !toggle.AutogenInternals() {
+	if !toggle.AutogenInternals.Enabled() {
 		if curP.Spec.Background == nil || curP.Spec.ValidationFailureAction == "" || missingAutoGenRules(curP, logger) {
 			pol, _ := utilscommon.MutatePolicy(curP, logger)
 			_, err := pc.kyvernoClient.KyvernoV1().ClusterPolicies().Update(context.TODO(), pol.(*kyvernov1.ClusterPolicy), metav1.UpdateOptions{})
@@ -269,7 +269,7 @@ func (pc *PolicyController) addNsPolicy(obj interface{}) {
 
 	logger.Info("policy created", "uid", p.UID, "kind", "Policy", "name", p.Name, "namespaces", p.Namespace)
 
-	if !toggle.AutogenInternals() {
+	if !toggle.AutogenInternals.Enabled() {
 		spec := p.GetSpec()
 		if spec.Background == nil || spec.ValidationFailureAction == "" || missingAutoGenRules(p, logger) {
 			nsPol, _ := utilscommon.MutatePolicy(p, logger)
@@ -297,7 +297,7 @@ func (pc *PolicyController) updateNsPolicy(old, cur interface{}) {
 	// register kyverno_policy_changes_total metric concurrently
 	go pc.registerPolicyChangesMetricUpdatePolicy(logger, oldP, curP)
 
-	if !toggle.AutogenInternals() {
+	if !toggle.AutogenInternals.Enabled() {
 		if curP.Spec.Background == nil || curP.Spec.ValidationFailureAction == "" || missingAutoGenRules(curP, logger) {
 			nsPol, _ := utilscommon.MutatePolicy(curP, logger)
 			_, err := pc.kyvernoClient.KyvernoV1().Policies(curP.GetNamespace()).Update(context.TODO(), nsPol.(*kyvernov1.Policy), metav1.UpdateOptions{})

pkg/policy/report.go

@@ -148,7 +148,7 @@ func eraseResultEntries(pclient kyvernoclient.Interface, reportLister policyrepo
 	var polrName string
 
 	if ns != nil {
-		if toggle.SplitPolicyReport() {
+		if toggle.SplitPolicyReport.Enabled() {
 			err = eraseSplitResultEntries(pclient, ns, selector)
 			if err != nil {
 				errors = append(errors, fmt.Sprintf("%v", err))

pkg/policymutation/policymutation.go

@@ -21,7 +21,7 @@ func GenerateJSONPatchesForDefaults(policy kyvernov1.PolicyInterface, log logr.L
 	var updateMsgs []string
 	spec := policy.GetSpec()
 	// if autogenInternals is enabled, we don't mutate most of the policy fields
-	if !toggle.AutogenInternals() {
+	if !toggle.AutogenInternals.Enabled() {
 		// default 'ValidationFailureAction'
 		if patch, updateMsg := defaultvalidationFailureAction(spec, log); patch != nil {
 			patches = append(patches, patch)

pkg/policyreport/builder.go

@@ -47,14 +47,14 @@ const (
 
 func GeneratePolicyReportName(ns, policyName string) string {
 	if ns == "" {
-		if toggle.SplitPolicyReport() {
+		if toggle.SplitPolicyReport.Enabled() {
 			return TrimmedName(clusterpolicyreport + "-" + policyName)
 		}
 		return clusterpolicyreport
 	}
 
 	var name string
-	if toggle.SplitPolicyReport() {
+	if toggle.SplitPolicyReport.Enabled() {
 		name = fmt.Sprintf("polr-ns-%s-%s", ns, policyName)
 	} else {
 		name = fmt.Sprintf("polr-ns-%s", ns)

pkg/policyreport/changerequestcreator.go

@@ -114,7 +114,7 @@ func (c *changeRequestCreator) run(stopChan <-chan struct{}) {
 	ticker := time.NewTicker(c.tickerInterval)
 	defer ticker.Stop()
 
-	if toggle.SplitPolicyReport() {
+	if toggle.SplitPolicyReport.Enabled() {
 		err := CleanupPolicyReport(c.client)
 		if err != nil {
 			c.log.Error(err, "failed to delete old reports")

pkg/policyreport/reportcontroller.go

@@ -130,7 +130,7 @@ func (g *ReportGenerator) generateCacheKey(changeRequest interface{}) string {
 		if ns == "" {
 			ns = "default"
 		}
-		if toggle.SplitPolicyReport() {
+		if toggle.SplitPolicyReport.Enabled() {
 			policy = label[policyLabel]
 			return strings.Join([]string{ns, policy}, "/")
 		} else {
@@ -143,7 +143,7 @@ func (g *ReportGenerator) generateCacheKey(changeRequest interface{}) string {
 		if rule != "" || policy != "" {
 			return strings.Join([]string{deletedPolicyKey, policy, rule}, "/")
 		}
-		if toggle.SplitPolicyReport() {
+		if toggle.SplitPolicyReport.Enabled() {
 			policy = label[policyLabel]
 			return strings.Join([]string{"", policy}, "/")
 		} else {
@@ -337,7 +337,7 @@ func (g *ReportGenerator) syncHandler(key string) (aggregatedRequests interface{
 		return g.removePolicyEntryFromReport(policy, rule)
 	}
 	var namespace, policyName string
-	if toggle.SplitPolicyReport() {
+	if toggle.SplitPolicyReport.Enabled() {
 		namespace = strings.Split(key, "/")[0]
 		policyName = strings.Split(key, "/")[1]
 	} else {
@@ -348,7 +348,7 @@ func (g *ReportGenerator) syncHandler(key string) (aggregatedRequests interface{
 		return aggregatedRequests, fmt.Errorf("failed to aggregate reportChangeRequest results %v", err)
 	}
 
-	if toggle.SplitPolicyReport() {
+	if toggle.SplitPolicyReport.Enabled() {
 		deleteResources := getDeletedResources(aggregatedRequests)
 		if len(deleteResources) != 0 {
 			for _, dr := range deleteResources {
@@ -494,7 +494,7 @@ func (g *ReportGenerator) removeFromClusterPolicyReport(policyName, ruleName str
 			if ruleName != "" && result.Rule == ruleName && result.Policy == policyName {
 				continue
 			} else if ruleName == "" && result.Policy == policyName {
-				if toggle.SplitPolicyReport() {
+				if toggle.SplitPolicyReport.Enabled() {
 					if err := g.pclient.Wgpolicyk8sV1alpha2().ClusterPolicyReports().Delete(context.TODO(), cpolr.GetName(), metav1.DeleteOptions{}); err != nil {
 						if apierrors.IsNotFound(err) {
 							return nil
@@ -538,7 +538,7 @@ func (g *ReportGenerator) removeFromPolicyReport(policyName, ruleName string) er
 			if ruleName != "" && result.Rule == ruleName && result.Policy == policyName {
 				continue
 			} else if ruleName == "" && result.Policy == policyName {
-				if toggle.SplitPolicyReport() {
+				if toggle.SplitPolicyReport.Enabled() {
 					if err := g.pclient.Wgpolicyk8sV1alpha2().PolicyReports(r.GetNamespace()).Delete(context.TODO(), r.GetName(), metav1.DeleteOptions{}); err != nil {
 						if apierrors.IsNotFound(err) {
 							return nil
@@ -579,7 +579,7 @@ func (g *ReportGenerator) aggregateReports(namespace, policyName string) (
 
 	var selector labels.Selector
 	if namespace == "" {
-		if toggle.SplitPolicyReport() {
+		if toggle.SplitPolicyReport.Enabled() {
 			selector = labels.SelectorFromSet(labels.Set(map[string]string{appVersion: version.BuildVersion, policyLabel: TrimmedName(policyName)}))
 		} else {
 			selector = labels.SelectorFromSet(labels.Set(map[string]string{appVersion: version.BuildVersion}))
@@ -605,7 +605,7 @@ func (g *ReportGenerator) aggregateReports(namespace, policyName string) (
 			ns.SetDeletionTimestamp(&now)
 		}
 
-		if toggle.SplitPolicyReport() {
+		if toggle.SplitPolicyReport.Enabled() {
 			selector = labels.SelectorFromSet(labels.Set(map[string]string{appVersion: version.BuildVersion, ResourceLabelNamespace: namespace, policyLabel: TrimmedName(policyName)}))
 		} else {
 			selector = labels.SelectorFromSet(labels.Set(map[string]string{appVersion: version.BuildVersion, ResourceLabelNamespace: namespace}))

pkg/toggle/toggle.go

@@ -8,20 +8,56 @@ import (
 const (
 	AutogenInternalsFlagName     = "autogenInternals"
 	AutogenInternalsDescription  = "Enables autogen internal policies. When this is 'true' policy rules should not be mutated."
-	AutogenInternalsEnvVar      = "FLAG_AUTOGEN_INTERNALS"
-	DefaultAutogenInternals     = true
-
-	// split policy report ...
+	autogenInternalsEnvVar       = "FLAG_AUTOGEN_INTERNALS"
+	defaultAutogenInternals      = true
 	SplitPolicyReportFlagName    = "splitPolicyReport"
-	SplitPolicyReportEnvVar   = "FLAG_SPLIT_POLICY_REPORT"
-	DefaultSplitPolicyReport  = false
+	SplitPolicyReportDescription = "Set the flag to 'true', to enable the split-up PolicyReports per policy."
+	splitPolicyReportEnvVar      = "FLAG_SPLIT_POLICY_REPORT"
+	defaultSplitPolicyReport     = false
 )
 
 var (
-	autogenInternals  *bool
-	splitPolicyReport *bool
+	AutogenInternals  = newToggle(defaultAutogenInternals, autogenInternalsEnvVar)
+	SplitPolicyReport = newToggle(defaultSplitPolicyReport, splitPolicyReportEnvVar)
 )
 
+type Toggle interface {
+	Enabled() bool
+	Parse(string) error
+}
+
+type toggle struct {
+	value        *bool
+	defaultValue bool
+	envVar       string
+}
+
+func newToggle(defaultValue bool, envVar string) *toggle {
+	return &toggle{
+		defaultValue: defaultValue,
+		envVar:       envVar,
+	}
+}
+
+func (t *toggle) Parse(in string) error {
+	if value, err := getBool(in); err != nil {
+		return err
+	} else {
+		t.value = value
+		return nil
+	}
+}
+
+func (t *toggle) Enabled() bool {
+	if t.value != nil {
+		return *t.value
+	}
+	if value, err := getBool(os.Getenv(t.envVar)); err == nil && value != nil {
+		return *value
+	}
+	return t.defaultValue
+}
+
 func getBool(in string) (*bool, error) {
 	if in == "" {
 		return nil, nil
@@ -32,41 +68,3 @@ func getBool(in string) (*bool, error) {
 	}
 	return &value, nil
 }
-
-func AutogenInternalsFlag(in string) error {
-	if value, err := getBool(in); err != nil {
-		return err
-	} else {
-		autogenInternals = value
-		return nil
-	}
-}
-
-func AutogenInternals() bool {
-	if autogenInternals != nil {
-		return *autogenInternals
-	}
-	if value, err := getBool(os.Getenv(AutogenInternalsEnvVar)); err == nil && value != nil {
-		return *value
-	}
-	return DefaultAutogenInternals
-}
-
-func SplitPolicyReportFlag(in string) error {
-	if value, err := getBool(in); err != nil {
-		return err
-	} else {
-		splitPolicyReport = value
-		return nil
-	}
-}
-
-func SplitPolicyReport() bool {
-	if splitPolicyReport != nil {
-		return *splitPolicyReport
-	}
-	if value, err := getBool(os.Getenv(SplitPolicyReportEnvVar)); err == nil && value != nil {
-		return *value
-	}
-	return DefaultSplitPolicyReport
-}

pkg/webhookconfig/configmanager.go

@@ -502,7 +502,7 @@ func (m *webhookConfigManager) updateStatus(namespace, name string, ready bool)
 	update := func(meta *metav1.ObjectMeta, p kyvernov1.PolicyInterface, status *kyvernov1.PolicyStatus) bool {
 		copy := status.DeepCopy()
 		status.SetReady(ready)
-		if toggle.AutogenInternals() {
+		if toggle.AutogenInternals.Enabled() {
 			var rules []kyvernov1.Rule
 			for _, rule := range autogen.ComputeRules(p) {
 				if strings.HasPrefix(rule.Name, "autogen-") {

pkg/webhooks/policy/handlers.go

@@ -53,7 +53,7 @@ func (h *handlers) Validate(logger logr.Logger, request *admissionv1.AdmissionRe
 }
 
 func (h *handlers) Mutate(logger logr.Logger, request *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse {
-	if toggle.AutogenInternals() {
+	if toggle.AutogenInternals.Enabled() {
 		return admissionutils.Response(true)
 	}
 	if request.SubResource != "" {