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.

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

feat: add geoip support to sentry deployment #1516

Merged
merged 2 commits into from
Oct 7, 2024
Merged

feat: add geoip support to sentry deployment #1516

merged 2 commits into from
Oct 7, 2024

Conversation

patsevanton
Copy link
Contributor

This pull request introduces support for GeoIP in the Sentry deployment. The changes include:

  1. New Configuration Options:

    • Added new configuration options for GeoIP in the values.yaml file.
    • The new options include accountID, licenseKey, editionIDs, persistence, volumeName, mountPath, and path.
  2. New Templates:

    • Added a new deployment-geoip-job.yaml template to handle the GeoIP database update job.
    • Added a new pvc-geoip.yaml template for the PersistentVolumeClaim (PVC) to store the GeoIP database.
    • Added a new secret-geoip-env.yaml template to manage the environment variables required for GeoIP configuration.
  3. Volume Mounts:

    • Added volume mounts for the GeoIP database in the deployment-relay.yaml, deployment-sentry-web.yaml, and deployment-sentry-worker-events.yaml templates.
  4. Documentation:

    • Updated the README.md file with an example of how to configure GeoIP in the Sentry deployment.

@Mokto Mokto merged commit 4f2429b into sentry-kubernetes:develop Oct 7, 2024
2 checks passed
@Mokto Mokto mentioned this pull request Oct 7, 2024
@swade1987
Copy link

swade1987 commented Oct 8, 2024

We should allow the user to provide the GeoIP configuration via a secret. Otherwise, we are adding tech-debt like we had when being unable to provide the Redis password via a secret. A license key is not something that should be stored in plain text anywhere as its a "secret".

@patsevanton
Copy link
Contributor Author

patsevanton commented Oct 8, 2024

@swade1987 You want to instead of indicating

geodata:
  licenseKey: “licenseKey”.

specify

geodata:
  licenseKey: secret name with licenseKey?

@swade1987
Copy link

@swade1987 You want to instead of indicating

geodata:
  licenseKey: “licenseKey”.

specify

geodata:
  licenseKey: secret name with licenseKey?

All the configuration options for the geodata could be passed as a single secret.
I would typically say ConfigMap but as the license key is a secret that outweighs it in this instance.

Comment on lines +17 to +21
{{- if (eq "-" .Values.geodata.persistence.storageClass) }}
storageClassName: ""
{{- else }}
storageClassName: "{{ .Values.geodata.persistence.storageClass }}"
{{- end }}

Choose a reason for hiding this comment

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

@patsevanton according to the comment in the values.yaml shouldn't that be:

{{- if .Values.geodata.persistence.storageClass }}
{{- if (eq "-" .Values.geodata.persistence.storageClass) }}
  storageClassName: ""
{{- else }}
  storageClassName: "{{ .Values.geodata.persistence.storageClass }}"
{{- end }}
{{- end }}

Copy link
Contributor Author

@patsevanton patsevanton Oct 8, 2024

Choose a reason for hiding this comment

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

@sdernbach-ionos please review PR #1524

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants