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

Feature/Use Zitadel Postgres Integration by default #2181

Merged
merged 20 commits into from
Jun 25, 2024
Merged

Feature/Use Zitadel Postgres Integration by default #2181

merged 20 commits into from
Jun 25, 2024

Conversation

r0b2g1t
Copy link
Contributor

@r0b2g1t r0b2g1t commented Jun 22, 2024

This PR replaces cockroachDB as default DB for Zitadel in the getting started script to deploy script. Users can switch back to cockroachDB by setting the environment variable ZITADEL_DATABASE to cockroach.

The PR fix some typos as well.

Issue ticket number and link

Checklist

  • Is it a bug fix
  • Is a feature enhancement

@mlsmaycon
Copy link
Collaborator

Awesome PR, @r0b2g1t; thanks. We actually had in our roadmap to make Postgres the default for the installer. Can you update the following check to reflect that?

  if [[ $ZITADEL_DATABASE == "" ]]; then
    CRDB=$(renderDockerComposeCockroachDB)
    ZITADEL_DB_ENV=$(renderZidatelCockroachDBEnv)
  elif [[ $DATABASE == "postgres" ]]; then
    CRDB=$(renderDockerComposePostgres)
    ZITADEL_DB_ENV=$(renderZidatelPostgresEnv)
  fi

@r0b2g1t r0b2g1t changed the title Feature/zitadel postgres integration Feature/zitadel postgres Integration Jun 22, 2024
@r0b2g1t
Copy link
Contributor Author

r0b2g1t commented Jun 22, 2024

@mlsmaycon thanks for the remark. I have set Postgres as default and added echos for the user about the usage of the environment variable.

@r0b2g1t r0b2g1t changed the title Feature/zitadel postgres Integration Feature/Zitadel Postgres Integration Jun 22, 2024
Copy link
Collaborator

@mlsmaycon mlsmaycon left a comment

Choose a reason for hiding this comment

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

infrastructure_files/getting-started-with-zitadel.sh Outdated Show resolved Hide resolved
infrastructure_files/getting-started-with-zitadel.sh Outdated Show resolved Hide resolved
infrastructure_files/getting-started-with-zitadel.sh Outdated Show resolved Hide resolved
infrastructure_files/getting-started-with-zitadel.sh Outdated Show resolved Hide resolved
infrastructure_files/getting-started-with-zitadel.sh Outdated Show resolved Hide resolved
@r0b2g1t
Copy link
Contributor Author

r0b2g1t commented Jun 22, 2024

Thanks @r0b2g1t .

Can you also update the https://github.com/netbirdio/netbird/blob/main/.github/workflows/test-infrastructure-files.yml#L172 file with cockroachdb tests?

I will take a look into it and will try to create tests. 🙂

@r0b2g1t
Copy link
Contributor Author

r0b2g1t commented Jun 22, 2024

Thanks @r0b2g1t .
Can you also update the https://github.com/netbirdio/netbird/blob/main/.github/workflows/test-infrastructure-files.yml#L172 file with cockroachdb tests?

I will take a look into it and will try to create tests. 🙂

@mlsmaycon I have added tests for psotgres and cockroach.

@r0b2g1t
Copy link
Contributor Author

r0b2g1t commented Jun 23, 2024

@mlsmaycon the tests are stable. May you can take a look into it and give feedback.

@mlsmaycon
Copy link
Collaborator

@r0b2g1t thanks again for the amazing contribution

@mlsmaycon mlsmaycon changed the title Feature/Zitadel Postgres Integration Feature/Use Zitadel Postgres Integration by default Jun 25, 2024
@mlsmaycon mlsmaycon merged commit 1787477 into netbirdio:main Jun 25, 2024
24 checks passed
lixmal pushed a commit that referenced this pull request Jul 22, 2024
replaces cockroachDB as default DB for Zitadel in the getting started script to deploy script. Users can switch back to cockroachDB by setting the environment variable ZITADEL_DATABASE to cockroach.
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.

2 participants