Skip to content

Commit

Permalink
feat(agent): add label for Java options variable name (#1035)
Browse files Browse the repository at this point in the history
  • Loading branch information
ebaron authored Jan 30, 2025
1 parent 2884ad3 commit 53d0618
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 21 deletions.
1 change: 1 addition & 0 deletions internal/controllers/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ const (
AgentLabelCallbackPort = agentLabelPrefix + "callback-port"
AgentLabelContainer = agentLabelPrefix + "container"
AgentLabelReadOnly = agentLabelPrefix + "read-only"
AgentLabelJavaOptionsVar = agentLabelPrefix + "java-options-var"

CryostatCATLSCommonName = "cryostat-ca-cert-manager"
CryostatTLSCommonName = "cryostat"
Expand Down
28 changes: 18 additions & 10 deletions internal/webhooks/agent/pod_defaulter.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const (
agentMaxSizeBytes = "50Mi"
agentInitCpuRequest = "10m"
agentInitMemoryRequest = "32Mi"
defaultJavaOptsVar = "JAVA_TOOL_OPTIONS"
)

// Default optionally mutates a pod to inject the Cryostat agent
Expand Down Expand Up @@ -274,8 +275,8 @@ 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)
// Inject agent using JAVA_TOOL_OPTIONS or specified variable, appending to any existing value
extended, err := extendJavaOptsVar(container.Env, getJavaOptionsVar(pod.Labels))
if err != nil {
return err
}
Expand Down Expand Up @@ -339,6 +340,15 @@ func hasWriteAccess(labels map[string]string) (*bool, error) {
return &result, nil
}

func getJavaOptionsVar(labels map[string]string) string {
result := defaultJavaOptsVar
value, pres := labels[constants.AgentLabelJavaOptionsVar]
if pres {
result = value
}
return result
}

func (r *podMutator) callbackEnv(cr *model.CryostatInstance, namespace string, tls bool, containerPort int32) []corev1.EnvVar {
scheme := "https"
if !tls {
Expand Down Expand Up @@ -409,8 +419,8 @@ func findNamedContainer(containers []corev1.Container, name string) (*corev1.Con
return nil, fmt.Errorf("no container found with name \"%s\"", name)
}

func extendJavaToolOptions(envs []corev1.EnvVar) ([]corev1.EnvVar, error) {
existing, err := findJavaToolOptions(envs)
func extendJavaOptsVar(envs []corev1.EnvVar, javaOptsVar string) ([]corev1.EnvVar, error) {
existing, err := findJavaOptsVar(envs, javaOptsVar)
if err != nil {
return nil, err
}
Expand All @@ -419,21 +429,19 @@ func extendJavaToolOptions(envs []corev1.EnvVar) ([]corev1.EnvVar, error) {
existing.Value += " " + agentArg
} else {
envs = append(envs, corev1.EnvVar{
Name: "JAVA_TOOL_OPTIONS",
Name: javaOptsVar,
Value: agentArg,
})
}

return envs, nil
}

var errJavaToolOptionsValueFrom error = errors.New("environment variable JAVA_TOOL_OPTIONS uses \"valueFrom\" and cannot be extended")

func findJavaToolOptions(envs []corev1.EnvVar) (*corev1.EnvVar, error) {
func findJavaOptsVar(envs []corev1.EnvVar, javaOptsVar string) (*corev1.EnvVar, error) {
for i, env := range envs {
if env.Name == "JAVA_TOOL_OPTIONS" {
if env.Name == javaOptsVar {
if env.ValueFrom != nil {
return nil, errJavaToolOptionsValueFrom
return nil, fmt.Errorf("environment variable %s uses \"valueFrom\" and cannot be extended", javaOptsVar)
}
return &envs[i], nil
}
Expand Down
10 changes: 10 additions & 0 deletions internal/webhooks/agent/pod_defaulter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,16 @@ var _ = Describe("PodDefaulter", func() {
ExpectPod()
})
})

Context("with a custom java options var label", func() {
BeforeEach(func() {
t.objs = append(t.objs, t.NewCryostat().Object)
originalPod = t.NewPodJavaOptsVar()
expectedPod = t.NewMutatedPodJavaOptsVarLabel()
})

ExpectPod()
})
})

Context("with a missing Cryostat CR", func() {
Expand Down
38 changes: 27 additions & 11 deletions internal/webhooks/agent/test/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,22 @@ func (r *AgentWebhookTestResources) NewPodReadOnlyLabelInvalid() *corev1.Pod {
return pod
}

func (r *AgentWebhookTestResources) NewPodJavaOptsVar() *corev1.Pod {
pod := r.NewPod()
pod.Labels["cryostat.io/java-options-var"] = "SOME_OTHER_VAR"
return pod
}

type mutatedPodOptions struct {
javaToolOptions string
namespace string
image string
pullPolicy corev1.PullPolicy
gatewayPort int32
callbackPort int32
writeAccess *bool
scheme string
javaOptionsName string
javaOptionsValue string
namespace string
image string
pullPolicy corev1.PullPolicy
gatewayPort int32
callbackPort int32
writeAccess *bool
scheme string
// Function to produce mutated container array
containersFunc func(*AgentWebhookTestResources, *mutatedPodOptions) []corev1.Container
}
Expand All @@ -212,6 +219,9 @@ func (r *AgentWebhookTestResources) setDefaultMutatedPodOptions(options *mutated
if options.writeAccess == nil {
options.writeAccess = &[]bool{true}[0]
}
if len(options.javaOptionsName) == 0 {
options.javaOptionsName = "JAVA_TOOL_OPTIONS"
}
options.scheme = "https"
if !r.TLS {
options.scheme = "http"
Expand All @@ -227,7 +237,7 @@ func (r *AgentWebhookTestResources) NewMutatedPod() *corev1.Pod {

func (r *AgentWebhookTestResources) NewMutatedPodJavaToolOptions() *corev1.Pod {
return r.newMutatedPod(&mutatedPodOptions{
javaToolOptions: "-Dexisting=var ",
javaOptionsValue: "-Dexisting=var ",
})
}

Expand Down Expand Up @@ -281,6 +291,12 @@ func (r *AgentWebhookTestResources) NewMutatedPodReadOnlyLabel() *corev1.Pod {
})
}

func (r *AgentWebhookTestResources) NewMutatedPodJavaOptsVarLabel() *corev1.Pod {
return r.newMutatedPod(&mutatedPodOptions{
javaOptionsName: "SOME_OTHER_VAR",
})
}

func (r *AgentWebhookTestResources) newMutatedPod(options *mutatedPodOptions) *corev1.Pod {
r.setDefaultMutatedPodOptions(options)
pod := &corev1.Pod{
Expand Down Expand Up @@ -403,8 +419,8 @@ func (r *AgentWebhookTestResources) newMutatedContainer(original *corev1.Contain
Value: strconv.Itoa(int(options.callbackPort)),
},
{
Name: "JAVA_TOOL_OPTIONS",
Value: options.javaToolOptions + "-javaagent:/tmp/cryostat-agent/cryostat-agent-shaded.jar",
Name: options.javaOptionsName,
Value: options.javaOptionsValue + "-javaagent:/tmp/cryostat-agent/cryostat-agent-shaded.jar",
},
}...),
Ports: []corev1.ContainerPort{
Expand Down

0 comments on commit 53d0618

Please sign in to comment.