Resource requests, more cleanup

bundle/manifests/cryostat-operator.clusterserviceversion.yaml

@@ -30,7 +30,7 @@ metadata:
     capabilities: Seamless Upgrades
     categories: Monitoring, Developer Tools
     containerImage: quay.io/cryostat/cryostat-operator:4.0.0-dev
-    createdAt: "2024-12-20T16:49:14Z"
+    createdAt: "2024-12-20T18:04:36Z"
     description: JVM monitoring and profiling tool
     operatorframework.io/initialization-resource: |-
       {

config/manager/kustomization.yaml

@@ -21,5 +21,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 images:
 - name: controller
-  newName: quay.io/ebaron/cryostat-operator
-  newTag: pod-mutation-01
+  newName: quay.io/cryostat/cryostat-operator
+  newTag: 4.0.0-dev

internal/controllers/common/naming.go

@@ -0,0 +1,32 @@
+// Copyright The Cryostat Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package common
+
+import (
+	"github.com/cryostatio/cryostat-operator/internal/controllers/model"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+func AgentHeadlessServiceName(gvk *schema.GroupVersionKind, cr *model.CryostatInstance) string {
+	return ClusterUniqueShortName(gvk, cr.Name, cr.InstallNamespace)
+}
+
+func AgentProxyServiceName(cr *model.CryostatInstance) string {
+	return cr.Name + "-agent"
+}
+
+func AgentCertificateName(gvk *schema.GroupVersionKind, cr *model.CryostatInstance, targetNamespace string) string {
+	return ClusterUniqueNameWithPrefixTargetNS(gvk, "agent", cr.Name, cr.InstallNamespace, targetNamespace)
+}

internal/controllers/common/resource_definitions/certificates.go

@@ -134,8 +134,8 @@ func NewReportsCert(cr *model.CryostatInstance) *certv1.Certificate {
 }
 
 func NewAgentCert(cr *model.CryostatInstance, namespace string, gvk *schema.GroupVersionKind) *certv1.Certificate {
-	svcName := common.ClusterUniqueShortName(gvk, cr.Name, cr.InstallNamespace) // TODO abstract
-	name := common.ClusterUniqueNameWithPrefixTargetNS(gvk, "agent", cr.Name, cr.InstallNamespace, namespace)
+	svcName := common.AgentHeadlessServiceName(gvk, cr)
+	name := common.AgentCertificateName(gvk, cr, namespace)
 	return &certv1.Certificate{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -159,6 +159,7 @@ func NewAgentCert(cr *model.CryostatInstance, namespace string, gvk *schema.Grou
 }
 
 func NewAgentProxyCert(cr *model.CryostatInstance) *certv1.Certificate {
+	svcName := common.AgentProxyServiceName(cr)
 	return &certv1.Certificate{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name + "-agent-proxy",
@@ -167,9 +168,9 @@ func NewAgentProxyCert(cr *model.CryostatInstance) *certv1.Certificate {
 		Spec: certv1.CertificateSpec{
 			CommonName: constants.AgentAuthProxyTLSCommonName,
 			DNSNames: []string{
-				cr.Name + "-agent",
-				fmt.Sprintf("%s-agent.%s.svc", cr.Name, cr.InstallNamespace),
-				fmt.Sprintf("%s-agent.%s.svc.cluster.local", cr.Name, cr.InstallNamespace),
+				svcName,
+				fmt.Sprintf("%s.%s.svc", svcName, cr.InstallNamespace),
+				fmt.Sprintf("%s.%s.svc.cluster.local", svcName, cr.InstallNamespace),
 			},
 			SecretName: cr.Name + "-agent-tls",
 			IssuerRef: certMeta.ObjectReference{

internal/controllers/services.go

@@ -112,7 +112,7 @@ func (r *Reconciler) reconcileReportsService(ctx context.Context, cr *model.Cryo
 	return nil
 }
 
-func NewAgentService(cr *model.CryostatInstance) *corev1.Service {
+func newAgentService(cr *model.CryostatInstance) *corev1.Service {
 	return &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name + "-agent",
@@ -122,8 +122,8 @@ func NewAgentService(cr *model.CryostatInstance) *corev1.Service {
 }
 
 func (r *Reconciler) reconcileAgentService(ctx context.Context, cr *model.CryostatInstance) error {
-	svc := NewAgentService(cr)
-	config := GetAgentServiceConfig(cr)
+	svc := newAgentService(cr)
+	config := configureAgentService(cr)
 
 	return r.createOrUpdateService(ctx, svc, cr.Object, &config.ServiceConfig, func() error {
 		svc.Spec.Selector = map[string]string{
@@ -144,7 +144,7 @@ func (r *Reconciler) reconcileAgentService(ctx context.Context, cr *model.Cryost
 func (r *Reconciler) newAgentHeadlessService(cr *model.CryostatInstance, namespace string) *corev1.Service {
 	return &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      common.ClusterUniqueShortName(r.gvk, cr.Name, cr.InstallNamespace),
+			Name:      common.AgentHeadlessServiceName(r.gvk, cr),
 			Namespace: namespace,
 		},
 	}
@@ -247,7 +247,7 @@ func configureReportsService(cr *model.CryostatInstance) *operatorv1beta2.Report
 	return config
 }
 
-func GetAgentServiceConfig(cr *model.CryostatInstance) *operatorv1beta2.AgentServiceConfig {
+func configureAgentService(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/pod_defaulter.go

@@ -20,7 +20,6 @@ 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"
@@ -46,10 +45,12 @@ type podMutator struct {
 var _ admission.CustomDefaulter = &podMutator{}
 
 const (
-	agentArg          = "-javaagent:/tmp/cryostat-agent/cryostat-agent-shaded.jar"
-	podNameEnvVar     = "CRYOSTAT_AGENT_POD_NAME"
-	podIPEnvVar       = "CRYOSTAT_AGENT_POD_IP"
-	agentMaxSizeBytes = 50 * 1024 * 1024
+	agentArg               = "-javaagent:/tmp/cryostat-agent/cryostat-agent-shaded.jar"
+	podNameEnvVar          = "CRYOSTAT_AGENT_POD_NAME"
+	podIPEnvVar            = "CRYOSTAT_AGENT_POD_IP"
+	agentMaxSizeBytes      = "50Mi"
+	agentInitCpuRequest    = "10m"
+	agentInitMemoryRequest = "32Mi"
 )
 
 // Default optionally mutates a pod to inject the Cryostat agent
@@ -91,7 +92,7 @@ func (r *podMutator) Default(ctx context.Context, obj runtime.Object) error {
 		// Should never happen, Kubernetes doesn't allow this
 		return errors.New("pod has no containers")
 	}
-	// TODO make configurable
+	// TODO make configurable with label
 	container := &pod.Spec.Containers[0]
 
 	// Add init container
@@ -115,15 +116,22 @@ func (r *podMutator) Default(ctx context.Context, obj runtime.Object) error {
 					constants.CapabilityAll,
 				},
 			},
-		}, // TODO Resources?
+		},
+		Resources: corev1.ResourceRequirements{ // TODO allow customization with CRD
+			Requests: corev1.ResourceList{
+				corev1.ResourceCPU:    resource.MustParse(agentInitCpuRequest),
+				corev1.ResourceMemory: resource.MustParse(agentInitMemoryRequest),
+			},
+		},
 	})
 
 	// Add emptyDir volume to copy agent into, and mount it
+	sizeLimit := resource.MustParse(agentMaxSizeBytes)
 	pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{
 		Name: "cryostat-agent-init",
 		VolumeSource: corev1.VolumeSource{
 			EmptyDir: &corev1.EmptyDirVolumeSource{
-				SizeLimit: resource.NewQuantity(agentMaxSizeBytes, resource.BinarySI),
+				SizeLimit: &sizeLimit,
 			},
 		},
 	})
@@ -166,7 +174,7 @@ func (r *podMutator) Default(ctx context.Context, obj runtime.Object) error {
 	)
 
 	// Append callback environment variables
-	container.Env = append(container.Env, r.callbackEnv(cr, pod.Namespace, tlsEnabled)...)
+	container.Env = append(container.Env, r.callbackEnv(crModel, pod.Namespace, tlsEnabled)...)
 
 	if tlsEnabled {
 		// Mount the certificate volume
@@ -175,8 +183,7 @@ func (r *podMutator) Default(ctx context.Context, obj runtime.Object) error {
 			Name: "cryostat-agent-tls",
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
-					// TODO abstract implementation
-					SecretName:  common.ClusterUniqueNameWithPrefixTargetNS(r.gvk, "agent", cr.Name, cr.Namespace, pod.Namespace),
+					SecretName:  common.AgentCertificateName(r.gvk, crModel, pod.Namespace),
 					DefaultMode: &readOnlyMode,
 				},
 			},
@@ -244,24 +251,14 @@ func (r *podMutator) Default(ctx context.Context, obj runtime.Object) error {
 			},
 		)
 	}
+	// Inject agent using JAVA_TOOL_OPTIONS, appending to any existing value
 	extended, err := extendJavaToolOptions(container.Env)
 	if err != nil {
 		return err
 	}
 	container.Env = extended
 
-	// TODO JAVA_TOOL_OPTIONS for -javaagent?
-
-	// TODO figure out authz. may not be suitable to create a service account in each target ns with pod/exec perms.
-	// someone with access to the namespace should not necessarily have access to that permission.
-	//
-	// TokenRequest API, can we autorenew? Non-expiring token would work but security warns against using it. If
-	// using non-expiring token. Remember RBAC is validated against install namespace. Need SA per target namespace,
-	// with a RoleBinding in install namespace.
-	//
-	// could do an authz check on webhook user. if they have pod/exec permissions, then create the sa and/or secret.
-	// this would count as a side effect of the webhook. this could still allow other ns users to escalate though.
-
+	// Use GenerateName for logging if no explicit Name is given
 	podName := pod.Name
 	if len(podName) == 0 {
 		podName = pod.GenerateName
@@ -277,13 +274,19 @@ func cryostatURL(cr *model.CryostatInstance, tls bool) string {
 	if !tls {
 		scheme = "http"
 	}
-	svc := controllers.NewAgentService(cr)
-	config := controllers.GetAgentServiceConfig(cr)
-	return fmt.Sprintf("%s://%s.%s.svc:%d", scheme, svc.Name, svc.Namespace,
-		*config.HTTPPort)
+	return fmt.Sprintf("%s://%s.%s.svc:%d", scheme, common.AgentProxyServiceName(cr), cr.InstallNamespace,
+		getAgentProxyHTTPPort(cr))
+}
+
+func getAgentProxyHTTPPort(cr *model.CryostatInstance) int32 {
+	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 port
 }
 
-func (r *podMutator) callbackEnv(cr *operatorv1beta2.Cryostat, namespace string, tls bool) []corev1.EnvVar {
+func (r *podMutator) callbackEnv(cr *model.CryostatInstance, namespace string, tls bool) []corev1.EnvVar {
 	scheme := "https"
 	if !tls {
 		scheme = "http"
@@ -299,7 +302,7 @@ func (r *podMutator) callbackEnv(cr *operatorv1beta2.Cryostat, namespace string,
 		},
 		{
 			Name:  "CRYOSTAT_AGENT_CALLBACK_DOMAIN_NAME",
-			Value: fmt.Sprintf("%s.%s.svc", common.ClusterUniqueShortName(r.gvk, cr.Name, cr.Namespace), namespace),
+			Value: fmt.Sprintf("%s.%s.svc", common.AgentHeadlessServiceName(r.gvk, cr), namespace),
 		},
 		{
 			Name:  "CRYOSTAT_AGENT_CALLBACK_PORT",

internal/webhooks/agent/pod_defaulter_test.go

@@ -16,7 +16,6 @@ package agent_test
 
 import (
 	"context"
-	"fmt"
 	"strconv"
 	"strings"
 
@@ -107,7 +106,6 @@ var _ = Describe("PodDefaulter", func() {
 					Expect(container.Args).To(Equal(expected.Args))
 					Expect(container.Env).To(Equal(expected.Env))
 					Expect(container.EnvFrom).To(Equal(expected.EnvFrom))
-					fmt.Println(expected.Image[:strings.Index(expected.Image, ":")]) // XXX
 					Expect(container.Image).To(HavePrefix(expected.Image[:strings.Index(expected.Image, ":")]))
 					Expect(container.ImagePullPolicy).To(Equal(expected.ImagePullPolicy))
 					Expect(container.VolumeMounts).To(Equal(expected.VolumeMounts))

internal/webhooks/agent/pod_webhook.go

@@ -26,7 +26,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
-// log is for logging in this package.
+// podWebhookLog is for logging in this package.
 var podWebhookLog = logf.Log.WithName("pod-webhook")
 
 // Environment variable to override the agent init container image

internal/webhooks/agent/test/resources.go

@@ -247,7 +247,7 @@ func (r *AgentWebhookTestResources) newMutatedPod(options *mutatedPodOptions) *c
 						},
 						{
 							Name:  "CRYOSTAT_AGENT_API_WRITES_ENABLED",
-							Value: "true", // TODO default to writes enabled, separate label?
+							Value: "true",
 						},
 
 						{
@@ -286,6 +286,12 @@ func (r *AgentWebhookTestResources) newMutatedPod(options *mutatedPodOptions) *c
 							ReadOnly:  true,
 						},
 					},
+					Resources: corev1.ResourceRequirements{
+						Requests: corev1.ResourceList{
+							corev1.ResourceCPU:    resource.MustParse("10m"),
+							corev1.ResourceMemory: resource.MustParse("32Mi"),
+						},
+					},
 				},
 			},
 			SecurityContext: &corev1.PodSecurityContext{