Refactor agent proxy service, add test

config/webhook/manifests.yaml

@@ -10,38 +10,38 @@ webhooks:
     service:
       name: webhook-service
       namespace: system
-      path: /mutate--v1-pod
-  failurePolicy: Ignore
-  name: mpod.cryostat.io
+      path: /mutate-operator-cryostat-io-v1beta2-cryostat
+  failurePolicy: Fail
+  name: mcryostat.kb.io
   rules:
   - apiGroups:
-    - ""
+    - operator.cryostat.io
     apiVersions:
-    - v1
+    - v1beta2
     operations:
     - CREATE
+    - UPDATE
     resources:
-    - pods
+    - cryostats
   sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:
     service:
       name: webhook-service
       namespace: system
-      path: /mutate-operator-cryostat-io-v1beta2-cryostat
-  failurePolicy: Fail
-  name: mcryostat.kb.io
+      path: /mutate--v1-pod
+  failurePolicy: Ignore
+  name: mpod.cryostat.io
   rules:
   - apiGroups:
-    - operator.cryostat.io
+    - ""
     apiVersions:
-    - v1beta2
+    - v1
     operations:
     - CREATE
-    - UPDATE
     resources:
-    - cryostats
+    - pods
   sideEffects: None
 ---
 apiVersion: admissionregistration.k8s.io/v1

internal/controllers/constants/constants.go

