Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add options to set additional nginx headers #3817

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from
+13 −17
Draft

Add options to set additional nginx headers #3817

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9d61e49
Add options to values.yaml and the nginx template for injecting extra…
nealormsbee Jan 22, 2025
f92effa
Signed-off-by: jesse goodier <31039225+jessegoodier@users.noreply.git…
jessegoodier Jan 23, 2025
f60409c
Add additional whitelist sources to CSP
nealormsbee Jan 23, 2025
eafbdeb
Remove support for per-route header configs. Only use global server h…
nealormsbee Jan 23, 2025
09513a2
Merge branch 'develop' into ndo/make-cors-and-csp-configurable
jessegoodier Jan 24, 2025
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
53 changes: 36 additions & 17 deletions cost-analyzer/templates/cost-analyzer-frontend-config-map-template.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ data:
root /var/www;
index index.html;

add_header Cache-Control "must-revalidate";
{{- range .Values.kubecostFrontend.nginxHeaders.server }}
add_header {{ . }}
{{- end }}

{{- if .Values.kubecostFrontend.extraServerConfig }}
{{- .Values.kubecostFrontend.extraServerConfig | toString | nindent 8 -}}
Expand All @@ -169,6 +171,9 @@ data:
{{- if or .Values.saml.enabled .Values.oidc.enabled }}
add_header Cache-Control "max-age=0";
location / {
{{- range .Values.kubecostFrontend.nginxHeaders.location.root }}
add_header {{ . }}
{{- end }}
auth_request /auth;
proxy_redirect off;
proxy_http_version 1.1;
Expand All @@ -180,6 +185,9 @@ data:
}
location /healthz {
add_header 'Content-Type' 'text/plain';
{{- range .Values.kubecostFrontend.nginxHeaders.location.healthz }}
add_header {{ . }}
{{- end }}
return 200 "healthy\n";
}
{{- else }}
Expand Down Expand Up @@ -218,6 +226,9 @@ data:
{{- end }}

