Skip to content

Commit 834af86

Browse files
vivekk16vivekk16
authored
fix: skip CA cert loading when insecureSkipVerify is true and no cert is provided (#925)
Signed-off-by: Vivek Karunai Kiri Ragavan <vkarunai@redhat.com>
1 parent 10a4ce1 commit 834af86

8 files changed

Lines changed: 47 additions & 16 deletions

File tree

‎charts/workload-variant-autoscaler/templates/manager/wva-configmap.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ data:
2121
# Prometheus Configuration (REQUIRED)
2222
# Base URL for Prometheus API (must use HTTPS).
2323
PROMETHEUS_BASE_URL: {{ .Values.wva.prometheus.baseURL | quote }}
24+
{{- if .Values.wva.prometheus.caCert }}
2425
# Filesystem path to the CA certificate used to verify Prometheus TLS cert.
2526
PROMETHEUS_CA_CERT_PATH: {{ .Values.wva.prometheus.tls.caCertPath | default "/etc/ssl/certs/prometheus-ca.crt" | quote }}
27+
{{- end }}
2628
# Whether to skip TLS certificate verification when connecting to Prometheus.
2729
PROMETHEUS_TLS_INSECURE_SKIP_VERIFY: {{ if and .Values.wva.prometheus.tls (hasKey .Values.wva.prometheus.tls "insecureSkipVerify") }}{{ .Values.wva.prometheus.tls.insecureSkipVerify | quote }}{{ else }}"true"{{ end }}
2830

‎charts/workload-variant-autoscaler/templates/manager/wva-deployment-controller-manager.yaml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,21 +105,24 @@ spec:
105105
mountPath: /etc/wva/config.yaml
106106
subPath: config.yaml
107107
readOnly: true
108+
{{- if .Values.wva.prometheus.caCert }}
108109
- name: prometheus-ca-cert
109110
mountPath: /etc/ssl/certs/prometheus-ca.crt
110111
subPath: ca.crt
111112
readOnly: true
113+
{{- end }}
112114
- name: epp-metrics-token
113115
mountPath: /var/run/secrets/epp-metrics
114116
readOnly: true
115117
volumes:
116118
- name: wva-config
117119
configMap:
118120
name: {{ include "workload-variant-autoscaler.fullname" . }}-variantautoscaling-config
121+
{{- if .Values.wva.prometheus.caCert }}
119122
- name: prometheus-ca-cert
120123
configMap:
121124
name: {{ include "workload-variant-autoscaler.fullname" . }}-prometheus-ca
122-
optional: true
125+
{{- end }}
123126
- name: epp-metrics-token
124127
secret:
125128
secretName: {{ include "workload-variant-autoscaler.fullname" . }}-epp-metrics-token

‎charts/workload-variant-autoscaler/templates/prometheus-ca-configmap-prom.yaml‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{{- if .Values.controller.enabled }}
1+
{{- if and .Values.controller.enabled .Values.wva.prometheus.caCert }}
22
apiVersion: v1
33
kind: ConfigMap
44
metadata:
@@ -8,9 +8,5 @@ metadata:
88
{{- include "workload-variant-autoscaler.labels" . | nindent 4 }}
99
data:
1010
ca.crt: |
11-
{{- if .Values.wva.prometheus.caCert }}
1211
{{ .Values.wva.prometheus.caCert | indent 4 }}
13-
{{- else }}
14-
# CA certificate not provided - using system CA bundle
15-
{{- end }}
1612
{{- end }}

‎charts/workload-variant-autoscaler/templates/prometheus-ca-configmap-wva.yaml‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
{{- if .Values.controller.enabled }}
1+
{{- if and .Values.controller.enabled .Values.wva.prometheus.caCert }}
22
apiVersion: v1
33
kind: ConfigMap
44
metadata:
@@ -8,9 +8,5 @@ metadata:
88
{{- include "workload-variant-autoscaler.labels" . | nindent 4 }}
99
data:
1010
ca.crt: |
11-
{{- if .Values.wva.prometheus.caCert }}
1211
{{ .Values.wva.prometheus.caCert | indent 4 }}
13-
{{- else }}
14-
# CA certificate not provided - using system CA bundle
15-
{{- end }}
1612
{{- end }}

‎charts/workload-variant-autoscaler/values-dev.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ wva:
2727
# Development security configuration (relaxed for easier development)
2828
tls:
2929
insecureSkipVerify: true # Development: true, Production: false
30-
caCertPath: "/etc/ssl/certs/prometheus-ca.crt"
30+
# caCertPath: "/etc/ssl/certs/prometheus-ca.crt" # Only used when caCert is provided
3131
# caCert: | # Uncomment and provide your CA certificate
3232
# -----BEGIN CERTIFICATE-----
3333
# YOUR_CA_CERTIFICATE_HERE

‎charts/workload-variant-autoscaler/values.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ wva:
3939
# Development security configuration (relaxed for easier development)
4040
tls:
4141
insecureSkipVerify: true # Development: true, Production: false
42-
caCertPath: "/etc/ssl/certs/prometheus-ca.crt"
42+
# caCertPath: "/etc/ssl/certs/prometheus-ca.crt" # Only used when caCert is provided
4343
# caCert: | # Uncomment and provide your CA certificate
4444
# -----BEGIN CERTIFICATE-----
4545
# YOUR_CA_CERTIFICATE_HERE

‎internal/utils/tls.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ func CreateTLSConfig(cfg *config.Config) (*tls.Config, error) {
3535
MinVersion: tls.VersionTLS12, // Enforce minimum TLS version - https://docs.redhat.com/en/documentation/openshift_container_platform/4.18/html/security_and_compliance/tls-security-profiles#:~:text=requires%20a%20minimum-,TLS%20version%20of%201.2,-.
3636
}
3737

38+
if insecureSkipVerify {
39+
ctrl.Log.V(logging.VERBOSE).Info("TLS certificate verification is disabled, skipping certificate loading")
40+
return config, nil
41+
}
42+
3843
// Load CA certificate if provided
3944
if caCertPath != "" {
4045
caCert, err := os.ReadFile(caCertPath)

‎internal/utils/tls_test.go‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,25 +86,54 @@ func TestCreateTLSConfig(t *testing.T) {
8686
}),
8787
expectError: false,
8888
},
89+
{
90+
name: "insecure skip verify with invalid CA cert path should not error",
91+
promConfig: testConfigFromEnv(t, map[string]string{
92+
"PROMETHEUS_BASE_URL": "https://prometheus:9090",
93+
"PROMETHEUS_TLS_INSECURE_SKIP_VERIFY": "true",
94+
"PROMETHEUS_CA_CERT_PATH": "/nonexistent/path/ca.crt",
95+
}),
96+
expectError: false,
97+
},
8998
}
9099

