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

SLVS-1396 Migration wizard: Integrate with the new binding and connection models #5700

Merged

Conversation

gabriela-trutan-sonarsource
Copy link
Contributor

@gabriela-trutan-sonarsource gabriela-trutan-sonarsource commented Sep 23, 2024

SLVS-1396

The old migration was working. I just moved the method that was dealing with migrating the ServerConnection for an old binding away from UnintrusiveBindingController and into the ConnectedModeMigration.

Also I am making sure that if the old migration is trying to migrate a server connection and the connections.json does not exist, that we throw an error. The order of the migrations is important.

Additional changes:

  • Renamed MigrateBindingToServerConnectionIfNeededAsync to MigrateAllBindingsToServerConnectionsIfNeededAsync
  • Adapt the ServerConnectionsRepository to not expose the file path, but only if the file exists. In this way, the callers do not need to know the name of the file and to check each time if the file exist, instead they ask directly the repository.

…nection away from the UnintrusiveBindingController and into the ConnectedModeMigration class
… path, but instead only if the file exists.

The callers do not need to know the name of the file, only if the file exists
…ion.json, which would prevent the new migration to migrate the existing bindings.
@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title SLVS-1396 Migration wizard: Integrate with the new binding and connection models SLVS-1396 Migration wizard: Integrate with the new binding and connection models Sep 23, 2024
Copy link
Contributor

@vnaskos-sonar vnaskos-sonar left a comment

Choose a reason for hiding this comment

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

As discussed, there is
one inconsistency: When there are no credentials on the credentials manager, previously it was succeeding to migrate, but currently it fails.
and one issue: When a user has only legacy projects, the connections.json will not be created and the migration will always be skipped.

…verConnection could not be migrated.

If it can not be migrate, due to missing credentials, for example, the old migration should still succeed. The user will just have to manually add the migration
…son does not exist and no bindings in the new binding exist.
Copy link

sonarcloud bot commented Sep 25, 2024

Copy link
Contributor

@vnaskos-sonar vnaskos-sonar left a comment

Choose a reason for hiding this comment

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

👍

@gabriela-trutan-sonarsource gabriela-trutan-sonarsource merged commit ca5fbca into feature/new-connected-mode Sep 25, 2024
4 checks passed
@gabriela-trutan-sonarsource gabriela-trutan-sonarsource deleted the gt/old-migration branch September 25, 2024 12:03
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