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: include "extraEnvVars" as part of the migration job #135

Merged
merged 8 commits into from
Jun 4, 2024

Conversation

Oscmage
Copy link
Contributor

@Oscmage Oscmage commented May 22, 2024

Description

From reading the code I see no way of providing the same extraEnvVars as done to the deployment for the migration job. The only way I can see is to directly add datastore.uri as part of the values file which is not something we want since it is a secret and something we want to commit to a git repository. It is from what I've found (please correct me if so) not possible to to use something like ExternalSecret to load the value dynamically rather than hardcoding it in the values file.

I'd like to be able to do something like

We are currently running this version of the chart in production and it works just fine.

openfga:
  extraEnvVars:
    - name: OPENFGA_DATASTORE_URI
      valueFrom:
        secretKeyRef:
          name: openfga-external-secrets
          key: OPENFGA_DATASTORE_URI

but it is currently not picked up by the migration job.

References

How it is done in the deployment file:

{{- with .Values.extraEnvVars }}

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

Copy link

linux-foundation-easycla bot commented May 22, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@Oscmage Oscmage closed this May 22, 2024
@Oscmage Oscmage reopened this May 22, 2024
@Oscmage Oscmage changed the title Replicate deployment behaviour Include "extraEnvVars" as part of the migration job May 22, 2024
@Oscmage Oscmage marked this pull request as ready for review May 22, 2024 20:41
@Oscmage Oscmage requested review from a team as code owners May 22, 2024 20:41
@Oscmage Oscmage closed this May 22, 2024
@Oscmage Oscmage deleted the replicate-deployment-behaviour branch May 22, 2024 20:47
@Oscmage Oscmage restored the replicate-deployment-behaviour branch May 22, 2024 20:48
@Oscmage Oscmage reopened this May 22, 2024
@ewanharris ewanharris changed the title Include "extraEnvVars" as part of the migration job feat: include "extraEnvVars" as part of the migration job May 28, 2024
ewanharris
ewanharris previously approved these changes May 28, 2024
Copy link
Member

@ewanharris ewanharris left a comment

Choose a reason for hiding this comment

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

This makes sense to me given #134, but I'd like to tag @jon-whit to make sure there's nothing I'm missing

@Oscmage
Copy link
Contributor Author

Oscmage commented May 30, 2024

@ewanharris Make sense, thanks!

@Oscmage
Copy link
Contributor Author

Oscmage commented Jun 3, 2024

Resolved merge conflict due to other changes.

rhamzeh
rhamzeh previously approved these changes Jun 3, 2024
@jon-whit
Copy link
Collaborator

jon-whit commented Jun 3, 2024

@Oscmage please don't forget to bump the chartVersion.

jon-whit
jon-whit previously approved these changes Jun 3, 2024
@Oscmage Oscmage dismissed stale reviews from jon-whit and rhamzeh via 05c2ecd June 4, 2024 06:06
@Oscmage
Copy link
Contributor Author

Oscmage commented Jun 4, 2024

@Oscmage please don't forget to bump the chartVersion.

Sorry might have gotten lost in the merge.

@jon-whit Do you mean charts/openfga/Chart.yaml version? Did that change now.

@ewanharris ewanharris requested review from rhamzeh and jon-whit June 4, 2024 16:55
@jon-whit jon-whit enabled auto-merge (squash) June 4, 2024 16:59
@jon-whit jon-whit merged commit e4f28fd into openfga:main Jun 4, 2024
2 checks passed
@Oscmage Oscmage deleted the replicate-deployment-behaviour branch June 5, 2024 07:37
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