use tls for prometheus metrics in ssp operator and template validator #1567

Ruclo · 2025-10-01T10:55:00Z

What this PR does / why we need it:
Creates a separate service monitor targeting the template validator metrics and verifies the metrics service tls certificate when querying prometheus metrics.

Which issue(s) this PR fixes:

Fixes CNV-55455

Release note:

NONE

openshift-ci · 2025-10-01T10:55:11Z

Hi @Ruclo. Thanks for your PR.

I'm waiting for a kubevirt member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

internal/operands/metrics/resources.go

0xFelix · 2025-10-02T11:23:16Z

internal/operands/metrics/resources.go

+	return &promv1.SecretOrConfigMap{
+		ConfigMap: &v1.ConfigMapKeySelector{
+			LocalObjectReference: v1.LocalObjectReference{
+				Name: "openshift-service-ca.crt",


Make name and key a const? Also, this introduces further hard dependencies to OCP. Is there no way to make it configurable?

0xFelix · 2025-10-02T11:23:56Z

internal/operands/metrics/resources.go

+			},
+		},
+	}
+	tlsConfig.ServerName = ptr.To("virt-template-validator.kubevirt.svc")


Read that from the cert to be sure? The namespace can vary.

0xFelix · 2025-10-02T11:25:05Z

internal/operands/metrics/resources.go

