Skip to content

SEC-5054-adding custom headers #142

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

Open
sjgupta19 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

SEC-5054-adding custom headers #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3eaa21
SEC-5054-adding custom headers
sjgupta19 Nov 2, 2023
50cf8fa
adding more custom headers and chart version
sjgupta19 Nov 2, 2023
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
2 changes: 1 addition & 1 deletion charts/common/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.8.6-beta.1
version: 0.8.6-beta.2


# This is the version number of the application being deployed. This version number should be
Expand Down
5 changes: 3 additions & 2 deletions charts/common/templates/service-cloudarmor.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ metadata:
{{- include "common.labels" . | nindent 4 }}
annotations:
{{- if .Values.cloudArmor.backendConfig.iap }}
cloud.google.com/backend-config: '{"ports": {"80":"{{ include "common.name" . }}", "443":"{{ include "common.name" . }}"},"default": "{{ include "common.name" . }}"}'
cloud.google.com/backend-config: '{"ports": {"80":"{{ include "common.name" . }}", "443":"{{ include "common.name" . }}"},"default": "{{ include "common.name" . }}", "customRequestHeaders": {{ .Values.cloudArmor.backendConfig.customHeaders | toJson }}}'

@TavoDave TavoDave Nov 2, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sjgupta19 I'm not sure toJson is a valid type conversion, Did you mean toYaml instead?

@sjgupta19 sjgupta19 Nov 2, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

seems like it is valid function - https://helm.sh/docs/chart_template_guide/function_list/

@TavoDave TavoDave Nov 2, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sjgupta19 ic, Can you share the helm template --dry-run where you tested this? Just to make sure does not error out

@sjgupta19 sjgupta19 Nov 2, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

hmm.. erroring out -- Error: file '/Users/srajangupta/Desktop/dave-github/charts/charts/common/templates/service-cloudarmor.yaml' seems to be a YAML file, but expected a gzipped archive

@sjgupta19 sjgupta19 Nov 2, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

thats the same error I am getting without these changes. So I guess its unrelated to my changes?

@sjgupta19 sjgupta19 Nov 2, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I am curious if there is any other way we can add these custom headers? I am following this guide - https://cloud.google.com/load-balancing/docs/https/custom-headers-global

@sjgupta19 sjgupta19 Nov 2, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

just bumped the chart version

@vafied vafied Nov 2, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

here is the documentation on the annotation you are modifying, doesn't look like it supports that directive https://cloud.google.com/kubernetes-engine/docs/how-to/ingress-configuration

@vafied vafied Nov 2, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks like you can use the backendconfig for this https://cloud.google.com/kubernetes-engine/docs/how-to/ingress-configuration#request_headers

@sjgupta19 sjgupta19 Nov 3, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

So do i have to specifically mention that in backendConfig? isn't .Values.cloudArmor.backendConfig.customHeaders this essentially adding it to backendConfig?

{{- else }}
cloud.google.com/backend-config: '{"ports": {"80":"{{ include "common.name" . }}", "443":"{{ include "common.name" . }}"}}'
cloud.google.com/backend-config: '{"ports": {"80":"{{ include "common.name" . }}", "443":"{{ include "common.name" . }}"}, "customRequestHeaders": {{ .Values.cloudArmor.backendConfig.customHeaders | toJson }}}'
{{- end }}
{{- end }}
spec:
type: {{ .Values.cloudArmor.service.type }}
Expand Down
12 changes: 12 additions & 0 deletions charts/common/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,18 @@ cloudArmor:
# enabled: true
# oauthclientCredentials:
# secretName: chart-iapsecret
customHeaders: []
# - name: X-Frame-Options
# value: DENY
# - name: X-XSS-Protection
# value: 1; mode=block
# - name: X-Content-Type-Options
# value: nosniff
# - name: Referrer-Policy
# value: no-referrer-when-downgrade
# - name: Content-Security-Policy
# - name: X-Client-Geo-Location
# value: {client.region_code}, {client.city}
# By default we always use redirectToHttps in frontendConfig if you want to define overwrite consider including it
frontendConfig:
redirectToHttps:
Expand Down