-
Notifications
You must be signed in to change notification settings - Fork 83
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.
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 additional logic for es restriction from airflow #2159
Conversation
# The following "location" rules limit airflow interactions to only their indices. Any further | ||
# additions should follow this pattern. | ||
location ~* /_count$ { | ||
rewrite /_count(.*) /{{ include "logging.indexNamePrefix" . }}.$remote_user.*/_count$1 break; | ||
{{- if or .Values.global.customLogging.awsSecretName .Values.global.customLogging.awsServiceAccountAnnotation .Values.global.customLogging.awsIAMRole }} | ||
proxy_pass http://localhost:{{ .Values.service.awsproxy }}; | ||
{{- else }} | ||
access_by_lua_file /usr/local/openresty/nginx/conf/setenv.lua; | ||
proxy_pass {{.Values.global.customLogging.scheme}}://{{.Values.global.customLogging.host}}:{{.Values.global.customLogging.port}}; | ||
{{- include "external-es-proxy-trustcerts" . | indent 8 }} | ||
{{- end }} | ||
} | ||
|
||
location ~* /_bulk$ { | ||
rewrite /_bulk(.*) /{{ include "logging.indexNamePrefix" . }}.$remote_user.*/_bulk$1 break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this is just copied code, can we put this whole 'nginx.confor
http` block into a template and include it in both external and non-external es configmaps ? that would be DRY, and as long as they're going to be the same then that would be helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only parts of the changes are copied like rewrite parts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then we need to move the the common changes to global helpers.tpl to share the common code
Co-authored-by: Daniel Hoherd <daniel.hoherd@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs tests. Unfortunately I haven't been able to find a maintained nginx configuration parser for python, so we'll have to test the contents of the nginx config as a string.
Does this PR have a gh issue? |
Description
Updates to external es nginx config with restricted location block.
Related Issues
https://github.com/astronomer/issues/issues/6241
Testing
Requires external elastic search to test this change
Merging
Yet to update