91100
for _, tt := range tests {
92101
t.Run(tt.name, func(t *testing.T) {
93-
config, err := CreateTLSConfig(tt.promConfig)
102+
tlsCfg, err := CreateTLSConfig(tt.promConfig)
94103
if tt.expectError {
95104
assert.Error(t, err)
96105
return
97106
}
98107
assert.NoError(t, err)
99108
if tt.promConfig != nil {
100-
assert.NotNil(t, config)
109+
assert.NotNil(t, tlsCfg)
101110
} else {
102-
assert.Nil(t, config)
111+
assert.Nil(t, tlsCfg)
103112
}
104113
})
105114
}
106115
}
107116

117+
func TestCreateTLSConfig_InsecureSkipVerifySkipsCertLoading(t *testing.T) {
118+
invalidCertFile, err := os.CreateTemp(t.TempDir(), "invalid-cert-*.crt")
119+
require.NoError(t, err)
120+
_, err = invalidCertFile.WriteString("# CA certificate not provided - using system CA bundle")
121+
require.NoError(t, err)
122+
require.NoError(t, invalidCertFile.Close())
123+
124+
cfg := testConfigFromEnv(t, map[string]string{
125+
"PROMETHEUS_BASE_URL": "https://prometheus:9090",
126+
"PROMETHEUS_TLS_INSECURE_SKIP_VERIFY": "true",
127+
"PROMETHEUS_CA_CERT_PATH": invalidCertFile.Name(),
128+
})
129+
130+
tlsCfg, err := CreateTLSConfig(cfg)
131+
assert.NoError(t, err)
132+
assert.NotNil(t, tlsCfg)
133+
assert.True(t, tlsCfg.InsecureSkipVerify)
134+
assert.Nil(t, tlsCfg.RootCAs, "RootCAs should be nil when insecureSkipVerify is true")
135+
}
136+
108137
func TestValidateTLSConfig(t *testing.T) {
109138
tests := []struct {
110139
name string

0 commit comments

Comments
 (0)