fix(auth-secrets): fix system update secrets #351
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
david-leifker
requested review from
pedro93,
iprentic and
anshbansal
and removed request for
pedro93
pedro93
reviewed
Aug 16, 2023
| @@ -39,15 +39,6 @@ Return the env variables for upgrade jobs | |||
| value: "{{ .Values.global.sql.datasource.url }}" | |||
| - name: EBEAN_DATASOURCE_DRIVER | |||
| value: "{{ .Values.global.sql.datasource.driver }}" | |||
| {{- if .Values.global.datahub.metadata_service_authentication.enabled }} | |||
| - name: DATAHUB_SYSTEM_CLIENT_ID | |||
There was a problem hiding this comment.
Why removing credentials from the upgrade job?
There was a problem hiding this comment.
I am a bit confused here too why this is causing the problem.
We have 4 jobs https://github.com/search?q=repo%3Aacryldata%2Fdatahub-helm%20%20include%20%22datahub.upgrade.env%22&type=code which include this datahub.upgrade.env and 3 of them are getting this new config. Which means something with [datahub-system-update-job.yml](https://github.com/acryldata/datahub-helm/blob/30eee991bbad5b398051758c1a97a5d028430427/charts/datahub/templates/datahub-upgrade/datahub-system-update-job.yml#L66) is causing the problem.
Now would system update job be able to work without these variables? I am not sure about that.
There was a problem hiding this comment.
The system update job runs early, before secrets are created. Second, the system update doesn't access any GMS services since it is not running at that early (or running a read-only version of the previous release). The non-existent secrets cannot be mounted in the job. Maybe there is another way to fix it, but this is the easiest, I think.
pedro93
approved these changes
Aug 16, 2023
ericnoam
added a commit
to ericnoam/datahub-helm
that referenced
this pull request
Sep 15, 2023
* feat: values for PSQL db name, elastic prefix (acryldata#313) * Values for PSQL database name, elastic prefix --------- Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> * feat(search,schema-registry): updates for v0.10.3 release (acryldata#311) * Update kafka chart to 22.1.3 for kafka 3.4.0 (acryldata#316) * feat: add parameters to cleanupJob resources settings (acryldata#317) * feat: Add ability to specify extraPodLabels per deployment (acryldata#310) * feat: Add ability to specify extraPodLabels per deployment * Update Chart.yaml --------- Co-authored-by: jorrick <jorrick.sleijster@adyen.com> Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> * fix: use common labels for (Cron)Jobs (acryldata#303) * fix(datahub): use common labels for (Cron)Jobs * chore: update version --------- Co-authored-by: Matthijs van der Loos <matthijs.vanderloos@utopiamusic.com> Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> * fix: add missing global values to subchart values (acryldata#302) * fix(datahub): add missing global values to subchart values * chore: update versions --------- Co-authored-by: Matthijs van der Loos <matthijs.vanderloos@utopiamusic.com> Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> * feat: allow pulling ebean username from secrets alongside password (acryldata#291) * chore(secrets): use configurable refs instead of fixed names (acryldata#323) * chore(secrets): use configurable refs instead of fixed names * Update Chart.yaml --------- Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> * Update Default version to v0.10.4 (acryldata#330) * fix: Fixed indentation in datahub-cleanup-job-template.yml (acryldata#328) * feat(healthcheck): use new healthcheck endpoint for GMS (acryldata#331) Co-authored-by: Indy Prentice <indy@Indys-MacBook-Pro.local> * chore(version): version bump & indent (acryldata#324) * feat: Default User Credentials (acryldata#321) Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> * feat(cloud-sql-proxy): add support for running gcloud sql proxy as prerequisite (acryldata#332) * feat: allow for overriding job annotations and adding init containers (acryldata#315) * allow for overriding hook annotations * allow for specifying init containers on all jobs --------- Signed-off-by: David van der Spek <vanderspek.david@gmail.com> * feat(cron): Adding more parameters to ingestion-cron (acryldata#336) * feat(cron): Adding more parameters to ingestion-cron --------- Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> * Update charts to include search and browse env variable flags (acryldata#337) * Update charts to include search and browse env variable flags * fix(config) Set search and browse flags default off (acryldata#339) * fix(config) Set search and browse flags default off * feat(cron): support nodeselector, affinity and toleration capabilities (acryldata#342) * feat : add tolerations parameter in datahub-ingestion-cron chart --------- Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> * Helm update for 0.10.5 release (acryldata#346) * Helm update for 0.10.5 release * use latest point release for ingestion * docs(readme): document secrets randomization (acryldata#350) * docs(readme): Add notes about randomized keys and credentials * fix(auth-secrets): fix system update secrets (acryldata#351) * fix(auth-secret): remove auth secret from common template, cannot be used by all jobs * fix(datahub-system-update-job): enable `SPRING_KAFKA_PROPERTIES_AUTO_REGISTER_SCHEMAS` (acryldata#358) * fix(datahub-system-update-job): enable SPRING_KAFKA_PROPERTIES_AUTO_REGISTER_SCHEMAS * feat(session): add session duration configuration (acryldata#361) * feat(session): add session duration configuration * fix(ingestion-cron): fix indentation in ingestion cron template (acryldata#356) * fix: remove unused and irrelevant sidecar configuration * fix: correct sidecar configuration in cron job template * fix: update ingestion cron sidecar parameter in README --------- Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> * docs(ingestion-cron): add documentation for ingestion cron job values (acryldata#355) * Helm changes for 0.11.0 release (acryldata#366) Co-authored-by: Indy Prentice <indy@Indys-MacBook-Pro.local> * Release v0.11.0 updates (acryldata#367) * feat(release): updates for release v0.11.0 --------- Co-authored-by: KonstantinVishnivetskii <120646195+KonstantinVishnivetskii@users.noreply.github.com> Co-authored-by: david-leifker <114954101+david-leifker@users.noreply.github.com> Co-authored-by: Jinlin Yang <86577891+jinlintt@users.noreply.github.com> Co-authored-by: Álvaro González <alvaro.gonzalez@auctane.com> Co-authored-by: Jorrick Sleijster <jorricks3@gmail.com> Co-authored-by: jorrick <jorrick.sleijster@adyen.com> Co-authored-by: Matthijs van der Loos <33864538+matthijsvanderloos@users.noreply.github.com> Co-authored-by: Matthijs van der Loos <matthijs.vanderloos@utopiamusic.com> Co-authored-by: Max Pospischil <maxpospischil@gmail.com> Co-authored-by: Sergio Gómez Villamor <sgomezvillamor@gmail.com> Co-authored-by: Pedro Silva <pedro@acryl.io> Co-authored-by: TusharM <tushar.madhukar@gmail.com> Co-authored-by: Indy Prentice <iprentic@users.noreply.github.com> Co-authored-by: Indy Prentice <indy@Indys-MacBook-Pro.local> Co-authored-by: seokyun.ha <127274415+Seokyun-Ha@users.noreply.github.com> Co-authored-by: Tony Ouyang <43738225+TonyOuyangGit@users.noreply.github.com> Co-authored-by: David van der Spek <vanderspek.david@gmail.com> Co-authored-by: miguelbirdie <86949255+miguelbirdie@users.noreply.github.com> Co-authored-by: Chris Collins <chriscollins3456@gmail.com> Co-authored-by: sachinsaju <33017477+sachinsaju@users.noreply.github.com> Co-authored-by: Kohei Watanabe <littlewat.dev@gmail.com> Co-authored-by: RyanHolstien <RyanHolstien@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The auth secrets are not common to all jobs. Moved from common function to individual jobs. Without this change the following values will not deploy successfully.
Checklist