Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions charts/stackrox-mcp/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,21 @@ TLS Secret name - returns existingSecretName if set, otherwise generates name
{{- include "stackrox-mcp.fullname" . }}-tls
{{- end }}
{{- end }}

{{/*
Central CA Secret name - returns existingSecretName if set, otherwise generates name
*/}}
{{- define "stackrox-mcp.centralCASecretName" -}}
{{- if .Values.centralCACert.existingSecretName }}
{{- .Values.centralCACert.existingSecretName }}
{{- else }}
{{- include "stackrox-mcp.fullname" . }}-central-ca
{{- end }}
{{- end }}
Comment on lines +87 to +93
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Truncate CA secret name after suffixing to avoid invalid Kubernetes names.

If stackrox-mcp.fullname is already near 63 chars, adding -central-ca can exceed the DNS label limit and break installs for long release names.

🔧 Proposed fix
 {{- define "stackrox-mcp.centralCASecretName" -}}
 {{- if .Values.centralCACert.existingSecretName }}
 {{- .Values.centralCACert.existingSecretName }}
 {{- else }}
-{{- include "stackrox-mcp.fullname" . }}-central-ca
+{{- printf "%s-central-ca" (include "stackrox-mcp.fullname" .) | trunc 63 | trimSuffix "-" }}
 {{- end }}
 {{- end }}
-  name: {{ include "stackrox-mcp.fullname" . }}-central-ca
+  name: {{ include "stackrox-mcp.centralCASecretName" . }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{{- define "stackrox-mcp.centralCASecretName" -}}
{{- if .Values.centralCACert.existingSecretName }}
{{- .Values.centralCACert.existingSecretName }}
{{- else }}
{{- include "stackrox-mcp.fullname" . }}-central-ca
{{- end }}
{{- end }}
{{- define "stackrox-mcp.centralCASecretName" -}}
{{- if .Values.centralCACert.existingSecretName }}
{{- .Values.centralCACert.existingSecretName }}
{{- else }}
{{- printf "%s-central-ca" (include "stackrox-mcp.fullname" .) | trunc 63 | trimSuffix "-" }}
{{- end }}
{{- end }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@charts/stackrox-mcp/templates/_helpers.tpl` around lines 87 - 93, The
template stackrox-mcp.centralCASecretName can produce names >63 chars when
appending "-central-ca" to include "stackrox-mcp.fullname"; truncate the
fullname portion before adding the suffix so the total DNS label length stays
<=63. Update the else branch to take include "stackrox-mcp.fullname" . and pipe
it through a truncation (e.g., sprig's trunc) to 52 characters (63 -
len("-central-ca") = 52), then append "-central-ca" so the resulting secret name
always fits Kubernetes DNS label limits.


{{/*
Central CA enabled - returns "true" if either cert or existingSecretName is set
*/}}
{{- define "stackrox-mcp.centralCAEnabled" -}}
{{- if or .Values.centralCACert.cert .Values.centralCACert.existingSecretName }}true{{- end }}
{{- end }}
14 changes: 14 additions & 0 deletions charts/stackrox-mcp/templates/central-ca-secret.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{{- if and .Values.centralCACert.cert .Values.centralCACert.existingSecretName }}
{{- fail "centralCACert: cannot set both 'cert' and 'existingSecretName' — use one or the other" }}
{{- end }}
{{- if and .Values.centralCACert.cert (not .Values.centralCACert.existingSecretName) }}
apiVersion: v1
kind: Secret
metadata:
name: {{ include "stackrox-mcp.fullname" . }}-central-ca
labels:
{{- include "stackrox-mcp.labels" . | nindent 4 }}
type: Opaque
data:
ca.crt: {{ .Values.centralCACert.cert | b64enc }}
{{- end }}
3 changes: 3 additions & 0 deletions charts/stackrox-mcp/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ data:
auth_type: "passthrough"
insecure_skip_tls_verify: {{ .Values.config.central.insecureSkipTLSVerify }}
force_http1: {{ .Values.config.central.forceHTTP1 }}
{{- if include "stackrox-mcp.centralCAEnabled" . }}
ca_cert_path: "/central-ca/ca.crt"
{{- end }}
request_timeout: {{ .Values.config.central.requestTimeout | quote }}
max_retries: {{ .Values.config.central.maxRetries }}
initial_backoff: {{ .Values.config.central.initialBackoff | quote }}
Expand Down
11 changes: 11 additions & 0 deletions charts/stackrox-mcp/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ spec:
mountPath: /certs
readOnly: true
{{- end }}
{{- if include "stackrox-mcp.centralCAEnabled" . }}
- name: central-ca
mountPath: /central-ca
readOnly: true
{{- end }}
volumes:
- name: config
configMap:
Expand All @@ -121,6 +126,12 @@ spec:
secretName: {{ include "stackrox-mcp.tlsSecretName" . }}
defaultMode: 0440
{{- end }}
{{- if include "stackrox-mcp.centralCAEnabled" . }}
- name: central-ca
secret:
secretName: {{ include "stackrox-mcp.centralCASecretName" . }}
defaultMode: 0440
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
Expand Down
6 changes: 6 additions & 0 deletions charts/stackrox-mcp/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ tlsSecret:
# Server TLS Private Key (PEM format)
key: ""

# CA certificate for verifying Central's TLS certificate (e.g., self-signed)
# Only one of cert or existingSecretName should be set.
centralCACert:
existingSecretName: ""
cert: ""

# Resource limits and requests
resources:
limits:
Expand Down
111 changes: 109 additions & 2 deletions internal/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ package client
import (
"context"
"crypto/tls"
"crypto/x509"
"encoding/pem"
"fmt"
"log/slog"
"os"
"sync"
"testing"
"time"
Expand All @@ -23,6 +27,7 @@ import (
const (
minConnectTimeout = 5 * time.Second
backoffJitter = 0.2
maxCACertFileSize = 1 << 20 // 1MB
)

// Client provides gRPC connection to StackRox Central API.
Expand Down Expand Up @@ -229,11 +234,113 @@ func (c *Client) tlsConfig() (*tls.Config, error) {
return nil, errors.Wrap(err, "failed to get central URL hostname")
}

return &tls.Config{
tlsCfg := &tls.Config{
InsecureSkipVerify: c.config.InsecureSkipTLSVerify, //nolint:gosec
MinVersion: tls.VersionTLS12,
ServerName: hostname,
}, nil
}

// There is no reason to load certificates if we allow InsecureSkipTLSVerify.
if !c.config.InsecureSkipTLSVerify && c.config.CACertPath != "" {
certPool, err := loadCACertPool(c.config.CACertPath)
if err != nil {
return nil, err
}

tlsCfg.RootCAs = certPool
}

return tlsCfg, nil
}

func loadCACertPool(caCertPath string) (*x509.CertPool, error) {
// File size guard
fileInfo, err := os.Stat(caCertPath)
if err != nil {
return nil, errors.Wrapf(err, "failed to access CA certificate at %s", caCertPath)
}

if !fileInfo.Mode().IsRegular() {
return nil, errors.Errorf("CA certificate path %s is not a regular file", caCertPath)
}

if fileInfo.Size() == 0 {
return nil, errors.Errorf("CA certificate file %s is empty", caCertPath)
}

if fileInfo.Size() > maxCACertFileSize {
return nil, errors.Errorf(
"CA certificate file %s is too large (%d bytes, max %d)",
caCertPath, fileInfo.Size(),
maxCACertFileSize,
)
}

//nolint: gosec
caCert, err := os.ReadFile(caCertPath)
if err != nil {
return nil, errors.Wrapf(err, "failed to read CA certificate from %s", caCertPath)
}

// Get system cert pool, warn on fallback
certPool, err := x509.SystemCertPool()
if err != nil {
slog.Warn("Failed to load system CA pool, using custom CA only", "error", err)

certPool = x509.NewCertPool()
}

if !certPool.AppendCertsFromPEM(caCert) {
return nil, errors.Errorf("failed to parse CA certificate from %s: no valid PEM data found", caCertPath)
}

showCertInfo(caCert)

return certPool, nil
}

// showCertInfo parses and logs certificate metadata.
func showCertInfo(caCert []byte) {
block, _ := pem.Decode(caCert)
if block == nil {
slog.Warn("Unable to decode CA certificate")

return
}

cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
slog.Warn("Failed to parse CA certificate", "error", err)

return
}

slog.Info("Loaded CA certificate",
"subject", cert.Subject.CommonName,
"issuer", cert.Issuer.CommonName,
"notAfter", cert.NotAfter,
"isCA", cert.IsCA,
)

if !cert.IsCA {
slog.Warn("Provided certificate does not have the CA basic constraint set — TLS verification may fail",
"subject", cert.Subject.CommonName,
)
}

if time.Now().After(cert.NotAfter) {
slog.Warn("CA certificate is expired — TLS verification will fail",
"subject", cert.Subject.CommonName,
"expiredAt", cert.NotAfter,
)
}

if time.Now().Before(cert.NotBefore) {
slog.Warn("CA certificate is not yet valid",
"subject", cert.Subject.CommonName,
"validFrom", cert.NotBefore,
)
}
}

func (c *Client) connectHTTP1(
Expand Down
Loading
Loading