From f9f01fc70d482cdd199005fb7ab39b62669efb43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Mon, 28 Nov 2022 16:53:26 +0100 Subject: [PATCH] chore: refactor metrics namespace check (#5489) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Charles-Edouard Brétéché --- pkg/config/metricsconfig.go | 20 ++++++++++++-- .../admissionrequests/admissionRequests.go | 13 ++------- .../admissionReviewDuration.go | 13 ++------- pkg/metrics/policychanges/policyChanges.go | 22 +++------------ .../policyExecutionDuration.go | 25 ++++------------- pkg/metrics/policyresults/policyResults.go | 24 ++++------------- pkg/metrics/policyruleinfo/policyRuleInfo.go | 27 +++++++------------ 7 files changed, 45 insertions(+), 99 deletions(-) diff --git a/pkg/config/metricsconfig.go b/pkg/config/metricsconfig.go index 6fff50c6e3..ddeea43a1c 100644 --- a/pkg/config/metricsconfig.go +++ b/pkg/config/metricsconfig.go @@ -6,6 +6,7 @@ import ( "os" "time" + "golang.org/x/exp/slices" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -24,8 +25,8 @@ type MetricsConfiguration interface { GetIncludeNamespaces() []string // GetMetricsRefreshInterval returns the refresh interval for the metrics GetMetricsRefreshInterval() time.Duration - // namespaces namespacesConfig - // metricsRefreshInterval time.Duration + // CheckNamespace returns `true` if the namespace has to be considered + CheckNamespace(string) bool } type namespacesConfig struct { @@ -54,6 +55,21 @@ func (mcd *metricsConfig) GetMetricsRefreshInterval() time.Duration { return mcd.metricsRefreshInterval } +// CheckNamespace returns `true` if the namespace has to be considered +func (mcd *metricsConfig) CheckNamespace(namespace string) bool { + // TODO(eddycharly): check we actually need `"-"` + if namespace == "" || namespace == "-" { + return true + } + if slices.Contains(mcd.namespaces.ExcludeNamespaces, namespace) { + return false + } + if len(mcd.namespaces.IncludeNamespaces) == 0 { + return true + } + return slices.Contains(mcd.namespaces.IncludeNamespaces, namespace) +} + func (cd *metricsConfig) load(cm *corev1.ConfigMap) { logger := logger.WithValues("name", cm.Name, "namespace", cm.Namespace) if cm.Data == nil { diff --git a/pkg/metrics/admissionrequests/admissionRequests.go b/pkg/metrics/admissionrequests/admissionRequests.go index 6e4459c198..4b76090cf2 100644 --- a/pkg/metrics/admissionrequests/admissionRequests.go +++ b/pkg/metrics/admissionrequests/admissionRequests.go @@ -2,25 +2,16 @@ package admissionrequests import ( "context" - "fmt" "strings" "github.com/kyverno/kyverno/pkg/metrics" - "github.com/kyverno/kyverno/pkg/utils" admissionv1 "k8s.io/api/admission/v1" ) func registerAdmissionRequestsMetric(ctx context.Context, m *metrics.MetricsConfig, resourceKind, resourceNamespace string, resourceRequestOperation metrics.ResourceRequestOperation, allowed bool) { - includeNamespaces, excludeNamespaces := m.Config.GetIncludeNamespaces(), m.Config.GetExcludeNamespaces() - if (resourceNamespace != "" && resourceNamespace != "-") && utils.ContainsString(excludeNamespaces, resourceNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_admission_requests_total metric as the operation belongs to the namespace '%s' which is one of 'namespaces.exclude' %+v in values.yaml", resourceNamespace, excludeNamespaces)) - return + if m.Config.CheckNamespace(resourceNamespace) { + m.RecordAdmissionRequests(ctx, resourceKind, resourceNamespace, resourceRequestOperation, allowed) } - if (resourceNamespace != "" && resourceNamespace != "-") && len(includeNamespaces) > 0 && !utils.ContainsString(includeNamespaces, resourceNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_admission_requests_total metric as the operation belongs to the namespace '%s' which is not one of 'namespaces.include' %+v in values.yaml", resourceNamespace, includeNamespaces)) - return - } - m.RecordAdmissionRequests(ctx, resourceKind, resourceNamespace, resourceRequestOperation, allowed) } func Process(ctx context.Context, m *metrics.MetricsConfig, request *admissionv1.AdmissionRequest, response *admissionv1.AdmissionResponse) { diff --git a/pkg/metrics/admissionreviewduration/admissionReviewDuration.go b/pkg/metrics/admissionreviewduration/admissionReviewDuration.go index 87ac1b0dda..96b31ccf6c 100644 --- a/pkg/metrics/admissionreviewduration/admissionReviewDuration.go +++ b/pkg/metrics/admissionreviewduration/admissionReviewDuration.go @@ -2,25 +2,16 @@ package admissionreviewduration import ( "context" - "fmt" "strings" "github.com/kyverno/kyverno/pkg/metrics" - "github.com/kyverno/kyverno/pkg/utils" admissionv1 "k8s.io/api/admission/v1" ) func registerAdmissionReviewDurationMetric(ctx context.Context, m *metrics.MetricsConfig, resourceKind, resourceNamespace string, resourceRequestOperation metrics.ResourceRequestOperation, admissionRequestLatency float64, allowed bool) { - includeNamespaces, excludeNamespaces := m.Config.GetIncludeNamespaces(), m.Config.GetExcludeNamespaces() - if (resourceNamespace != "" && resourceNamespace != "-") && utils.ContainsString(excludeNamespaces, resourceNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_admission_review_duration_seconds metric as the operation belongs to the namespace '%s' which is one of 'namespaces.exclude' %+v in values.yaml", resourceNamespace, excludeNamespaces)) - return + if m.Config.CheckNamespace(resourceNamespace) { + m.RecordAdmissionReviewDuration(ctx, resourceKind, resourceNamespace, string(resourceRequestOperation), admissionRequestLatency, allowed) } - if (resourceNamespace != "" && resourceNamespace != "-") && len(includeNamespaces) > 0 && !utils.ContainsString(includeNamespaces, resourceNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_admission_review_duration_seconds metric as the operation belongs to the namespace '%s' which is not one of 'namespaces.include' %+v in values.yaml", resourceNamespace, includeNamespaces)) - return - } - m.RecordAdmissionReviewDuration(ctx, resourceKind, resourceNamespace, string(resourceRequestOperation), admissionRequestLatency, allowed) } func Process(ctx context.Context, m *metrics.MetricsConfig, request *admissionv1.AdmissionRequest, response *admissionv1.AdmissionResponse, latency int64) { diff --git a/pkg/metrics/policychanges/policyChanges.go b/pkg/metrics/policychanges/policyChanges.go index b772a624c6..3bb3b25d64 100644 --- a/pkg/metrics/policychanges/policyChanges.go +++ b/pkg/metrics/policychanges/policyChanges.go @@ -2,11 +2,9 @@ package policychanges import ( "context" - "fmt" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/metrics" - "github.com/kyverno/kyverno/pkg/utils" ) func registerPolicyChangesMetric( @@ -17,23 +15,13 @@ func registerPolicyChangesMetric( policyBackgroundMode metrics.PolicyBackgroundMode, policyNamespace, policyName string, policyChangeType PolicyChangeType, -) error { +) { if policyType == metrics.Cluster { policyNamespace = "-" } - includeNamespaces, excludeNamespaces := m.Config.GetIncludeNamespaces(), m.Config.GetExcludeNamespaces() - if (policyNamespace != "" && policyNamespace != "-") && utils.ContainsString(excludeNamespaces, policyNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_policy_changes_total metric as the operation belongs to the namespace '%s' which is one of 'namespaces.exclude' %+v in values.yaml", policyNamespace, excludeNamespaces)) - return nil + if m.Config.CheckNamespace(policyNamespace) { + m.RecordPolicyChanges(ctx, policyValidationMode, policyType, policyBackgroundMode, policyNamespace, policyName, string(policyChangeType)) } - if (policyNamespace != "" && policyNamespace != "-") && len(includeNamespaces) > 0 && !utils.ContainsString(includeNamespaces, policyNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_policy_changes_total metric as the operation belongs to the namespace '%s' which is not one of 'namespaces.include' %+v in values.yaml", policyNamespace, includeNamespaces)) - return nil - } - - m.RecordPolicyChanges(ctx, policyValidationMode, policyType, policyBackgroundMode, policyNamespace, policyName, string(policyChangeType)) - - return nil } func RegisterPolicy(ctx context.Context, m *metrics.MetricsConfig, policy kyvernov1.PolicyInterface, policyChangeType PolicyChangeType) error { @@ -41,8 +29,6 @@ func RegisterPolicy(ctx context.Context, m *metrics.MetricsConfig, policy kyvern if err != nil { return err } - if err = registerPolicyChangesMetric(ctx, m, validationMode, policyType, backgroundMode, namespace, name, policyChangeType); err != nil { - return err - } + registerPolicyChangesMetric(ctx, m, validationMode, policyType, backgroundMode, namespace, name, policyChangeType) return nil } diff --git a/pkg/metrics/policyexecutionduration/policyExecutionDuration.go b/pkg/metrics/policyexecutionduration/policyExecutionDuration.go index 8c2db750f7..eb54c41880 100644 --- a/pkg/metrics/policyexecutionduration/policyExecutionDuration.go +++ b/pkg/metrics/policyexecutionduration/policyExecutionDuration.go @@ -2,12 +2,10 @@ package policyexecutionduration import ( "context" - "fmt" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/engine/response" "github.com/kyverno/kyverno/pkg/metrics" - "github.com/kyverno/kyverno/pkg/utils" ) func registerPolicyExecutionDurationMetric( @@ -23,24 +21,13 @@ func registerPolicyExecutionDurationMetric( ruleType metrics.RuleType, ruleExecutionCause metrics.RuleExecutionCause, ruleExecutionLatency float64, -) error { +) { if policyType == metrics.Cluster { policyNamespace = "-" } - - includeNamespaces, excludeNamespaces := m.Config.GetIncludeNamespaces(), m.Config.GetExcludeNamespaces() - if (resourceNamespace != "" && resourceNamespace != "-") && utils.ContainsString(excludeNamespaces, resourceNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_policy_execution_duration_seconds metric as the operation belongs to the namespace '%s' which is one of 'namespaces.exclude' %+v in values.yaml", resourceNamespace, excludeNamespaces)) - return nil + if m.Config.CheckNamespace(policyNamespace) { + m.RecordPolicyExecutionDuration(ctx, policyValidationMode, policyType, policyBackgroundMode, policyNamespace, policyName, ruleName, ruleResult, ruleType, ruleExecutionCause, ruleExecutionLatency) } - if (resourceNamespace != "" && resourceNamespace != "-") && len(includeNamespaces) > 0 && !utils.ContainsString(includeNamespaces, resourceNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_policy_execution_duration_seconds metric as the operation belongs to the namespace '%s' which is not one of 'namespaces.include' %+v in values.yaml", resourceNamespace, includeNamespaces)) - return nil - } - - m.RecordPolicyExecutionDuration(ctx, policyValidationMode, policyType, policyBackgroundMode, policyNamespace, policyName, ruleName, ruleResult, ruleType, ruleExecutionCause, ruleExecutionLatency) - - return nil } // policy - policy related data @@ -72,7 +59,7 @@ func ProcessEngineResponse(ctx context.Context, m *metrics.MetricsConfig, policy ruleResult = metrics.Fail } ruleExecutionLatencyInSeconds := float64(rule.RuleStats.ProcessingTime) / float64(1000*1000*1000) - if err := registerPolicyExecutionDurationMetric( + registerPolicyExecutionDurationMetric( ctx, m, validationMode, @@ -85,9 +72,7 @@ func ProcessEngineResponse(ctx context.Context, m *metrics.MetricsConfig, policy ruleType, executionCause, ruleExecutionLatencyInSeconds, - ); err != nil { - return err - } + ) } return nil } diff --git a/pkg/metrics/policyresults/policyResults.go b/pkg/metrics/policyresults/policyResults.go index b95df71014..6b0a52ddb5 100644 --- a/pkg/metrics/policyresults/policyResults.go +++ b/pkg/metrics/policyresults/policyResults.go @@ -2,12 +2,10 @@ package policyresults import ( "context" - "fmt" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/engine/response" "github.com/kyverno/kyverno/pkg/metrics" - "github.com/kyverno/kyverno/pkg/utils" ) func registerPolicyResultsMetric( @@ -23,23 +21,13 @@ func registerPolicyResultsMetric( ruleResult metrics.RuleResult, ruleType metrics.RuleType, ruleExecutionCause metrics.RuleExecutionCause, -) error { +) { if policyType == metrics.Cluster { policyNamespace = "-" } - includeNamespaces, excludeNamespaces := m.Config.GetIncludeNamespaces(), m.Config.GetExcludeNamespaces() - if (resourceNamespace != "" && resourceNamespace != "-") && utils.ContainsString(excludeNamespaces, resourceNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_policy_results_total metric as the operation belongs to the namespace '%s' which is one of 'namespaces.exclude' %+v in values.yaml", resourceNamespace, excludeNamespaces)) - return nil + if m.Config.CheckNamespace(policyNamespace) { + m.RecordPolicyResults(ctx, policyValidationMode, policyType, policyBackgroundMode, policyNamespace, policyName, resourceKind, resourceNamespace, resourceRequestOperation, ruleName, ruleResult, ruleType, ruleExecutionCause) } - if (resourceNamespace != "" && resourceNamespace != "-") && len(includeNamespaces) > 0 && !utils.ContainsString(includeNamespaces, resourceNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_policy_results_total metric as the operation belongs to the namespace '%s' which is not one of 'namespaces.include' %+v in values.yaml", resourceNamespace, includeNamespaces)) - return nil - } - - m.RecordPolicyResults(ctx, policyValidationMode, policyType, policyBackgroundMode, policyNamespace, policyName, resourceKind, resourceNamespace, resourceRequestOperation, ruleName, ruleResult, ruleType, ruleExecutionCause) - - return nil } // policy - policy related data @@ -71,7 +59,7 @@ func ProcessEngineResponse(ctx context.Context, m *metrics.MetricsConfig, policy default: ruleResult = metrics.Fail } - if err := registerPolicyResultsMetric( + registerPolicyResultsMetric( ctx, m, validationMode, @@ -84,9 +72,7 @@ func ProcessEngineResponse(ctx context.Context, m *metrics.MetricsConfig, policy ruleResult, ruleType, executionCause, - ); err != nil { - return err - } + ) } return nil } diff --git a/pkg/metrics/policyruleinfo/policyRuleInfo.go b/pkg/metrics/policyruleinfo/policyRuleInfo.go index 3c96114d82..84e9048f8e 100644 --- a/pkg/metrics/policyruleinfo/policyRuleInfo.go +++ b/pkg/metrics/policyruleinfo/policyRuleInfo.go @@ -7,7 +7,6 @@ import ( kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/autogen" "github.com/kyverno/kyverno/pkg/metrics" - "github.com/kyverno/kyverno/pkg/utils" ) func registerPolicyRuleInfoMetric( @@ -30,24 +29,16 @@ func registerPolicyRuleInfoMetric( default: return fmt.Errorf("unknown metric change type found: %s", metricChangeType) } - includeNamespaces, excludeNamespaces := m.Config.GetIncludeNamespaces(), m.Config.GetExcludeNamespaces() - if (policyNamespace != "" && policyNamespace != "-") && utils.ContainsString(excludeNamespaces, policyNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_policy_rule_info_total metric as the operation belongs to the namespace '%s' which is one of 'namespaces.exclude' %+v in values.yaml", policyNamespace, excludeNamespaces)) - return nil + if m.Config.CheckNamespace(policyNamespace) { + if policyType == metrics.Cluster { + policyNamespace = "-" + } + status := "false" + if ready { + status = "true" + } + m.RecordPolicyRuleInfo(ctx, policyValidationMode, policyType, policyBackgroundMode, policyNamespace, policyName, ruleName, ruleType, status, metricValue) } - if (policyNamespace != "" && policyNamespace != "-") && len(includeNamespaces) > 0 && !utils.ContainsString(includeNamespaces, policyNamespace) { - m.Log.V(2).Info(fmt.Sprintf("Skipping the registration of kyverno_policy_rule_info_total metric as the operation belongs to the namespace '%s' which is not one of 'namespaces.include' %+v in values.yaml", policyNamespace, includeNamespaces)) - return nil - } - if policyType == metrics.Cluster { - policyNamespace = "-" - } - status := "false" - if ready { - status = "true" - } - m.RecordPolicyRuleInfo(ctx, policyValidationMode, policyType, policyBackgroundMode, policyNamespace, policyName, ruleName, ruleType, status, metricValue) - return nil }