location /model/ {
{{- range .Values.kubecostFrontend.nginxHeaders.location.model }}
add_header {{ . }}
{{- end }}
proxy_connect_timeout {{ .Values.kubecostFrontend.timeoutSeconds | default 300 }};
proxy_send_timeout {{ .Values.kubecostFrontend.timeoutSeconds | default 300 }};
proxy_read_timeout {{ .Values.kubecostFrontend.timeoutSeconds | default 300 }};
Expand All @@ -234,8 +245,9 @@ data:

location ~ ^/(turndown|cluster)/ {

add_header 'Access-Control-Allow-Origin' '*' always;
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;
{{- range .Values.kubecostFrontend.nginxHeaders.location.clusterTurndown }}
add_header {{ . }}
{{- end }}
{{- if .Values.clusterController }}
{{- if .Values.clusterController.enabled }}
{{- if or .Values.saml .Values.oidc }}
Expand Down Expand Up @@ -1407,8 +1419,9 @@ data:
{{- end }}
location = /model/hideOrphanedResources {
default_type 'application/json';
add_header 'Access-Control-Allow-Origin' '*' always;
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;
{{- range .Values.kubecostFrontend.nginxHeaders.location.hideOrphanedResources }}
add_header {{ . }}
{{- end }}
{{- if .Values.kubecostFrontend.hideOrphanedResources }}
return 200 '{"hideOrphanedResources": "true"}';
{{- else }}
Expand All @@ -1417,8 +1430,9 @@ data:
}
location = /model/hideDiagnostics {
default_type 'application/json';
add_header 'Access-Control-Allow-Origin' '*' always;
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;
{{- range .Values.kubecostFrontend.nginxHeaders.location.hideDiagnostics }}
add_header {{ . }}
{{- end }}
{{- if .Values.kubecostFrontend.hideDiagnostics }}
return 200 '{"hideDiagnostics": "true"}';
{{- else }}
Expand All @@ -1434,8 +1448,9 @@ data:

location /model/multi-cluster-diagnostics-enabled {
default_type 'application/json';
add_header 'Access-Control-Allow-Origin' '*' always;
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;
{{- range .Values.kubecostFrontend.nginxHeaders.location.multiClusterDiagnosticsEnabled }}
add_header {{ . }}
{{- end }}
{{- if and .Values.diagnostics.enabled .Values.diagnostics.primary.enabled }}
{{- if or (not (empty .Values.kubecostModel.federatedStorageConfigSecret )) .Values.kubecostModel.federatedStorageConfig }}
return 200 '{"multiClusterDiagnosticsEnabled": true}';
Expand All @@ -1455,8 +1470,9 @@ data:
# Deployment, we should forward that path to the K8s Service.
location /model/diagnostics/multicluster {
default_type 'application/json';
add_header 'Access-Control-Allow-Origin' '*' always;
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;
{{- range .Values.kubecostFrontend.nginxHeaders.location.multiClusterDiagnostics }}
add_header {{ . }}
{{- end }}
proxy_read_timeout 300;
proxy_pass http://multi-cluster-diagnostics/status;
proxy_redirect off;
Expand All @@ -1467,8 +1483,9 @@ data:
# simple alias for support
location /mcd {
default_type 'application/json';
add_header 'Access-Control-Allow-Origin' '*' always;
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;
{{- range .Values.kubecostFrontend.nginxHeaders.location.multiClusterDiagnostics }}
add_header {{ . }}
{{- end }}
proxy_read_timeout 300;
proxy_pass http://multi-cluster-diagnostics/status?window=7d;
proxy_redirect off;
Expand All @@ -1488,8 +1505,9 @@ data:
{{- if .Values.forecasting.enabled }}
location /forecasting {
default_type 'application/json';
add_header 'Access-Control-Allow-Origin' '*' always;
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;
{{- range .Values.kubecostFrontend.nginxHeaders.location.forecastingEnabled }}
add_header {{ . }}
{{- end }}
proxy_read_timeout 300;
proxy_pass http://forecasting/;
proxy_redirect off;
Expand All @@ -1506,8 +1524,9 @@ data:

location /model/productConfigs {
default_type 'application/json';
add_header 'Access-Control-Allow-Origin' '*' always;
add_header 'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;
{{- range .Values.kubecostFrontend.nginxHeaders.location.productConfigs }}
add_header {{ . }}
{{- end }}
return 200 '\n
{
"ssoConfigured": "{{ template "ssoEnabled" . }}",
Expand Down
34 changes: 34 additions & 0 deletions cost-analyzer/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,40 @@ kubecostFrontend:
# fqdn: kubecost-multi-diag.kubecost.svc.cluster.local:9007
# clusterController:
# fqdn: cluster-controller.kubecost.svc.cluster.local:9731
#

# Configurable headers for nginx responses.
nginxHeaders:
# applied to all route locations
server:
- Content-Security-Policy "default-src 'self' cdn.userway.org; frame-ancestors 'none';";
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it'd be a new header. I'm not super familiar with CSPs, do we know this will work?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I still need to test it 😅

The idea is that sources like scripts, images, styles, etc can only be loaded from the same domain the UI is being accessed through, and cdn.userway.org. And also that the application cannot be embedded in an iframe. MDN has great CSP documentation if you feel like digging in more to check my understanding

Copy link
Member Author

Choose a reason for hiding this comment

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

Just pushed an update that I have tested, and it actually does work now.

I had to include directives for font-src data:; and I added unsafe-inline to default-src. I'm not sure whether these will still be flagged. It's safer than having no CSP, and we have something, somewhere, that is adding inline styles to the document.

- Cache-Control "must-revalidate";
# per-location headers
location:
root: []
healthz: []
model: []
clusterTurndown:
- "'Access-Control-Allow-Origin' '*' always;"
- "'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;"
hideOrphanedResources:
- "'Access-Control-Allow-Origin' '*' always;"
- "'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;"
hideDiagnostics:
- "'Access-Control-Allow-Origin' '*' always;"
- "'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;"
multiClusterDiagnosticsEnabled:
- "'Access-Control-Allow-Origin' '*' always;"
- "'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;"
multiClusterDiagnostics:
- "'Access-Control-Allow-Origin' '*' always;"
- "'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;"
forecastingEnabled:
- "'Access-Control-Allow-Origin' '*' always;"
- "'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;"
productConfigs:
- "'Access-Control-Allow-Origin' '*' always;"
- "'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;"
Copy link
Member

Choose a reason for hiding this comment

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

I'm worried that one configuration per route could quickly become unwieldy. Would it be possible start with:

nginxHeaders:
  routes:
    - "'Access-Control-Allow-Origin' '*' always;"
    - "'Access-Control-Allow-Methods' 'GET, PUT, POST, DELETE, OPTIONS' always;"

These headers would then be added to all routes. Do you see an issue with this from a frontend or security perspective?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that's an issue. I was attempting to keep the same behavior we have today, where some location directives don't support CORS...personally I have no problem with broadly applying it! I can update.


# Kubecost Metrics deploys a separate pod which will emit kubernetes specific metrics required
# by the cost-model. This pod is designed to remain active and decoupled from the cost-model itself.
Expand Down
Loading