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(vaults) remove beta label, migrate tables and enable bundled vaults #8871

Merged
merged 2 commits into from
May 31, 2022

Conversation

bungle
Copy link
Member

@bungle bungle commented May 27, 2022

Summary

This is quite awful, but it seems like there is no clear way to migrate over in single migration.

The migration has to do two things:

  1. rename possible (ee only - vault auth plugin) vaults table to vault_auth_vaults
  2. rename vaults_beta table to vaults table

On Postgres it happens to do it in place on up migration. It is not blue-green, but it is impossible to make this blue-green in single migration without a lot of hacks in code itself. On Cassandra everything happens on finish migration as you don't have DDL like what Postgres offers. On Cassandra it is not blue-green either.

Is this good enough?

  1. the problem is mostly with people who happen to use EE only vault_auth plugin, which we think there are not that many
  2. it will migrate data over, just that it is not blue-green

We might want to add some migration tests (outside this PR).
The EE Vault auth plugin needs to be modified too (outside this PR).

### Summmary

Kong `2.8.0` came with Vaults Beta, and that used `:8001/vaults-beta` Admin API
endpoint and `kong.db.vaults_beta` DAO. This commit removes the beta labels from
those. This commit also contains migrations to remove `_beta` label from underlying
database tables.
### Summary

On Kong `2.8.0` we released Vaults as `beta`. We also made it so that no Vaults were
enabled by default. Now that we are getting to `3.0.0`, and we remove the `beta` label,
the bundled Vaults should be enabled by default. This commit changes that.
@bungle bungle force-pushed the feat/vaults-remove-beta-label branch from dd8376a to 5bef165 Compare May 27, 2022 12:49
Copy link
Contributor

@jschmid1 jschmid1 left a comment

Choose a reason for hiding this comment

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

We might want to add some migration tests (outside this PR).
The EE Vault auth plugin needs to be modified too (outside this PR).

+1

@bungle bungle merged commit 830bbe8 into master May 31, 2022
@bungle bungle deleted the feat/vaults-remove-beta-label branch May 31, 2022 12:56
@aboudreault
Copy link
Contributor

For EE and version compatibility, we might need to rename the entity to vaults_beta before sending the entities to the DPs.

@bungle
Copy link
Member Author

bungle commented Jun 3, 2022

For EE and version compatibility, we might need to rename the entity to vaults_beta before sending the entities to the DPs.

Good point. This is definitely hairy.

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

Successfully merging this pull request may close these issues.

3 participants