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 parameters to allow migrate job to use its own database account and service account #98

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

Conversation

evankanderson
Copy link

Fixes #95

Description

Add parameters to enable running the migration job with a different database access level.

  • We are using AWS RDS IAM authentication using a sidecar to populate a PGPASSFILE file with credentials (which rotate every 15 minutes).
  • This means we need:
    • A separate database URI (for a different database user with schema-change permissions)
    • A separate service account (for an independent IAM role assignment for the migration job)
    • Separate sidecar, volumes, and volumeMounts to be able to share a 1MB shared in-memory volume between the sidecar and the migration process
    • The ability to set PGPASSFILE in the migration container

References

(Unfortunately, our other examples of doing this aren't public)

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

@evankanderson evankanderson changed the title Add parameters to allow migrate job to use its own database account and service account feat: add parameters to allow migrate job to use its own database account and service account Jan 10, 2024
@evankanderson evankanderson marked this pull request as ready for review January 10, 2024 22:17
@evankanderson evankanderson requested a review from a team as a code owner January 10, 2024 22:17
@evankanderson
Copy link
Author

Interesting... the Contribution Guidelines say the CLA is required, but don't also mention DCO (which seems belt & suspenders to me).

…ount and service account

Signed-off-by: Evan Anderson <evan@stacklok.com>
@evankanderson
Copy link
Author

I'm not sure how this is supposed to be tested. I did some manual testing, but didn't see any testing directions.

Signed-off-by: Evan Anderson <evan@stacklok.com>
@evankanderson
Copy link
Author

I updated the values.schema.json -- I can squash that to a single PR if needed.

@jon-whit
Copy link
Member

@evankanderson thanks for the contribution!

Do you mind doing a screen capture of this working in action? That will help me manually verify it a little better and it could help future eyes landing on this to know how to use it a little better.

@rhamzeh
Copy link
Member

rhamzeh commented Jan 12, 2024

Interesting... the Contribution Guidelines say the CLA is required, but don't also mention DCO (which seems belt & suspenders to me).

Apologies for the confusion - we just disabled the DCO, it was from the time the CLA was not enabled on this repo. Thanks for pointing it out!

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.

Migration job and deployment use the same service account
3 participants