+		return &promv1.SecretOrConfigMap{
+			Secret: &v1.SecretKeySelector{
+				LocalObjectReference: v1.LocalObjectReference{
+					Name: "ssp-operator-service-cert",


Names of objects that are not reconciled by SSP should be configurable.

akrejcir · 2025-10-06T12:40:40Z

internal/operands/metrics/resources.go

+// Variable to store OLM deployment info (set from main)
+var isOLMDeployment bool
+
+func SetOLMDeployment(isOLM bool) {
+	isOLMDeployment = isOLM
+}


Using a global variable is an antipattern. Can you instead pass the value into functions that need it?

akrejcir · 2025-10-06T12:43:13Z

internal/operands/metrics/resources.go

+
+// getCAConfigForServiceMonitor returns the appropriate CA configuration
+func getCAConfigForServiceMonitor() *promv1.SecretOrConfigMap {
+	if isOLMDeployment {


Can you pass this global as a parameter to this function?

akrejcir · 2025-10-06T12:44:36Z

internal/operands/metrics/resources.go

+}
+
+func newSspServiceMonitor(namespace string) *promv1.ServiceMonitor {
+	certPath := filepath.Join(DefaultCertsDirectory, CertFilename)


Hardcoding a path like this makes this code hard to unit test. The path will not exist on the machine where tests run. Can you pass the hostname as a parameter to this function?

internal/operands/metrics/resources.go

akrejcir

Thanks, looks good. I mostly have code style comments.

The CI is failing, because some files need to be regenerated using make bundle.

Can you please write more information into the commit message? A good commit message explains the reason why is the change done, not only what is changed.
It will be useful when someone looks at git history, to find out why a code is written in a certain way.

akrejcir · 2025-10-20T10:48:16Z

internal/operands/metrics/resources.go

 	PrometheusServiceAccountName = "prometheus-k8s"
 	MetricsPortName              = "http-metrics"
+
+	TemplateValidatorMetricsServiceName = "template-validator-metrics"


Please use this constant also in template-validator/resources.go file, so that we are sure that it is the same string.

The above comment still applies.

The string template-validator-metrics is used to connect 2 resources in the cluster: Service and ServiceMonitor. It is a good code style if the 2 strings are also the same constant in the code. Then we can be sure, that there is not a typo in one of them.

Can you extract the constant to a different package, so it does not cause import loop?

akrejcir · 2025-10-20T10:48:42Z

internal/operands/metrics/resources.go

 	MetricsPortName              = "http-metrics"
+
+	TemplateValidatorMetricsServiceName = "template-validator-metrics"
+	MetricsServiceName                  = "ssp-operator-metrics"


Use this constant also in controllers/services_contorller.go.

The same reason as in my comment above.

akrejcir · 2025-10-20T10:49:46Z

internal/operands/metrics/resources.go

+	ServiceCABundle                     = "openshift-service-ca.crt"
+	ServiceCABUndleKey                  = "service-ca.crt"
+	OLMManagedCert                      = "ssp-operator-service-cert"
+	OLMManagedCertKey                   = "olmCAKey"


These 4 constants don't need to be exported, because they are only used in a single function in this file.
Can you move them to the functions where they are used?

This is still valid.

akrejcir · 2025-10-20T10:58:57Z

internal/operands/metrics/resources.go

+	}
+}
+
+func olmManagedCABundle() promv1.SecretOrConfigMap {


Can you inline this function? It is only used in one place, and it is short.

akrejcir · 2025-10-20T11:00:36Z

internal/operands/metrics/resources.go

+
+func olmManagedCABundle() promv1.SecretOrConfigMap {
+	return promv1.SecretOrConfigMap{
+		Secret: &v1.SecretKeySelector{


Please add a comment, that OLM uses a self-signed CA, and that it is not available in a ConfigMap.
So that future developers know why we use a secret.

internal/operands/metrics/resources.go

main.go

sourcery-ai

Hey there - I've reviewed your changes - here's some feedback:

There’s a typo in ServiceCABUndleKey (should be ServiceCABundleKey) and you may want to correct it to avoid confusion.
The TLSConfig construction in newValidatorServiceMonitor and newSspServiceMonitor is almost identical—consider extracting a shared helper to reduce duplication.
extractHostnameFromCert references sdkTLSDir which no longer exists—ensure you introduce or update the constant/path so the code compiles and reads the intended cert.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- There’s a typo in ServiceCABUndleKey (should be ServiceCABundleKey) and you may want to correct it to avoid confusion.
- The TLSConfig construction in newValidatorServiceMonitor and newSspServiceMonitor is almost identical—consider extracting a shared helper to reduce duplication.
- extractHostnameFromCert references sdkTLSDir which no longer exists—ensure you introduce or update the constant/path so the code compiles and reads the intended cert.

## Individual Comments

### Comment 1
<location> `internal/operands/metrics/resources.go:110` </location>
<code_context>
+	)
+}
+
+func newValidatorServiceMonitor(request common.Request) *promv1.ServiceMonitor {
+	tlsConfig := &promv1.TLSConfig{
+		SafeTLSConfig: promv1.SafeTLSConfig{
</code_context>

<issue_to_address>
**issue (complexity):** Consider refactoring the three ServiceMonitor constructors into a single, parameterized function to reduce duplication and centralize logic.

Here’s one way to collapse all three builders into a single, parameterized constructor.  You keep the same inputs (name, namespace, label sets, port, CA bundle, serverName), but only write the service‐monitor shape once:

```go
// newServiceMonitor constructs any ServiceMonitor.
//  - labels: service labels to merge with the defaults (cluster-monitoring & k8s-app)
//  - selector: the MatchLabels for the ServiceMonitor selector
//  - port: the metrics port name
//  - ca:   the CA bundle source
//  - serverName: optional server name override
func newServiceMonitor(
    name, namespace string,
    labels, selector map[string]string,
    port string,
    ca promv1.SecretOrConfigMap,
    serverName *string,
) *promv1.ServiceMonitor {
    return &promv1.ServiceMonitor{
        ObjectMeta: metav1.ObjectMeta{
            Name:      name,
            Namespace: namespace,
            Labels:    serviceMonitorLabels(labels),
        },
        Spec: promv1.ServiceMonitorSpec{
            NamespaceSelector: promv1.NamespaceSelector{Any: true},
            Selector:          metav1.LabelSelector{MatchLabels: selector},
            Endpoints: []promv1.Endpoint{{
                Port:   port,
                Scheme: "https",
                TLSConfig: &promv1.TLSConfig{
                    SafeTLSConfig: promv1.SafeTLSConfig{
                        CA:         ca,
                        ServerName: serverName,
                    },
                },
                HonorLabels: true,
            }},
        },
    }
}
```

Then each specialized builder becomes a one‐liner:

```go
func newValidatorServiceMonitor(req common.Request) *promv1.ServiceMonitor {
    svcLabels := template_validator.PrometheusServiceLabels()
    return newServiceMonitor(
        template_validator.MetricsServiceName,
        req.Namespace,
        svcLabels,
        svcLabels,
        template_validator.MetricsPortName,
        serviceCABundle(),
        ptr.To(fmt.Sprintf("%s.%s.svc", template_validator.VirtTemplateValidator, req.Namespace)),
    )
}

func newSspServiceMonitor(req common.Request) *promv1.ServiceMonitor {
    svcLabels := resources.PrometheusServiceLabels()
    return newServiceMonitor(
        resources.MetricsServiceName,
        req.Namespace,
        svcLabels,
        svcLabels,
        resources.MetricsPortName,
        getCAConfigForServiceMonitor(req.OLMDeployment),
        ptr.To(req.SSPServiceHostname),
    )
}
```

And you can drop the old `newServiceMonitorCR`, `TemplateValidatorServiceMonitorLabels` and `SspServiceMonitorLabels` helpers completely—everything funnels through that single `newServiceMonitor`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

akrejcir

I've only done partial review.

Can you please put the refactoring commit that fixes import cycle before the commit that implements new functionality, or can you merge the 2 commits?

A commit should be a code change that compiles and preferably passes tests. So having a second commit that fixes the first commit is unusual.

Can you also add more text to the commit messages? Mainly in the first commit, the message should explain why is the symlink removed.

akrejcir · 2025-11-03T12:09:44Z

internal/common/labels.go

+func PrometheusServiceLabels(serviceName string) map[string]string {
+	return map[string]string{
+		PrometheusLabelKey: serviceName,
+	}
+}


This function is trivial, and can be inlined. PrometheusLabelKey is exported variable, so it can be accessed from other packages.

perhaps exporting only the function would be better? i also renamed the function to MetricsServiceLabels().

internal/common/resource.go

internal/controllers/services_controller.go

akrejcir · 2025-11-03T12:15:33Z

internal/operands/metrics/resources.go

+func serviceMonitorLabels(serviceLabels map[string]string) map[string]string {
+	labels := map[string]string{
 		"openshift.io/cluster-monitoring": "true",
-		PrometheusLabelKey:                PrometheusLabelValue,
 		"k8s-app":                         "kubevirt",
 	}
+
+	for k, v := range serviceLabels {
+		labels[k] = v
+	}
+
+	return labels


I think this is unnecessarily complex. Can you use the same function that was here, but make the PrometheusLabelValue a parameter?

akrejcir · 2025-11-03T12:16:55Z

internal/resources/metrics_service.go

+	return common.PrometheusServiceLabels(MetricsServiceName)
+}
+
+func SspMetricsService(namespace string) *v1.Service {


This function is only used in one file, and a functional test file. I think it would not cause an import cycle, if it is left in the services_controller.go where it is used.

it causes an import cycle so i had to extract it out of the controllers package.

main.go

internal/constants.go

akrejcir · 2025-11-03T12:41:41Z

internal/constants.go

 const (
 	GoldenImagesNamespace  = "kubevirt-os-images"
 	DefaultOperatorVersion = "devel"
+	SspDeploymentName      = "ssp-operator"


This constant is only used in one place and does not need to be here.

This file contains only constants that change value in the build scripts for downstream. The name of SSP deployment is the same in upstream and downstream, so this constant doesn't need to be here.

now, it's used in more places. I think this constant could be extracted somewhere else if this file is not adequate.

akrejcir · 2025-11-07T15:28:02Z

main.go


 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))

-	err := createCertificateSymlinks()


Can you please add a text to the commit message explaining why the symlinks are not needed anymore?
It is not obvious to me, and it would be helpful for others, when they look at commit history.

akrejcir · 2025-11-07T15:29:03Z

internal/operands/metrics/resources.go

 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
-
+	"kubevirt.io/ssp-operator/internal/common"


Add a newline above this line to split imports into 3 blocks.

akrejcir · 2025-11-07T15:32:14Z

internal/operands/metrics/resources.go

 	PrometheusServiceAccountName = "prometheus-k8s"
 	MetricsPortName              = "http-metrics"
+
+	TemplateValidatorMetricsServiceName = "template-validator-metrics"


The above comment still applies.

The string template-validator-metrics is used to connect 2 resources in the cluster: Service and ServiceMonitor. It is a good code style if the 2 strings are also the same constant in the code. Then we can be sure, that there is not a typo in one of them.

Can you extract the constant to a different package, so it does not cause import loop?

akrejcir · 2025-11-07T15:32:36Z

internal/operands/metrics/resources.go

 	MetricsPortName              = "http-metrics"
+
+	TemplateValidatorMetricsServiceName = "template-validator-metrics"
+	MetricsServiceName                  = "ssp-operator-metrics"


The same reason as in my comment above.

akrejcir · 2025-11-07T15:32:54Z

internal/operands/metrics/resources.go

+	ServiceCABundle                     = "openshift-service-ca.crt"
+	ServiceCABUndleKey                  = "service-ca.crt"
+	OLMManagedCert                      = "ssp-operator-service-cert"
+	OLMManagedCertKey                   = "olmCAKey"


This is still valid.

akrejcir · 2025-11-07T15:33:50Z

internal/operands/metrics/resources.go

+	}
+	tlsConfig.ServerName = ptr.To(request.SSPServiceHostname)
+
+	serviceMonitor := newServiceMonitor(rules.RuleName, request.Namespace, tlsConfig, metav1.LabelSelector{


rules.RuleName us not a good name for ServiceMonitor. Rules are not related to metrics collection.

True, the Service monitor can be named after the service, just as it's done in the template validator.

Ruclo · 2025-11-10T09:24:41Z

I tried to extract the related functionality in the third commit that was part of this branch. Was this a bad thing ? Do you mind giving me a review on that ?

akrejcir · 2025-11-11T16:10:18Z

I tried to extract the related functionality in the third commit that was part of this branch. Was this a bad thing ? Do you mind giving me a review on that ?

I meant to only extract the constants to a separate file, and use it in multiple files. Like this:
akrejcir@1525fa0

In the previous commit, you've extracted a function, but that was not needed, because it was only used in a single file.

config/default/kustomization.yaml

internal/controllers/ssp_controller.go

internal/operands/metrics/resources.go

0xFelix · 2025-11-13T10:31:06Z

main.go

+		return cert.Subject.CommonName, nil
+	}
+
+	return cert.DNSNames[0], nil


CommonName and DNSNames[0] should match, no?

I think they should match, can this be kept in place as a fallback?

Can you add a check to verify that?

Can you do that? (CommonName == DNSNames[0])

akrejcir

I have only a few code style comments. Otherwise it looks good.

Can you also please add more text to the second commit message?

akrejcir · 2025-11-20T15:29:18Z

internal/operands/metrics/resources.go

+	ServiceCABundle              = "openshift-service-ca.crt"
+	ServiceCABUndleKey           = "service-ca.crt"
+	OLMManagedCert               = "ssp-operator-service-cert"
+	OLMManagedCertKey            = "olmCAKey"


These constants are only used in one function in this file, so they don't need to be exported.
Can you please inline them?

akrejcir · 2025-11-20T15:31:09Z

internal/constants.go

+	SspOperatorMetricsServiceName       = "ssp-operator-metrics"
+	TemplateValidatorMetricsServiceName = "template-validator-metrics"
+	VirtTemplateValidator               = "virt-template-validator"


Please move these constants to a different file.
This file only contains constants that are changed by the build process downstream. We use a simple sed command to change the values, so it would be best if there are small number of constants here.

akrejcir · 2025-11-20T15:31:31Z

tests/misc_test.go


 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"kubevirt.io/ssp-operator/internal"


Move this import to the last block.

akrejcir · 2025-11-20T15:31:39Z

tests/service_controller_test.go


 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"kubevirt.io/ssp-operator/internal"


The same as above.

akrejcir · 2025-11-20T15:31:50Z

tests/validator_test.go


 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"kubevirt.io/ssp-operator/internal"


The same as above.

0xFelix

/ok-to-test

0xFelix · 2025-11-24T10:01:56Z

main.go

+		return cert.Subject.CommonName, nil
+	}
+
+	return cert.DNSNames[0], nil


Can you do that? (CommonName == DNSNames[0])

0xFelix · 2025-11-24T10:05:53Z

main.go

 	}
 }
-
-func createCertificateSymlinks() error {


Do we test CSV deployment in the upstream lanes?

No, we don't.

@rucle Can you please make sure this will not break stuff?

I tested the metrics, both when manually deployed and also OLM-deployed and the metrics were correctly reported in both scenarios.

akrejcir · 2025-11-24T11:09:12Z

The CI failures are unrelated. Recently golangci-lint either OOMs or takes too long and times out. I will look into it.

The certificates get mounted to the correct mount points in the filesystem, Symlink creation is unnecessary. Signed-off-by: Michal Vavrinec <[email protected]>

akrejcir · 2025-12-08T12:44:55Z

Can you update the failing functional tests, and include a new test for the second ServiceMonitor?

tests/metrics_test.go

This commit implements TLS encryption for both SSP operator and template validator metrics services. The implementation handles both OLM and non-OLM deployments with proper certificate management. Changes: - Add --olm-deployment flag to distinguish OLM-managed deployments - Create separate ServiceMonitors for SSP and template validator metrics - Configure TLS settings with appropriate CA bundles based on deployment type - Move common service names to constants package for reusability - Update ServiceMonitor selector labels to differentiate between services Technical details: - Non-OLM deployments use service-ca.crt from ConfigMap - OLM deployments use ssp-operator-service-cert Secret with olmCAKey Signed-off-by: Michal Vavrinec <[email protected]>

sonarqubecloud · 2025-12-09T11:53:40Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

akrejcir · 2025-12-09T14:09:44Z

Looks good now.

/approve

kubevirt-bot · 2025-12-09T14:09:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akrejcir

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [akrejcir]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

akrejcir · 2025-12-12T13:15:59Z

/cc @0xFelix @jcanocan

kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Oct 1, 2025

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 1, 2025

kubevirt-bot added the size/L label Oct 1, 2025

Ruclo force-pushed the metrics-tls branch from 7204d52 to f5bac9e Compare October 1, 2025 11:04

0xFelix reviewed Oct 2, 2025

View reviewed changes

akrejcir reviewed Oct 6, 2025

View reviewed changes

Ruclo force-pushed the metrics-tls branch 4 times, most recently from 5d3cb20 to 1893326 Compare October 20, 2025 08:09

akrejcir reviewed Oct 20, 2025

View reviewed changes

kubevirt-bot added size/XL and removed size/L labels Oct 27, 2025

Ruclo force-pushed the metrics-tls branch 2 times, most recently from 596b681 to f527836 Compare October 29, 2025 12:23

kubevirt-bot added size/L and removed size/XL labels Oct 29, 2025

Ruclo marked this pull request as ready for review October 30, 2025 09:11

kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 30, 2025

kubevirt-bot requested review from codingben and jcanocan October 30, 2025 09:11

sourcery-ai bot reviewed Oct 30, 2025

View reviewed changes

Ruclo force-pushed the metrics-tls branch from f527836 to 8757f88 Compare October 30, 2025 09:28

akrejcir reviewed Nov 3, 2025

View reviewed changes

Ruclo force-pushed the metrics-tls branch 2 times, most recently from a7edeb2 to 884d98e Compare November 5, 2025 09:57

akrejcir reviewed Nov 7, 2025

View reviewed changes

0xFelix reviewed Nov 13, 2025

View reviewed changes

Ruclo force-pushed the metrics-tls branch from 884d98e to 087f2b9 Compare November 13, 2025 11:33

akrejcir reviewed Nov 20, 2025

View reviewed changes

Ruclo force-pushed the metrics-tls branch from 087f2b9 to 02938f4 Compare November 21, 2025 11:21

0xFelix reviewed Nov 24, 2025

View reviewed changes

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 24, 2025

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2025

remove unnecessary symlink creation

fb9f170

The certificates get mounted to the correct mount points in the filesystem, Symlink creation is unnecessary. Signed-off-by: Michal Vavrinec <[email protected]>

Ruclo force-pushed the metrics-tls branch from 02938f4 to d7aa2a8 Compare December 8, 2025 10:15

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 8, 2025

Ruclo force-pushed the metrics-tls branch from d7aa2a8 to fd31edc Compare December 9, 2025 09:18

akrejcir reviewed Dec 9, 2025

View reviewed changes

tests/metrics_test.go Outdated Show resolved Hide resolved

Ruclo force-pushed the metrics-tls branch from fd31edc to a102da0 Compare December 9, 2025 11:52

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 9, 2025

kubevirt-bot requested a review from 0xFelix December 12, 2025 13:16


		ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))

		err := createCertificateSymlinks()

use tls for prometheus metrics in ssp operator and template validator #1567

Are you sure you want to change the base?

use tls for prometheus metrics in ssp operator and template validator #1567

Uh oh!

Conversation

Ruclo commented Oct 1, 2025

Uh oh!

openshift-ci bot commented Oct 1, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akrejcir left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

akrejcir left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!