@@ -50,10 +50,16 @@ const (
 	AgentProxyConfigFilePath string = "/etc/nginx-cryostat"
 	AgentProxyConfigFileName string = "nginx.conf"
 
+	// Labels applied by operator to track cross-namespace ownership
 	targetNamespaceCRLabelPrefix    = "operator.cryostat.io/"
 	TargetNamespaceCRNameLabel      = targetNamespaceCRLabelPrefix + "name"
 	TargetNamespaceCRNamespaceLabel = targetNamespaceCRLabelPrefix + "namespace"
 
+	// Labels for agent auto-configuration
+	agentLabelPrefix            = "cryostat.io/"
+	AgentLabelCryostatName      = agentLabelPrefix + "name"
+	AgentLabelCryostatNamespace = agentLabelPrefix + "namespace"
+
 	CryostatCATLSCommonName     = "cryostat-ca-cert-manager"
 	CryostatTLSCommonName       = "cryostat"
 	ReportsTLSCommonName        = "cryostat-reports"

internal/controllers/services.go

@@ -18,14 +18,12 @@ import (
 	"context"
 	"fmt"
 	"net/url"
-	"strconv"
 
 	operatorv1beta2 "github.com/cryostatio/cryostat-operator/api/v1beta2"
 	common "github.com/cryostatio/cryostat-operator/internal/controllers/common"
 	"github.com/cryostatio/cryostat-operator/internal/controllers/common/resource_definitions"
 	"github.com/cryostatio/cryostat-operator/internal/controllers/constants"
 	"github.com/cryostatio/cryostat-operator/internal/controllers/model"
-	"github.com/cryostatio/cryostat-operator/internal/webhooks/agent"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -51,7 +49,7 @@ func (r *Reconciler) reconcileCoreService(ctx context.Context, cr *model.Cryosta
 		appProtocol := "http"
 		svc.Spec.Ports = []corev1.ServicePort{
 			{
-				Name:        "http",
+				Name:        constants.HttpPortName,
 				Port:        *config.HTTPPort,
 				TargetPort:  intstr.IntOrString{IntVal: constants.AuthProxyHttpContainerPort},
 				AppProtocol: &appProtocol,
@@ -91,7 +89,7 @@ func (r *Reconciler) reconcileReportsService(ctx context.Context, cr *model.Cryo
 		}
 		svc.Spec.Ports = []corev1.ServicePort{
 			{
-				Name:       "http",
+				Name:       constants.HttpPortName,
 				Port:       *config.HTTPPort,
 				TargetPort: intstr.IntOrString{IntVal: constants.ReportsContainerPort},
 			},
@@ -109,19 +107,23 @@ func (r *Reconciler) reconcileReportsService(ctx context.Context, cr *model.Cryo
 	}
 	specs.ReportsURL = &url.URL{
 		Scheme: scheme,
-		Host:   svc.Name + ":" + strconv.Itoa(int(svc.Spec.Ports[0].Port)), // TODO use getHTTPPort?
+		Host:   fmt.Sprintf("%s:%d", svc.Name, *config.HTTPPort),
 	}
 	return nil
 }
 
-func (r *Reconciler) reconcileAgentService(ctx context.Context, cr *model.CryostatInstance) error {
-	config := configureAgentService(cr)
-	svc := &corev1.Service{
+func NewAgentService(cr *model.CryostatInstance) *corev1.Service {
+	return &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name + "-agent",
 			Namespace: cr.InstallNamespace,
 		},
 	}
+}
+
+func (r *Reconciler) reconcileAgentService(ctx context.Context, cr *model.CryostatInstance) error {
+	svc := NewAgentService(cr)
+	config := GetAgentServiceConfig(cr)
 
 	return r.createOrUpdateService(ctx, svc, cr.Object, &config.ServiceConfig, func() error {
 		svc.Spec.Selector = map[string]string{
@@ -130,7 +132,7 @@ func (r *Reconciler) reconcileAgentService(ctx context.Context, cr *model.Cryost
 		}
 		svc.Spec.Ports = []corev1.ServicePort{
 			{
-				Name:       "http",
+				Name:       constants.HttpPortName,
 				Port:       *config.HTTPPort,
 				TargetPort: intstr.IntOrString{IntVal: constants.AgentProxyContainerPort},
 			},
@@ -162,12 +164,12 @@ func (r *Reconciler) reconcileAgentHeadlessServices(ctx context.Context, cr *mod
 		err := r.createOrUpdateService(ctx, svc, nil, config, func() error {
 			// Select agent auto-configuration labels
 			svc.Spec.Selector = map[string]string{
-				agent.LabelCryostatName:      cr.Name,
-				agent.LabelCryostatNamespace: cr.InstallNamespace,
+				constants.AgentLabelCryostatName:      cr.Name,
+				constants.AgentLabelCryostatNamespace: cr.InstallNamespace,
 			}
 			svc.Spec.Ports = []corev1.ServicePort{
 				{
-					Name:       "http",
+					Name:       constants.HttpPortName,
 					Port:       9977, // TODO make configurable
 					TargetPort: intstr.IntOrString{IntVal: 9977},
 				},
@@ -245,7 +247,7 @@ func configureReportsService(cr *model.CryostatInstance) *operatorv1beta2.Report
 	return config
 }
 
-func configureAgentService(cr *model.CryostatInstance) *operatorv1beta2.AgentServiceConfig {
+func GetAgentServiceConfig(cr *model.CryostatInstance) *operatorv1beta2.AgentServiceConfig {
 	// Check CR for config
 	var config *operatorv1beta2.AgentServiceConfig
 	if cr.Spec.ServiceOptions == nil || cr.Spec.ServiceOptions.AgentConfig == nil {

internal/webhooks/agent/const.go

@@ -15,7 +15,7 @@
 package agent
 
 const (
-	labelprefix            = "cryostat.io/"
-	LabelCryostatName      = labelprefix + "name"
-	LabelCryostatNamespace = labelprefix + "namespace"
+	agentLabelPrefix            = "cryostat.io/"
+	AgentLabelCryostatName      = agentLabelPrefix + "name"
+	AgentLabelCryostatNamespace = agentLabelPrefix + "namespace"
 )

internal/webhooks/agent/pod_defaulter.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	operatorv1beta2 "github.com/cryostatio/cryostat-operator/api/v1beta2"
+	"github.com/cryostatio/cryostat-operator/internal/controllers"
 	"github.com/cryostatio/cryostat-operator/internal/controllers/common"
 	"github.com/cryostatio/cryostat-operator/internal/controllers/constants"
 	"github.com/cryostatio/cryostat-operator/internal/controllers/model"
@@ -58,16 +59,16 @@ func (r *podMutator) Default(ctx context.Context, obj runtime.Object) error {
 
 	// TODO do this with objectSelector: https://github.com/kubernetes-sigs/controller-tools/issues/553
 	// Check for required labels and return early if missing
-	if !metav1.HasLabel(pod.ObjectMeta, LabelCryostatName) || !metav1.HasLabel(pod.ObjectMeta, LabelCryostatNamespace) {
+	if !metav1.HasLabel(pod.ObjectMeta, constants.AgentLabelCryostatName) || !metav1.HasLabel(pod.ObjectMeta, constants.AgentLabelCryostatNamespace) {
 		fmt.Println(pod.Labels)
 		return nil
 	}
 
 	// Look up Cryostat
 	cr := &operatorv1beta2.Cryostat{}
 	err := r.client.Get(ctx, types.NamespacedName{
-		Name:      pod.Labels[LabelCryostatName],
-		Namespace: pod.Labels[LabelCryostatNamespace],
+		Name:      pod.Labels[constants.AgentLabelCryostatName],
+		Namespace: pod.Labels[constants.AgentLabelCryostatNamespace],
 	}, cr)
 	if err != nil {
 		return err
@@ -80,7 +81,8 @@ func (r *podMutator) Default(ctx context.Context, obj runtime.Object) error {
 	}
 
 	// Check whether TLS is enabled for this CR
-	tlsEnabled := r.IsCertManagerEnabled(model.FromCryostat(cr))
+	crModel := model.FromCryostat(cr)
+	tlsEnabled := r.IsCertManagerEnabled(crModel)
 
 	// Select target container
 	if len(pod.Spec.Containers) == 0 {
@@ -131,7 +133,7 @@ func (r *podMutator) Default(ctx context.Context, obj runtime.Object) error {
 	container.Env = append(container.Env,
 		corev1.EnvVar{
 			Name:  "CRYOSTAT_AGENT_BASEURI",
-			Value: cryostatURL(cr, tlsEnabled),
+			Value: cryostatURL(crModel, tlsEnabled),
 		},
 		corev1.EnvVar{
 			Name: podNameEnvVar,
@@ -265,18 +267,16 @@ func (r *podMutator) Default(ctx context.Context, obj runtime.Object) error {
 	return nil
 }
 
-func cryostatURL(cr *operatorv1beta2.Cryostat, tls bool) string {
+func cryostatURL(cr *model.CryostatInstance, tls bool) string {
+	// Build the URL to the agent proxy service
 	scheme := "https"
 	if !tls {
 		scheme = "http"
 	}
-	// TODO see if this can be easily refactored
-	port := constants.AgentProxyContainerPort
-	if cr.Spec.ServiceOptions != nil && cr.Spec.ServiceOptions.AgentConfig != nil && cr.Spec.ServiceOptions.AgentConfig.HTTPPort != nil {
-		port = *cr.Spec.ServiceOptions.AgentConfig.HTTPPort
-	}
-	return fmt.Sprintf("%s://%s-agent.%s.svc:%d", scheme, cr.Name, cr.Namespace, // TODO maybe use agent service instead of CR meta
-		port)
+	svc := controllers.NewAgentService(cr)
+	config := controllers.GetAgentServiceConfig(cr)
+	return fmt.Sprintf("%s://%s.%s.svc:%d", scheme, svc.Name, svc.Namespace,
+		*config.HTTPPort)
 }
 
 func (r *podMutator) callbackEnv(cr *operatorv1beta2.Cryostat, namespace string, tls bool) []corev1.EnvVar {

internal/webhooks/agent/pod_defaulter_test.go

@@ -255,7 +255,9 @@ var _ = Describe("PodDefaulter", func() {
 				}
 
 				JustAfterEach(func() {
+					// Reset state
 					agentWebhookConfig.OSUtils = saveOSUtils
+					agentWebhookConfig.InitImageTag = nil
 				})
 
 				Context("for development", func() {
@@ -276,6 +278,16 @@ var _ = Describe("PodDefaulter", func() {
 					ExpectPod()
 				})
 			})
+
+			Context("with a custom proxy port", func() {
+				BeforeEach(func() {
+					t.objs = append(t.objs, t.NewCryostatWithAgentSvc().Object)
+					originalPod = t.NewPod()
+					expectedPod = t.NewMutatedPodProxyPort()
+				})
+
+				ExpectPod()
+			})
 		})
 
 		Context("with a missing Cryostat CR", func() {